[PATCH 1/3] mtd: spi-nor: Export SFDP access to sysfs

[PATCH 1/3] mtd: spi-nor: Export SFDP access to sysfs

[Alexander Williams]

Add the ability to specify a technology-specific attribute_group, and
expose the SFDP through spi-nor's private group. This enables user space
to have full access to the contents of the SFDP, so all information is
available without regard for whether the kernel supports parsing it.

Signed-off-by: Alexander Williams <awill@google.com>
---
 drivers/mtd/mtdcore.c      | 17 ++++++++++++++++-
 drivers/mtd/spi-nor/core.c | 39 ++++++++++++++++++++++++++++++++++++++
 drivers/mtd/spi-nor/core.h |  2 ++
 drivers/mtd/spi-nor/sfdp.c | 10 ++++++++--
 drivers/mtd/spi-nor/sfdp.h |  2 ++
 include/linux/mtd/mtd.h    |  1 +
 6 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 9aaeadd53eb4..65dc956e7d47 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -667,10 +667,20 @@ int add_mtd_device(struct mtd_info *mtd)
 	dev_set_name(&mtd->dev, "mtd%d", i);
 	dev_set_drvdata(&mtd->dev, mtd);
 	of_node_get(mtd_get_of_node(mtd));
-	error = device_register(&mtd->dev);
+	device_initialize(&mtd->dev);
+	dev_set_uevent_suppress(&mtd->dev, 1);
+	error = device_add(&mtd->dev);
 	if (error)
 		goto fail_added;
 
+	if (master->priv_attr_group) {
+		error = devm_device_add_group(&mtd->dev, master->priv_attr_group);
+		WARN_ON(error);
+		error = 0;
+	}
+	dev_set_uevent_suppress(&mtd->dev, 0);
+	kobject_uevent(&mtd->dev.kobj, KOBJ_ADD);
+
 	/* Add the nvmem provider */
 	error = mtd_nvmem_add(mtd);
 	if (error)
@@ -719,6 +729,7 @@ int del_mtd_device(struct mtd_info *mtd)
 {
 	int ret;
 	struct mtd_notifier *not;
+	struct mtd_info *master = mtd_get_master(mtd);
 
 	mutex_lock(&mtd_table_mutex);
 
@@ -743,6 +754,10 @@ int del_mtd_device(struct mtd_info *mtd)
 		if (mtd->nvmem)
 			nvmem_unregister(mtd->nvmem);
 
+		if (master->priv_attr_group) {
+			devm_device_remove_group(&mtd->dev,
+						 master->priv_attr_group);
+		}
 		device_unregister(&mtd->dev);
 
 		idr_remove(&mtd_idr, mtd->index);
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index bd2c7717eb10..1eec1ce44882 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3058,6 +3058,44 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
 	return info;
 }
 
