Hi,
On 9/16/21 9:50 AM, Patrick DELAUNAY wrote:
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.
V3 sent => mtd: spi: nor: force mtd name to "nor%d"
http://patchwork.ozlabs.org/project/uboot/list/?series=262632&state=*
I need to solve some compilation issue when I use CONFIG_SYS_MAX_FLASH_BANKS
Patrick