[PATCH u-boot-dm + u-boot-spi v3 10/11] mtd: compare also with OF path and device name in get_mtd_device_nm()

[Simon Glass]

Hi Marek,

On Thu, 25 Feb 2021 at 09:14, Marek Behún <marek.behun at nic.cz> wrote:
>
> The get_mtd_device_nm() function (code imported from Linux) simply
> iterates all registered MTD devices and compares the given name with
> all MTDs' names.
>
> With SPI_FLASH_MTD enabled U-Boot registers a SPI-NOR as a MTD device
> with name identical to the SPI flash chip name (from SPI ID table). Thus
> for a board with multiple same SPI-NORs it registers multiple MTDs, but
> all with the same name (such as "s25fl164k"). We do not want to change
> this behaviour, since such a change could break existing boot scripts,
> which can rely on a hardcoded name.
>
> In order to allow somehow to uniqely select a MTD device, change
> get_mtd_device_nm() function as such:
> - if first character of name is '/', interpret it as OF path
> - otherwise compare the name with MTDs name and MTDs device name.
>
> In the following example a board has two "s25fl164k" SPI-NORs. They both
> have name "s25fl164k", thus cannot be uniquely selected via this name.
> With this change, the user can select the second SPI-NOR either with
> "spi-nor at 1" or "/soc/spi at 10600/spi-nor at 1".
>
> Example:
>   => mtd list
>   List of MTD devices:
>   * s25fl164k
>     - device: spi-nor at 0
>     - parent: spi at 10600
>     - driver: jedec_spi_nor
>     - path: /soc/spi at 10600/spi-nor at 0
>     - type: NOR flash
>     - block size: 0x1000 bytes
>     - min I/O: 0x1 bytes
>     - 0x000000000000-0x000000800000 : "s25fl164k"
>   * s25fl164k
>     - device: spi-nor at 1
>     - parent: spi at 10600
>     - driver: jedec_spi_nor
>     - path: /soc/spi at 10600/spi-nor at 1
>     - type: NOR flash
>     - block size: 0x1000 bytes
>     - min I/O: 0x1 bytes
>     - 0x000000000000-0x000000800000 : "s25fl164k"
>
> This change adds code that depends on CONFIG_DM. Although CONFIG_DM
> is compulsory since v2020.01, there are still some boards (for example
> tqma6s_wru4_mmc_defconfig) that don't enable it. Therefore the code
> guards this parts by #ifdefs.
>
> Signed-off-by: Marek Behún <marek.behun at nic.cz>
> Cc: Jagan Teki <jagan at amarulasolutions.com>
> Cc: Priyanka Jain <priyanka.jain at nxp.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Heiko Schocher <hs at denx.de>
> Cc: Jagan Teki <jagan at amarulasolutions.com>
> Cc: Patrick Delaunay <patrick.delaunay at st.com>
> Cc: Patrice CHOTARD <patrice.chotard at foss.st.com>
> Cc: Miquel Raynal <miquel.raynal at bootlin.com>
> ---
>  drivers/mtd/mtdcore.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 0d1f94c6cb..7c894b2e41 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -780,13 +780,42 @@ struct mtd_info *get_mtd_device_nm(const char *name)
>  {
>         int err = -ENODEV;
>         struct mtd_info *mtd = NULL, *other;
> +#ifdef CONFIG_DM
> +       struct udevice *dev = NULL;
> +#endif
> +
> +#ifdef CONFIG_DM

We should not need CONFIG_DM here...it should be enabled for all
boards. You can always disable MTD for a board if not, or send a
removable patch.

If for some reason you do, please use if (IS_ENABLED() so that 'dev'
can always be declared.

> +       /*
> +        * If the first character of mtd name is '/', interpret it as OF path.
> +        * Otherwise try comparing by mtd->name and mtd->dev->name.
> +        */
> +       if (*name == '/') {
> +               err = device_get_global_by_ofnode(ofnode_path(name), &dev);
> +               if (err)
> +                       return ERR_PTR(err);
> +       }
> +#endif
>
>         mutex_lock(&mtd_table_mutex);
>
>         mtd_for_each_device(other) {
> +#ifdef CONFIG_DM
> +               if ((dev && !mtd_is_partition(other) && other->dev == dev) ||
> +                   !strcmp(name, other->name) ||
> +                   (!mtd_is_partition(other) && other->dev &&
> +                    !strcmp(name, other->dev->name))) {
> +#else
>                 if (!strcmp(name, other->name)) {
> +#endif
> +#ifdef __UBOOT__
> +                       if (mtd)
> +                               printf("\nWarning: MTD name \"%s\" is not unique!\n\n",
> +                                      name);
> +                       mtd = other;
> +#else /* !__UBOOT__ */
>                         mtd = other;
>                         break;
> +#endif /* !__UBOOT__ */
>                 }
>         }
>
> --
> 2.26.2
>

Regards,
Simon