diff mbox series

[V2] nvmem: add driver handling U-Boot environment variables

Message ID 20220503165658.13932-1-zajec5@gmail.com (mailing list archive)
State New, archived
Headers show
Series [V2] nvmem: add driver handling U-Boot environment variables | expand

Commit Message

Rafał Miłecki May 3, 2022, 4:56 p.m. UTC
From: Rafał Miłecki <rafal@milecki.pl>

U-Boot stores its setup as environment variables. It's a list of
key-value pairs stored on flash device with a custom header.

This commit adds an NVMEM driver that:
1. Provides NVMEM access to environment vars binary data
2. Extracts variables as NVMEM cells

It can be used for:
1. Accessing env variables from user-space
2. Reading NVMEM cells by Linux drivers

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Drop ARCH_BCM4908 dependency as there are plenty architectures using
    U-Boot bootloader. Thanks Srinivas.
---
 MAINTAINERS                |   1 +
 drivers/nvmem/Kconfig      |  11 ++
 drivers/nvmem/Makefile     |   2 +
 drivers/nvmem/u-boot-env.c | 236 +++++++++++++++++++++++++++++++++++++
 4 files changed, 250 insertions(+)
 create mode 100644 drivers/nvmem/u-boot-env.c

Comments

Ahmad Fatoum May 4, 2022, 9:23 a.m. UTC | #1
Hello Rafał,

On 03.05.22 18:56, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> U-Boot stores its setup as environment variables. It's a list of
> key-value pairs stored on flash device with a custom header.
> 
> This commit adds an NVMEM driver that:
> 1. Provides NVMEM access to environment vars binary data
> 2. Extracts variables as NVMEM cells
> 
> It can be used for:
> 1. Accessing env variables from user-space

Is this already possible? The only interface I know of is the /nvmem
file in sysfs, but that one is not per cell, but per device.

> +	label = of_get_property(np->parent, "label", NULL);
> +	if (!label)
> +		label = np->parent->name;
> +
> +	priv->mtd = get_mtd_device_nm(label);
> +	if (IS_ERR(priv->mtd)) {
> +		dev_err(dev, "Failed to find \"%s\" MTD device: %ld\n", label, PTR_ERR(priv->mtd));
> +		return PTR_ERR(priv->mtd);
> +	}

I am trying to make sense of this using the binding, but I can't.
Do you have an example device tree fragment?

Cheers,
Ahmad
Rafał Miłecki May 5, 2022, 5:46 a.m. UTC | #2
On 4.05.2022 11:23, Ahmad Fatoum wrote:
> Hello Rafał,
> 
> On 03.05.22 18:56, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> U-Boot stores its setup as environment variables. It's a list of
>> key-value pairs stored on flash device with a custom header.
>>
>> This commit adds an NVMEM driver that:
>> 1. Provides NVMEM access to environment vars binary data
>> 2. Extracts variables as NVMEM cells
>>
>> It can be used for:
>> 1. Accessing env variables from user-space
> 
> Is this already possible? The only interface I know of is the /nvmem
> file in sysfs, but that one is not per cell, but per device.

Maybe that wasn't precise enough, I should probably write:
1. Parsing binary data from user-space

In future I'd like to extend U-Boot's "printenv" tool to support reading
env variables blob using Linux's sysfs as documented in the
Documentation/ABI/stable/sysfs-bus-nvmem


>> +	label = of_get_property(np->parent, "label", NULL);
>> +	if (!label)
>> +		label = np->parent->name;
>> +
>> +	priv->mtd = get_mtd_device_nm(label);
>> +	if (IS_ERR(priv->mtd)) {
>> +		dev_err(dev, "Failed to find \"%s\" MTD device: %ld\n", label, PTR_ERR(priv->mtd));
>> +		return PTR_ERR(priv->mtd);
>> +	}
> 
> I am trying to make sense of this using the binding, but I can't.
> Do you have an example device tree fragment?

This comes from unreleased yet board I'm working on.

It stores U-Boot env variables in the middle of U-Boot binary.

partitions {
	compatible = "fixed-partitions";
	#address-cells = <1>;
	#size-cells = <1>;

	partition@0 {
		label = "loader";
		reg = <0x0 0x100000>;

		partition@40000 {
			compatible = "u-boot,env";
			label = "u-boot-env";
			reg = <0x40000 0x4000>;
		};
	};

	partition@100000 {
		label = "image";
		reg = <0x100000 0x1fe00000>;
	};
};
Ahmad Fatoum May 6, 2022, 12:04 p.m. UTC | #3
Hello Rafał,

