Hi,
On 9/15/21 2:10 PM, Marek Vasut wrote:
On 9/15/21 2:00 PM, Patrick DELAUNAY wrote:
Hi,
On 9/15/21 10:57 AM, Marek Vasut wrote:
On 9/15/21 10:53 AM, Patrice CHOTARD wrote:
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 marex@denx.de >>> Cc: Heiko Schocher hs@denx.de >>> Cc: Jagan Teki jagan@amarulasolutions.com >>> Cc: Marek Behún marek.behun@nic.cz >>> Cc: Miquel Raynal miquel.raynal@bootlin.com >>> Cc: Pali Rohár pali@kernel.org >>> Cc: Patrice Chotard patrice.chotard@foss.st.com >>> Cc: Patrick Delaunay patrick.delaunay@st.com >>> Cc: Priyanka Jain priyanka.jain@nxp.com >>> Cc: Simon Glass sjg@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.I73dae4... >> >> >> I find your approach cleaner, so: >> >> Acked-by: Heiko Schocher hs@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"
Right , that's what I was afraid of. The patch from Patrick would fail if there are multiple disparate NOR technologies (parallel NOR and SPI NOR). So we need a third option.
And also HyperFlash, which today, are seen as "nor".
Dang ... that too. I have a system with HF and SPI NOR which I could use for testing I _think_. Do you expect Patrick to send out a V2 of his patch with this stuff above addressed ?
Hi,
I will change the name to "spi-nor%d" in V2 (alligned with "spi-nand")
to avoid conflict with other "nor" device.
No, that is not an option, because that will break existing devices, grep for MTDIDS.*nor in configs/ .
I need to check if that solve my issue.
[...]
Yes, after check / test it is not a option and that is not working
https://github.com/patrickdelaunay/u-boot/pull/new/mtd_nor
STM32MP> env print mtdids mtdids=nand0=nand0,nor0=spi-nor0
STM32MP> env print mtdparts mtdparts=mtdparts=nand0:2m(fsbl),4m(fip1),4m(fip2),-(UBI);nor0:256k(fsbl1),256k(fsbl2),4m(fip),512k(u-boot-env),-(nor_user)
STM32MP> mtd list 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" - 0x000000000000-0x000000200000 : "fsbl" - 0x000000200000-0x000000600000 : "fip1" - 0x000000600000-0x000000a00000 : "fip2" - 0x000000a00000-0x000040000000 : "UBI" * spi-nor0 - 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 : "spi-nor0" * spi-nor1 - 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 : "spi-nor1"
But I get the error:
STM32MP> mtdparts Device nor0 not found!
it is linked to get_mtd_info() in cmd/mtdparts.c
=> search for "nor%d" device associated to MTD_DEV_TYPE_NOR
MTD_DEV_TYPE(type), num
New solution:
to align the device name in cfi and sf part
in cfi_mtd_init(void)
for (i = 0; i < CONFIG_SYS_MAX_FLASH_BANKS; i++) {
...
sprintf(cfi_mtd_names[i], "nor%d", i); mtd->name = cfi_mtd_names[i];
...
}
so for CFI NOR => "nor0" ..... "norN"
and I update my patch to handle the spi nor AFTER the CFI nor....
based on CONFIG_SYS_MAX_FLASH_BANKS
CFI NOR => "norM" ..... "norZ"
with M = N+1
I will propose it soon.
Patrick