[4/8] block: implement NVMEM provider

Commit Message

Daniel Golle March 21, 2024, 7:34 p.m. UTC

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 <[e-mail]>
---
 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

Comments

Bart Van Assche March 21, 2024, 7:44 p.m. UTC | #1

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:	[e-mail]
>   S:	Maintained
>   F:	drivers/mtd/devices/block2mtd.c
>   
> +BLOCK NVMEM DRIVER
> +M:	Daniel Golle <[e-mail]>
> +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.

Daniel Golle March 21, 2024, 8:22 p.m. UTC | #2

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:	[e-mail]
> >   S:	Maintained
> >   F:	drivers/mtd/devices/block2mtd.c
> > +BLOCK NVMEM DRIVER
> > +M:	Daniel Golle <[e-mail]>
> > +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.

Bart Van Assche March 22, 2024, 5:52 p.m. UTC | #3

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.

Daniel Golle March 22, 2024, 6:11 p.m. UTC | #4

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.

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 8c88f362feb55..242a0a139c00a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3662,6 +3662,11 @@  L:	[e-mail]
 S:	Maintained
 F:	drivers/mtd/devices/block2mtd.c
 
+BLOCK NVMEM DRIVER
+M:	Daniel Golle <[e-mail]>
+S:	Maintained
+F:	block/blk-nvmem.c
+
 BLUETOOTH DRIVERS
 M:	Marcel Holtmann <[e-mail]>
 M:	Luiz Augusto von Dentz <[e-mail]>
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 <[e-mail]>
+ *
+ * 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);