On Mon, Jun 8, 2026 at 1:17 PM Bartosz Golaszewski <[email protected]> wrote:
>
> On Mon, 8 Jun 2026 12:50:41 +0200, Loic Poulain
> <[email protected]> said:
> > From: Daniel Golle <[email protected]>
> >
> > 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.
> >
> > To safely defer the freeing of the provider private data until all
> > consumers have released their cells, a nvmem_dev() accessor is added to
> > the NVMEM core to expose the struct device embedded in struct nvmem_device.
> > This allows registering a devm action on the nvmem device itself, ensuring
> > the private data outlives any active cell references.
> >
> > Signed-off-by: Daniel Golle <[email protected]>
> > Co-developed-by: Loic Poulain <[email protected]>
> > Signed-off-by: Loic Poulain <[email protected]>
> > ---
> > block/Kconfig | 9 +++
> > block/Makefile | 1 +
> > block/blk-nvmem.c | 171
> > +++++++++++++++++++++++++++++++++++++++++
> > drivers/nvmem/core.c | 13 ++++
> > include/linux/nvmem-consumer.h | 6 ++
> > 5 files changed, 200 insertions(+)
> >
> > diff --git a/block/Kconfig b/block/Kconfig
> > index
> > 15027963472d7b40e27b9097a5993c457b5b3054..0b33747e16dc33473683706f75c92bdf8b648f7c
> > 100644
> > --- a/block/Kconfig
> > +++ b/block/Kconfig
> > @@ -209,6 +209,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 providers,
> > + typically used with eMMC to store MAC addresses or Wi-Fi
> > + calibration data on embedded devices.
> > +
> > source "block/partitions/Kconfig"
> >
> > config BLK_PM
> > diff --git a/block/Makefile b/block/Makefile
> > index
> > 7dce2e44276c4274c11a0a61121c83d9c43d6e0c..d7ac389e71902bc091a8800ea266190a43b3e63d
> > 100644
> > --- a/block/Makefile
> > +++ b/block/Makefile
> > @@ -36,3 +36,4 @@ obj-$(CONFIG_BLK_INLINE_ENCRYPTION) += blk-crypto.o
> > blk-crypto-profile.o \
> > blk-crypto-sysfs.o
> > obj-$(CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK) += blk-crypto-fallback.o
> > obj-$(CONFIG_BLOCK_HOLDER_DEPRECATED) += holder.o
> > +obj-$(CONFIG_BLK_NVMEM) += blk-nvmem.o
> > diff --git a/block/blk-nvmem.c b/block/blk-nvmem.c
> > new file mode 100644
> > index
> > 0000000000000000000000000000000000000000..99c7728fb7bccdc2216780a73a89a9210f925049
> > --- /dev/null
> > +++ b/block/blk-nvmem.c
> > @@ -0,0 +1,171 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * block device NVMEM provider
> > + *
> > + * Copyright (c) 2024 Daniel Golle <[email protected]>
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + *
> > + * Useful on devices using a partition on an eMMC for MAC addresses or
> > + * Wi-Fi calibration EEPROM data.
> > + */
> > +
> > +#include <linux/cleanup.h>
> > +#include <linux/mutex.h>
> > +#include <linux/nvmem-provider.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/of.h>
> > +#include <linux/pagemap.h>
> > +#include <linux/property.h>
> > +
> > +#include "blk.h"
> > +
> > +
>
> Stray newline?
>
> > +/* List of all NVMEM devices */
> > +static LIST_HEAD(nvmem_devices);
> > +static DEFINE_MUTEX(devices_mutex);
> > +
> > +struct blk_nvmem {
> > + struct nvmem_device *nvmem;
> > + dev_t devt;
> > + 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;
> > + struct blk_nvmem *bnv = priv;
> > + size_t bytes_left = bytes;
> > + struct file *bdev_file;
> > + loff_t pos = from;
> > + int ret = 0;
> > +
> > + bdev_file = bdev_file_open_by_dev(bnv->devt, mode, priv, NULL);
> > + if (!bdev_file)
> > + return -ENODEV;
> > +
> > + if (IS_ERR(bdev_file))
> > + return PTR_ERR(bdev_file);
> > +
> > + while (bytes_left) {
> > + pgoff_t f_index = pos >> PAGE_SHIFT;
> > + struct folio *folio;
> > + size_t folio_off;
> > + size_t to_read;
> > +
> > + folio = read_mapping_folio(bdev_file->f_mapping, f_index,
> > NULL);
> > + if (IS_ERR(folio)) {
> > + ret = PTR_ERR(folio);
> > + goto err_release_bdev;
> > + }
> > +
> > + folio_off = offset_in_folio(folio, pos);
> > + to_read = min(bytes_left, folio_size(folio) - folio_off);
> > + memcpy_from_folio(val, folio, folio_off, to_read);
> > + pos += to_read;
> > + bytes_left -= to_read;
> > + val += to_read;
> > + folio_put(folio);
> > + }
> > +
> > +err_release_bdev:
> > + fput(bdev_file);
>
> There's a __free() action defined in linux/file.h so you can use:
>
> struct file *bdev_file __free(fput) = ...
>
> and avoid this label.
Ok, thanks, will use.
>
> > +
> > + return ret;
> > +}
> > +
> > +static int blk_nvmem_register(struct device *dev)
> > +{
> > + struct device_node *child, *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 */
> > + child = of_get_child_by_name(np, "nvmem-layout");
> > + if (!child)
> > + return 0;
> > + of_node_put(child);
>
> Same as above, can be:
>
> struct device_node *child __free(device_node) == ...
Ack.
>
> > +
> > + /*
> > + * skip block device too large to be represented as NVMEM devices,
> > + * the NVMEM reg_read callback uses an unsigned int offset
> > + */
> > + if (bdev_nr_bytes(bdev) > UINT_MAX)
> > + return -EFBIG;
>
> This will mean a failed probe. Wouldn't it be better to use -ENODEV?
That would indeed be an appropriate error.
>
> > +
> > + bnv = kzalloc_obj(*bnv);
> > + 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->devt = bdev->bd_device.devt;
> > + 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);
> > + }
> > +
> > + scoped_guard(mutex, &devices_mutex)
> > + list_add_tail(&bnv->list, &nvmem_devices);
>
> I'm not sure I understand the need to store these? Whatever you need to do in
> remove() can be scheduled in a devres action here.
I think the devm_ approach would work fine in practice. The only
difference is that NVMEM unregistration would be delayed from
device_del() to device_release(), but during that window any read
attempt would simply return -ENODEV, so there is no real race or
safety concern AFAIU. I guess the explicit list was initially kept to
mirror the add_dev/remove_dev symmetry of the class interface. But,
except if there is no strong technical argument against devm_, I will
switch to that simpler approach in the next version.
Daniel, feel free to nack or ask for authorship removal if needed.
This patch submitted in your name has accumulated enough changes since
the original submission that the current shape may no longer reflect
your intent.
>
> > +
> > + return 0;
> > +}
> > +
> > +static void blk_nvmem_unregister(struct device *dev)
> > +{
> > + struct blk_nvmem *bnv_c, *bnv_t, *bnv = NULL;
> > +
> > + scoped_guard(mutex, &devices_mutex) {
> > + list_for_each_entry_safe(bnv_c, bnv_t, &nvmem_devices, list) {
> > + if (bnv_c->devt == dev_to_bdev(dev)->bd_device.devt) {
> > + bnv = bnv_c;
> > + list_del(&bnv->list);
> > + break;
> > + }
> > + }
> > +
> > + if (!bnv)
> > + return;
> > + }
> > +
> > + 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);
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index
> > 311cb2e5a5c02d2c6979d7c9bbb7f94abdfbdad1..ee3481229c20b3063c86d0dd66aabbf6b5e29169
> > 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -2154,6 +2154,19 @@ const char *nvmem_dev_name(struct nvmem_device
> > *nvmem)
> > }
> > EXPORT_SYMBOL_GPL(nvmem_dev_name);
> >
> > +/**
> > + * nvmem_dev() - Get the struct device of a given nvmem device.
> > + *
> > + * @nvmem: nvmem device.
> > + *
> > + * Return: pointer to the struct device of the nvmem device.
> > + */
> > +struct device *nvmem_dev(struct nvmem_device *nvmem)
> > +{
> > + return &nvmem->dev;
> > +}
> > +EXPORT_SYMBOL_GPL(nvmem_dev);
>
> This should still be a separate patch.
Well yes, actually I should even remove this as this is no more needed.
Regards,
Loic
