[RFC PATCH v2 0/8] nvmem: add block device NVMEM provider

[RFC PATCH v2 0/8] nvmem: add block device NVMEM provider

[Daniel Golle]

On embedded devices using an eMMC it is common that one or more (hw/sw)
partitions on the eMMC are used to store MAC addresses and Wi-Fi
calibration EEPROM data.

Implement an NVMEM provider backed by block devices as typically the
NVMEM framework is used to have kernel drivers read and use binary data
from EEPROMs, efuses, flash memory (MTD), ...

In order to be able to reference hardware partitions on an eMMC, add code
to bind each hardware partition to a specific firmware subnode.

This series is meant to open the discussion on how exactly the device
tree schema for block devices and partitions may look like, and even
if using the block layer to back the NVMEM device is at all the way to
go -- to me it seemed to be a good solution because it will be reuable
e.g. for (normal, software GPT or MBR) partitions of an NVMe SSD.

This series has previously been submitted on July 19th 2023[1] and most of
the basic idea did not change since.

However, the recent introduction of bdev_file_open_by_dev() allow to
get rid of most use of block layer internals which supposedly was the
main objection raised by Christoph Hellwig back then.

Most of the other comments received for in the first RFC have also
been addressed, however, what remains is the use of class_interface
(lacking an alternative way to get notifications about addition or
removal of block devices from the system). As this has been criticized
in the past I'm specifically interested in suggestions on how to solve
this in another way -- ideally without having to implement a whole new
way for in-kernel notifications of appearing or disappearing block
devices...

And, in a way just like in case of MTD and UBI, I believe acting as an
NVMEM provider *is* a functionality which belongs to the block layer
itself and, other than e.g. filesystems, is inconvenient to implement
elsewhere.

[1]: https://patchwork.kernel.org/project/linux-block/list/?series=767565

Daniel Golle (8):
  dt-bindings: block: add basic bindings for block devices
  block: partitions: populate fwnode
  block: add new genhd flag GENHD_FL_NVMEM
  block: implement NVMEM provider
  dt-bindings: mmc: mmc-card: add block device nodes
  mmc: core: set card fwnode_handle
  mmc: block: set fwnode of disk devices
  mmc: block: set GENHD_FL_NVMEM

 .../bindings/block/block-device.yaml          |  22 +++
 .../devicetree/bindings/block/partition.yaml  |  51 +++++
 .../devicetree/bindings/block/partitions.yaml |  20 ++
 .../devicetree/bindings/mmc/mmc-card.yaml     |  45 +++++
 block/Kconfig                                 |   9 +
 block/Makefile                                |   1 +
 block/blk-nvmem.c                             | 175 ++++++++++++++++++
 block/partitions/core.c                       |  41 ++++
 drivers/mmc/core/block.c                      |   8 +
 drivers/mmc/core/bus.c                        |   2 +
 include/linux/blkdev.h                        |   2 +
 11 files changed, 376 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/block/block-device.yaml
 create mode 100644 Documentation/devicetree/bindings/block/partition.yaml
 create mode 100644 Documentation/devicetree/bindings/block/partitions.yaml
 create mode 100644 block/blk-nvmem.c

-- 
2.44.0

[RFC PATCH v2 1/8] dt-bindings: block: add basic bindings for block devices

[Daniel Golle]

Add bindings for block devices which are used to allow referencing
nvmem bits on them.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 .../bindings/block/block-device.yaml          | 22 ++++++++
 .../devicetree/bindings/block/partition.yaml  | 51 +++++++++++++++++++
 .../devicetree/bindings/block/partitions.yaml | 20 ++++++++
 3 files changed, 93 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/block/block-device.yaml
 create mode 100644 Documentation/devicetree/bindings/block/partition.yaml
 create mode 100644 Documentation/devicetree/bindings/block/partitions.yaml

diff --git a/Documentation/devicetree/bindings/block/block-device.yaml b/Documentation/devicetree/bindings/block/block-device.yaml
new file mode 100644
index 0000000000000..c83ea525650ba
--- /dev/null
+++ b/Documentation/devicetree/bindings/block/block-device.yaml
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/block/block-device.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: block storage device
+
+description: |
+  This binding is generic and describes a block-oriented storage device.
+
+maintainers:
+  - Daniel Golle <daniel@makrotopia.org>
+
+properties:
+  partitions:
+    $ref: /schemas/block/partitions.yaml
+
+  nvmem-layout:
+    $ref: /schemas/nvmem/layouts/nvmem-layout.yaml#
+
+unevaluatedProperties: false
diff --git a/Documentation/devicetree/bindings/block/partition.yaml b/Documentation/devicetree/bindings/block/partition.yaml
new file mode 100644
index 0000000000000..df561dd33cbc9
--- /dev/null
+++ b/Documentation/devicetree/bindings/block/partition.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/block/partition.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Partition on a block device
+
+description: |
+  This binding describes a partition on a block storage device.
+  Partitions may be matched by a combination of partition number, name,
+  and UUID.
+
+maintainers:
+  - Daniel Golle <daniel@makrotopia.org>
+
+properties:
+  $nodename:
+    pattern: '^block-partition-.+$'
+
+  partnum:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Matches partition by number if present.
+
+  partname:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Matches partition by PARTNAME if present.
+
+  uuid:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Matches partition by PARTUUID if present.
+
+  nvmem-layout:
+    $ref: /schemas/nvmem/layouts/nvmem-layout.yaml#
+    description:
+      This container may reference an NVMEM layout parser.
+
+anyOf:
+  - required:
+      - partnum
+
+  - required:
+      - partname
+
+  - required:
+      - uuid
+
+unevaluatedProperties: false
diff --git a/Documentation/devicetree/bindings/block/partitions.yaml b/Documentation/devicetree/bindings/block/partitions.yaml
new file mode 100644
index 0000000000000..fd84c3ba8493b
--- /dev/null
+++ b/Documentation/devicetree/bindings/block/partitions.yaml
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/block/partitions.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Partitions on block devices
+
+description: |
+  This binding is generic and describes the content of the partitions container
+  node.
+
+maintainers:
+  - Daniel Golle <daniel@makrotopia.org>
+
+patternProperties:
+  "^block-partition-.+$":
+    $ref: partition.yaml
+
+unevaluatedProperties: false
-- 
2.44.0

[RFC PATCH v2 2/8] block: partitions: populate fwnode

[Daniel Golle]

Let block partitions to be represented by a firmware node and hence
allow them to being referenced e.g. for use with blk-nvmem.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 block/partitions/core.c | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/block/partitions/core.c b/block/partitions/core.c
index 5f5ed5c75f04d..48d7f01d76c6a 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -10,6 +10,8 @@
 #include <linux/ctype.h>
 #include <linux/vmalloc.h>
 #include <linux/raid/detect.h>
