Re: [PATCH u-boot-dm + u-boot-spi v4 04/10] mtd: spi-nor: allow registering multiple MTDs when DM is enabled

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 3431 bytes --]

On Thu, Jul 08, 2021 at 04:15:17PM +0200, Marek Behún wrote:
> On Thu, 8 Jul 2021 08:54:51 +0900
> Masami Hiramatsu <masami.hiramatsu@linaro.org> wrote:
> 
> > Hi Marek,
> > 
> > I found that this changes the mtd device name and makes 'mtdparts'
> > doesn't work on my developerbox platform.
> > 
> > Before this change,
> > -------
> > => sf probe  
> > SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB,
> > total 64 MiB
> > => mtd list  
> > List of MTD devices:
> > * nor1
> >   - type: NOR flash
> >   - block size: 0x1000 bytes
> >   - min I/O: 0x1 bytes
> >   - 0x000000000000-0x000004000000 : "nor1"
> >           - 0x000000000000-0x000000070000 : "BootStrap-BL1"
> >           - 0x000000070000-0x000000100000 : "Flash-Writer"
> >           - 0x000000100000-0x000000180000 : "SCP-BL2"
> >           - 0x000000180000-0x0000001f8000 : "FIP-TFA"
> >           - 0x0000001f8000-0x000000200000 : "Stg2-Tables"
> >           - 0x000000200000-0x000000400000 : "EDK2"
> >           - 0x000000400000-0x000000500000 : "UEFI-Vars"
> >           - 0x000000500000-0x000000700000 : "OPTEE"
> >           - 0x000000700000-0x000000800000 : "UBoot-Env"
> >           - 0x000000800000-0x000000900000 : "U-Boot"
> >           - 0x000000900000-0x000004000000 : "Free"
> > =>  
> > -------
> > after this change,
> > -------
> > => sf probe  
> > SF: Detected mx66u51235f with page size 256 Bytes, erase size 4 KiB,
> > total 64 MiB
> > => mtd list  
> > Could not find a valid device for nor1
> > List of MTD devices:
> > * mx66u51235f
> >   - device: spi-flash@0
> >   - parent: spi@54800000
> >   - driver: jedec_spi_nor
> >   - path: /spi@54800000/spi-flash@0
> >   - type: NOR flash
> >   - block size: 0x1000 bytes
> >   - min I/O: 0x1 bytes
> >   - 0x000000000000-0x000004000000 : "mx66u51235f"
> > -------
> > 
> > I think I should update CONFIG_MTDIDS_DEFAULT and
> > CONFIG_MTDPARTS_DEFAULT. But before that, I would like to confirm
> > that this is an intended change, and what should I do. (replace nor1
> > with mx66u51235f ?)
> 
> Hi Masami,
> 
> no. The intended solution here for you is to remove MTDIDS / MTDPARTS
> completely and instead define the partitions in your device tree:
>   https://source.denx.de/u-boot/u-boot/-/blob/master/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
> 
> You should add something like this into the spi-flash@0 node:
> 
>   partitions {
>     compatible = "fixed-partitions";
>     #address-cells = <1>;
>     #size-cells = <1>;
> 
>     partition@0 {
>       label = "BootStrap-BL1";
>       reg = <0x0 0x70000>;
>     };
> 
>     partition@70000 {
>       label = "Flash-Writer";
>       reg = <0x70000 0x90000>;
>     };
> 
>     partition@100000 {
>       label = "SCP-BL2";
>       reg = <0x100000 0x80000>;
>     };
> 
>     ...
>   };
> 
> I wonder though now whether we should force other boards to do this or
> whether we should fix the code to be backwards compatible with the old
> names.
> 
> Tom, Miquel, Jagan, what do you think?

I think we need for passing mtdparts/mtdids via the kernel command line
to continue to work like they used it.  And some checkpatch magic to
catch people introducing new ones?  It certainly would be best for
people to define them in dts as allowed, but there's a good size
existing base that doesn't.  Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]