+static ssize_t sfdp_read(struct file *file, struct kobject *kobj,
+			 struct bin_attribute *attr, char *buf, loff_t offset,
+			 size_t len)
+{
+	int ret;
+	struct spi_nor *nor;
+	struct device *dev = kobj_to_dev(kobj);
+	struct mtd_info *mtd = container_of(dev, struct mtd_info, dev);
+
+	mtd = mtd_get_master(mtd);
+	nor = mtd_to_spi_nor(mtd);
+
+	if (nor->params->sfdp_size == 0)
+		return 0;
+
+	if (nor->info->flags & SPI_NOR_SKIP_SFDP)
+		return -EOPNOTSUPP;
+
+	len = min_t(size_t, len, nor->params->sfdp_size - offset);
+	ret = spi_nor_read_sfdp_dma_unsafe(nor, offset, len, buf);
+
+	if (ret)
+		return ret;
+	return len;
+}
+
+static BIN_ATTR_RO(sfdp, 0);
+
+static struct bin_attribute *spi_nor_bin_attrs[] = {
+	&bin_attr_sfdp,
+	NULL,
+};
+
+static struct attribute_group spi_nor_group = {
+	.name		= "spi-nor",
+	.bin_attrs	= spi_nor_bin_attrs,
+};
+
 int spi_nor_scan(struct spi_nor *nor, const char *name,
 		 const struct spi_nor_hwcaps *hwcaps)
 {
@@ -3122,6 +3160,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	if (!mtd->name)
 		mtd->name = dev_name(dev);
 	mtd->priv = nor;
+	mtd->priv_attr_group = &spi_nor_group;
 	mtd->type = MTD_NORFLASH;
 	mtd->writesize = nor->params->writesize;
 	mtd->flags = MTD_CAP_NORFLASH;
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 28a2e0be97a3..18e27725fafb 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -240,6 +240,7 @@ struct spi_nor_otp {
  * @rdsr_dummy:		dummy cycles needed for Read Status Register command.
  * @rdsr_addr_nbytes:	dummy address bytes needed for Read Status Register
  *			command.
+ * @sfdp_size		discovered size of the SFDP.
  * @hwcaps:		describes the read and page program hardware
  *			capabilities.
  * @reads:		read capabilities ordered by priority: the higher index
@@ -268,6 +269,7 @@ struct spi_nor_flash_parameter {
 	u32				page_size;
 	u8				rdsr_dummy;
 	u8				rdsr_addr_nbytes;
+	u32				sfdp_size;
 
 	struct spi_nor_hwcaps		hwcaps;
 	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index 23c28e91f698..221c7dc1aee1 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -210,8 +210,8 @@ static int spi_nor_read_sfdp(struct spi_nor *nor, u32 addr,
  * Return: -ENOMEM if kmalloc() fails, the return code of spi_nor_read_sfdp()
  *          otherwise.
  */
-static int spi_nor_read_sfdp_dma_unsafe(struct spi_nor *nor, u32 addr,
-					size_t len, void *buf)
+int spi_nor_read_sfdp_dma_unsafe(struct spi_nor *nor, u32 addr, size_t len,
+				 void *buf)
 {
 	void *dma_safe_buf;
 	int ret;
@@ -1247,6 +1247,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 	struct device *dev = nor->dev;
 	size_t psize;
 	int i, err;
+	u32 param_limit;
 
 	/* Get the SFDP header. */
 	err = spi_nor_read_sfdp_dma_unsafe(nor, 0, sizeof(header), &header);
@@ -1266,6 +1267,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 	if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
 	    bfpt_header->major != SFDP_JESD216_MAJOR)
 		return -EINVAL;
+	nor->params->sfdp_size = sizeof(header);
 
 	/*
 	 * Allocate memory then read all parameter headers with a single
@@ -1299,6 +1301,10 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
 	 */
 	for (i = 0; i < header.nph; i++) {
 		param_header = &param_headers[i];
+		param_limit = SFDP_PARAM_HEADER_PTP(param_header)
+				+ param_header->length * 4;
+		nor->params->sfdp_size = max(param_limit,
+					     nor->params->sfdp_size);
 
 		if (SFDP_PARAM_HEADER_ID(param_header) == SFDP_BFPT_ID &&
 		    param_header->major == SFDP_JESD216_MAJOR &&
diff --git a/drivers/mtd/spi-nor/sfdp.h b/drivers/mtd/spi-nor/sfdp.h
index bbf80d2990ab..17548b320397 100644
--- a/drivers/mtd/spi-nor/sfdp.h
+++ b/drivers/mtd/spi-nor/sfdp.h
@@ -108,5 +108,7 @@ struct sfdp_parameter_header {
 };
 
 int spi_nor_parse_sfdp(struct spi_nor *nor);
+int spi_nor_read_sfdp_dma_unsafe(struct spi_nor *nor, u32 addr, size_t len,
+				 void *buf);
 
 #endif /* __LINUX_MTD_SFDP_H */
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index a89955f3cbc8..2ef1f7e2db76 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -374,6 +374,7 @@ struct mtd_info {
 	int subpage_sft;
 
 	void *priv;
+	struct attribute_group *priv_attr_group;
 
 	struct module *owner;
 	struct device dev;
-- 
2.31.1.527.g47e6f16901-goog

Re: [PATCH 1/3] mtd: spi-nor: Export SFDP access to sysfs

[Michael Walle]

Hi Alexander,

Am 2021-04-28 07:27, schrieb Alexander Williams:
> Add the ability to specify a technology-specific attribute_group, and
> expose the SFDP through spi-nor's private group. This enables user 
> space
> to have full access to the contents of the SFDP, so all information is
> available without regard for whether the kernel supports parsing it.

Please see:
https://lore.kernel.org/linux-mtd/20210318092406.5340-1-michael@walle.cc/-

-michael

Re: [PATCH 1/3] mtd: spi-nor: Export SFDP access to sysfs

[Alexander Williams]

On Wed, Apr 28, 2021 at 12:58 AM Michael Walle <michael@walle.cc> wrote:
>
> Hi Alexander,
>
> Am 2021-04-28 07:27, schrieb Alexander Williams:
> > Add the ability to specify a technology-specific attribute_group, and
> > expose the SFDP through spi-nor's private group. This enables user
> > space
> > to have full access to the contents of the SFDP, so all information is
> > available without regard for whether the kernel supports parsing it.
>
> Please see:
> https://lore.kernel.org/linux-mtd/20210318092406.5340-1-michael@walle.cc/

Thanks for the pointer! Sorry I didn't see that patch series already: My search
skills need some work... In any case, there's no good reason for me to duplicate
work.

I should probably comment on that series directly, but I'll note that one thing
I tried to do with the sysfs attributes here was avoid racing with user space.
The uevent is suppressed until all the related files are created in sysfs. See
http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/ for
Greg's explanation of a scenario.

- Alex

Re: [PATCH 1/3] mtd: spi-nor: Export SFDP access to sysfs

[Michael Walle]

Am 2021-04-28 17:37, schrieb Alexander Williams:
> On Wed, Apr 28, 2021 at 12:58 AM Michael Walle <michael@walle.cc> 
> wrote:
>> 
>> Hi Alexander,
>> 
>> Am 2021-04-28 07:27, schrieb Alexander Williams:
>> > Add the ability to specify a technology-specific attribute_group, and
>> > expose the SFDP through spi-nor's private group. This enables user
>> > space
>> > to have full access to the contents of the SFDP, so all information is
>> > available without regard for whether the kernel supports parsing it.
>> 
>> Please see:
>> https://lore.kernel.org/linux-mtd/20210318092406.5340-1-michael@walle.cc/
> 
> Thanks for the pointer! Sorry I didn't see that patch series already: 
> My search
> skills need some work... In any case, there's no good reason for me to 
> duplicate
> work.
> 
> I should probably comment on that series directly, but I'll note that 
> one thing
> I tried to do with the sysfs attributes here was avoid racing with user 
> space.
> The uevent is suppressed until all the related files are created in 
> sysfs. See
> http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/ 
> for
> Greg's explanation of a scenario.

Sure, reviews and remarks are always welcome. Btw. what is patch 2/3 and 
3/3?
I couldn't find it on the LKML.

-michael

Re: [PATCH 1/3] mtd: spi-nor: Export SFDP access to sysfs

[Alexander Williams]

On Wed, Apr 28, 2021 at 8:51 AM Michael Walle <michael@walle.cc> wrote:
>
> Am 2021-04-28 17:37, schrieb Alexander Williams:
> > On Wed, Apr 28, 2021 at 12:58 AM Michael Walle <michael@walle.cc>
> > wrote:
> >>
> >> Hi Alexander,
> >>
> >> Am 2021-04-28 07:27, schrieb Alexander Williams:
> >> > Add the ability to specify a technology-specific attribute_group, and
> >> > expose the SFDP through spi-nor's private group. This enables user
> >> > space
> >> > to have full access to the contents of the SFDP, so all information is
> >> > available without regard for whether the kernel supports parsing it.
> >>
> >> Please see:
> >> https://lore.kernel.org/linux-mtd/20210318092406.5340-1-michael@walle.cc/
> >
> > Thanks for the pointer! Sorry I didn't see that patch series already:
> > My search
> > skills need some work... In any case, there's no good reason for me to
> > duplicate
> > work.
> >
> > I should probably comment on that series directly, but I'll note that
> > one thing
> > I tried to do with the sysfs attributes here was avoid racing with user
> > space.
> > The uevent is suppressed until all the related files are created in
> > sysfs. See
> > http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
> > for
> > Greg's explanation of a scenario.
>
> Sure, reviews and remarks are always welcome. Btw. what is patch 2/3 and
> 3/3?
> I couldn't find it on the LKML.

There are none. I goofed and forgot to fix up the subject line. The two commits
that followed merely added quick-and-dirty SFDP support to intel-spi.c to enable
testing on one of the platforms I had lying around.

--Alex