+#include <linux/property.h>
+
 #include "check.h"
 
 static int (*const check_part[])(struct parsed_partitions *) = {
@@ -281,6 +283,43 @@ static ssize_t whole_disk_show(struct device *dev,
 }
 static const DEVICE_ATTR(whole_disk, 0444, whole_disk_show, NULL);
 
+static struct fwnode_handle *find_partition_fwnode(struct block_device *bdev)
+{
+	struct fwnode_handle *fw_parts, *fw_part;
+	struct device *ddev = disk_to_dev(bdev->bd_disk);
+	const char *partname, *uuid;
+	u32 partno;
+
+	fw_parts = device_get_named_child_node(ddev, "partitions");
+	if (!fw_parts)
+		fw_parts = device_get_named_child_node(ddev->parent, "partitions");
+
+	if (!fw_parts)
+		return NULL;
+
+	fwnode_for_each_child_node(fw_parts, fw_part) {
+		if (!fwnode_property_read_string(fw_part, "uuid", &uuid) &&
+		    (!bdev->bd_meta_info || strncmp(uuid,
+						    bdev->bd_meta_info->uuid,
+						    PARTITION_META_INFO_UUIDLTH)))
+			continue;
+
+		if (!fwnode_property_read_string(fw_part, "partname", &partname) &&
+		    (!bdev->bd_meta_info || strncmp(partname,
+						    bdev->bd_meta_info->volname,
+						    PARTITION_META_INFO_VOLNAMELTH)))
+			continue;
+
+		if (!fwnode_property_read_u32(fw_part, "partno", &partno) &&
+		    bdev->bd_partno != partno)
+			continue;
+
+		return fw_part;
+	}
+
+	return NULL;
+}
+
 /*
  * Must be called either with open_mutex held, before a disk can be opened or
  * after all disk users are gone.
@@ -355,6 +394,8 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
 			goto out_put;
 	}
 
+	device_set_node(pdev, find_partition_fwnode(bdev));
+
 	/* delay uevent until 'holders' subdir is created */
 	dev_set_uevent_suppress(pdev, 1);
 	err = device_add(pdev);
-- 
2.44.0

[RFC PATCH v2 3/8] block: add new genhd flag GENHD_FL_NVMEM

[Daniel Golle]

Add new flag to destinguish block devices which may act as an NVMEM
provider.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 include/linux/blkdev.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 715c9201008a6..03e4b1fd5060d 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -81,11 +81,13 @@ struct partition_meta_info {
  * ``GENHD_FL_NO_PART``: partition support is disabled.  The kernel will not
  * scan for partitions from add_disk, and users can't add partitions manually.
  *
+ * ``GENHD_FL_NVMEM``: the block device should be considered as NVMEM provider.
  */
 enum {
 	GENHD_FL_REMOVABLE			= 1 << 0,
 	GENHD_FL_HIDDEN				= 1 << 1,
 	GENHD_FL_NO_PART			= 1 << 2,
+	GENHD_FL_NVMEM				= 1 << 3,
 };
 
 enum {
-- 
2.44.0

[RFC PATCH v2 4/8] block: implement NVMEM provider

[Daniel Golle]

On embedded devices using an eMMC it is common that one or more partitions
on the eMMC are used to store MAC addresses and Wi-Fi calibration EEPROM
data. Allow referencing the partition in device tree for the kernel and
Wi-Fi drivers accessing it via the NVMEM layer.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 block/Kconfig     |   9 +++
 block/Makefile    |   1 +
 block/blk-nvmem.c | 175 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 185 insertions(+)
 create mode 100644 block/blk-nvmem.c

diff --git a/block/Kconfig b/block/Kconfig
index 1de4682d48ccb..b1d4c88c70040 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -229,6 +229,15 @@ config BLK_INLINE_ENCRYPTION_FALLBACK
 	  by falling back to the kernel crypto API when inline
 	  encryption hardware is not present.
 
+config BLK_NVMEM
+	bool "Block device NVMEM provider"
+	depends on OF
+	depends on NVMEM
+	help
+	  Allow block devices (or partitions) to act as NVMEM prodivers,
+	  typically used with eMMC to store MAC addresses or Wi-Fi
+	  calibration data on embedded devices.
+
 source "block/partitions/Kconfig"
 
 config BLK_MQ_PCI
diff --git a/block/Makefile b/block/Makefile
index 46ada9dc8bbfe..03c0bfa8642df 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_BLK_DEV_ZONED)	+= blk-zoned.o
 obj-$(CONFIG_BLK_WBT)		+= blk-wbt.o
 obj-$(CONFIG_BLK_DEBUG_FS)	+= blk-mq-debugfs.o
 obj-$(CONFIG_BLK_DEBUG_FS_ZONED)+= blk-mq-debugfs-zoned.o
+obj-$(CONFIG_BLK_NVMEM)		+= blk-nvmem.o
 obj-$(CONFIG_BLK_SED_OPAL)	+= sed-opal.o
 obj-$(CONFIG_BLK_PM)		+= blk-pm.o
 obj-$(CONFIG_BLK_INLINE_ENCRYPTION)	+= blk-crypto.o blk-crypto-profile.o \
diff --git a/block/blk-nvmem.c b/block/blk-nvmem.c
new file mode 100644
index 0000000000000..6deb2f251d813
--- /dev/null
+++ b/block/blk-nvmem.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * block device NVMEM provider
+ *
+ * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
+ *
+ * Useful on devices using a partition on an eMMC for MAC addresses or
+ * Wi-Fi calibration EEPROM data.
+ */
+
+#include "blk.h"
+#include <linux/nvmem-provider.h>
+#include <linux/of.h>
+#include <linux/pagemap.h>
+#include <linux/property.h>
+
+/* List of all NVMEM devices */
+static LIST_HEAD(nvmem_devices);
+static DEFINE_MUTEX(devices_mutex);
+
+struct blk_nvmem {
+	struct nvmem_device	*nvmem;
+	struct device		*dev;
+	struct list_head	list;
+};
+
+static int blk_nvmem_reg_read(void *priv, unsigned int from,
+			      void *val, size_t bytes)
+{
+	blk_mode_t mode = BLK_OPEN_READ | BLK_OPEN_RESTRICT_WRITES;
+	unsigned long offs = from & ~PAGE_MASK, to_read;
+	pgoff_t f_index = from >> PAGE_SHIFT;
+	struct blk_nvmem *bnv = priv;
+	size_t bytes_left = bytes;
+	struct file *bdev_file;
+	struct folio *folio;
+	void *p;
+	int ret = 0;
+
+	bdev_file = bdev_file_open_by_dev(bnv->dev->devt, mode, priv, NULL);
+	if (!bdev_file)
+		return -ENODEV;
+
+	if (IS_ERR(bdev_file))
+		return PTR_ERR(bdev_file);
+
+	while (bytes_left) {
+		folio = read_mapping_folio(bdev_file->f_mapping, f_index++, NULL);
+		if (IS_ERR(folio)) {
+			ret = PTR_ERR(folio);
+			goto err_release_bdev;
+		}
+		to_read = min_t(unsigned long, bytes_left, PAGE_SIZE - offs);
+		p = folio_address(folio) + offset_in_folio(folio, offs);
+		memcpy(val, p, to_read);
+		offs = 0;
+		bytes_left -= to_read;
+		val += to_read;
+		folio_put(folio);
+	}
+
+err_release_bdev:
+	fput(bdev_file);
+
+	return ret;
+}
+
+static int blk_nvmem_register(struct device *dev)
+{
+	struct device_node *np = dev_of_node(dev);
+	struct block_device *bdev = dev_to_bdev(dev);
+	struct nvmem_config config = {};
+	struct blk_nvmem *bnv;
+
+	/* skip devices which do not have a device tree node */
+	if (!np)
+		return 0;
+
+	/* skip devices without an nvmem layout defined */
+	if (!of_get_child_by_name(np, "nvmem-layout"))
+		return 0;
+
+	/*
+	 * skip devices which don't have GENHD_FL_NVMEM set
+	 *
+	 * This flag is used for mtdblock and ubiblock devices because
+	 * both, MTD and UBI already implement their own NVMEM provider.
+	 * To avoid registering multiple NVMEM providers for the same
+	 * device node, don't register the block NVMEM provider for them.
+	 */
+	if (!(bdev->bd_disk->flags & GENHD_FL_NVMEM))
+		return 0;
+
+	/*
+	 * skip block device too large to be represented as NVMEM devices
+	 * which are using an 'int' as address
+	 */
+	if (bdev_nr_bytes(bdev) > INT_MAX)
+		return -EFBIG;
+
+	bnv = kzalloc(sizeof(struct blk_nvmem), GFP_KERNEL);
+	if (!bnv)
+		return -ENOMEM;
+
+	config.id = NVMEM_DEVID_NONE;
+	config.dev = &bdev->bd_device;
+	config.name = dev_name(&bdev->bd_device);
+	config.owner = THIS_MODULE;
+	config.priv = bnv;
+	config.reg_read = blk_nvmem_reg_read;
+	config.size = bdev_nr_bytes(bdev);
+	config.word_size = 1;
+	config.stride = 1;
+	config.read_only = true;
+	config.root_only = true;
+	config.ignore_wp = true;
+	config.of_node = to_of_node(dev->fwnode);
+
+	bnv->dev = &bdev->bd_device;
+	bnv->nvmem = nvmem_register(&config);
+	if (IS_ERR(bnv->nvmem)) {
+		dev_err_probe(&bdev->bd_device, PTR_ERR(bnv->nvmem),
+			      "Failed to register NVMEM device\n");
+
+		kfree(bnv);
+		return PTR_ERR(bnv->nvmem);
+	}
+
+	mutex_lock(&devices_mutex);
+	list_add_tail(&bnv->list, &nvmem_devices);
+	mutex_unlock(&devices_mutex);
+
+	return 0;
+}
+
+static void blk_nvmem_unregister(struct device *dev)
+{
+	struct blk_nvmem *bnv_c, *bnv = NULL;
+
+	mutex_lock(&devices_mutex);
+	list_for_each_entry(bnv_c, &nvmem_devices, list) {
+		if (bnv_c->dev == dev) {
+			bnv = bnv_c;
+			break;
+		}
+	}
+
+	if (!bnv) {
+		mutex_unlock(&devices_mutex);
+		return;
+	}
+
+	list_del(&bnv->list);
+	mutex_unlock(&devices_mutex);
+	nvmem_unregister(bnv->nvmem);
+	kfree(bnv);
+}
+
+static struct class_interface blk_nvmem_bus_interface __refdata = {
+	.class = &block_class,
+	.add_dev = &blk_nvmem_register,
+	.remove_dev = &blk_nvmem_unregister,
+};
+
+static int __init blk_nvmem_init(void)
+{
+	int ret;
+
+	ret = class_interface_register(&blk_nvmem_bus_interface);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+device_initcall(blk_nvmem_init);
-- 
2.44.0

[RFC PATCH v2 5/8] dt-bindings: mmc: mmc-card: add block device nodes

[Daniel Golle]

Add nodes representing the block devices exposed by an MMC device
including an example involving nvmem-cells.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 .../devicetree/bindings/mmc/mmc-card.yaml     | 45 +++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/mmc-card.yaml b/Documentation/devicetree/bindings/mmc/mmc-card.yaml
index fd347126449ac..95ccbda871d24 100644
--- a/Documentation/devicetree/bindings/mmc/mmc-card.yaml
+++ b/Documentation/devicetree/bindings/mmc/mmc-card.yaml
@@ -26,6 +26,18 @@ properties:
       Use this to indicate that the mmc-card has a broken hpi
       implementation, and that hpi should not be used.
 
+  block:
+    $ref: /schemas/block/block-device.yaml#
+    description:
+      Represents the block storage provided by an SD card or the
+      main hardware partition of an eMMC.
+
+patternProperties:
+  '^boot[0-9]+':
+    $ref: /schemas/block/block-device.yaml#
+    description:
+      Represents a boot hardware partition on an eMMC.
+
 required:
   - compatible
   - reg
@@ -42,6 +54,39 @@ examples:
             compatible = "mmc-card";
             reg = <0>;
             broken-hpi;
+
+            block {
+                partitions {
+                    cal_data: block-partition-rf {
+                        partnum = <3>;
+                        partname = "rf";
+
+                        nvmem-layout {
+                            compatible = "fixed-layout";
+                            #address-cells = <1>;
+                            #size-cells = <1>;
+
+                            eeprom@0 {
+                                reg = <0x0 0x1000>;
+                            };
+                        };
+                    };
+                };
+            };
+
+            boot1 {
+                nvmem-layout {
+                    compatible = "fixed-layout";
+                    #address-cells = <1>;
+                    #size-cells = <1>;
+
+                    macaddr: macaddr@a {
+                        compatible = "mac-base";
+                        reg = <0xa 0x6>;
+                        #nvmem-cell-cells = <1>;
+                    };
+                };
+            };
         };
     };
 
-- 
2.44.0

[RFC PATCH v2 6/8] mmc: core: set card fwnode_handle

[Daniel Golle]

Set fwnode in case it isn't set yet and of_node is present.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/mmc/core/bus.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index 0ddaee0eae54f..e1c5fc1b3ce4b 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -364,6 +364,8 @@ int mmc_add_card(struct mmc_card *card)
 
 	mmc_add_card_debugfs(card);
 	card->dev.of_node = mmc_of_find_child_device(card->host, 0);
+	if (card->dev.of_node && !card->dev.fwnode)
+		card->dev.fwnode = &card->dev.of_node->fwnode;
 
 	device_enable_async_suspend(&card->dev);
 
-- 
2.44.0

[RFC PATCH v2 7/8] mmc: block: set fwnode of disk devices

[Daniel Golle]

Set fwnode of disk devices to 'block', 'boot0' and 'boot1' subnodes of
the mmc-card. This is done in preparation for having the eMMC act as
NVMEM provider.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/mmc/core/block.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index bd165cef2bbc3..c3e9477743515 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2461,6 +2461,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 					      int area_type,
 					      unsigned int part_type)
 {
+	struct fwnode_handle *fwnode;
 	struct mmc_blk_data *md;
 	int devidx, ret;
 	char cap_str[10];
@@ -2557,6 +2558,12 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 
 	blk_queue_write_cache(md->queue.queue, cache_enabled, fua_enabled);
 
+	fwnode = device_get_named_child_node(subname ? md->parent->parent :
+						       md->parent,
+					     subname ? subname : "block");
+	if (fwnode)
+		device_set_node(disk_to_dev(md->disk), fwnode);
+
 	string_get_size((u64)size, 512, STRING_UNITS_2,
 			cap_str, sizeof(cap_str));
 	pr_info("%s: %s %s %s%s\n",
-- 
2.44.0

[RFC PATCH v2 8/8] mmc: block: set GENHD_FL_NVMEM

[Daniel Golle]

Set flag to consider MMC block devices as NVMEM providers.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/mmc/core/block.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index c3e9477743515..8cd0400c9887d 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2514,6 +2514,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
 	md->disk->major	= MMC_BLOCK_MAJOR;
 	md->disk->minors = perdev_minors;
 	md->disk->first_minor = devidx * perdev_minors;
+	md->disk->flags = GENHD_FL_NVMEM;
 	md->disk->fops = &mmc_bdops;
 	md->disk->private_data = md;
 	md->parent = parent;
-- 
2.44.0

Re: [RFC PATCH v2 1/8] dt-bindings: block: add basic bindings for block devices

[Sascha Hauer]

Hi Daniel,

On Tue, Mar 05, 2024 at 08:23:20PM +0000, Daniel Golle wrote:
> diff --git a/Documentation/devicetree/bindings/block/partition.yaml b/Documentation/devicetree/bindings/block/partition.yaml
> new file mode 100644
> index 0000000000000..df561dd33cbc9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/block/partition.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/block/partition.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Partition on a block device
> +
> +description: |
> +  This binding describes a partition on a block storage device.
> +  Partitions may be matched by a combination of partition number, name,
> +  and UUID.
> +
> +maintainers:
> +  - Daniel Golle <daniel@makrotopia.org>
> +
> +properties:
> +  $nodename:
> +    pattern: '^block-partition-.+$'
> +
> +  partnum:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Matches partition by number if present.
> +
> +  partname:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      Matches partition by PARTNAME if present.

In the mtd world we originally had the partition nodes directly under
the hardware device node as well. That was changed to put a
partitions subnode between the hardware device node and the partitions.

From fe2585e9c29a ("doc: dt: mtd: support partitions in a special
'partitions' subnode"):

    To avoid conflict with other drivers using subnodes of the mtd device
    create only one ofpart-specific node rather than any number of
    arbitrary partition subnodes.

Does it make sense to do the same for block devices?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [RFC PATCH v2 1/8] dt-bindings: block: add basic bindings for block devices

[Sascha Hauer]

On Wed, Mar 06, 2024 at 08:22:59AM +0100, Sascha Hauer wrote:
> Hi Daniel,
> 
> On Tue, Mar 05, 2024 at 08:23:20PM +0000, Daniel Golle wrote:
> > diff --git a/Documentation/devicetree/bindings/block/partition.yaml b/Documentation/devicetree/bindings/block/partition.yaml
> > new file mode 100644
> > index 0000000000000..df561dd33cbc9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/block/partition.yaml
> > @@ -0,0 +1,51 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/block/partition.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Partition on a block device
> > +
> > +description: |
> > +  This binding describes a partition on a block storage device.
> > +  Partitions may be matched by a combination of partition number, name,
> > +  and UUID.
> > +
> > +maintainers:
> > +  - Daniel Golle <daniel@makrotopia.org>
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: '^block-partition-.+$'
> > +
> > +  partnum:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Matches partition by number if present.
> > +
> > +  partname:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    description:
> > +      Matches partition by PARTNAME if present.
> 
> In the mtd world we originally had the partition nodes directly under
> the hardware device node as well. That was changed to put a
> partitions subnode between the hardware device node and the partitions.
> 
> From fe2585e9c29a ("doc: dt: mtd: support partitions in a special
> 'partitions' subnode"):
> 
>     To avoid conflict with other drivers using subnodes of the mtd device
>     create only one ofpart-specific node rather than any number of
>     arbitrary partition subnodes.
> 
> Does it make sense to do the same for block devices?

Hm, looking at the example in 5/8 it seems you've already done that. I
think I have misread the binding.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [RFC PATCH v2 4/8] block: implement NVMEM provider

[Ricardo B. Marliere]

Hi Daniel,

On  5 Mar 20:23, Daniel Golle wrote:
> static int __init blk_nvmem_init(void)
> {
> 	int ret;
>  
> 	ret = class_interface_register(&blk_nvmem_bus_interface);
> 	if (ret)
> 		return ret;
>  
> 	return 0;

Just an idea, why not just:

return class_interface_register(&blk_nvmem_bus_interface);

Re: [RFC PATCH v2 1/8] dt-bindings: block: add basic bindings for block devices

[Rob Herring]

On Tue, Mar 05, 2024 at 08:23:20PM +0000, Daniel Golle wrote:
> Add bindings for block devices which are used to allow referencing
> nvmem bits on them.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  .../bindings/block/block-device.yaml          | 22 ++++++++
>  .../devicetree/bindings/block/partition.yaml  | 51 +++++++++++++++++++
>  .../devicetree/bindings/block/partitions.yaml | 20 ++++++++
>  3 files changed, 93 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/block/block-device.yaml
>  create mode 100644 Documentation/devicetree/bindings/block/partition.yaml
>  create mode 100644 Documentation/devicetree/bindings/block/partitions.yaml
> 
> diff --git a/Documentation/devicetree/bindings/block/block-device.yaml b/Documentation/devicetree/bindings/block/block-device.yaml
> new file mode 100644
> index 0000000000000..c83ea525650ba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/block/block-device.yaml
> @@ -0,0 +1,22 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/block/block-device.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: block storage device
> +
> +description: |
> +  This binding is generic and describes a block-oriented storage device.
> +
> +maintainers:
> +  - Daniel Golle <daniel@makrotopia.org>
> +
> +properties:
> +  partitions:
> +    $ref: /schemas/block/partitions.yaml
> +
> +  nvmem-layout:
> +    $ref: /schemas/nvmem/layouts/nvmem-layout.yaml#
> +
> +unevaluatedProperties: false
> diff --git a/Documentation/devicetree/bindings/block/partition.yaml b/Documentation/devicetree/bindings/block/partition.yaml
> new file mode 100644
> index 0000000000000..df561dd33cbc9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/block/partition.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/block/partition.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Partition on a block device
> +
> +description: |
> +  This binding describes a partition on a block storage device.
> +  Partitions may be matched by a combination of partition number, name,
> +  and UUID.
> +
> +maintainers:
> +  - Daniel Golle <daniel@makrotopia.org>
> +
> +properties:
> +  $nodename:
> +    pattern: '^block-partition-.+$'
> +
> +  partnum:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Matches partition by number if present.
> +
> +  partname:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      Matches partition by PARTNAME if present.

Why do we need something new here? The existing fixed-partitions can 
already define block device partitions. It just matches by 
address/offset which works whether its MBR or GPT. Also, in DT we always 
have an address when there is an address.

I'm sure you want to statically define this and have it work even if the 
partitions move, but sorry...

> +
> +  uuid:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      Matches partition by PARTUUID if present.

If this remains it will need some work in the dtschema tools. The reason 
is json-schema already has support for UUIDs as a defined 'format' key 
value and we should use that.

> +
> +  nvmem-layout:
> +    $ref: /schemas/nvmem/layouts/nvmem-layout.yaml#
> +    description:
> +      This container may reference an NVMEM layout parser.
> +
> +anyOf:
> +  - required:
> +      - partnum
> +
> +  - required:
> +      - partname
> +
> +  - required:
> +      - uuid
> +
> +unevaluatedProperties: false

RE: [RFC PATCH v2 6/8] mmc: core: set card fwnode_handle

[Avri Altman]

 
> Set fwnode in case it isn't set yet and of_node is present.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  drivers/mmc/core/bus.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c index
> 0ddaee0eae54f..e1c5fc1b3ce4b 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -364,6 +364,8 @@ int mmc_add_card(struct mmc_card *card)
> 
>         mmc_add_card_debugfs(card);
>         card->dev.of_node = mmc_of_find_child_device(card->host, 0);
> +       if (card->dev.of_node && !card->dev.fwnode)
> +               card->dev.fwnode = &card->dev.of_node->fwnode;
Should this be restricted to eMMC only, or is it fine to be called for SD as well?

Thanks,
Avri

> 
>         device_enable_async_suspend(&card->dev);
> 
> --
> 2.44.0

Re: [RFC PATCH v2 6/8] mmc: core: set card fwnode_handle

[Daniel Golle]

On Fri, Mar 08, 2024 at 08:04:54AM +0000, Avri Altman wrote:
>  
> > Set fwnode in case it isn't set yet and of_node is present.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  drivers/mmc/core/bus.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c index
> > 0ddaee0eae54f..e1c5fc1b3ce4b 100644
> > --- a/drivers/mmc/core/bus.c
> > +++ b/drivers/mmc/core/bus.c
> > @@ -364,6 +364,8 @@ int mmc_add_card(struct mmc_card *card)
> > 
> >         mmc_add_card_debugfs(card);
> >         card->dev.of_node = mmc_of_find_child_device(card->host, 0);
> > +       if (card->dev.of_node && !card->dev.fwnode)
> > +               card->dev.fwnode = &card->dev.of_node->fwnode;
> Should this be restricted to eMMC only, or is it fine to be called for SD as well?

It's always odd to have of_node set and fwnode unset. And also SD
cards can be referenced in device tree, resulting in of_node being set
but fwnode being unpopulated, which is no more or less weird than for
an eMMC.

So imho it should always be called and shouldn't hurt.

RE: [EXTERNAL] Re: [RFC PATCH v2 1/8] dt-bindings: block: add basic bindings for block devices

[Chad Monroe]

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Thursday, March 7, 2024 6:51 AM
> To: Daniel Golle <daniel@makrotopia.org>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Ulf Hansson <ulf.hansson@linaro.org>; Jens Axboe
> <axboe@kernel.dk>; Dave Chinner <dchinner@redhat.com>; Jan Kara
> <jack@suse.cz>; Thomas Weißschuh <linux@weissschuh.net>; Christian Brauner
> <brauner@kernel.org>; Li Lingfeng <lilingfeng3@huawei.com>; Damien Le Moal
> <dlemoal@kernel.org>; Min Li <min15.li@samsung.com>; Adrian Hunter
> <adrian.hunter@intel.com>; Hannes Reinecke <hare@suse.de>; Christian Loehle
> <CLoehle@hyperstone.com>; Avri Altman <avri.altman@wdc.com>; Bean Huo
> <beanhuo@micron.com>; Yeqi Fu <asuk4.q@gmail.com>; Victor Shih
> <victor.shih@genesyslogic.com.tw>; Christophe JAILLET
> <christophe.jaillet@wanadoo.fr>; Ricardo B. Marliere <ricardo@marliere.net>;
> Greg Kroah-Hartman <gregkh@linuxfoundation.org>; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux-
> block@vger.kernel.org; Diping Zhang <diping.zhang@gl-inet.com>; Jianhui Zhao
> <zhaojh329@gmail.com>; Jieying Zeng <jieying.zeng@gl-inet.com>; Chad Monroe
> <chad.monroe@adtran.com>; Adam Fox <adam.fox@adtran.com>; John Crispin
> <john@phrozen.org>
> Subject: [EXTERNAL] Re: [RFC PATCH v2 1/8] dt-bindings: block: add basic bindings
> for block devices
> 
> On Tue, Mar 05, 2024 at 08:23:20PM +0000, Daniel Golle wrote:
> > Add bindings for block devices which are used to allow referencing
> > nvmem bits on them.
> >
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> > .../bindings/block/block-device.yaml | 22 ++++++++
> > .../devicetree/bindings/block/partition.yaml | 51 +++++++++++++++++++
> > .../devicetree/bindings/block/partitions.yaml | 20 ++++++++
> > 3 files changed, 93 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/block/block-
> device.yaml
> > create mode 100644 Documentation/devicetree/bindings/block/partition.yaml
> > create mode 100644 Documentation/devicetree/bindings/block/partitions.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/block/block-device.yaml
> b/Documentation/devicetree/bindings/block/block-device.yaml
> > new file mode 100644
> > index 0000000000000..c83ea525650ba
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/block/block-device.yaml
> > @@ -0,0 +1,22 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/block/block-device.yaml# <https://protect-
> de.mimecast.com/s/gI6FCDqGk9uBM0gMFZVG39?domain=devicetree.org>
> > +$schema: http://devicetree.org/meta-schemas/core.yaml# <https://protect-
> de.mimecast.com/s/rGEGCEqJR9uWnMRnHZK6FQ?domain=devicetree.org>
> > +
> > +title: block storage device
> > +
> > +description: |
> > + This binding is generic and describes a block-oriented storage device.
> > +
> > +maintainers:
> > + - Daniel Golle <daniel@makrotopia.org>
> > +
> > +properties:
> > + partitions:
> > + $ref: /schemas/block/partitions.yaml
> > +
> > + nvmem-layout:
> > + $ref: /schemas/nvmem/layouts/nvmem-layout.yaml#
> > +
> > +unevaluatedProperties: false
> > diff --git a/Documentation/devicetree/bindings/block/partition.yaml
> b/Documentation/devicetree/bindings/block/partition.yaml
> > new file mode 100644
> > index 0000000000000..df561dd33cbc9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/block/partition.yaml
> > @@ -0,0 +1,51 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/block/partition.yaml# <https://protect-
> de.mimecast.com/s/8Tf9CGRL65UJqGjqu07YGR?domain=devicetree.org>
> > +$schema: http://devicetree.org/meta-schemas/core.yaml# <https://protect-
> de.mimecast.com/s/rGEGCEqJR9uWnMRnHZK6FQ?domain=devicetree.org>
> > +
> > +title: Partition on a block device
> > +
> > +description: |
> > + This binding describes a partition on a block storage device.
> > + Partitions may be matched by a combination of partition number, name,
> > + and UUID.
> > +
> > +maintainers:
> > + - Daniel Golle <daniel@makrotopia.org>
> > +
> > +properties:
> > + $nodename:
> > + pattern: '^block-partition-.+$'
> > +
> > + partnum:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description:
> > + Matches partition by number if present.
> > +
> > + partname:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + description:
> > + Matches partition by PARTNAME if present.
> 
> Why do we need something new here? The existing fixed-partitions can
> already define block device partitions. It just matches by
> address/offset which works whether its MBR or GPT. Also, in DT we always
> have an address when there is an address.
> 
> I'm sure you want to statically define this and have it work even if the
> partitions move, but sorry...

The partitions which hold this data are typically defined as a MBR or GPT
partition and referenced by PARTNAME, PARTUUID or PARTNO. The data is
referenced as an offset within that partition. It's possible for the offset
of the RF/calibration partition to change if the size of eMMC chip changes
between builds of the same device for example. Within the RF partition the
data is always available at the same offsets.

Based on this, we don't always know the offset of the RF partition and
simply want to use the partition table to point us at the right location.

> 
> > +
> > + uuid:
> > + $ref: /schemas/types.yaml#/definitions/string
> > + description:
> > + Matches partition by PARTUUID if present.
> 
> If this remains it will need some work in the dtschema tools. The reason
> is json-schema already has support for UUIDs as a defined 'format' key
> value and we should use that.
> 
> > +
> > + nvmem-layout:
> > + $ref: /schemas/nvmem/layouts/nvmem-layout.yaml#
> > + description:
> > + This container may reference an NVMEM layout parser.
> > +
> > +anyOf:
> > + - required:
> > + - partnum
> > +
> > + - required:
> > + - partname
> > +
> > + - required:
> > + - uuid
> > +
> > +unevaluatedProperties: false

Re: [RFC PATCH v2 0/8] nvmem: add block device NVMEM provider

[Ulf Hansson]

On Tue, 5 Mar 2024 at 21:23, Daniel Golle <daniel@makrotopia.org> wrote:
>
> On embedded devices using an eMMC it is common that one or more (hw/sw)
> partitions on the eMMC are used to store MAC addresses and Wi-Fi
> calibration EEPROM data.
>
> Implement an NVMEM provider backed by block devices as typically the
> NVMEM framework is used to have kernel drivers read and use binary data
> from EEPROMs, efuses, flash memory (MTD), ...
>
> In order to be able to reference hardware partitions on an eMMC, add code
> to bind each hardware partition to a specific firmware subnode.
>
> This series is meant to open the discussion on how exactly the device
> tree schema for block devices and partitions may look like, and even
> if using the block layer to back the NVMEM device is at all the way to
> go -- to me it seemed to be a good solution because it will be reuable
> e.g. for (normal, software GPT or MBR) partitions of an NVMe SSD.
>
> This series has previously been submitted on July 19th 2023[1] and most of
> the basic idea did not change since.
>
> However, the recent introduction of bdev_file_open_by_dev() allow to
> get rid of most use of block layer internals which supposedly was the
> main objection raised by Christoph Hellwig back then.
>
> Most of the other comments received for in the first RFC have also
> been addressed, however, what remains is the use of class_interface
> (lacking an alternative way to get notifications about addition or
> removal of block devices from the system). As this has been criticized
> in the past I'm specifically interested in suggestions on how to solve
> this in another way -- ideally without having to implement a whole new
> way for in-kernel notifications of appearing or disappearing block
> devices...
>
> And, in a way just like in case of MTD and UBI, I believe acting as an
> NVMEM provider *is* a functionality which belongs to the block layer
> itself and, other than e.g. filesystems, is inconvenient to implement
> elsewhere.

I don't object to the above, however to keep things scalable at the
block device driver level, such as the MMC subsystem, I think we
should avoid having *any* knowledge about the binary format at these
kinds of lower levels.

Even if most of the NVMEM format is managed elsewhere, the support for
NVMEM partitions seems to be dealt with from the MMC subsystem too.
Why can't NVMEM partitions be managed the usual way via the MBR/GPT?

[...]

Kind regards
Uffe

Re: [RFC PATCH v2 0/8] nvmem: add block device NVMEM provider

[Daniel Golle]

Hi Ulf,

On Tue, Mar 12, 2024 at 01:22:49PM +0100, Ulf Hansson wrote:
> On Tue, 5 Mar 2024 at 21:23, Daniel Golle <daniel@makrotopia.org> wrote:
> >
> > On embedded devices using an eMMC it is common that one or more (hw/sw)
> > partitions on the eMMC are used to store MAC addresses and Wi-Fi
> > calibration EEPROM data.
> >
> > Implement an NVMEM provider backed by block devices as typically the
> > NVMEM framework is used to have kernel drivers read and use binary data
> > from EEPROMs, efuses, flash memory (MTD), ...
> >
> > In order to be able to reference hardware partitions on an eMMC, add code
> > to bind each hardware partition to a specific firmware subnode.
> >
> > This series is meant to open the discussion on how exactly the device
> > tree schema for block devices and partitions may look like, and even
> > if using the block layer to back the NVMEM device is at all the way to
> > go -- to me it seemed to be a good solution because it will be reuable
> > e.g. for (normal, software GPT or MBR) partitions of an NVMe SSD.
> >
> > This series has previously been submitted on July 19th 2023[1] and most of
> > the basic idea did not change since.
> >
> > However, the recent introduction of bdev_file_open_by_dev() allow to
> > get rid of most use of block layer internals which supposedly was the
> > main objection raised by Christoph Hellwig back then.
> >
> > Most of the other comments received for in the first RFC have also
> > been addressed, however, what remains is the use of class_interface
> > (lacking an alternative way to get notifications about addition or
> > removal of block devices from the system). As this has been criticized
> > in the past I'm specifically interested in suggestions on how to solve
> > this in another way -- ideally without having to implement a whole new
> > way for in-kernel notifications of appearing or disappearing block
> > devices...
> >
> > And, in a way just like in case of MTD and UBI, I believe acting as an
> > NVMEM provider *is* a functionality which belongs to the block layer
> > itself and, other than e.g. filesystems, is inconvenient to implement
> > elsewhere.
> 
> I don't object to the above, however to keep things scalable at the
> block device driver level, such as the MMC subsystem, I think we
> should avoid having *any* knowledge about the binary format at these
> kinds of lower levels.
> 
> Even if most of the NVMEM format is managed elsewhere, the support for
> NVMEM partitions seems to be dealt with from the MMC subsystem too.

In an earlier iteration of this RFC it was requested to make NVMEM
support opt-in (instead of opt-out for mtdblock and ubiblock, which
already got their own NVMEM provider implementation).
Hence at least a change to opt-in for NVMEM support is required in the
MMC subsystem, together with making sure that MMC devices have their
fwnode assigned.

> Why can't NVMEM partitions be managed the usual way via the MBR/GPT?

Absolutely, maybe my wording was not clear, but that's exactly what
I'm suggesting here. There are no added parsers nor any knowledge
about binary formats in this patchset.

Or did I misunderstand your comment?

Re: [RFC PATCH v2 0/8] nvmem: add block device NVMEM provider

[Ulf Hansson]

On Tue, 12 Mar 2024 at 13:30, Daniel Golle <daniel@makrotopia.org> wrote:
>
> Hi Ulf,
>
> On Tue, Mar 12, 2024 at 01:22:49PM +0100, Ulf Hansson wrote:
> > On Tue, 5 Mar 2024 at 21:23, Daniel Golle <daniel@makrotopia.org> wrote:
> > >
> > > On embedded devices using an eMMC it is common that one or more (hw/sw)
> > > partitions on the eMMC are used to store MAC addresses and Wi-Fi
> > > calibration EEPROM data.
> > >
> > > Implement an NVMEM provider backed by block devices as typically the
> > > NVMEM framework is used to have kernel drivers read and use binary data
> > > from EEPROMs, efuses, flash memory (MTD), ...
> > >
> > > In order to be able to reference hardware partitions on an eMMC, add code
> > > to bind each hardware partition to a specific firmware subnode.
> > >
> > > This series is meant to open the discussion on how exactly the device
> > > tree schema for block devices and partitions may look like, and even
> > > if using the block layer to back the NVMEM device is at all the way to
> > > go -- to me it seemed to be a good solution because it will be reuable
> > > e.g. for (normal, software GPT or MBR) partitions of an NVMe SSD.
> > >
> > > This series has previously been submitted on July 19th 2023[1] and most of
> > > the basic idea did not change since.
> > >
> > > However, the recent introduction of bdev_file_open_by_dev() allow to
> > > get rid of most use of block layer internals which supposedly was the
> > > main objection raised by Christoph Hellwig back then.
> > >
> > > Most of the other comments received for in the first RFC have also
> > > been addressed, however, what remains is the use of class_interface
> > > (lacking an alternative way to get notifications about addition or
> > > removal of block devices from the system). As this has been criticized
> > > in the past I'm specifically interested in suggestions on how to solve
> > > this in another way -- ideally without having to implement a whole new
> > > way for in-kernel notifications of appearing or disappearing block
> > > devices...
> > >
> > > And, in a way just like in case of MTD and UBI, I believe acting as an
> > > NVMEM provider *is* a functionality which belongs to the block layer
> > > itself and, other than e.g. filesystems, is inconvenient to implement
> > > elsewhere.
> >
> > I don't object to the above, however to keep things scalable at the
> > block device driver level, such as the MMC subsystem, I think we
> > should avoid having *any* knowledge about the binary format at these
> > kinds of lower levels.
> >
> > Even if most of the NVMEM format is managed elsewhere, the support for
> > NVMEM partitions seems to be dealt with from the MMC subsystem too.
>
> In an earlier iteration of this RFC it was requested to make NVMEM
> support opt-in (instead of opt-out for mtdblock and ubiblock, which
> already got their own NVMEM provider implementation).
> Hence at least a change to opt-in for NVMEM support is required in the
> MMC subsystem, together with making sure that MMC devices have their
> fwnode assigned.

So, the NVMEM support needs to be turned on (opt-in) for each and
every block device driver?

It's not a big deal for me - and I would be happy to apply such a
change. On the other hand, it is just some binary data that is stored
on the flash, why should MMC have to opt-in or opt-out at all? It
should be the upper layers who decide what to store on the flash, not
the MMC subsystem, if you get my point.

>
> > Why can't NVMEM partitions be managed the usual way via the MBR/GPT?
>
> Absolutely, maybe my wording was not clear, but that's exactly what
> I'm suggesting here. There are no added parsers nor any knowledge
> about binary formats in this patchset.

Right, but there are new DT bindings added in the $subject series that
allows us to describe NVMEM partitions for an eMMC. Why isn't that
parsed from the MBR/GPT, etc, rather than encoded in DT?

>
> Or did I misunderstand your comment?

Maybe. I am just trying to understand this, so apologize if you find
my questions silly. :-)

Kind regards
Uffe

Re: [RFC PATCH v2 0/8] nvmem: add block device NVMEM provider

[Daniel Golle]

On Tue, Mar 12, 2024 at 01:57:39PM +0100, Ulf Hansson wrote:
> On Tue, 12 Mar 2024 at 13:30, Daniel Golle <daniel@makrotopia.org> wrote:
> >
> > Hi Ulf,
> >
> > On Tue, Mar 12, 2024 at 01:22:49PM +0100, Ulf Hansson wrote:
> > > On Tue, 5 Mar 2024 at 21:23, Daniel Golle <daniel@makrotopia.org> wrote:
> > > >
> > > > On embedded devices using an eMMC it is common that one or more (hw/sw)
> > > > partitions on the eMMC are used to store MAC addresses and Wi-Fi
> > > > calibration EEPROM data.
> > > >
> > > > Implement an NVMEM provider backed by block devices as typically the
> > > > NVMEM framework is used to have kernel drivers read and use binary data
> > > > from EEPROMs, efuses, flash memory (MTD), ...
> > > >
> > > > In order to be able to reference hardware partitions on an eMMC, add code
> > > > to bind each hardware partition to a specific firmware subnode.
> > > >
> > > > This series is meant to open the discussion on how exactly the device
> > > > tree schema for block devices and partitions may look like, and even
> > > > if using the block layer to back the NVMEM device is at all the way to
> > > > go -- to me it seemed to be a good solution because it will be reuable
> > > > e.g. for (normal, software GPT or MBR) partitions of an NVMe SSD.
> > > >
> > > > This series has previously been submitted on July 19th 2023[1] and most of
> > > > the basic idea did not change since.
> > > >
> > > > However, the recent introduction of bdev_file_open_by_dev() allow to
> > > > get rid of most use of block layer internals which supposedly was the
> > > > main objection raised by Christoph Hellwig back then.
> > > >
> > > > Most of the other comments received for in the first RFC have also
> > > > been addressed, however, what remains is the use of class_interface
> > > > (lacking an alternative way to get notifications about addition or
> > > > removal of block devices from the system). As this has been criticized
> > > > in the past I'm specifically interested in suggestions on how to solve
> > > > this in another way -- ideally without having to implement a whole new
> > > > way for in-kernel notifications of appearing or disappearing block
> > > > devices...
> > > >
> > > > And, in a way just like in case of MTD and UBI, I believe acting as an
> > > > NVMEM provider *is* a functionality which belongs to the block layer
> > > > itself and, other than e.g. filesystems, is inconvenient to implement
> > > > elsewhere.
> > >
> > > I don't object to the above, however to keep things scalable at the
> > > block device driver level, such as the MMC subsystem, I think we
> > > should avoid having *any* knowledge about the binary format at these
> > > kinds of lower levels.
> > >
> > > Even if most of the NVMEM format is managed elsewhere, the support for
> > > NVMEM partitions seems to be dealt with from the MMC subsystem too.
> >
> > In an earlier iteration of this RFC it was requested to make NVMEM
> > support opt-in (instead of opt-out for mtdblock and ubiblock, which
> > already got their own NVMEM provider implementation).
> > Hence at least a change to opt-in for NVMEM support is required in the
> > MMC subsystem, together with making sure that MMC devices have their
> > fwnode assigned.
> 
> So, the NVMEM support needs to be turned on (opt-in) for each and
> every block device driver?
> 
> It's not a big deal for me - and I would be happy to apply such a
> change. On the other hand, it is just some binary data that is stored
> on the flash, why should MMC have to opt-in or opt-out at all? It
> should be the upper layers who decide what to store on the flash, not
> the MMC subsystem, if you get my point.
> 

I agree, and that's exactly how I originally wrote it. However, in the
first round of rewiew it was requested to be in that way (ie. opt-in
for each subsystem; rather than opt-out for subsystems already
providing NVMEM in another way, such as MTD or UBI), see here:

https://patchwork.kernel.org/comment/25432948/

> >
> > > Why can't NVMEM partitions be managed the usual way via the MBR/GPT?
> >
> > Absolutely, maybe my wording was not clear, but that's exactly what
> > I'm suggesting here. There are no added parsers nor any knowledge
> > about binary formats in this patchset.
> 
> Right, but there are new DT bindings added in the $subject series that
> allows us to describe NVMEM partitions for an eMMC. Why isn't that
> parsed from the MBR/GPT, etc, rather than encoded in DT?

The added dt-bindings merely allow to **identify** the partition by
it's PARTNAME, PARTNO or PARTUUID, so we can reference them in DT.
We'd still rely on MBR or GPT to do the actual parsing of the on-disk
format.

> 
> >
> > Or did I misunderstand your comment?
> 
> Maybe. I am just trying to understand this, so apologize if you find
> my questions silly. :-)

Let's make sure to all be on the same page and everything is fully
understood by everyone. Everyone has to bare the noise, but I guess
that's ok ;)


Cheers


Daniel

Re: [RFC PATCH v2 0/8] nvmem: add block device NVMEM provider

[Ulf Hansson]

On Tue, 12 Mar 2024 at 14:12, Daniel Golle <daniel@makrotopia.org> wrote:
>
> On Tue, Mar 12, 2024 at 01:57:39PM +0100, Ulf Hansson wrote:
> > On Tue, 12 Mar 2024 at 13:30, Daniel Golle <daniel@makrotopia.org> wrote:
> > >
> > > Hi Ulf,
> > >
> > > On Tue, Mar 12, 2024 at 01:22:49PM +0100, Ulf Hansson wrote:
> > > > On Tue, 5 Mar 2024 at 21:23, Daniel Golle <daniel@makrotopia.org> wrote:
> > > > >
> > > > > On embedded devices using an eMMC it is common that one or more (hw/sw)
> > > > > partitions on the eMMC are used to store MAC addresses and Wi-Fi
> > > > > calibration EEPROM data.
> > > > >
> > > > > Implement an NVMEM provider backed by block devices as typically the
> > > > > NVMEM framework is used to have kernel drivers read and use binary data
> > > > > from EEPROMs, efuses, flash memory (MTD), ...
> > > > >
> > > > > In order to be able to reference hardware partitions on an eMMC, add code
> > > > > to bind each hardware partition to a specific firmware subnode.
> > > > >
> > > > > This series is meant to open the discussion on how exactly the device
> > > > > tree schema for block devices and partitions may look like, and even
> > > > > if using the block layer to back the NVMEM device is at all the way to
> > > > > go -- to me it seemed to be a good solution because it will be reuable
> > > > > e.g. for (normal, software GPT or MBR) partitions of an NVMe SSD.
> > > > >
> > > > > This series has previously been submitted on July 19th 2023[1] and most of
> > > > > the basic idea did not change since.
> > > > >
> > > > > However, the recent introduction of bdev_file_open_by_dev() allow to
> > > > > get rid of most use of block layer internals which supposedly was the
> > > > > main objection raised by Christoph Hellwig back then.
> > > > >
> > > > > Most of the other comments received for in the first RFC have also
> > > > > been addressed, however, what remains is the use of class_interface
> > > > > (lacking an alternative way to get notifications about addition or
> > > > > removal of block devices from the system). As this has been criticized
> > > > > in the past I'm specifically interested in suggestions on how to solve
> > > > > this in another way -- ideally without having to implement a whole new
> > > > > way for in-kernel notifications of appearing or disappearing block
> > > > > devices...
> > > > >
> > > > > And, in a way just like in case of MTD and UBI, I believe acting as an
> > > > > NVMEM provider *is* a functionality which belongs to the block layer
> > > > > itself and, other than e.g. filesystems, is inconvenient to implement
> > > > > elsewhere.
> > > >
> > > > I don't object to the above, however to keep things scalable at the
> > > > block device driver level, such as the MMC subsystem, I think we
> > > > should avoid having *any* knowledge about the binary format at these
> > > > kinds of lower levels.
> > > >
> > > > Even if most of the NVMEM format is managed elsewhere, the support for
> > > > NVMEM partitions seems to be dealt with from the MMC subsystem too.
> > >
> > > In an earlier iteration of this RFC it was requested to make NVMEM
> > > support opt-in (instead of opt-out for mtdblock and ubiblock, which
> > > already got their own NVMEM provider implementation).
> > > Hence at least a change to opt-in for NVMEM support is required in the
> > > MMC subsystem, together with making sure that MMC devices have their
> > > fwnode assigned.
> >
> > So, the NVMEM support needs to be turned on (opt-in) for each and
> > every block device driver?
> >
> > It's not a big deal for me - and I would be happy to apply such a
> > change. On the other hand, it is just some binary data that is stored
> > on the flash, why should MMC have to opt-in or opt-out at all? It
> > should be the upper layers who decide what to store on the flash, not
> > the MMC subsystem, if you get my point.
> >
>
> I agree, and that's exactly how I originally wrote it. However, in the
> first round of rewiew it was requested to be in that way (ie. opt-in
> for each subsystem; rather than opt-out for subsystems already
> providing NVMEM in another way, such as MTD or UBI), see here:
>
> https://patchwork.kernel.org/comment/25432948/

Okay, got it, thanks!

>
> > >
> > > > Why can't NVMEM partitions be managed the usual way via the MBR/GPT?
> > >
> > > Absolutely, maybe my wording was not clear, but that's exactly what
> > > I'm suggesting here. There are no added parsers nor any knowledge
> > > about binary formats in this patchset.
> >
> > Right, but there are new DT bindings added in the $subject series that
> > allows us to describe NVMEM partitions for an eMMC. Why isn't that
> > parsed from the MBR/GPT, etc, rather than encoded in DT?
>
> The added dt-bindings merely allow to **identify** the partition by
> it's PARTNAME, PARTNO or PARTUUID, so we can reference them in DT.
> We'd still rely on MBR or GPT to do the actual parsing of the on-disk
> format.

Thanks for clarifying!

So, it looks like this all relies on what DT maintainers think then.

[...]

Kind regards
Uffe