u-boot and mtdparts

u-boot and mtdparts

[Marek Behún]

Hello Tom and others,

many boards still use the obsolete mtdparts command.
What is the plan with this command? Do we still want to support it?

Recently as discovered by Masami, my patch
  mtd: spi-nor: allow  registering multiple MTDs when DM is enabled
caused that mtds may have different name than that defined in
CONFIG_MTDPARTS_DEFAULT.
Masami fixed his board by putting the partition definitions into
devicetree, but there are still many boards that may be broken.

The easy fix here is to make mtd_device_matches_name() in
drivers/mtd/mtdcore.c to support the old names. Most of them are in
format "type.id", for example "nor.1", "nand.1", but some are different.

The problem is that I do not know how exactly to match these names with
the devices. Does the number represent the bus, or chip select or
something else?

There are also multiple parsers of the mtdparts string (at least two,
one in cmd/mtdparts.c and the other in drivers/mtd/mtd_uboot.c).

IMO the proper solution would be to get rid of the mtdparts entirely,
move it to devicetree, and add code that generates the mtdparts string
for Linux' command line from devicetree.

What do you think?

Marek

Re: u-boot and mtdparts

[Simon Glass]

Hi Marek,

On Thu, 15 Jul 2021 at 12:13, Marek Behún <marek.behun@nic.cz> wrote:
>
> Hello Tom and others,
>
> many boards still use the obsolete mtdparts command.
> What is the plan with this command? Do we still want to support it?
>
> Recently as discovered by Masami, my patch
>   mtd: spi-nor: allow  registering multiple MTDs when DM is enabled
> caused that mtds may have different name than that defined in
> CONFIG_MTDPARTS_DEFAULT.
> Masami fixed his board by putting the partition definitions into
> devicetree, but there are still many boards that may be broken.
>
> The easy fix here is to make mtd_device_matches_name() in
> drivers/mtd/mtdcore.c to support the old names. Most of them are in
> format "type.id", for example "nor.1", "nand.1", but some are different.
>
> The problem is that I do not know how exactly to match these names with
> the devices. Does the number represent the bus, or chip select or
> something else?
>
> There are also multiple parsers of the mtdparts string (at least two,
> one in cmd/mtdparts.c and the other in drivers/mtd/mtd_uboot.c).
>
> IMO the proper solution would be to get rid of the mtdparts entirely,
> move it to devicetree, and add code that generates the mtdparts string
> for Linux' command line from devicetree.
>
> What do you think?

SGTM. Let's add some tests too.

Regards,
Simon

Re: u-boot and mtdparts

[Tom Rini]

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

On Thu, Jul 15, 2021 at 08:13:55PM +0200, Marek Behún wrote:

> Hello Tom and others,
> 
> many boards still use the obsolete mtdparts command.
> What is the plan with this command? Do we still want to support it?
> 
> Recently as discovered by Masami, my patch
>   mtd: spi-nor: allow  registering multiple MTDs when DM is enabled
> caused that mtds may have different name than that defined in
> CONFIG_MTDPARTS_DEFAULT.
> Masami fixed his board by putting the partition definitions into
> devicetree, but there are still many boards that may be broken.
> 
> The easy fix here is to make mtd_device_matches_name() in
> drivers/mtd/mtdcore.c to support the old names. Most of them are in
> format "type.id", for example "nor.1", "nand.1", but some are different.
> 
> The problem is that I do not know how exactly to match these names with
> the devices. Does the number represent the bus, or chip select or
> something else?
> 
> There are also multiple parsers of the mtdparts string (at least two,
> one in cmd/mtdparts.c and the other in drivers/mtd/mtd_uboot.c).
> 
> IMO the proper solution would be to get rid of the mtdparts entirely,
> move it to devicetree, and add code that generates the mtdparts string
> for Linux' command line from devicetree.
> 
> What do you think?

I think there's a few parts to this.  Checking the help for
CMD_MTDPARTS, it's been saying it's deprecated since:
commit 938db6fe5da3581ed2c17d64c7e016a7a8df5237
Author: Miquel Raynal <miquel.raynal@bootlin.com>
Date:   Sat Sep 29 12:58:30 2018 +0200

    cmd: mtdparts: describe as legacy

So yes, lets kill off cmd/mtdparts.c.  Linux has taken the partition
layout from the device tree itself, one way or another, for seemingly
forever.  I don't think we need to have logic to construct the cmdline
option for the kernel at this point.  If platforms need it still, for
legacy kernels, they can still of course construct and pass it in
manually.

Now, what to do about parsing of CONFIG_MTDPARTS_DEFAULT /
CONFIG_MTDIDS_DEFAULT?  Well, I think at this point we what we care
about in terms of "U-Boot wants to know how MTD is partitioned"
everything should come in from the device tree.  Or, we can parse
CONFIG_MTD{PARTS,IDS}_DEFAULT and assume it describes what we need
perhaps?

-- 
Tom

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