[PATCH v2 4/8] block: implement NVMEM provider
Bartosz Golaszewski
brgl at kernel.org
Mon May 11 04:27:44 PDT 2026
On Thu, 7 May 2026 17:24:39 +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>
> ---
...
> diff --git a/block/blk-nvmem.c b/block/blk-nvmem.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..96c0ffc51b1862a75644f3f94add35d59577d86b
> --- /dev/null
> +++ b/block/blk-nvmem.c
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * block device NVMEM provider
> + *
> + * Copyright (c) 2024 Daniel Golle <daniel at makrotopia.org>
> + *
> + * Useful on devices using a partition on an eMMC for MAC addresses or
> + * Wi-Fi calibration EEPROM data.
> + */
> +
> +#include "blk.h"
Local includes typically go after system-wide ones. I thought that maybe it's
a block subsystem thing but no, it too goes after here.
> +#include <linux/nvmem-provider.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/of.h>
> +#include <linux/pagemap.h>
> +#include <linux/property.h>
> +
> +static void blk_nvmem_free(void *data)
> +{
> + kfree(data);
> +}
> +
> +/* 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;
> + bool removed;
> + 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;
> +
> + if (bnv->removed)
> + return -ENODEV;
> +
> + 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) {
> + 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 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_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);
> + }
> +
> + /*
> + * Free bnv only when the nvmem device is fully released (i.e. when
> + * its kref hits zero), not at unregister time. This prevents a
> + * use-after-free if a consumer still holds an nvmem_cell reference
> + * when the block device is removed: nvmem_unregister() only does a
> + * kref_put(), so reg_read could still be called with bnv as priv
> + * until the last consumer drops its cell.
> + */
> + if (devm_add_action(nvmem_dev(bnv->nvmem), blk_nvmem_free, bnv)) {
> + nvmem_unregister(bnv->nvmem);
> + kfree(bnv);
> + return -ENOMEM;
> + }
Please take a look at the series[1] I sent, it seems to address this issue at
the nvmem core level. Help reviewing it is appreciated. :)
In any case: I don't think it will work the way you intend it to: the devres
action will be executed when the nvmem device is "unbound" not when it's
"released". Not only that but the device you retrieve here is not the "parent"
device that's actually bound to the driver but the "logical" nvmem device, the
nvmem core creates to back the struct nvmem_device's functionality and reference
counting. In other words: I believe the devres action will never be called.
In general, it's a very bad idea to schedule devres actions on devices you
don't control and in context different than at probe() time.
> +
> + 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->devt == dev_to_bdev(dev)->bd_device.devt) {
> + bnv = bnv_c;
> + break;
> + }
> + }
> +
> + if (!bnv) {
> + mutex_unlock(&devices_mutex);
> + return;
> + }
> +
> + list_del(&bnv->list);
> + mutex_unlock(&devices_mutex);
> + bnv->removed = true;
> + nvmem_unregister(bnv->nvmem);
> +}
> +
> +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);
> +
That should be done in a separate patch but see my response above.
> /**
> * nvmem_dev_size() - Get the size of a given nvmem device.
> *
> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
> index 34c0e58dfa26636d2804fcc7e0bc4a875ee73dae..ce387c89dc8e4bc1241f3b6f36be8c6c95e282ed 100644
> --- a/include/linux/nvmem-consumer.h
> +++ b/include/linux/nvmem-consumer.h
> @@ -82,6 +82,7 @@ int nvmem_device_cell_write(struct nvmem_device *nvmem,
>
> const char *nvmem_dev_name(struct nvmem_device *nvmem);
> size_t nvmem_dev_size(struct nvmem_device *nvmem);
> +struct device *nvmem_dev(struct nvmem_device *nvmem);
>
> void nvmem_add_cell_lookups(struct nvmem_cell_lookup *entries,
> size_t nentries);
> @@ -220,6 +221,11 @@ static inline const char *nvmem_dev_name(struct nvmem_device *nvmem)
> return NULL;
> }
>
> +static inline struct device *nvmem_dev(struct nvmem_device *nvmem)
> +{
> + return NULL;
> +}
> +
> static inline void
> nvmem_add_cell_lookups(struct nvmem_cell_lookup *entries, size_t nentries) {}
> static inline void
>
> --
> 2.34.1
>
>
Thanks,
Bartosz
[1] https://lore.kernel.org/all/20260429-nvmem-unbind-v3-0-2a694f95395b@oss.qualcomm.com/
More information about the ath10k
mailing list