Hi All
On 9/15/21 7:59 AM, Marek Vasut wrote:
> On 9/15/21 6:23 AM, Heiko Schocher wrote:
>> Hi Marek,
>>
>> On 15.09.21 01:06, Marek Vasut wrote:
>>> The flash->mtd.name used to be nor%d before, now it is the type of the
>>> SPI NOR like e.g. mt25ql02g. It is possible to find plenty of examples
>>> of the former in U-Boot configs by searching for MTDIDS.*nor.*spi, while
>>> the later is ambiguous if there are multiple flashes of the same type in
>>> the system and breaks existing environments.
>>>
>>> This does no longer get recognized when running 'mtdparts' for example:
>>> CONFIG_MTDIDS_DEFAULT="nor0=47040000.spi.0"
>>>
>>> Fix this by setting the correct mtd.name to nor%d.
>>>
>>> Fixes: b7f060565e3 ("mtd: spi-nor: allow registering multiple MTDs when DM
>>> is enabled")
>>> Signed-off-by: Marek Vasut <ma...@denx.de>
>>> Cc: Heiko Schocher <h...@denx.de>
>>> Cc: Jagan Teki <ja...@amarulasolutions.com>
>>> Cc: Marek Behún <marek.be...@nic.cz>
>>> Cc: Miquel Raynal <miquel.ray...@bootlin.com>
>>> Cc: Pali Rohár <p...@kernel.org>
>>> Cc: Patrice Chotard <patrice.chot...@foss.st.com>
>>> Cc: Patrick Delaunay <patrick.delau...@st.com>
>>> Cc: Priyanka Jain <priyanka.j...@nxp.com>
>>> Cc: Simon Glass <s...@chromium.org>
>>> ---
>>> drivers/mtd/spi/sf_mtd.c | 5 ++++-
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> Seem fixes the same problem as Patrick already posted here:
>>
>> https://patchwork.ozlabs.org/project/uboot/patch/20210913095742.v2.1.I73dae4b93f0587dc130e512e95a1f4794e0b0233@changeid/
>>
>> I find your approach cleaner, so:
>>
>> Acked-by: Heiko Schocher <h...@denx.de>
>>
>> @Patrick: Could you test this patch please?
>
> The option from Patrick does look more predictable, but looking at the old
> implementation of spi_flash_mtd_number(), that one handled even cases of
> parallel-nor and spi-nor (both using the nor%d) present on the same system.
> This:
>
> static int spi_flash_mtd_number(void)
> {
> #ifdef CONFIG_SYS_MAX_FLASH_BANKS
> return CONFIG_SYS_MAX_FLASH_BANKS;
> #else
> return 0;
> #endif
> }
> ...
> sprintf(sf_mtd_name, "nor%d", spi_flash_mtd_number());
>
> The patch from Patrick does not, it does per-spi-nor-only counting using the
> dev_seq() there, which is more predictable, but local to spi-nor.
>
> The MTD core has its own IDR counting, which is used by this patch and is
> global to the MTD core. That has two issues, one is that it is possibly too
> global and counts both nor%d and nand%d, which means it would fail on systems
> with both SPI NOR and regular NAND. The other is it is likely less
> predictable (what happens if the SPI NOR and parallel NOR gets probed in the
> reverse order ? I know it is unlikely, but it can happen in the future).
>
> So I think we need a third option which I would suggest be based on the patch
> from Patrick, but which would take into account the other nor%d devices like
> parallel NOR flashes.
>
> That would likely have to happen in the mtd core using some modified IDR
> counting. I think you might need to modify it such that you would pass in the
> prefix of the device (like 'nor') and then do per-prefix counting, with the
> special case that parallel NORs are counted first. Also, that would also have
> to consider probing of multiple SPI NORs in either order (say you have two,
> if you do 'sf probe 0:0 ; sf probe 0:1' vs. 'sf probe 0:1 ; sf probe 0:0'),
> and make sure they get enumerated with the same nor%d value either way, that
> might be where the dev_seq() would come in.
I just got a try with this patch and unfortunately, it doesn't solve the issue.
I tested it on STM32MP1-ev1 board which embedds 1 NAND and 2 SPI-NORs.
Here is the output of mtd list command:
U-Boot 2021.10-rc4-00001-g1923b5a21d (Sep 15 2021 - 10:48:48 +0200)
CPU: STM32MP157AAA Rev.B
Model: STMicroelectronics STM32MP157C eval daughter on eval mother
Board: stm32mp1 in basic mode (st,stm32mp157c-ev1)
Board: MB1263 Var1.0 Rev.C-01
DRAM: 1 GiB
Clocks:
- MPU : 650 MHz
- MCU : 208.878 MHz
- AXI : 266.500 MHz
- PER : 24 MHz
- DDR : 533 MHz
WDT: Started with servicing (32s timeout)
NAND: 1024 MiB
MMC: STM32 SD/MMC: 0, STM32 SD/MMC: 1
Loading Environment from MMC... *** Warning - bad CRC, using default environment
In: serial
Out: serial
Err: serial
Net: eth0: ethernet@5800a000
Hit any key to stop autoboot: 0
STM32MP> mtd list
SF: Detected mx66l51235l with page size 256 Bytes, erase size 64 KiB, total 64
MiB
SF: Detected nor1 with page size 256 Bytes, erase size 64 KiB, total 64 MiB
List of MTD devices:
* nand0
- type: NAND flash
- block size: 0x40000 bytes
- min I/O: 0x1000 bytes
- OOB size: 224 bytes
- OOB available: 118 bytes
- ECC strength: 8 bits
- ECC step size: 512 bytes
- bitflip threshold: 6 bits
- 0x000000000000-0x000040000000 : "nand0"
* nor2
- device: mx66l51235l@0
- parent: spi@58003000
- driver: jedec_spi_nor
- path: /soc/spi@58003000/mx66l51235l@0
- type: NOR flash
- block size: 0x10000 bytes
- min I/O: 0x1 bytes
- 0x000000000000-0x000004000000 : "nor2"
* nor2
- device: mx66l51235l@1
- parent: spi@58003000
- driver: jedec_spi_nor
- path: /soc/spi@58003000/mx66l51235l@1
- type: NOR flash
- block size: 0x10000 bytes
- min I/O: 0x1 bytes
- 0x000000000000-0x000004000000 : "nor2"
Patrice