On 05.05.22 07:46, Rafał Miłecki wrote:
> On 4.05.2022 11:23, Ahmad Fatoum wrote:
>> Hello Rafał,
>>
>> On 03.05.22 18:56, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> U-Boot stores its setup as environment variables. It's a list of
>>> key-value pairs stored on flash device with a custom header.
>>>
>>> This commit adds an NVMEM driver that:
>>> 1. Provides NVMEM access to environment vars binary data
>>> 2. Extracts variables as NVMEM cells
>>>
>>> It can be used for:
>>> 1. Accessing env variables from user-space
>>
>> Is this already possible? The only interface I know of is the /nvmem
>> file in sysfs, but that one is not per cell, but per device.
> 
> Maybe that wasn't precise enough, I should probably write:
> 1. Parsing binary data from user-space
> 
> In future I'd like to extend U-Boot's "printenv" tool to support reading
> env variables blob using Linux's sysfs as documented in the
> Documentation/ABI/stable/sysfs-bus-nvmem

So, would you use this interface just to save fw_printenv the hassle
of finding the environment (but redoing parsing) or do you intend
to preprocess the data too? (e.g. only show the active environment) 

For your use case, it sound like teaching NVMEM core to export
cells as binary sysfs files would be very useful.

>>> +    label = of_get_property(np->parent, "label", NULL);
>>> +    if (!label)
>>> +        label = np->parent->name;
>>> +
>>> +    priv->mtd = get_mtd_device_nm(label);
>>> +    if (IS_ERR(priv->mtd)) {
>>> +        dev_err(dev, "Failed to find \"%s\" MTD device: %ld\n", label, PTR_ERR(priv->mtd));
>>> +        return PTR_ERR(priv->mtd);
>>> +    }
>>
>> I am trying to make sense of this using the binding, but I can't.
>> Do you have an example device tree fragment?
> 
> This comes from unreleased yet board I'm working on.
> 
> It stores U-Boot env variables in the middle of U-Boot binary.

Huh, that's an odd layout. I am not sure whether of_get_property to
arrive at the parent is such a good idea though. Doesn't it enforce
a limitation that there must not exist two partitions with the same label?

Some systems can have a second recovery bootloader for example.
Given that these are device tree nodes, wouldn't it be possible
to find the MTD by device tree parent instead of via its label?

> partitions {
>     compatible = "fixed-partitions";
>     #address-cells = <1>;
>     #size-cells = <1>;
> 
>     partition@0 {
>         label = "loader";
>         reg = <0x0 0x100000>;
> 
>         partition@40000 {
>             compatible = "u-boot,env";
>             label = "u-boot-env";
>             reg = <0x40000 0x4000>;
>         };
>     };
> 
>     partition@100000 {
>         label = "image";
>         reg = <0x100000 0x1fe00000>;
>     };
> };


Thanks,
Ahmad
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 2869a958f5e4..368e532444d0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20193,6 +20193,7 @@  U-BOOT ENVIRONMENT VARIABLES
 M:	Rafał Miłecki <rafal@milecki.pl>
 S:	Maintained
 F:	Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
+F:	drivers/nvmem/u-boot-env.c
 
 UACCE ACCELERATOR FRAMEWORK
 M:	Zhangfei Gao <zhangfei.gao@linaro.org>
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
index 967d0084800e..eee3ff668629 100644
--- a/drivers/nvmem/Kconfig
+++ b/drivers/nvmem/Kconfig
@@ -337,4 +337,15 @@  config NVMEM_APPLE_EFUSES
 	  This driver can also be built as a module. If so, the module will
 	  be called nvmem-apple-efuses.
 
