Re: [PATCH v4 0/2] mtd: spi: nor: force mtd name to "nor%d"

["Marek Behún" <marek.behun@nic.cz>]

On Fri, 24 Sep 2021 14:22:57 -0400
Tom Rini <trini@konsulko.com> wrote:

> On Thu, Sep 23, 2021 at 11:04:28AM +0200, Patrick DELAUNAY wrote:
> > Hi,
> > 
> > On 9/23/21 3:32 AM, Marek Vasut wrote:  
> > > On 9/22/21 10:00 PM, Tom Rini wrote:  
> > > > On Wed, Sep 22, 2021 at 09:56:26PM +0200, Marek Vasut wrote:  
> > > > > On 9/22/21 9:46 PM, Tom Rini wrote:  
> > > > > > On Wed, Sep 22, 2021 at 09:24:24PM +0200, Marek Vasut wrote:
> > > > > >   
> > > > > > > I am talking about using nor%d in MTDIDS in U-Boot UBI
> > > > > > > code to look up from
> > > > > > > which device to attach UBI in U-Boot.  
> > > > > > 
> > > > > > OK, so are we not able to pass in the correct name now?  Or
> > > > > > just worried
> > > > > > about old environment and new U-Boot?  
> > > > > 
> > > > > Say you have the following in board config:
> > > > > 
> > > > > CONFIG_MTDIDS_DEFAULT="nor0=ff705000.spi.0"
> > > > > CONFIG_MTDPARTS_DEFAULT="mtdparts=ff705000.spi.0:-(fs);"
> > > > > 
> > > > > Then run "=> ubi part fs", which will fail to find nor0, because
> > > > > now that
> > > > > nor0 is called something else. That is what this series tries to fix.  
> > > > 
> > > > Yes, and what is nor0 now, and what happens if you use it?  
> > > 
> > > Now it is "mt25ql02g", for all mt25ql02g on the board, so ... I cannot
> > > even select the one SPI NOR I want to use, since they are not even
> > > enumerated in any way, they are all the same. You might want to look at
> > > get_mtd_device_nm() in drivers/mtd/mtdcore.c .  
> > 
> > 
> > To comple me use case, on EV1 board can boot from NOR / NAND / SPI-NAND
> > 
> > so mtdparts and mtdids are buidl dynamically with
> > CONFIG_SYS_MTDPARTS_RUNTIME in
> > 
> > afraided board/st/common/stm32mp_mtdparts.c::board_mtdparts_default()
> > 
> > 
> > I don't use MTDIDS_DEFAULT / MTDPARTS_DEFAULT.
> > 
> > 
> > For example, when I force NOR / NAND presence, I create the MTD variables:
> > 
> > mtdids=nand0=nand0,nor0=nor0
> > 
> > mtdparts=mtdparts=nand0:2m(fsbl),2m(ssbl1),2m(ssbl2),-(UBI);nor0:256k(fsbl1),256k(fsbl2),2m(ssbl),512k(u-boot-env),-(nor_user)
> > 
> > 
> > The command "mtdparts" is working in previous U-Boot releaseafraided
> > 
> > and it is not more working as the name of MTD device change
> > 
> > 
> > Today, without my patch I have
> >   
> > STM32MP> mtd list  
> > SF: Detected mx66l51235l with page size 256 Bytes, erase size 64 KiB, total
> > 64 MiB
> > Could not find a valid device for nor0
> > 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-0x000000400000 : "ssbl1"
> >       - 0x000000400000-0x000000600000 : "ssbl2"
> >       - 0x000000600000-0x000040000000 : "UBI"
> > * mx66l51235l
> >   - 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 : "mx66l51235l"
> > * mx66l51235l
> >   - 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 : "mx66l51235l"
> > 
> > 
> > 
> > before my patch, Ihave always the error "Device nor0 not found!" on mtdparts
> > command
> >   
> > => get_mtd_info  
> >   
> > ==> get_mtd_device_nm("nor0")   build with MTD_DEV_TYPE(type)  
> >   
> > ===> mtd_device_matches_name()  
> > 
> >             and here "nor0" must be  mtd->name acoring the code
> > 
> > 
> > or I miss something...
> > 
> > 
> > I don't found any way to solve my issue only with "mtdids" variable.
> > 
> > so I restore the previous behavior as I expect the mtd name
> > 
> > modification can impact many other boards.
> > 
> > 
> > A other solution can be change get_mtd_info(),
> > 
> > but I was also afraid of side effect.  
> 
> Thanks for explaining more.  Marek, any ideas on how to resolve this
> best, other than logic to restore some form of nor%d being created here?

So in board/st/common/stm32mp_mtdparts.c the mtd partitions are defined
in a options configurable in the Kconfig file in that directory:
  config MTDPARTS_NAND0_BOOT
    default "2m(fsbl),2m(ssbl1),2m(ssbl2)" if STM32MP15x_STM32IMAGE ||
                                              !TFABOOT
    default "2m(fsbl),4m(fip1),4m(fip2)"

  config MTDPARTS_NAND0_TEE
    default "512k(teeh),512k(teed),512k(teex)"
and so on.

This should be moved to device tree, and if needed, fixed up via
board_fix_fdt() if some needed fixes can only be discovered in runtime.

When mtd partitions are defined this way, the mtd command will be able
to work with them all.

The other thing is that the boot device is chosen according to the
result of get_bootmode() function, and can be UART, USB, NAND, SPINAND,
NOR.

This could be implemented via distro boot by setting some env variables
to needed values. include/configs/stm32mp1.h already does define
bootcmd_stm32mp, for example.

Anyway, this whole thing can be converted to not use mtdparts. I can
elaborate more to Patrick if he has some questions. He can also look
at how board/CZ.NIC/turris_omnia.c changes distro boot in
handle_reset_button() function.



I think all the other code using mtdparts can be converted in a similar
way. The question is how hard would it be to convert things such as UBI.

Marek