Re: [PATCH] mtd: fsl-quadspi: Distinguish the mtd device names

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Mon, 8 Jan 2018 17:13:17 +0000
Han Xu <han.xu@nxp.com> wrote:

> ________________________________________
> From: Fabio Estevam <festevam@gmail.com>
> Sent: Friday, January 5, 2018 1:46 PM
> To: Han Xu
> Cc: cyrille.pitchen@wedev4u.fr; David Wolfe; Frank Li; linux-mtd@lists.infradead.org; Fabio Estevam
> Subject: [PATCH] mtd: fsl-quadspi: Distinguish the mtd device names
> 
> From: Fabio Estevam <fabio.estevam@nxp.com>
> 
> Currently on a imx6sx-sdb board, which has two SPI NOR chips connected
> to QSPI2 the following output from /proc/mtd is seen:
> 
> # cat /proc/mtd
> dev:    size   erasesize  name
> mtd0: 01000000 00010000 "21e4000.qspi"
> mtd1: 01000000 00010000 "21e4000.qspi"
> 
> Attempts to partition them on the kernel command line result in both
> chips with identical (and identically named) partitions, which is
> an inconvenient behavior.
> 
> Assign a different mtd->name for each mtd device to avoid this problem.
> 
> After this change the output from /proc/mtd becomes:
> 
> # cat /proc/mtd
> dev:    size   erasesize  name
> mtd0: 01000000 00010000 "fsl-quadspi.0"
> mtd1: 01000000 00010000 "fsl-quadspi.1"

What happens if you end up having 2 qspi IPs in the same SoC? IMHO,
it's more future proof to have something like
"<reg-addr>.qspi:<chipidx>" or "<reg-addr>.qspi:<chipidx>".

> 
> Reported-by: David Wolfe <david.wolfe@nxp.com>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index f17d224..7712c63 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -967,7 +967,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>         struct resource *res;
>         struct spi_nor *nor;
>         struct mtd_info *mtd;
> -       int ret, i = 0;
> +       int ret, i = 0, mtd_index = 0;

It's not the MTD index if you have other MTD devices. How about
spiflash_idx?

> 
>         q = devm_kzalloc(dev, sizeof(*q), GFP_KERNEL);
>         if (!q)
> @@ -1047,6 +1047,13 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>                 nor = &q->nor[i];
>                 mtd = &nor->mtd;
> 
> +               mtd->name = devm_kasprintf(dev, GFP_KERNEL, "fsl-quadspi.%d",
> +                                          mtd_index);

With the alternative naming scheme suggested above, it gives:

		mtd->name = devm_kasprintf(dev, GFP_KERNEL, "%s:%d"
					   dev_name(&pdev->dev),
					   mtd_index);

> +               if (!mtd->name) {
> +                       ret = -ENOMEM;
> +                       goto mutex_failed;
> +               }
> +
>                 nor->dev = dev;
>                 spi_nor_set_flash_node(nor, np);
>                 nor->priv = q;
> @@ -1098,6 +1105,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>                         nor->page_size = q->devtype_data->txfifo;
> 
>                 i++;
> +               mtd_index++;
>         }
> 
>         /* finish the rest init. */
> --
> 2.7.4
> 
> Acked-by: Han Xu <han.xu@nxp.com>
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/