Re: [PATCH v1] mtd: core: add sysfs nodes for querying the flash name and id

[Zhuohao Lee <zhuohao@xxxxxxxxxxxx>]

Dear reviewers,

May i have your professional feedback for this patch? Could we land
this patch? Do you have any concern?
Please let me know what you think. Thank you.


On Thu, Mar 28, 2019 at 12:59 PM Zhuohao Lee <zhuohao@xxxxxxxxxxxx> wrote:
>
> Currently, we don't have sysfs nodes for querying the underlying flash
> name and flash id. This information is important especially when we
> want to know the flash detail of the defective system. In order to
> support the query, we add two pointers (*flashname, *id) into the
> mtd_info structure and create two sysfs nodes (flashname, id). This
> patch is modified based on the SPI-NOR flash system as we only have
> that system now. But the idea should be applied to the other flash
> driver like NAND flash.
>
> The output of new sysfs nodes on my device are:
> cat /sys/devices/platform/soc/11010000.spi/spi_master/spi1/spi1.0/mtd/mtd0/flashname
> w25q64dw
> cat /sys/devices/platform/soc/11010000.spi/spi_master/spi1/spi1.0/mtd/mtd0/id
> ef6017
>
> Signed-off-by: Zhuohao Lee <zhuohao@xxxxxxxxxxxx>
> ---
>  drivers/mtd/mtdcore.c         | 24 ++++++++++++++++++++++++
>  drivers/mtd/spi-nor/spi-nor.c |  3 +++
>  include/linux/mtd/mtd.h       |  3 +++
>  3 files changed, 30 insertions(+)
>
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 3ef01baef9b6..dcbe6719ad67 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -241,6 +241,28 @@ static ssize_t mtd_name_show(struct device *dev,
>  }
>  static DEVICE_ATTR(name, S_IRUGO, mtd_name_show, NULL);
>
> +static ssize_t mtd_flashname_show(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       struct mtd_info *mtd = dev_get_drvdata(dev);
> +
> +       if (!mtd->flashname)
> +               return 0;
> +       return snprintf(buf, PAGE_SIZE, "%s\n", mtd->flashname);
> +}
> +static DEVICE_ATTR(flashname, S_IRUGO, mtd_flashname_show, NULL);
> +
> +static ssize_t mtd_id_show(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +       struct mtd_info *mtd = dev_get_drvdata(dev);
> +
> +       if (!mtd->id)
> +               return 0;
> +       return snprintf(buf, PAGE_SIZE, "%*phN\n", mtd->id_size, mtd->id);
> +}
> +static DEVICE_ATTR(id, S_IRUGO, mtd_id_show, NULL);
> +
>  static ssize_t mtd_ecc_strength_show(struct device *dev,
>                                      struct device_attribute *attr, char *buf)
>  {
> @@ -340,6 +362,8 @@ static struct attribute *mtd_attrs[] = {
>         &dev_attr_oobavail.attr,
>         &dev_attr_numeraseregions.attr,
>         &dev_attr_name.attr,
> +       &dev_attr_flashname.attr,
> +       &dev_attr_id.attr,
>         &dev_attr_ecc_strength.attr,
>         &dev_attr_ecc_step_size.attr,
>         &dev_attr_corrected_bits.attr,
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 6e13bbd1aaa5..0e10858e532c 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -4027,6 +4027,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>
>         if (!mtd->name)
>                 mtd->name = dev_name(dev);
> +       mtd->flashname = info->name;
> +       mtd->id = info->id;
> +       mtd->id_size = info->id_len;
>         mtd->priv = nor;
>         mtd->type = MTD_NORFLASH;
>         mtd->writesize = 1;
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 677768b21a1d..0a81569fa4f6 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -210,6 +210,9 @@ struct mtd_info {
>         uint32_t flags;
>         uint32_t orig_flags; /* Flags as before running mtd checks */
>         uint64_t size;   // Total size of the MTD
> +       const char *flashname; /* The underlying flash name */
> +       const char *id; /* The ID of the flash */
> +       int id_size; /* Number of bytes of id array */
>
>         /* "Major" erase size for the device. Naïve users may take this
>          * to be the only erase size available, or may use the more detailed
> --
> 2.21.0.392.gf8f6787159e-goog
>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/