Re: [PATCH v2 02/35] mtd: spi-nor: core: Report correct name in case of ID collisions

Pratyush Yadav
2021-08-04T08:25:08Z

On 27/07/21 07:51AM, Tudor Ambarus wrote:
> Provide a way to report the correct flash name in case of ID collisions.
> There will be a single flash_info entry when flash IDs collide, and the
> differentiation between the flash types will be made at runtime
> if possible.

I have the same comments as last time around. I am not convinced that
this approach is better than having multiple entries, one for each
colliding flash. I wonder how you will handle different fixups for the
colliding flashes for example, since nor->info is const.

Maybe I will change my mind once I read through the rest of the series,
in which case I will report back here.

>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> drivers/mtd/spi-nor/core.c | 7 +++++--
> drivers/mtd/spi-nor/sysfs.c | 2 +-
> include/linux/mtd/spi-nor.h | 2 ++
> 3 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 3d9f3698fb7b..1a278d33b02d 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3027,7 +3027,7 @@ static void spi_nor_debugfs_init(struct spi_nor *nor,
> {
> struct mtd_info *mtd = &nor->mtd;
>
> - mtd->dbg.partname = info->name;
> + mtd->dbg.partname = nor->name;
> mtd->dbg.partid = devm_kasprintf(nor->dev, GFP_KERNEL, "spi-nor:%*phN",
> info->id_len, info->id);
> }
> @@ -3208,7 +3208,10 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> /* Configure OTP parameters and ops */
> spi_nor_otp_init(nor);
>
> - dev_info(dev, "%s (%lld Kbytes)\n", info->name,
> + if (!nor->name)
> + nor->name = info->name;
> +
> + dev_info(dev, "%s (%lld Kbytes)\n", nor->name,
> (long long)mtd->size >> 10);
>
> dev_dbg(dev,
> diff --git a/drivers/mtd/spi-nor/sysfs.c b/drivers/mtd/spi-nor/sysfs.c
> index 9aec9d8a98ad..017119768f32 100644
> --- a/drivers/mtd/spi-nor/sysfs.c
> +++ b/drivers/mtd/spi-nor/sysfs.c
> @@ -25,7 +25,7 @@ static ssize_t partname_show(struct device *dev,
> struct spi_mem *spimem = spi_get_drvdata(spi);
> struct spi_nor *nor = spi_mem_get_drvdata(spimem);
>
> - return sysfs_emit(buf, "%s\n", nor->info->name);
> + return sysfs_emit(buf, "%s\n", nor->name);
> }
> static DEVICE_ATTR_RO(partname);
>
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index f67457748ed8..bd92acdd1899 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -369,6 +369,7 @@ struct spi_nor_flash_parameter;
> * @bouncebuf: bounce buffer used when the buffer passed by the MTD
> * layer is not DMA-able
> * @bouncebuf_size: size of the bounce buffer
> + * @name: used to point to correct name in case of ID collisions.
> * @info: SPI NOR part JEDEC MFR ID and other info
> * @manufacturer: SPI NOR manufacturer
> * @page_size: the page size of the SPI NOR
> @@ -399,6 +400,7 @@ struct spi_nor {
> struct spi_mem *spimem;
> u8 *bouncebuf;
> size_t bouncebuf_size;
> + const char *name;
> const struct flash_info *info;
> const struct spi_nor_manufacturer *manufacturer;
> u32 page_size;
> --
> 2.25.1
>

--
Regards,
Pratyush Yadav
Texas Instruments Inc.