From: Boris Brezillon <boris.brezillon@collabora.com>
To: Zhuohao Lee <zhuohao@chromium.org>
Cc: drinkcat@chromium.org, bbrezillon@kernel.org, richard@nod.at,
briannorris@chromium.org, marek.vasut@gmail.com,
linux-mtd@lists.infradead.org, computersforpeace@gmail.com,
dwmw2@infradead.org
Subject: Re: [PATCH v1] mtd: core: add sysfs nodes for querying the flash name and id
Date: Mon, 1 Apr 2019 11:27:48 +0200 [thread overview]
Message-ID: <20190401112748.4a3d5539@collabora.com> (raw)
In-Reply-To: <20190328045910.98250-1-zhuohao@chromium.org>
On Thu, 28 Mar 2019 12:59:10 +0800
Zhuohao Lee <zhuohao@chromium.org> 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
I'm not sure I like the idea of exposing this kind of info through
sysfs as it then makes part of the ABI. Did you consider exposing that
through debugfs?
>
> Signed-off-by: Zhuohao Lee <zhuohao@chromium.org>
> ---
> 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);
MTD also deals with things that are not flashes (SRAMs, ROM, ...). How
about partname?
> +
> +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);
I'd recommend making mtd->id a string so that each flash type can
decide of the formatting, and maybe have a prefix that tells which kind
of ID this is: "spi-nor:xxxxx", "nand:xxxx", "spi-nand:xxxx".
> +}
> +static DEVICE_ATTR(id, S_IRUGO, mtd_id_show, NULL);
id is bit vague, how about partid.
> +
> 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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
next prev parent reply other threads:[~2019-04-01 9:27 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-28 4:59 [PATCH v1] mtd: core: add sysfs nodes for querying the flash name and id Zhuohao Lee
2019-04-01 8:43 ` Zhuohao Lee
2019-04-01 9:27 ` Boris Brezillon [this message]
2019-04-02 7:39 ` Zhuohao Lee
2019-04-02 7:56 ` Boris Brezillon
2019-04-02 8:27 ` Vignesh Raghavendra
2019-04-02 11:06 ` Zhuohao Lee
2019-04-02 12:01 ` Boris Brezillon
2019-04-02 13:03 ` Zhuohao Lee
2019-04-03 8:31 ` Miquel Raynal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190401112748.4a3d5539@collabora.com \
--to=boris.brezillon@collabora.com \
--cc=bbrezillon@kernel.org \
--cc=briannorris@chromium.org \
--cc=computersforpeace@gmail.com \
--cc=drinkcat@chromium.org \
--cc=dwmw2@infradead.org \
--cc=linux-mtd@lists.infradead.org \
--cc=marek.vasut@gmail.com \
--cc=richard@nod.at \
--cc=zhuohao@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).