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

[Marek Vasut <marex@denx.de>]

On 9/24/21 9:25 PM, Marek Behún wrote:
> 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.

I have a feeling the discussion is again banking toward "mtdparts" and 
"DT", which really is a solved problem, or at least we agreed upon the 
solution.

The patch is trying to fix MTDIDS and their look up in 
get_mtd_device_nm(). This is all U-Boot stuff, it has nothing to do with 
passing mtd partitions to Linux at all.