[PATCH v3 4/7] block: implement NVMEM provider
Loic Poulain
loic.poulain at oss.qualcomm.com
Mon Jun 8 06:00:51 PDT 2026
On Mon, Jun 8, 2026 at 1:17 PM Bartosz Golaszewski <brgl at kernel.org> wrote:
>
> On Mon, 8 Jun 2026 12:50:41 +0200, Loic Poulain
> <loic.poulain at oss.qualcomm.com> said:
> > From: Daniel Golle <daniel at makrotopia.org>
> >
> > 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 <daniel at makrotopia.org>
> > Co-developed-by: Loic Poulain <loic.poulain at oss.qualcomm.com>
> > Signed-off-by: Loic Poulain <loic.poulain at oss.qualcomm.com>
> > ---
> > 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 <daniel at makrotopia.org>
> > + * 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
More information about the ath10k
mailing list