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 a block device 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.
Overall, this enables uniform handling across practially all flash
storage types used for this purpose (MTD, UBI, and now also MMC).
As part of this series it was necessary to define a device tree schema
for block devices and partitions on them, which (similar to how it now
works also for UBI volumes) can be matched by one or more properties.
---
This series has previously been submitted as RFC on July 19th 2023[1]
and most of the basic idea did not change since. Another round of RFC
was submitted on March 5th 2024[2] which has received overall positive
feedback and only minor corrections have been done since (see
changelog below).
[1]: https://patchwork.kernel.org/project/linux-block/list/?series=767565
[2]: https://patchwork.kernel.org/project/linux-block/list/?series=832705
Changes since RFC:
* Use 'partuuid' instead of reserved 'uuid' keyword to match against
PARTUUID.
* Simplify blk_nvmem_init(void) function.
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 +++++
MAINTAINERS | 5 +
block/Kconfig | 9 +
block/Makefile | 1 +
block/blk-nvmem.c | 169 ++++++++++++++++++
block/partitions/core.c | 41 +++++
drivers/mmc/core/block.c | 8 +
drivers/mmc/core/bus.c | 2 +
include/linux/blkdev.h | 2 +
12 files changed, 375 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
Add bindings for block devices which are used to allow referencing
nvmem bits on them.
Signed-off-by: Daniel Golle <[email protected]>
---
.../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 <[email protected]>
+
+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..86b61e30f9a41
--- /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 device.
+ Partitions may be matched by a combination of partition number, name,
+ and UUID.
+
+maintainers:
+ - Daniel Golle <[email protected]>
+
+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.
+
+ partuuid:
+ $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:
+ - partuuid
+
+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 <[email protected]>
+
+patternProperties:
+ "^block-partition-.+$":
+ $ref: partition.yaml
+
+unevaluatedProperties: false
--
2.44.0
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 <[email protected]>
---
block/partitions/core.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/block/partitions/core.c b/block/partitions/core.c
index b11e88c82c8cf..c40ba88837373 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, *partuuid;
+ 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, "partuuid", &partuuid) &&
+ (!bdev->bd_meta_info || strncmp(partuuid,
+ 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
Add new flag to destinguish block devices which may act as an NVMEM
provider.
Signed-off-by: Daniel Golle <[email protected]>
---
include/linux/blkdev.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c3e8f7cf96be9..f2c4f280d7619 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
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 <[email protected]>
---
MAINTAINERS | 5 ++
block/Kconfig | 9 +++
block/Makefile | 1 +
block/blk-nvmem.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 184 insertions(+)
create mode 100644 block/blk-nvmem.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 8c88f362feb55..242a0a139c00a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3662,6 +3662,11 @@ L: [email protected]
S: Maintained
F: drivers/mtd/devices/block2mtd.c
+BLOCK NVMEM DRIVER
+M: Daniel Golle <[email protected]>
+S: Maintained
+F: block/blk-nvmem.c
+
BLUETOOTH DRIVERS
M: Marcel Holtmann <[email protected]>
M: Luiz Augusto von Dentz <[email protected]>
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..29f8ac041d76e
--- /dev/null
+++ b/block/blk-nvmem.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * block device NVMEM provider
+ *
+ * Copyright (c) 2024 Daniel Golle <[email protected]>
+ *
+ * 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)
+{
+ return class_interface_register(&blk_nvmem_bus_interface);
+}
+device_initcall(blk_nvmem_init);
--
2.44.0
Add nodes representing the block devices exposed by an MMC device
including an example involving nvmem-cells.
Signed-off-by: Daniel Golle <[email protected]>
---
.../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
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 <[email protected]>
---
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 64a3492e8002f..5b7dfd4a21fa5 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2463,6 +2463,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];
@@ -2559,6 +2560,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
Set fwnode in case it isn't set yet and of_node is present.
Signed-off-by: Daniel Golle <[email protected]>
---
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
Set flag to consider MMC block devices as NVMEM providers.
Signed-off-by: Daniel Golle <[email protected]>
---
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 5b7dfd4a21fa5..157e6d0aed495 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -2516,6 +2516,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
On 3/21/24 12:32, Daniel Golle wrote:
> +$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 device.
> + Partitions may be matched by a combination of partition number, name,
> + and UUID.
> +
> +maintainers:
> + - Daniel Golle <[email protected]>
> +
> +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.
> +
> + partuuid:
> + $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.
Does the above imply that only systems with a single block device are
supported?
Supporting partition numbers seems unfortunate to me. Partition numbers
will change if the partition scheme changes.
Bart.
On 3/21/24 12:34, Daniel Golle wrote:
> 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.
Why to store calibration data in a partition instead of in a file on a
filesystem?
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8c88f362feb55..242a0a139c00a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3662,6 +3662,11 @@ L: [email protected]
> S: Maintained
> F: drivers/mtd/devices/block2mtd.c
>
> +BLOCK NVMEM DRIVER
> +M: Daniel Golle <[email protected]>
> +S: Maintained
> +F: block/blk-nvmem.c
Why to add this functionality to the block layer instead of somewhere
in the drivers/ directory?
Thanks,
Bart.
Hi Bart,
thank you for looking at the patches!
On Thu, Mar 21, 2024 at 12:44:19PM -0700, Bart Van Assche wrote:
> On 3/21/24 12:34, Daniel Golle wrote:
> > 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.
>
> Why to store calibration data in a partition instead of in a file on a
> filesystem?
First of all, it's just how it is already in the practical world out
there. The same methods for mass-production are used independently of
the type of flash memory, so vendors don't care if in Linux the flash
ends up as MMC/block (in case of an eMMC) device or MTD device (in
case of SPI-NOR, for example). I can name countless devices of
numerous vendors following this generally very common practise (and
then ending up extracting that using ugly custom drivers, or poking
around in the block devices in early userland, ... none of it is nice,
which is the motivation for this series).
Adtran, GL-iNet, Netgear, ... to name just a few very popular vendors.
The devices are already out there, and the way they store those
details is considered part of the low level firmware which will never
change. Yet it would be nice to run vanilla Linux on them (or
OpenWrt), and make sure things like NFS root can work, and for that
the MAC address needs to be in place already, ie. extracting it in
userland would be too late.
However, I also believe there is nothing wrong with that and using a
filesystem comes with many additional pitfalls, such as being possibly
not cleanly unmounted, the file could be renamed or deleted by the
user, .... All that should not result in a device not having it's
proper MAC address any more.
Why have all the complexity for something as simple as storing 6 bytes
of MAC address?
I will not re-iterate over all that discussion now, you may look at
list archives where this has been explained and discussed also for the
first run of the RFC series last year.
>
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 8c88f362feb55..242a0a139c00a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3662,6 +3662,11 @@ L: [email protected]
> > S: Maintained
> > F: drivers/mtd/devices/block2mtd.c
> > +BLOCK NVMEM DRIVER
> > +M: Daniel Golle <[email protected]>
> > +S: Maintained
> > +F: block/blk-nvmem.c
>
> Why to add this functionality to the block layer instead of somewhere
> in the drivers/ directory?
Simply because we need notifications about appearing and disappearing
block devices, or a way to iterate over all block devices in a system.
For both there isn't currently any other interface than using a
class_interface for that, and that requires access to &block_class
which is considered a block subsystem internal.
Also note that the same is true for the MTD NVMEM provider (in
drivers/mtd/mtdcore.c) as well as the UBI NVMEM provider (in
drivers/mtd/ubi/nvmem.c), both are considered an integral part of
their corresponding subsystems -- despite the fact that in those cases
this wouldn't even be stricktly needed as for MTD we got
register_mtd_user() and for UBI we'd have
ubi_register_volume_notifier().
Doing it differently for block devices would hence not only complicate
things unnessesarily, it would also be inconsistent.
On Thu, Mar 21, 2024 at 12:39:33PM -0700, Bart Van Assche wrote:
> On 3/21/24 12:32, Daniel Golle wrote:
> > +$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 device.
> > + Partitions may be matched by a combination of partition number, name,
> > + and UUID.
> > +
> > +maintainers:
> > + - Daniel Golle <[email protected]>
> > +
> > +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.
> > +
> > + partuuid:
> > + $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.
>
> Does the above imply that only systems with a single block device are
> supported?
Absolutely not. Of course also such devices often have multiple block
devices, typically eMMC, NVMe and SD card are supported, some also
come with SATA ports. The block device(s) relevant as NVMEM providers
has/have to be referenced and the 'partitions' node is a child node of
a specific block device, of course.
>
> Supporting partition numbers seems unfortunate to me. Partition numbers
> will change if the partition scheme changes.
I fully argee with that, and using partnum as an identifier is not
very smart. However, this is what some vendors are doing (in custom
downstream drivers or scripts running in early userland) and hence the
kernel implementation should allow to identify the relevant location
in exactly the same way to be sure we are always compatible.
On 3/21/24 12:33, Daniel Golle wrote:
> Add new flag to destinguish block devices which may act as an NVMEM
> provider.
>
> Signed-off-by: Daniel Golle <[email protected]>
> ---
> include/linux/blkdev.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index c3e8f7cf96be9..f2c4f280d7619 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,
> };
What would break if this flag wouldn't exist?
Thanks,
Bart.
On 3/21/24 12:31, Daniel Golle 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 a block device 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.
>
> Overall, this enables uniform handling across practially all flash
> storage types used for this purpose (MTD, UBI, and now also MMC).
>
> As part of this series it was necessary to define a device tree schema
> for block devices and partitions on them, which (similar to how it now
> works also for UBI volumes) can be matched by one or more properties.
Since this patch series adds code that opens partitions and reads
from partitions, can that part of the functionality be implemented in
user space? There is already a mechanism for notifying user space about
block device changes, namely udev.
Thanks,
Bart.
On 3/21/24 13:22, Daniel Golle wrote:
> On Thu, Mar 21, 2024 at 12:44:19PM -0700, Bart Van Assche wrote:
>> Why to add this functionality to the block layer instead of somewhere
>> in the drivers/ directory?
>
> Simply because we need notifications about appearing and disappearing
> block devices, or a way to iterate over all block devices in a system.
> For both there isn't currently any other interface than using a
> class_interface for that, and that requires access to &block_class
> which is considered a block subsystem internal.
That's an argument for adding an interface to the block layer that
implements this functionality but not for adding this code in the block
layer.
Thanks,
Bart.
On Fri, Mar 22, 2024 at 10:52:17AM -0700, Bart Van Assche wrote:
> On 3/21/24 12:31, Daniel Golle 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 a block device 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.
> >
> > Overall, this enables uniform handling across practially all flash
> > storage types used for this purpose (MTD, UBI, and now also MMC).
> >
> > As part of this series it was necessary to define a device tree schema
> > for block devices and partitions on them, which (similar to how it now
> > works also for UBI volumes) can be matched by one or more properties.
>
> Since this patch series adds code that opens partitions and reads
> from partitions, can that part of the functionality be implemented in
> user space? There is already a mechanism for notifying user space about
> block device changes, namely udev.
No. Because it has to happen (e.g. for nfsroot to work) before
userland gets initiated: Without Ethernet MAC address (which if often
stored at some raw offset on a partition or hw-partition of an eMMC),
we don't have a way to use nfsroot (because that requires functional
Ethernet), hence userland won't come up. It's a circular dependency
problem which can only be addressed by making sure that everything
needed for Ethernet to come up is provided by the kernel **before**
rootfs (which can be nfsroot) is mounted.
On Fri, Mar 22, 2024 at 10:49:48AM -0700, Bart Van Assche wrote:
> On 3/21/24 12:33, Daniel Golle wrote:
> > Add new flag to destinguish block devices which may act as an NVMEM
> > provider.
> >
> > Signed-off-by: Daniel Golle <[email protected]>
> > ---
> > include/linux/blkdev.h | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index c3e8f7cf96be9..f2c4f280d7619 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,
> > };
>
> What would break if this flag wouldn't exist?
As both, MTD and UBI already act as NVMEM providers themselves, once
the user creates a ubiblock device or got CONFIG_MTD_BLOCK=y set in their
kernel configuration, we would run into problems because both, the block
layer as well as MTD or UBI would try to be an NVMEM provider for the same
device tree node.
I intially suggested the invert of this flag, GENHD_FL_NO_NVMEM which
would be set only for mtdblock and ubiblock devices to opt-out of acting
as NVMEM proviers. However, in a previous comment [1] on the RFC it was
requested to make this opt-in instead.
[1]: https://patchwork.kernel.org/comment/25432948/
On Fri, Mar 22, 2024 at 10:52:36AM -0700, Bart Van Assche wrote:
> On 3/21/24 13:22, Daniel Golle wrote:
> > On Thu, Mar 21, 2024 at 12:44:19PM -0700, Bart Van Assche wrote:
> > > Why to add this functionality to the block layer instead of somewhere
> > > in the drivers/ directory?
> >
> > Simply because we need notifications about appearing and disappearing
> > block devices, or a way to iterate over all block devices in a system.
> > For both there isn't currently any other interface than using a
> > class_interface for that, and that requires access to &block_class
> > which is considered a block subsystem internal.
>
> That's an argument for adding an interface to the block layer that
> implements this functionality but not for adding this code in the block
> layer.
Fine with me. I can implement such an interface, similar to how it is
implemented for MTD devices or UBI volumes for the block layer.
I would basically add a subscription and callback interface utilizing
a class_interface inside the block subsystem similar to how the same
is done in this series for registering block-device-backed NVMEM
providers.
However, given that this is a bigger task, I'd like to know from more
than one block subsystem maintainer that this approach would be
agreeable before spending time and effort in this direction.
Also note that obviously it would be much more intrusive and affect
*all* users of the block subsystem, while the current approach would
only affect those users who got CONFIG_BLOCK_NVMEM enabled.
On 3/22/24 11:02, Daniel Golle wrote:
> On Fri, Mar 22, 2024 at 10:52:17AM -0700, Bart Van Assche wrote:
>> On 3/21/24 12:31, Daniel Golle 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 a block device 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.
>>>
>>> Overall, this enables uniform handling across practially all flash
>>> storage types used for this purpose (MTD, UBI, and now also MMC).
>>>
>>> As part of this series it was necessary to define a device tree schema
>>> for block devices and partitions on them, which (similar to how it now
>>> works also for UBI volumes) can be matched by one or more properties.
>>
>> Since this patch series adds code that opens partitions and reads
>> from partitions, can that part of the functionality be implemented in
>> user space? There is already a mechanism for notifying user space about
>> block device changes, namely udev.
>
> No. Because it has to happen (e.g. for nfsroot to work) before
> userland gets initiated: Without Ethernet MAC address (which if often
> stored at some raw offset on a partition or hw-partition of an eMMC),
> we don't have a way to use nfsroot (because that requires functional
> Ethernet), hence userland won't come up. It's a circular dependency
> problem which can only be addressed by making sure that everything
> needed for Ethernet to come up is provided by the kernel **before**
> rootfs (which can be nfsroot) is mounted.
How about the initial RAM disk? I think that's where code should occur
that reads calibration data from local storage.
Thanks,
Bart.
On 3/22/24 11:07, Daniel Golle wrote:
> On Fri, Mar 22, 2024 at 10:49:48AM -0700, Bart Van Assche wrote:
>> On 3/21/24 12:33, Daniel Golle wrote:
>>> enum {
>>> GENHD_FL_REMOVABLE = 1 << 0,
>>> GENHD_FL_HIDDEN = 1 << 1,
>>> GENHD_FL_NO_PART = 1 << 2,
>>> + GENHD_FL_NVMEM = 1 << 3,
>>> };
>>
>> What would break if this flag wouldn't exist?
>
> As both, MTD and UBI already act as NVMEM providers themselves, once
> the user creates a ubiblock device or got CONFIG_MTD_BLOCK=y set in their
> kernel configuration, we would run into problems because both, the block
> layer as well as MTD or UBI would try to be an NVMEM provider for the same
> device tree node.
Why would both MTD and UBI try to be an NVMEM provider for the same
device tree node? Why can't this patch series be implemented such that
a partition UUID occurs in the device tree and such that other code
scans for that partition UUID?
Thanks,
Bart.
On Thu, Mar 21, 2024 at 07:31:48PM +0000, Daniel Golle 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 a block device 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.
>
> Overall, this enables uniform handling across practially all flash
> storage types used for this purpose (MTD, UBI, and now also MMC).
>
> As part of this series it was necessary to define a device tree schema
> for block devices and partitions on them, which (similar to how it now
> works also for UBI volumes) can be matched by one or more properties.
>
> ---
> This series has previously been submitted as RFC on July 19th 2023[1]
> and most of the basic idea did not change since. Another round of RFC
> was submitted on March 5th 2024[2] which has received overall positive
> feedback and only minor corrections have been done since (see
> changelog below).
Also, please version your patches. 'RFC' is a tag, not a version. v1 was
July. v2 was March 5th. This is v3.
Rob
On Mon, Mar 25, 2024 at 10:10:46AM -0500, Rob Herring wrote:
> On Thu, Mar 21, 2024 at 07:31:48PM +0000, Daniel Golle 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 a block device 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.
> >
> > Overall, this enables uniform handling across practially all flash
> > storage types used for this purpose (MTD, UBI, and now also MMC).
> >
> > As part of this series it was necessary to define a device tree schema
> > for block devices and partitions on them, which (similar to how it now
> > works also for UBI volumes) can be matched by one or more properties.
> >
> > ---
> > This series has previously been submitted as RFC on July 19th 2023[1]
> > and most of the basic idea did not change since. Another round of RFC
> > was submitted on March 5th 2024[2] which has received overall positive
> > feedback and only minor corrections have been done since (see
> > changelog below).
>
> I don't recall giving positive feedback.
>
> I still think this should use offsets rather than partition specific
> information. Not wanting to have to update the offsets if they change is
> not reason enough to not use them.
Using raw offsets on the block device (rather than the partition)
won't work for most existing devices and boot firmware out there. They
always reference the partition, usually by the name of a GPT
partition (but sometimes also PARTUUID or even PARTNO) which is then
used in the exact same way as an MTD partition or UBI volume would be
on devices with NOR or NAND flash. Just on eMMC we usually use a GPT
or MBR partition table rather than defining partitions in DT or cmdline,
which is rather rare (for historic reasons, I suppose, but it is what it
is now).
Depending on the eMMC chip used, that partition may not even be at the
same offset for different batches of the same device and hence I'd
like to just do it in the same way vendor firmware does it as well.
Chad of Adtran has previously confirmed that [1], which was the
positive feedback I was refering to. Other vendors like GL-iNet or
Netgear are doing the exact same thing.
As of now, we support this in OpenWrt by adding a lot of
board-specific knowledge to userland, which is ugly and also prevents
using things like PXE-initiated nfsroot on those devices.
The purpose of this series is to be able to properly support such devices
(ie. practially all consumer-grade routers out there using an eMMC for
storing firmware).
Also, those devices have enough resources to run a general purpose
distribution like Debian instead of OpenWrt, and all the userland
hacks to set MAC addresses and extract WiFi-EEPROM-data in a
board-specific ways will most certainly never find their way into
Debian. It's just not how embedded Linux works, unless you are looking
only at the RaspberryPi which got that data stored in a textfile
which is shipped by the distribution -- something very weird and very
different from literally all of-the-shelf routers, access-points or
switches I have ever seen (and I've seen many). Maybe Felix who has
seen even more of them can tell us more about that.
[1]: https://patchwork.kernel.org/project/linux-block/patch/f70bb480aef6f55228a25ce20ff0e88e670e1b70.1709667858.git.daniel@makrotopia.org/#25756072
On Mon, Mar 25, 2024 at 10:12:59AM -0500, Rob Herring wrote:
> On Thu, Mar 21, 2024 at 07:31:48PM +0000, Daniel Golle 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 a block device 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.
> >
> > Overall, this enables uniform handling across practially all flash
> > storage types used for this purpose (MTD, UBI, and now also MMC).
> >
> > As part of this series it was necessary to define a device tree schema
> > for block devices and partitions on them, which (similar to how it now
> > works also for UBI volumes) can be matched by one or more properties.
> >
> > ---
> > This series has previously been submitted as RFC on July 19th 2023[1]
> > and most of the basic idea did not change since. Another round of RFC
> > was submitted on March 5th 2024[2] which has received overall positive
> > feedback and only minor corrections have been done since (see
> > changelog below).
>
> Also, please version your patches. 'RFC' is a tag, not a version. v1 was
> July. v2 was March 5th. This is v3.
According to "Submitting patches: the essential guide to getting your
code into the kernel" [1] a version is also a tag.
Quote:
Common tags might include a version descriptor if the [sic] multiple
versions of the patch have been sent out in response to comments
(i.e., “v1, v2, v3”), or “RFC” to indicate a request for comments.
Maybe this should be clarified, exclusive or inclusive "or" is up to
the reader to interpret at this point, and I've often seen RFC, RFCv2,
v1, v2, ... as a sequence of tags applied for the same series, which
is why I followed what I used to believe was the most common
interpretation of the guidelines.
In any way, thank you for pointing it out, I assume the next iteration
should then be v4.
[1]: https://docs.kernel.org/process/submitting-patches.html
On Thu, Mar 21, 2024 at 07:31:48PM +0000, Daniel Golle 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 a block device 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.
>
> Overall, this enables uniform handling across practially all flash
> storage types used for this purpose (MTD, UBI, and now also MMC).
>
> As part of this series it was necessary to define a device tree schema
> for block devices and partitions on them, which (similar to how it now
> works also for UBI volumes) can be matched by one or more properties.
>
> ---
> This series has previously been submitted as RFC on July 19th 2023[1]
> and most of the basic idea did not change since. Another round of RFC
> was submitted on March 5th 2024[2] which has received overall positive
> feedback and only minor corrections have been done since (see
> changelog below).
I don't recall giving positive feedback.
I still think this should use offsets rather than partition specific
information. Not wanting to have to update the offsets if they change is
not reason enough to not use them.
Rob
+boot-architecture list
On Mon, Mar 25, 2024 at 03:38:19PM +0000, Daniel Golle wrote:
> On Mon, Mar 25, 2024 at 10:10:46AM -0500, Rob Herring wrote:
> > On Thu, Mar 21, 2024 at 07:31:48PM +0000, Daniel Golle 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 a block device 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.
> > >
> > > Overall, this enables uniform handling across practially all flash
> > > storage types used for this purpose (MTD, UBI, and now also MMC).
> > >
> > > As part of this series it was necessary to define a device tree schema
> > > for block devices and partitions on them, which (similar to how it now
> > > works also for UBI volumes) can be matched by one or more properties.
> > >
> > > ---
> > > This series has previously been submitted as RFC on July 19th 2023[1]
> > > and most of the basic idea did not change since. Another round of RFC
> > > was submitted on March 5th 2024[2] which has received overall positive
> > > feedback and only minor corrections have been done since (see
> > > changelog below).
> >
> > I don't recall giving positive feedback.
> >
> > I still think this should use offsets rather than partition specific
> > information. Not wanting to have to update the offsets if they change is
> > not reason enough to not use them.
>
> Using raw offsets on the block device (rather than the partition)
> won't work for most existing devices and boot firmware out there. They
> always reference the partition, usually by the name of a GPT
> partition (but sometimes also PARTUUID or even PARTNO) which is then
> used in the exact same way as an MTD partition or UBI volume would be
> on devices with NOR or NAND flash.
MTD normally uses offsets hence why I'd like some alignment. UBI is
special because raw NAND is, well, special.
> Just on eMMC we usually use a GPT
> or MBR partition table rather than defining partitions in DT or cmdline,
> which is rather rare (for historic reasons, I suppose, but it is what it
> is now).
Yes, I understand how eMMC works. I don't understand why if you have
part #, uuid, or name you can't get to the offset or vice-versa. You
need only 1 piece of identification to map partition table entries to DT
nodes. Sure, offsets can change, but surely the firmware can handle
adjusting the DT?
An offset would also work for the case of random firmware data on the
disk that may or may not have a partition associated with it. There are
certainly cases of that. I don't think we have much of a solution for
that other than trying to educate vendors to not do that or OS
installers only supporting installing to something other than eMMC. This
is something EBBR[1] is trying to address.
> Depending on the eMMC chip used, that partition may not even be at the
> same offset for different batches of the same device and hence I'd
> like to just do it in the same way vendor firmware does it as well.
Often vendor firmware is not a model to follow...
> Chad of Adtran has previously confirmed that [1], which was the
> positive feedback I was refering to. Other vendors like GL-iNet or
> Netgear are doing the exact same thing.
>
> As of now, we support this in OpenWrt by adding a lot of
> board-specific knowledge to userland, which is ugly and also prevents
> using things like PXE-initiated nfsroot on those devices.
>
> The purpose of this series is to be able to properly support such devices
> (ie. practially all consumer-grade routers out there using an eMMC for
> storing firmware).
>
> Also, those devices have enough resources to run a general purpose
> distribution like Debian instead of OpenWrt, and all the userland
> hacks to set MAC addresses and extract WiFi-EEPROM-data in a
> board-specific ways will most certainly never find their way into
> Debian. It's just not how embedded Linux works, unless you are looking
> only at the RaspberryPi which got that data stored in a textfile
> which is shipped by the distribution -- something very weird and very
> different from literally all of-the-shelf routers, access-points or
> switches I have ever seen (and I've seen many). Maybe Felix who has
> seen even more of them can tell us more about that.
General purpose distros want to partition the disk themselves. Adding
anything to the DT for disk partitions would require the installer to be
aware of it. There's various distro folks on the boot-arch list, so
maybe one of them can comment.
Rob
[1] https://arm-software.github.io/ebbr/index.html#document-chapter4-firmware-media
Hi Rob,
On Tue, Mar 26, 2024 at 03:24:49PM -0500, Rob Herring wrote:
> +boot-architecture list
Good idea, thank you :)
>
> On Mon, Mar 25, 2024 at 03:38:19PM +0000, Daniel Golle wrote:
> > On Mon, Mar 25, 2024 at 10:10:46AM -0500, Rob Herring wrote:
> > > On Thu, Mar 21, 2024 at 07:31:48PM +0000, Daniel Golle 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 a block device 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.
> > > >
> > > > Overall, this enables uniform handling across practially all flash
> > > > storage types used for this purpose (MTD, UBI, and now also MMC).
> > > >
> > > > As part of this series it was necessary to define a device tree schema
> > > > for block devices and partitions on them, which (similar to how it now
> > > > works also for UBI volumes) can be matched by one or more properties.
> > > >
> > > > ---
> > > > This series has previously been submitted as RFC on July 19th 2023[1]
> > > > and most of the basic idea did not change since. Another round of RFC
> > > > was submitted on March 5th 2024[2] which has received overall positive
> > > > feedback and only minor corrections have been done since (see
> > > > changelog below).
> > >
> > > I don't recall giving positive feedback.
> > >
> > > I still think this should use offsets rather than partition specific
> > > information. Not wanting to have to update the offsets if they change is
> > > not reason enough to not use them.
> >
> > Using raw offsets on the block device (rather than the partition)
> > won't work for most existing devices and boot firmware out there. They
> > always reference the partition, usually by the name of a GPT
> > partition (but sometimes also PARTUUID or even PARTNO) which is then
> > used in the exact same way as an MTD partition or UBI volume would be
> > on devices with NOR or NAND flash.
>
> MTD normally uses offsets hence why I'd like some alignment. UBI is
> special because raw NAND is, well, special.
I get the point and in a way this is also already intended and
supported by this series. You can already just add an 'nvmem-layout'
node directly to a disk device rather than to a partition and define a
layout in this way.
Making this useful in practice will require some improvements to the
nvmem system in Linux though, because that currently uses signed 32-bit
integers as addresses which is not sufficient for the size of the
user-part of an eMMC. However, that needs to be done then and should
of course not be read as an excuse.
>
> > Just on eMMC we usually use a GPT
> > or MBR partition table rather than defining partitions in DT or cmdline,
> > which is rather rare (for historic reasons, I suppose, but it is what it
> > is now).
>
> Yes, I understand how eMMC works. I don't understand why if you have
> part #, uuid, or name you can't get to the offset or vice-versa. You
> need only 1 piece of identification to map partition table entries to DT
> nodes.
Yes, either of them (or a combination) is fine. In practise I've mostly
seen PARTNAME as identifier used in userland scripts, and only adding
this for now will probably cover most devices (and existing boot firmware)
out there. Notable exceptions are devices which are using MBR partitions
because the BootROM expects the bootloader to be at the same block as
we would usually have the primary GPT. In this case we can only use the
PARTNO, of course, and it stinks.
MediaTek's MT7623A/N is such an example, but it's a slingly outdated
and pretty weird niche SoC I admit.
> Sure, offsets can change, but surely the firmware can handle
> adjusting the DT?
Future firmware may be able to do this, of course. Current existing
firmware already out there on devices such as the quite popular
GL.iNet MT-6000, Netgear's Orbi and Orbi Pro series as well as all
Adtran SmartRG devices does not. Updating or changing the boot
firmware of devices already out there is not intended and quite
challenging, and will make the device incompatible with its vendor
firmware. Hence it would be better to support replacing only the
Linux-based firmware (eg. with OpenWrt or even Debian or any
general-purpose Linux, the eMMC is large enough...) while not having
to touch the boot firmware (and risking to brick the device if that
goes wrong).
Personally, I'm rather burdened and unhappy with vendor attempts to
have the boot firmware mess around too much in (highly customized,
downstream) DT, it may look like a good solution at the moment, but
can totally become an obstacle in an unpredictable future (no offense
ASUS...)
>
> An offset would also work for the case of random firmware data on the
> disk that may or may not have a partition associated with it. There are
> certainly cases of that. I don't think we have much of a solution for
> that other than trying to educate vendors to not do that or OS
> installers only supporting installing to something other than eMMC. This
> is something EBBR[1] is trying to address.
Absolutely. Actually *early* GL-iNet devices did exactly that: Use the
eMMC boot hw-partitions to store boot firmware as well as MAC
addresses and potentially also Wi-Fi calibration data.
The MT-2500 is the example I'm aware of and got sitting on my desk for
testing with this very series (which allows to also reference eMMC
hardware partitions, see "[7/8] mmc: block: set fwnode of disk
devices").
Unfortunately later devices such the the flag-ship MT-6000 moved MAC
addresses and WiFi-EEPROMs into a GPT partition on the user-part of
the eMMC.
>
> > Depending on the eMMC chip used, that partition may not even be at the
> > same offset for different batches of the same device and hence I'd
> > like to just do it in the same way vendor firmware does it as well.
>
> Often vendor firmware is not a model to follow...
I totally agree. However, I don't see a good reason for not supporting
those network-appliance-type embedded devices which even ship with
(outdated, downstream) Linux by default while going through great
lengths for things like broken ACPI tables in many laptops which
require lots of work-arounds to have features like suspend-to-disk
working, or even be able to run Linux at all.
>
> > Chad of Adtran has previously confirmed that [1], which was the
> > positive feedback I was refering to. Other vendors like GL-iNet or
> > Netgear are doing the exact same thing.
> >
> > As of now, we support this in OpenWrt by adding a lot of
> > board-specific knowledge to userland, which is ugly and also prevents
> > using things like PXE-initiated nfsroot on those devices.
> >
> > The purpose of this series is to be able to properly support such devices
> > (ie. practially all consumer-grade routers out there using an eMMC for
> > storing firmware).
> >
> > Also, those devices have enough resources to run a general purpose
> > distribution like Debian instead of OpenWrt, and all the userland
> > hacks to set MAC addresses and extract WiFi-EEPROM-data in a
> > board-specific ways will most certainly never find their way into
> > Debian. It's just not how embedded Linux works, unless you are looking
> > only at the RaspberryPi which got that data stored in a textfile
> > which is shipped by the distribution -- something very weird and very
> > different from literally all of-the-shelf routers, access-points or
> > switches I have ever seen (and I've seen many). Maybe Felix who has
> > seen even more of them can tell us more about that.
>
> General purpose distros want to partition the disk themselves. Adding
> anything to the DT for disk partitions would require the installer to be
> aware of it. There's various distro folks on the boot-arch list, so
> maybe one of them can comment.
Usually the installers are already aware to not touch partitions when
unaware of their purpose. Repartitioning the disk from scratch is not
what (modern) distributions are doing, at least the EFI System
partition is kept, as well as typical rescue/recovery partitions many
vendors put on their (Windows, Mac) laptops to allow to "factory
reset" them.
Installers usually offer to replace (or resize) the "large" partition
used by the currently installed OS instead.
And well, the DT reference to a partition holding e.g. MAC addresses
does make the installer aware of it, obviously.
Thank you for the constructive debate!
Cheers
Daniel
>
> Rob
>
> [1] https://arm-software.github.io/ebbr/index.html#document-chapter4-firmware-media
On Tue, Mar 26, 2024 at 4:29 PM Daniel Golle <[email protected]> wrote:
>
> Hi Rob,
>
> On Tue, Mar 26, 2024 at 03:24:49PM -0500, Rob Herring wrote:
> > +boot-architecture list
>
> Good idea, thank you :)
Now really adding it. :(
Will reply to rest later.
> >
> > On Mon, Mar 25, 2024 at 03:38:19PM +0000, Daniel Golle wrote:
> > > On Mon, Mar 25, 2024 at 10:10:46AM -0500, Rob Herring wrote:
> > > > On Thu, Mar 21, 2024 at 07:31:48PM +0000, Daniel Golle 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 a block device 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.
> > > > >
> > > > > Overall, this enables uniform handling across practially all flash
> > > > > storage types used for this purpose (MTD, UBI, and now also MMC).
> > > > >
> > > > > As part of this series it was necessary to define a device tree schema
> > > > > for block devices and partitions on them, which (similar to how it now
> > > > > works also for UBI volumes) can be matched by one or more properties.
> > > > >
> > > > > ---
> > > > > This series has previously been submitted as RFC on July 19th 2023[1]
> > > > > and most of the basic idea did not change since. Another round of RFC
> > > > > was submitted on March 5th 2024[2] which has received overall positive
> > > > > feedback and only minor corrections have been done since (see
> > > > > changelog below).
> > > >
> > > > I don't recall giving positive feedback.
> > > >
> > > > I still think this should use offsets rather than partition specific
> > > > information. Not wanting to have to update the offsets if they change is
> > > > not reason enough to not use them.
> > >
> > > Using raw offsets on the block device (rather than the partition)
> > > won't work for most existing devices and boot firmware out there. They
> > > always reference the partition, usually by the name of a GPT
> > > partition (but sometimes also PARTUUID or even PARTNO) which is then
> > > used in the exact same way as an MTD partition or UBI volume would be
> > > on devices with NOR or NAND flash.
> >
> > MTD normally uses offsets hence why I'd like some alignment. UBI is
> > special because raw NAND is, well, special.
>
> I get the point and in a way this is also already intended and
> supported by this series. You can already just add an 'nvmem-layout'
> node directly to a disk device rather than to a partition and define a
> layout in this way.
>
> Making this useful in practice will require some improvements to the
> nvmem system in Linux though, because that currently uses signed 32-bit
> integers as addresses which is not sufficient for the size of the
> user-part of an eMMC. However, that needs to be done then and should
> of course not be read as an excuse.
>
> >
> > > Just on eMMC we usually use a GPT
> > > or MBR partition table rather than defining partitions in DT or cmdline,
> > > which is rather rare (for historic reasons, I suppose, but it is what it
> > > is now).
> >
> > Yes, I understand how eMMC works. I don't understand why if you have
> > part #, uuid, or name you can't get to the offset or vice-versa. You
> > need only 1 piece of identification to map partition table entries to DT
> > nodes.
>
> Yes, either of them (or a combination) is fine. In practise I've mostly
> seen PARTNAME as identifier used in userland scripts, and only adding
> this for now will probably cover most devices (and existing boot firmware)
> out there. Notable exceptions are devices which are using MBR partitions
> because the BootROM expects the bootloader to be at the same block as
> we would usually have the primary GPT. In this case we can only use the
> PARTNO, of course, and it stinks.
> MediaTek's MT7623A/N is such an example, but it's a slingly outdated
> and pretty weird niche SoC I admit.
>
> > Sure, offsets can change, but surely the firmware can handle
> > adjusting the DT?
>
> Future firmware may be able to do this, of course. Current existing
> firmware already out there on devices such as the quite popular
> GL.iNet MT-6000, Netgear's Orbi and Orbi Pro series as well as all
> Adtran SmartRG devices does not. Updating or changing the boot
> firmware of devices already out there is not intended and quite
> challenging, and will make the device incompatible with its vendor
> firmware. Hence it would be better to support replacing only the
> Linux-based firmware (eg. with OpenWrt or even Debian or any
> general-purpose Linux, the eMMC is large enough...) while not having
> to touch the boot firmware (and risking to brick the device if that
> goes wrong).
>
> Personally, I'm rather burdened and unhappy with vendor attempts to
> have the boot firmware mess around too much in (highly customized,
> downstream) DT, it may look like a good solution at the moment, but
> can totally become an obstacle in an unpredictable future (no offense
> ASUS...)
>
> >
> > An offset would also work for the case of random firmware data on the
> > disk that may or may not have a partition associated with it. There are
> > certainly cases of that. I don't think we have much of a solution for
> > that other than trying to educate vendors to not do that or OS
> > installers only supporting installing to something other than eMMC. This
> > is something EBBR[1] is trying to address.
>
> Absolutely. Actually *early* GL-iNet devices did exactly that: Use the
> eMMC boot hw-partitions to store boot firmware as well as MAC
> addresses and potentially also Wi-Fi calibration data.
>
> The MT-2500 is the example I'm aware of and got sitting on my desk for
> testing with this very series (which allows to also reference eMMC
> hardware partitions, see "[7/8] mmc: block: set fwnode of disk
> devices").
> Unfortunately later devices such the the flag-ship MT-6000 moved MAC
> addresses and WiFi-EEPROMs into a GPT partition on the user-part of
> the eMMC.
>
> >
> > > Depending on the eMMC chip used, that partition may not even be at the
> > > same offset for different batches of the same device and hence I'd
> > > like to just do it in the same way vendor firmware does it as well.
> >
> > Often vendor firmware is not a model to follow...
>
> I totally agree. However, I don't see a good reason for not supporting
> those network-appliance-type embedded devices which even ship with
> (outdated, downstream) Linux by default while going through great
> lengths for things like broken ACPI tables in many laptops which
> require lots of work-arounds to have features like suspend-to-disk
> working, or even be able to run Linux at all.
>
> >
> > > Chad of Adtran has previously confirmed that [1], which was the
> > > positive feedback I was refering to. Other vendors like GL-iNet or
> > > Netgear are doing the exact same thing.
> > >
> > > As of now, we support this in OpenWrt by adding a lot of
> > > board-specific knowledge to userland, which is ugly and also prevents
> > > using things like PXE-initiated nfsroot on those devices.
> > >
> > > The purpose of this series is to be able to properly support such devices
> > > (ie. practially all consumer-grade routers out there using an eMMC for
> > > storing firmware).
> > >
> > > Also, those devices have enough resources to run a general purpose
> > > distribution like Debian instead of OpenWrt, and all the userland
> > > hacks to set MAC addresses and extract WiFi-EEPROM-data in a
> > > board-specific ways will most certainly never find their way into
> > > Debian. It's just not how embedded Linux works, unless you are looking
> > > only at the RaspberryPi which got that data stored in a textfile
> > > which is shipped by the distribution -- something very weird and very
> > > different from literally all of-the-shelf routers, access-points or
> > > switches I have ever seen (and I've seen many). Maybe Felix who has
> > > seen even more of them can tell us more about that.
> >
> > General purpose distros want to partition the disk themselves. Adding
> > anything to the DT for disk partitions would require the installer to be
> > aware of it. There's various distro folks on the boot-arch list, so
> > maybe one of them can comment.
>
> Usually the installers are already aware to not touch partitions when
> unaware of their purpose. Repartitioning the disk from scratch is not
> what (modern) distributions are doing, at least the EFI System
> partition is kept, as well as typical rescue/recovery partitions many
> vendors put on their (Windows, Mac) laptops to allow to "factory
> reset" them.
>
> Installers usually offer to replace (or resize) the "large" partition
> used by the currently installed OS instead.
>
> And well, the DT reference to a partition holding e.g. MAC addresses
> does make the installer aware of it, obviously.
>
>
> Thank you for the constructive debate!
>
>
> Cheers
>
>
> Daniel
>
>
> >
> > Rob
> >
> > [1] https://arm-software.github.io/ebbr/index.html#document-chapter4-firmware-media
On Fri, Mar 22, 2024 at 12:22:32PM -0700, Bart Van Assche wrote:
> On 3/22/24 11:07, Daniel Golle wrote:
> > On Fri, Mar 22, 2024 at 10:49:48AM -0700, Bart Van Assche wrote:
> > > On 3/21/24 12:33, Daniel Golle wrote:
> > > > enum {
> > > > GENHD_FL_REMOVABLE = 1 << 0,
> > > > GENHD_FL_HIDDEN = 1 << 1,
> > > > GENHD_FL_NO_PART = 1 << 2,
> > > > + GENHD_FL_NVMEM = 1 << 3,
> > > > };
> > >
> > > What would break if this flag wouldn't exist?
> >
> > As both, MTD and UBI already act as NVMEM providers themselves, once
> > the user creates a ubiblock device or got CONFIG_MTD_BLOCK=y set in their
> > kernel configuration, we would run into problems because both, the block
> > layer as well as MTD or UBI would try to be an NVMEM provider for the same
> > device tree node.
>
> Why would both MTD and UBI try to be an NVMEM provider for the same
> device tree node?
I didn't mean that both MTD and UBI would **simultanously** try to act
as NVMEM providers for the same device tree node. What I meant was
that either of them can act as an NVMEM provider while at the same time
also providing an emulated block device (mtdblock xor ubiblock).
Hence those emulated block devices will have to be excluded from acting
as NVMEM providers. In this patch I suggest to do this by opt-in of
block drivers which should potentially provide NVMEM (typically mmcblk).
I apologize for the confusion and assume that wasn't clear from the
wording I've used. I hope it's more clear now.
Alternatively it could also be solved via opt-out of ubiblock and
mtdblock devices using the inverted flag (GENHD_FL_NO_NVMEM) --
however, this has previously been criticized and I was asked to rather
make it opt-in.[1]
> Why can't this patch series be implemented such that
> a partition UUID occurs in the device tree and such that other code
> scans for that partition UUID?
This is actually one way this very series allows one to handle this:
by identifying a partition using its partuuid.
However, it's also quite common that the MMC boot **hardware**
partitions are used to store MAC addresses and/or Wi-Fi calibration
data. In this case there is no partition table and the NVMEM provider
has to act directly on the whole disk device (which is only a few
megabytes in size in case of those mmcblkXbootY devices and never has
a partition table).
[1]: https://patchwork.kernel.org/comment/25432948/