+config NVMEM_U_BOOT_ENV
+	tristate "U-Boot environment variables support"
+	depends on OF && MTD
+	select CRC32
+	help
+	  U-Boot stores its setup as environment variables. This driver adds
+	  support for verifying & exporting such data. It also exposes variables
+	  as NVMEM cells so they can be referenced by other drivers.
+
+	  If compiled as module it will be called nvmem_u-boot-env.
+
 endif
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
index 00e136a0a123..d201a02a09ec 100644
--- a/drivers/nvmem/Makefile
+++ b/drivers/nvmem/Makefile
@@ -67,3 +67,5 @@  obj-$(CONFIG_NVMEM_SUNPLUS_OCOTP)	+= nvmem_sunplus_ocotp.o
 nvmem_sunplus_ocotp-y		:= sunplus-ocotp.o
 obj-$(CONFIG_NVMEM_APPLE_EFUSES)	+= nvmem-apple-efuses.o
 nvmem-apple-efuses-y 		:= apple-efuses.o
+obj-$(CONFIG_NVMEM_U_BOOT_ENV)	+= nvmem_u-boot-env.o
+nvmem_u-boot-env-y		:= u-boot-env.o
diff --git a/drivers/nvmem/u-boot-env.c b/drivers/nvmem/u-boot-env.c
new file mode 100644
index 000000000000..c7945d7d5750
--- /dev/null
+++ b/drivers/nvmem/u-boot-env.c
@@ -0,0 +1,236 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 Rafał Miłecki <rafal@milecki.pl>
+ */
+
+#include <linux/crc32.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mtd/mtd.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/nvmem-provider.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+enum u_boot_env_format {
+	U_BOOT_FORMAT_SINGLE,
+	U_BOOT_FORMAT_REDUNDANT,
+};
+
+struct u_boot_env {
+	struct device *dev;
+	enum u_boot_env_format format;
+
+	/* Parent device */
+	struct mtd_info *mtd;
+	size_t offset;
+	size_t size;
+
+	/* Cells */
+	struct nvmem_cell_info *cells;
+	int ncells;
+};
+
+struct u_boot_env_image_single {
+	__le32 crc32;
+	uint8_t data[0];
+} __packed;
+
+struct u_boot_env_image_redundant {
+	__le32 crc32;
+	u8 mark;
+	uint8_t data[0];
+} __packed;
+
+static int u_boot_env_read(void *context, unsigned int offset, void *val,
+			   size_t bytes)
+{
+	struct u_boot_env *priv = context;
+	struct device *dev = priv->dev;
+	size_t bytes_read;
+	int err;
+
+	err = mtd_read(priv->mtd, priv->offset + offset, bytes, &bytes_read, val);
+	if (err && !mtd_is_bitflip(err)) {
+		dev_err(dev, "Failed to read from mtd: %d\n", err);
+		return err;
+	}
+
+	if (bytes_read != bytes) {
+		dev_err(dev, "Failed to read %zd bytes\n", bytes);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int u_boot_env_add_cells(struct u_boot_env *priv, uint8_t *buf,
+				size_t data_offset, size_t data_len)
+{
+	struct device *dev = priv->dev;
+	char *data = buf + data_offset;
+	char *var, *value, *eq;
+	int idx;
+
+	priv->ncells = 0;
+	for (var = data; var < data + data_len && *var; var += strlen(var) + 1)
+		priv->ncells++;
+
+	priv->cells = devm_kcalloc(dev, priv->ncells, sizeof(*priv->cells), GFP_KERNEL);
+	if (!priv->cells)
+		return -ENOMEM;
+
+	for (var = data, idx = 0;
+	     var < data + data_len && *var;
+	     var = value + strlen(value) + 1, idx++) {
+		eq = strchr(var, '=');
+		if (!eq)
+			break;
+		*eq = '\0';
+		value = eq + 1;
+
+		priv->cells[idx].name = devm_kstrdup(dev, var, GFP_KERNEL);
+		if (!priv->cells[idx].name)
+			return -ENOMEM;
+		priv->cells[idx].offset = data_offset + value - data;
+		priv->cells[idx].bytes = strlen(value);
+	}
+
+	if (WARN_ON(idx != priv->ncells))
+		priv->ncells = idx;
+
+	return 0;
+}
+
+static int u_boot_env_parse(struct u_boot_env *priv)
+{
+	struct device *dev = priv->dev;
+	size_t crc32_data_offset;
+	size_t crc32_data_len;
+	size_t crc32_offset;
+	size_t data_offset;
+	size_t data_len;
+	uint32_t crc32;
+	uint32_t calc;
+	size_t bytes;
+	uint8_t *buf;
+	int err;
+
+	buf = kcalloc(1, priv->size, GFP_KERNEL);
+	if (!buf) {
+		err = -ENOMEM;
+		goto err_out;
+	}
+
+	err = mtd_read(priv->mtd, priv->offset, priv->size, &bytes, buf);
+	if ((err && !mtd_is_bitflip(err)) || bytes != priv->size) {
+		dev_err(dev, "Failed to read from mtd: %d\n", err);
+		goto err_kfree;
+	}
+
+	switch (priv->format) {
+	case U_BOOT_FORMAT_SINGLE:
+		crc32_offset = offsetof(struct u_boot_env_image_single, crc32);
+		crc32_data_offset = offsetof(struct u_boot_env_image_single, data);
+		data_offset = offsetof(struct u_boot_env_image_single, data);
+		break;
+	case U_BOOT_FORMAT_REDUNDANT:
+		crc32_offset = offsetof(struct u_boot_env_image_redundant, crc32);
+		crc32_data_offset = offsetof(struct u_boot_env_image_redundant, mark);
+		data_offset = offsetof(struct u_boot_env_image_redundant, data);
+		break;
+	}
+	crc32 = le32_to_cpu(*(uint32_t *)(buf + crc32_offset));
+	crc32_data_len = priv->size - crc32_data_offset;
+	data_len = priv->size - data_offset;
+
+	calc = crc32(~0, buf + crc32_data_offset, crc32_data_len) ^ ~0L;
+	if (calc != crc32) {
+		dev_err(dev, "Invalid calculated CRC32: 0x%08x (expected: 0x%08x)\n", calc, crc32);
+		err = -EINVAL;
+		goto err_kfree;
+	}
+
+	buf[priv->size - 1] = '\0';
+	err = u_boot_env_add_cells(priv, buf, data_offset, data_len);
+	if (err)
+		dev_err(dev, "Failed to add cells: %d\n", err);
+
+err_kfree:
+	kfree(buf);
+err_out:
+	return err;
+}
+
+static const struct of_device_id u_boot_env_of_match_table[] = {
+	{ .compatible = "u-boot,env", .data = (void *)U_BOOT_FORMAT_SINGLE, },
+	{ .compatible = "u-boot,env-redundant-bool", .data = (void *)U_BOOT_FORMAT_REDUNDANT, },
+	{ .compatible = "u-boot,env-redundant-count", .data = (void *)U_BOOT_FORMAT_REDUNDANT, },
+	{},
+};
+
+static int u_boot_env_probe(struct platform_device *pdev)
+{
+	struct nvmem_config config = {
+		.name = "u-boot-env",
+		.reg_read = u_boot_env_read,
+	};
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	const struct of_device_id *of_id;
+	struct u_boot_env *priv;
+	const char *label;
+	int err;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+	priv->dev = dev;
+
+	of_id = of_match_device(u_boot_env_of_match_table, dev);
+	if (!of_id)
+		return -EINVAL;
+	priv->format = (uintptr_t)of_id->data;
+
+	if (of_property_read_u32(np, "reg", (u32 *)&priv->offset) ||
+	    of_property_read_u32_index(np, "reg", 1, (u32 *)&priv->size)) {
+		dev_err(dev, "Failed to read \"reg\" property\n");
+		return -EINVAL;
+	}
+
+	label = of_get_property(np->parent, "label", NULL);
+	if (!label)
+		label = np->parent->name;
+
+	priv->mtd = get_mtd_device_nm(label);
+	if (IS_ERR(priv->mtd)) {
+		dev_err(dev, "Failed to find \"%s\" MTD device: %ld\n", label, PTR_ERR(priv->mtd));
+		return PTR_ERR(priv->mtd);
+	}
+
+	err = u_boot_env_parse(priv);
+	if (err)
+		return err;
+
+	config.dev = dev;
+	config.cells = priv->cells;
+	config.ncells = priv->ncells;
+	config.priv = priv;
+	config.size = priv->size;
+
+	return PTR_ERR_OR_ZERO(devm_nvmem_register(dev, &config));
+}
+
+static struct platform_driver u_boot_env_driver = {
+	.probe = u_boot_env_probe,
+	.driver = {
+		.name = "u_boot_env",
+		.of_match_table = u_boot_env_of_match_table,
+	},
+};
+module_platform_driver(u_boot_env_driver);
+
+MODULE_AUTHOR("Rafał Miłecki");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(of, u_boot_env_of_match_table);