[PATCH 1/6] nvmem: core: add nvmem_dev_size() helper

From: Rafał Miłecki rafal@milecki.pl
This is required by layouts that need to read whole NVMEM space. It applies to NVMEM devices without hardcoded layout (like U-Boot environment data block).
Signed-off-by: Rafał Miłecki rafal@milecki.pl --- drivers/nvmem/core.c | 13 +++++++++++++ include/linux/nvmem-consumer.h | 1 + 2 files changed, 14 insertions(+)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index 1f05f0a50d86..81743ae8793b 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -2062,6 +2062,19 @@ void nvmem_del_cell_lookups(struct nvmem_cell_lookup *entries, size_t nentries) } EXPORT_SYMBOL_GPL(nvmem_del_cell_lookups);
+/** + * nvmem_dev_size() - Get the size of a given nvmem device. + * + * @nvmem: nvmem device. + * + * Return: size of the nvmem device. + */ +const size_t nvmem_dev_size(struct nvmem_device *nvmem) +{ + return nvmem->size; +} +EXPORT_SYMBOL_GPL(nvmem_dev_size); + /** * nvmem_dev_name() - Get the name of a given nvmem device. * diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h index fa030d93b768..d88294ddf562 100644 --- a/include/linux/nvmem-consumer.h +++ b/include/linux/nvmem-consumer.h @@ -78,6 +78,7 @@ ssize_t nvmem_device_cell_read(struct nvmem_device *nvmem, int nvmem_device_cell_write(struct nvmem_device *nvmem, struct nvmem_cell_info *info, void *buf);
+const size_t nvmem_dev_size(struct nvmem_device *nvmem); const char *nvmem_dev_name(struct nvmem_device *nvmem);
void nvmem_add_cell_lookups(struct nvmem_cell_lookup *entries,

From: Rafał Miłecki rafal@milecki.pl
Sometimes reading NVMEM cell value involves some data reformatting. It requires passing updated size value to the caller. Support that.
It's required e.g. to provide properly formatted MAC address in case it's stored in a non-binary format (e.g. using ASCII).
Signed-off-by: Rafał Miłecki rafal@milecki.pl --- drivers/nvmem/core.c | 5 +++-- drivers/nvmem/imx-ocotp.c | 6 +++--- drivers/nvmem/layouts/onie-tlv.c | 2 +- drivers/nvmem/layouts/sl28vpd.c | 4 ++-- include/linux/nvmem-provider.h | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index 81743ae8793b..f4040776e96c 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -1541,6 +1541,7 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem, struct nvmem_cell_entry *cell, void *buf, size_t *len, const char *id, int index) { + size_t bytes = cell->bytes; int rc;
rc = nvmem_reg_read(nvmem, cell->offset, buf, cell->bytes); @@ -1554,13 +1555,13 @@ static int __nvmem_cell_read(struct nvmem_device *nvmem,
if (cell->read_post_process) { rc = cell->read_post_process(cell->priv, id, index, - cell->offset, buf, cell->bytes); + cell->offset, buf, &bytes); if (rc) return rc; }
if (len) - *len = cell->bytes; + *len = bytes;
return 0; } diff --git a/drivers/nvmem/imx-ocotp.c b/drivers/nvmem/imx-ocotp.c index ac0edb6398f1..ebd0e9e0314e 100644 --- a/drivers/nvmem/imx-ocotp.c +++ b/drivers/nvmem/imx-ocotp.c @@ -223,15 +223,15 @@ static int imx_ocotp_read(void *context, unsigned int offset, }
static int imx_ocotp_cell_pp(void *context, const char *id, int index, - unsigned int offset, void *data, size_t bytes) + unsigned int offset, void *data, size_t *bytes) { u8 *buf = data; int i;
/* Deal with some post processing of nvmem cell data */ if (id && !strcmp(id, "mac-address")) - for (i = 0; i < bytes / 2; i++) - swap(buf[i], buf[bytes - i - 1]); + for (i = 0; i < *bytes / 2; i++) + swap(buf[i], buf[*bytes - i - 1]);
return 0; } diff --git a/drivers/nvmem/layouts/onie-tlv.c b/drivers/nvmem/layouts/onie-tlv.c index 767f39fff717..1e1f8935abe7 100644 --- a/drivers/nvmem/layouts/onie-tlv.c +++ b/drivers/nvmem/layouts/onie-tlv.c @@ -76,7 +76,7 @@ static const char *onie_tlv_cell_name(u8 type)
static int onie_tlv_mac_read_cb(void *priv, const char *id, int index, unsigned int offset, void *buf, - size_t bytes) + size_t *bytes) { eth_addr_add(buf, index);
diff --git a/drivers/nvmem/layouts/sl28vpd.c b/drivers/nvmem/layouts/sl28vpd.c index a36800f201a3..63c0da58ad60 100644 --- a/drivers/nvmem/layouts/sl28vpd.c +++ b/drivers/nvmem/layouts/sl28vpd.c @@ -23,9 +23,9 @@ struct sl28vpd_v1 {
static int sl28vpd_mac_address_pp(void *priv, const char *id, int index, unsigned int offset, void *buf, - size_t bytes) + size_t *bytes) { - if (bytes != ETH_ALEN) + if (*bytes != ETH_ALEN) return -EINVAL;
if (index < 0) diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h index 0cf9f9490514..5d896eec2f1c 100644 --- a/include/linux/nvmem-provider.h +++ b/include/linux/nvmem-provider.h @@ -21,7 +21,7 @@ typedef int (*nvmem_reg_write_t)(void *priv, unsigned int offset, /* used for vendor specific post processing of cell data */ typedef int (*nvmem_cell_post_process_t)(void *priv, const char *id, int index, unsigned int offset, void *buf, - size_t bytes); + size_t *bytes);
enum nvmem_type { NVMEM_TYPE_UNKNOWN = 0,

From: Rafał Miłecki rafal@milecki.pl
U-Boot environment variables can be found of various underlaying storage entities. This binding should be defined as a layout on top on NVMEM device not a NVMEM device itself.
Signed-off-by: Rafał Miłecki rafal@milecki.pl --- .../bindings/nvmem/layouts/nvmem-layout.yaml | 1 + .../nvmem/{ => layouts}/u-boot,env.yaml | 29 ++++++++++--------- MAINTAINERS | 2 +- 3 files changed, 17 insertions(+), 15 deletions(-) rename Documentation/devicetree/bindings/nvmem/{ => layouts}/u-boot,env.yaml (77%)
diff --git a/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml b/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml index 8512ee538c4c..8835b1781a9f 100644 --- a/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml +++ b/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml @@ -20,6 +20,7 @@ description: | oneOf: - $ref: kontron,sl28-vpd.yaml - $ref: onie,tlv-layout.yaml + - $ref: u-boot,env.yaml
properties: compatible: true diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml similarity index 77% rename from Documentation/devicetree/bindings/nvmem/u-boot,env.yaml rename to Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml index cbc5c69fd405..fb273b174fe7 100644 --- a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml +++ b/Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml @@ -1,10 +1,10 @@ # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2 --- -$id: http://devicetree.org/schemas/nvmem/u-boot,env.yaml# +$id: http://devicetree.org/schemas/nvmem/layouts/u-boot,env.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml#
-title: U-Boot environment variables +title: NVMEM layout of U-Boot environment variables
description: | U-Boot uses environment variables to store device parameters and @@ -14,16 +14,11 @@ description: | Data is stored using U-Boot specific formats (variant specific header and NUL separated key-value pairs).
- Environment data can be stored on various storage entities, e.g.: + Environment data can be stored on NVMEM devices of various underlaying storage + entities, e.g.: 1. Raw flash partition 2. UBI volume
- This binding allows marking storage device (as containing env data) and - specifying used format. - - Right now only flash partition case is covered but it may be extended to e.g. - UBI volumes in the future. - Variables can be defined as NVMEM device subnodes.
maintainers: @@ -67,11 +62,14 @@ examples: read-only; };
- env: partition@40000 { - compatible = "u-boot,env"; + partition@40000 { reg = <0x40000 0x10000>;
- mac: ethaddr { + nvmem-layout { + compatible = "u-boot,env"; + + mac: ethaddr { + }; }; }; }; @@ -87,9 +85,12 @@ examples: label = "u-boot";
partition-u-boot-env { - compatible = "brcm,env";
- ethaddr { + nvmem-layout { + compatible = "brcm,env"; + + ethaddr { + }; }; }; }; diff --git a/MAINTAINERS b/MAINTAINERS index 2a1368722c45..2e13ee875c77 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -21136,7 +21136,7 @@ F: drivers/media/pci/tw686x/ U-BOOT ENVIRONMENT VARIABLES M: Rafał Miłecki rafal@milecki.pl S: Maintained -F: Documentation/devicetree/bindings/nvmem/u-boot,env.yaml +F: Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml F: drivers/nvmem/u-boot-env.c
UACCE ACCELERATOR FRAMEWORK

From: Rafał Miłecki rafal@milecki.pl
U-Boot environment variables can be found on various NVMEM devices (not just MTD) so convert this driver to a generic layout one.
This way - thanks to using NVMEM generic API - this driver can be reused in other scenarios.
For backward DT compatibility we need to support the old compatible brinding string for now. Luckily it's just a one line of code.
Signed-off-by: Rafał Miłecki rafal@milecki.pl --- MAINTAINERS | 2 +- drivers/mtd/mtdcore.c | 7 +- drivers/nvmem/Kconfig | 14 +- drivers/nvmem/Makefile | 2 - drivers/nvmem/layouts/Kconfig | 10 ++ drivers/nvmem/layouts/Makefile | 1 + drivers/nvmem/layouts/u-boot-env.c | 176 ++++++++++++++++++++++ drivers/nvmem/u-boot-env.c | 233 ----------------------------- 8 files changed, 199 insertions(+), 246 deletions(-) create mode 100644 drivers/nvmem/layouts/u-boot-env.c delete mode 100644 drivers/nvmem/u-boot-env.c
diff --git a/MAINTAINERS b/MAINTAINERS index 2e13ee875c77..ace29081c613 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -21137,7 +21137,7 @@ U-BOOT ENVIRONMENT VARIABLES M: Rafał Miłecki rafal@milecki.pl S: Maintained F: Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml -F: drivers/nvmem/u-boot-env.c +F: drivers/nvmem/layouts/u-boot-env.c
UACCE ACCELERATOR FRAMEWORK M: Zhangfei Gao zhangfei.gao@linaro.org diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index 0feacb9fbdac..621e0b87b781 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -518,6 +518,11 @@ static int mtd_nvmem_add(struct mtd_info *mtd) { struct device_node *node = mtd_get_of_node(mtd); struct nvmem_config config = {}; + bool use_dev_of_node = false; + + if (of_device_is_compatible(node, "nvmem-cells") || + (IS_ENABLED(CONFIG_NVMEM_U_BOOT_ENV) && of_device_is_compatible(node, "brcm,env"))) + use_dev_of_node = true;
config.id = -1; config.dev = &mtd->dev; @@ -530,7 +535,7 @@ static int mtd_nvmem_add(struct mtd_info *mtd) config.read_only = true; config.root_only = true; config.ignore_wp = true; - config.no_of_node = !of_device_is_compatible(node, "nvmem-cells"); + config.no_of_node = !use_dev_of_node; config.priv = mtd;
mtd->nvmem = nvmem_register(&config); diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 0e10b5b094b9..980734ba8ddc 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -327,17 +327,13 @@ config NVMEM_SUNXI_SID will be called nvmem_sunxi_sid.
config NVMEM_U_BOOT_ENV - tristate "U-Boot environment variables support" + bool "U-Boot environment variables deprecated binding support" depends on OF && MTD - select CRC32 + select NVMEM_LAYOUT_U_BOOT_ENV 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. - - Currently this drivers works only with env variables on top of MTD. - - If compiled as module it will be called nvmem_u-boot-env. + This option enables support for deprecated DT binding for U-Boot + environment variables. It was used by DT files before introducing + nvmem-layout node based syntax.
config NVMEM_UNIPHIER_EFUSE tristate "UniPhier SoCs eFuse support" diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile index 4cf87ef6c24d..3b827ce96f6d 100644 --- a/drivers/nvmem/Makefile +++ b/drivers/nvmem/Makefile @@ -66,8 +66,6 @@ obj-$(CONFIG_NVMEM_SUNPLUS_OCOTP) += nvmem_sunplus_ocotp.o nvmem_sunplus_ocotp-y := sunplus-ocotp.o obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem_sunxi_sid.o nvmem_sunxi_sid-y := sunxi_sid.o -obj-$(CONFIG_NVMEM_U_BOOT_ENV) += nvmem_u-boot-env.o -nvmem_u-boot-env-y := u-boot-env.o obj-$(CONFIG_NVMEM_UNIPHIER_EFUSE) += nvmem-uniphier-efuse.o nvmem-uniphier-efuse-y := uniphier-efuse.o obj-$(CONFIG_NVMEM_VF610_OCOTP) += nvmem-vf610-ocotp.o diff --git a/drivers/nvmem/layouts/Kconfig b/drivers/nvmem/layouts/Kconfig index 9ad50474cb77..8a38c514943a 100644 --- a/drivers/nvmem/layouts/Kconfig +++ b/drivers/nvmem/layouts/Kconfig @@ -20,4 +20,14 @@ config NVMEM_LAYOUT_ONIE_TLV
If unsure, say N.
+config NVMEM_LAYOUT_U_BOOT_ENV + bool "U-Boot environment variables support" + 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 unsure, say N. + endmenu diff --git a/drivers/nvmem/layouts/Makefile b/drivers/nvmem/layouts/Makefile index 2974bd7d33ed..4940c9db0665 100644 --- a/drivers/nvmem/layouts/Makefile +++ b/drivers/nvmem/layouts/Makefile @@ -5,3 +5,4 @@
obj-$(CONFIG_NVMEM_LAYOUT_SL28_VPD) += sl28vpd.o obj-$(CONFIG_NVMEM_LAYOUT_ONIE_TLV) += onie-tlv.o +obj-$(CONFIG_NVMEM_LAYOUT_U_BOOT_ENV) += u-boot-env.o diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c new file mode 100644 index 000000000000..95c314553952 --- /dev/null +++ b/drivers/nvmem/layouts/u-boot-env.c @@ -0,0 +1,176 @@ +// 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/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, + U_BOOT_FORMAT_BROADCOM, +}; + +struct u_boot_env_image_single { + __le32 crc32; + uint8_t data[]; +} __packed; + +struct u_boot_env_image_redundant { + __le32 crc32; + u8 mark; + uint8_t data[]; +} __packed; + +struct u_boot_env_image_broadcom { + __le32 magic; + __le32 len; + __le32 crc32; + uint8_t data[]; +} __packed; + +static int u_boot_env_parse_data(struct device *dev, struct nvmem_device *nvmem, uint8_t *buf, + size_t data_offset, size_t data_len) +{ + struct device_node *np; + char *data = buf + data_offset; + char *var, *value, *eq; + int err = 0; + + np = of_nvmem_layout_get_container(nvmem); + if (!np) + return -ENOENT; + + for (var = data; + var < data + data_len && *var; + var = value + strlen(value) + 1) { + struct nvmem_cell_info info = {}; + + eq = strchr(var, '='); + if (!eq) + break; + *eq = '\0'; + value = eq + 1; + + info.name = devm_kstrdup(dev, var, GFP_KERNEL); + if (!info.name) { + err = -ENOMEM; + break; + } + info.offset = data_offset + value - data; + info.bytes = strlen(value); + info.np = of_get_child_by_name(np, info.name); + + err = nvmem_add_one_cell(nvmem, &info); + if (err) { + dev_err(dev, "Failed to add "%s" cell: %d\n", info.name, err); + break; + } + } + + of_node_put(np); + + return err; +} + +static int u_boot_env_add_cells(struct device *dev, struct nvmem_device *nvmem, + struct nvmem_layout *layout) +{ + enum u_boot_env_format format; + size_t crc32_data_offset; + size_t crc32_data_len; + size_t crc32_offset; + size_t data_offset; + size_t data_len; + size_t dev_size; + uint32_t crc32; + uint32_t calc; + size_t bytes; + uint8_t *buf; + int err; + + format = (uintptr_t)nvmem_layout_get_match_data(nvmem, layout); + + dev_size = nvmem_dev_size(nvmem); + + buf = kcalloc(1, dev_size, GFP_KERNEL); + if (!buf) { + err = -ENOMEM; + goto err_out; + } + + bytes = nvmem_device_read(nvmem, 0, dev_size, buf); + if (bytes < 0 || bytes != dev_size) { + dev_err(dev, "Failed to read from NVMEM device: %zd\n", bytes); + goto err_kfree; + } + + switch (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, data); + data_offset = offsetof(struct u_boot_env_image_redundant, data); + break; + case U_BOOT_FORMAT_BROADCOM: + crc32_offset = offsetof(struct u_boot_env_image_broadcom, crc32); + crc32_data_offset = offsetof(struct u_boot_env_image_broadcom, data); + data_offset = offsetof(struct u_boot_env_image_broadcom, data); + break; + } + crc32 = le32_to_cpu(*(__le32 *)(buf + crc32_offset)); + crc32_data_len = dev_size - crc32_data_offset; + data_len = dev_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[dev_size - 1] = '\0'; + err = u_boot_env_parse_data(dev, nvmem, buf, data_offset, data_len); + if (err) + dev_err(dev, "Failed to parse 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, }, + { .compatible = "brcm,env", .data = (void *)U_BOOT_FORMAT_BROADCOM, }, + {}, +}; + +static struct nvmem_layout u_boot_env_layout = { + .name = "U-Boot environment variables layout", + .of_match_table = u_boot_env_of_match_table, + .add_cells = u_boot_env_add_cells, +}; + +static int __init u_boot_env_init(void) +{ + return nvmem_layout_register(&u_boot_env_layout); +} +subsys_initcall(u_boot_env_init); + +MODULE_AUTHOR("Rafał Miłecki"); +MODULE_LICENSE("GPL"); +MODULE_DEVICE_TABLE(of, u_boot_env_of_match_table); diff --git a/drivers/nvmem/u-boot-env.c b/drivers/nvmem/u-boot-env.c deleted file mode 100644 index 29b1d87a3c51..000000000000 --- a/drivers/nvmem/u-boot-env.c +++ /dev/null @@ -1,233 +0,0 @@ -// 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, - U_BOOT_FORMAT_BROADCOM, -}; - -struct u_boot_env { - struct device *dev; - enum u_boot_env_format format; - - struct mtd_info *mtd; - - /* Cells */ - struct nvmem_cell_info *cells; - int ncells; -}; - -struct u_boot_env_image_single { - __le32 crc32; - uint8_t data[]; -} __packed; - -struct u_boot_env_image_redundant { - __le32 crc32; - u8 mark; - uint8_t data[]; -} __packed; - -struct u_boot_env_image_broadcom { - __le32 magic; - __le32 len; - __le32 crc32; - 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, 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 %zu 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); - priv->cells[idx].np = of_get_child_by_name(dev->of_node, priv->cells[idx].name); - } - - 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->mtd->size, GFP_KERNEL); - if (!buf) { - err = -ENOMEM; - goto err_out; - } - - err = mtd_read(priv->mtd, 0, priv->mtd->size, &bytes, buf); - if ((err && !mtd_is_bitflip(err)) || bytes != priv->mtd->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, data); - data_offset = offsetof(struct u_boot_env_image_redundant, data); - break; - case U_BOOT_FORMAT_BROADCOM: - crc32_offset = offsetof(struct u_boot_env_image_broadcom, crc32); - crc32_data_offset = offsetof(struct u_boot_env_image_broadcom, data); - data_offset = offsetof(struct u_boot_env_image_broadcom, data); - break; - } - crc32 = le32_to_cpu(*(__le32 *)(buf + crc32_offset)); - crc32_data_len = priv->mtd->size - crc32_data_offset; - data_len = priv->mtd->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->mtd->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 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; - struct u_boot_env *priv; - int err; - - priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); - if (!priv) - return -ENOMEM; - priv->dev = dev; - - priv->format = (uintptr_t)of_device_get_match_data(dev); - - priv->mtd = of_get_mtd_device_by_node(np); - if (IS_ERR(priv->mtd)) { - dev_err_probe(dev, PTR_ERR(priv->mtd), "Failed to get %pOF MTD\n", np); - 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->mtd->size; - - return PTR_ERR_OR_ZERO(devm_nvmem_register(dev, &config)); -} - -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, }, - { .compatible = "brcm,env", .data = (void *)U_BOOT_FORMAT_BROADCOM, }, - {}, -}; - -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);

From: Rafał Miłecki rafal@milecki.pl
U-Boot's "ethaddr" environment variable is very often used to store *base* MAC address. It's used as a base for calculating addresses for multiple interfaces. It's done by adding proper values. Actual offsets are picked by manufacturers and vary across devices.
Signed-off-by: Rafał Miłecki rafal@milecki.pl --- .../devicetree/bindings/nvmem/layouts/u-boot,env.yaml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml index fb273b174fe7..dbff702f2e5d 100644 --- a/Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml +++ b/Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml @@ -45,7 +45,11 @@ properties:
ethaddr: type: object - description: Ethernet interface's MAC address + description: Ethernet interfaces base MAC address. + properties: + "#nvmem-cell-cells": + description: The first argument is a MAC address offset. + const: 1
additionalProperties: false
@@ -69,6 +73,7 @@ examples: compatible = "u-boot,env";
mac: ethaddr { + #nvmem-cell-cells = <1>; }; }; };

From: Rafał Miłecki rafal@milecki.pl
U-Boot environment variables are stored in ASCII format so "ethaddr" requires parsing into binary to make it work with Ethernet interfaces.
This includes support for indexes to support #nvmem-cell-cells = <1>.
Signed-off-by: Rafał Miłecki rafal@milecki.pl --- drivers/nvmem/layouts/Kconfig | 1 + drivers/nvmem/layouts/u-boot-env.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+)
diff --git a/drivers/nvmem/layouts/Kconfig b/drivers/nvmem/layouts/Kconfig index 8a38c514943a..566b4f25630d 100644 --- a/drivers/nvmem/layouts/Kconfig +++ b/drivers/nvmem/layouts/Kconfig @@ -23,6 +23,7 @@ config NVMEM_LAYOUT_ONIE_TLV config NVMEM_LAYOUT_U_BOOT_ENV bool "U-Boot environment variables support" select CRC32 + select GENERIC_NET_UTILS help U-Boot stores its setup as environment variables. This driver adds support for verifying & exporting such data. It also exposes variables diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c index 95c314553952..63baeb18bd56 100644 --- a/drivers/nvmem/layouts/u-boot-env.c +++ b/drivers/nvmem/layouts/u-boot-env.c @@ -4,6 +4,8 @@ */
#include <linux/crc32.h> +#include <linux/etherdevice.h> +#include <linux/if_ether.h> #include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/nvmem-consumer.h> @@ -36,6 +38,26 @@ struct u_boot_env_image_broadcom { uint8_t data[]; } __packed;
+static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, int index, + unsigned int offset, void *data, size_t *bytes) +{ + u8 mac[ETH_ALEN]; + + if (*bytes != 3 * ETH_ALEN - 1) + return -EINVAL; + + if (!mac_pton(data, mac)) + return -EINVAL; + + if (index) + eth_addr_add(mac, index); + + ether_addr_copy(data, mac); + *bytes = ETH_ALEN; + + return 0; +} + static int u_boot_env_parse_data(struct device *dev, struct nvmem_device *nvmem, uint8_t *buf, size_t data_offset, size_t data_len) { @@ -67,6 +89,8 @@ static int u_boot_env_parse_data(struct device *dev, struct nvmem_device *nvmem, info.offset = data_offset + value - data; info.bytes = strlen(value); info.np = of_get_child_by_name(np, info.name); + if (!strcmp(var, "ethaddr")) + info.read_post_process = u_boot_env_read_post_process_ethaddr;
err = nvmem_add_one_cell(nvmem, &info); if (err) {

Hi,
Am 2023-01-10 11:54, schrieb Rafał Miłecki:
From: Rafał Miłecki rafal@milecki.pl
U-Boot environment variables are stored in ASCII format so "ethaddr" requires parsing into binary to make it work with Ethernet interfaces.
This includes support for indexes to support #nvmem-cell-cells = <1>.
Signed-off-by: Rafał Miłecki rafal@milecki.pl
drivers/nvmem/layouts/Kconfig | 1 + drivers/nvmem/layouts/u-boot-env.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+)
diff --git a/drivers/nvmem/layouts/Kconfig b/drivers/nvmem/layouts/Kconfig index 8a38c514943a..566b4f25630d 100644 --- a/drivers/nvmem/layouts/Kconfig +++ b/drivers/nvmem/layouts/Kconfig @@ -23,6 +23,7 @@ config NVMEM_LAYOUT_ONIE_TLV config NVMEM_LAYOUT_U_BOOT_ENV bool "U-Boot environment variables support" select CRC32
- select GENERIC_NET_UTILS help U-Boot stores its setup as environment variables. This driver adds support for verifying & exporting such data. It also exposes
variables diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c index 95c314553952..63baeb18bd56 100644 --- a/drivers/nvmem/layouts/u-boot-env.c +++ b/drivers/nvmem/layouts/u-boot-env.c @@ -4,6 +4,8 @@ */
#include <linux/crc32.h> +#include <linux/etherdevice.h> +#include <linux/if_ether.h> #include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/nvmem-consumer.h> @@ -36,6 +38,26 @@ struct u_boot_env_image_broadcom { uint8_t data[]; } __packed;
+static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, int index,
unsigned int offset, void *data, size_t *bytes)
+{
- u8 mac[ETH_ALEN];
- if (*bytes != 3 * ETH_ALEN - 1)
return -EINVAL;
- if (!mac_pton(data, mac))
return -EINVAL;
- if (index)
eth_addr_add(mac, index);
- ether_addr_copy(data, mac);
- *bytes = ETH_ALEN;
I don't know how to feel about this. This will only work if the new size is smaller than the old one. Can't we have a correct size in the first place? I.e. while adding the cells.
-michael
- return 0;
+}
static int u_boot_env_parse_data(struct device *dev, struct nvmem_device *nvmem, uint8_t *buf, size_t data_offset, size_t data_len) { @@ -67,6 +89,8 @@ static int u_boot_env_parse_data(struct device *dev, struct nvmem_device *nvmem, info.offset = data_offset + value - data; info.bytes = strlen(value); info.np = of_get_child_by_name(np, info.name);
if (!strcmp(var, "ethaddr"))
info.read_post_process = u_boot_env_read_post_process_ethaddr;
err = nvmem_add_one_cell(nvmem, &info); if (err) {

On 10.01.2023 13:19, Michael Walle wrote:
Hi,
Am 2023-01-10 11:54, schrieb Rafał Miłecki:
From: Rafał Miłecki rafal@milecki.pl
U-Boot environment variables are stored in ASCII format so "ethaddr" requires parsing into binary to make it work with Ethernet interfaces.
This includes support for indexes to support #nvmem-cell-cells = <1>.
Signed-off-by: Rafał Miłecki rafal@milecki.pl
drivers/nvmem/layouts/Kconfig | 1 + drivers/nvmem/layouts/u-boot-env.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+)
diff --git a/drivers/nvmem/layouts/Kconfig b/drivers/nvmem/layouts/Kconfig index 8a38c514943a..566b4f25630d 100644 --- a/drivers/nvmem/layouts/Kconfig +++ b/drivers/nvmem/layouts/Kconfig @@ -23,6 +23,7 @@ config NVMEM_LAYOUT_ONIE_TLV config NVMEM_LAYOUT_U_BOOT_ENV bool "U-Boot environment variables support" select CRC32 + select GENERIC_NET_UTILS help U-Boot stores its setup as environment variables. This driver adds support for verifying & exporting such data. It also exposes variables diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c index 95c314553952..63baeb18bd56 100644 --- a/drivers/nvmem/layouts/u-boot-env.c +++ b/drivers/nvmem/layouts/u-boot-env.c @@ -4,6 +4,8 @@ */
#include <linux/crc32.h> +#include <linux/etherdevice.h> +#include <linux/if_ether.h> #include <linux/mod_devicetable.h> #include <linux/module.h> #include <linux/nvmem-consumer.h> @@ -36,6 +38,26 @@ struct u_boot_env_image_broadcom { uint8_t data[]; } __packed;
+static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, int index, + unsigned int offset, void *data, size_t *bytes) +{ + u8 mac[ETH_ALEN];
+ if (*bytes != 3 * ETH_ALEN - 1) + return -EINVAL;
+ if (!mac_pton(data, mac)) + return -EINVAL;
+ if (index) + eth_addr_add(mac, index);
+ ether_addr_copy(data, mac); + *bytes = ETH_ALEN;
I don't know how to feel about this. This will only work if the new size is smaller than the old one. Can't we have a correct size in the first place? I.e. while adding the cells.
I didn't think about such solution. I like that idea. Will do.

Hi Rafał,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20230110] [also build test WARNING on v6.2-rc3] [cannot apply to robh/for-next shawnguo/for-next mtd/mtd/next mtd/mtd/fixes linus/master v6.2-rc3 v6.2-rc2 v6.2-rc1] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Rafa-Mi-ecki/nvmem-core-allow... patch link: https://lore.kernel.org/r/20230110105425.13188-1-zajec5%40gmail.com patch subject: [PATCH 1/6] nvmem: core: add nvmem_dev_size() helper config: m68k-allyesconfig compiler: m68k-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/4d5cc61f8d02a82344468f172a852f... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Rafa-Mi-ecki/nvmem-core-allow-read_post_process-callbacks-to-adjust-data-length/20230110-185915 git checkout 4d5cc61f8d02a82344468f172a852ffc56cf0d5c # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/nvmem/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
In file included from drivers/nvmem/core.c:16:
include/linux/nvmem-consumer.h:81:1: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
81 | const size_t nvmem_dev_size(struct nvmem_device *nvmem); | ^~~~~
drivers/nvmem/core.c:2070:1: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
2070 | const size_t nvmem_dev_size(struct nvmem_device *nvmem) | ^~~~~ -- In file included from drivers/nvmem/brcm_nvram.c:10:
include/linux/nvmem-consumer.h:81:1: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
81 | const size_t nvmem_dev_size(struct nvmem_device *nvmem); | ^~~~~
vim +81 include/linux/nvmem-consumer.h
49 50 /* Cell based interface */ 51 struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *id); 52 struct nvmem_cell *devm_nvmem_cell_get(struct device *dev, const char *id); 53 void nvmem_cell_put(struct nvmem_cell *cell); 54 void devm_nvmem_cell_put(struct device *dev, struct nvmem_cell *cell); 55 void *nvmem_cell_read(struct nvmem_cell *cell, size_t *len); 56 int nvmem_cell_write(struct nvmem_cell *cell, void *buf, size_t len); 57 int nvmem_cell_read_u8(struct device *dev, const char *cell_id, u8 *val); 58 int nvmem_cell_read_u16(struct device *dev, const char *cell_id, u16 *val); 59 int nvmem_cell_read_u32(struct device *dev, const char *cell_id, u32 *val); 60 int nvmem_cell_read_u64(struct device *dev, const char *cell_id, u64 *val); 61 int nvmem_cell_read_variable_le_u32(struct device *dev, const char *cell_id, 62 u32 *val); 63 int nvmem_cell_read_variable_le_u64(struct device *dev, const char *cell_id, 64 u64 *val); 65 66 /* direct nvmem device read/write interface */ 67 struct nvmem_device *nvmem_device_get(struct device *dev, const char *name); 68 struct nvmem_device *devm_nvmem_device_get(struct device *dev, 69 const char *name); 70 void nvmem_device_put(struct nvmem_device *nvmem); 71 void devm_nvmem_device_put(struct device *dev, struct nvmem_device *nvmem); 72 int nvmem_device_read(struct nvmem_device *nvmem, unsigned int offset, 73 size_t bytes, void *buf); 74 int nvmem_device_write(struct nvmem_device *nvmem, unsigned int offset, 75 size_t bytes, void *buf); 76 ssize_t nvmem_device_cell_read(struct nvmem_device *nvmem, 77 struct nvmem_cell_info *info, void *buf); 78 int nvmem_device_cell_write(struct nvmem_device *nvmem, 79 struct nvmem_cell_info *info, void *buf); 80
81 const size_t nvmem_dev_size(struct nvmem_device *nvmem);
82 const char *nvmem_dev_name(struct nvmem_device *nvmem); 83

Hi Rafał,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20230110] [also build test WARNING on v6.2-rc3] [cannot apply to robh/for-next shawnguo/for-next mtd/mtd/next mtd/mtd/fixes linus/master v6.2-rc3 v6.2-rc2 v6.2-rc1] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Rafa-Mi-ecki/nvmem-core-allow... patch link: https://lore.kernel.org/r/20230110105425.13188-1-zajec5%40gmail.com patch subject: [PATCH 1/6] nvmem: core: add nvmem_dev_size() helper config: hexagon-randconfig-r041-20230110 compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 8d9828ef5aa9688500657d36cd2aefbe12bbd162) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/4d5cc61f8d02a82344468f172a852f... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Rafa-Mi-ecki/nvmem-core-allow-read_post_process-callbacks-to-adjust-data-length/20230110-185915 git checkout 4d5cc61f8d02a82344468f172a852ffc56cf0d5c # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/nvmem/ drivers/thermal/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
In file included from drivers/nvmem/core.c:16:
include/linux/nvmem-consumer.h:81:1: warning: 'const' type qualifier on return type has no effect [-Wignored-qualifiers]
const size_t nvmem_dev_size(struct nvmem_device *nvmem); ^~~~~~
drivers/nvmem/core.c:2070:1: warning: 'const' type qualifier on return type has no effect [-Wignored-qualifiers]
const size_t nvmem_dev_size(struct nvmem_device *nvmem) ^~~~~~ 2 warnings generated. -- In file included from drivers/thermal/mtk_thermal.c:12: In file included from include/linux/interrupt.h:11: In file included from include/linux/hardirq.h:11: In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: In file included from include/asm-generic/hardirq.h:17: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:334: include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __raw_readb(PCI_IOBASE + addr); ~~~~~~~~~~ ^ include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); ~~~~~~~~~~ ^ include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu' #define __le16_to_cpu(x) ((__force __u16)(__le16)(x)) ^ In file included from drivers/thermal/mtk_thermal.c:12: In file included from include/linux/interrupt.h:11: In file included from include/linux/hardirq.h:11: In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: In file included from include/asm-generic/hardirq.h:17: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:334: include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~~~~~~~~~ ^ include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) ((__force __u32)(__le32)(x)) ^ In file included from drivers/thermal/mtk_thermal.c:12: In file included from include/linux/interrupt.h:11: In file included from include/linux/hardirq.h:11: In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1: In file included from include/asm-generic/hardirq.h:17: In file included from include/linux/irq.h:20: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:334: include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writeb(value, PCI_IOBASE + addr); ~~~~~~~~~~ ^ include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr); ~~~~~~~~~~ ^ include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr); ~~~~~~~~~~ ^ In file included from drivers/thermal/mtk_thermal.c:15:
include/linux/nvmem-consumer.h:81:1: warning: 'const' type qualifier on return type has no effect [-Wignored-qualifiers]
const size_t nvmem_dev_size(struct nvmem_device *nvmem); ^~~~~~ 7 warnings generated.
vim +/const +81 include/linux/nvmem-consumer.h
49 50 /* Cell based interface */ 51 struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *id); 52 struct nvmem_cell *devm_nvmem_cell_get(struct device *dev, const char *id); 53 void nvmem_cell_put(struct nvmem_cell *cell); 54 void devm_nvmem_cell_put(struct device *dev, struct nvmem_cell *cell); 55 void *nvmem_cell_read(struct nvmem_cell *cell, size_t *len); 56 int nvmem_cell_write(struct nvmem_cell *cell, void *buf, size_t len); 57 int nvmem_cell_read_u8(struct device *dev, const char *cell_id, u8 *val); 58 int nvmem_cell_read_u16(struct device *dev, const char *cell_id, u16 *val); 59 int nvmem_cell_read_u32(struct device *dev, const char *cell_id, u32 *val); 60 int nvmem_cell_read_u64(struct device *dev, const char *cell_id, u64 *val); 61 int nvmem_cell_read_variable_le_u32(struct device *dev, const char *cell_id, 62 u32 *val); 63 int nvmem_cell_read_variable_le_u64(struct device *dev, const char *cell_id, 64 u64 *val); 65 66 /* direct nvmem device read/write interface */ 67 struct nvmem_device *nvmem_device_get(struct device *dev, const char *name); 68 struct nvmem_device *devm_nvmem_device_get(struct device *dev, 69 const char *name); 70 void nvmem_device_put(struct nvmem_device *nvmem); 71 void devm_nvmem_device_put(struct device *dev, struct nvmem_device *nvmem); 72 int nvmem_device_read(struct nvmem_device *nvmem, unsigned int offset, 73 size_t bytes, void *buf); 74 int nvmem_device_write(struct nvmem_device *nvmem, unsigned int offset, 75 size_t bytes, void *buf); 76 ssize_t nvmem_device_cell_read(struct nvmem_device *nvmem, 77 struct nvmem_cell_info *info, void *buf); 78 int nvmem_device_cell_write(struct nvmem_device *nvmem, 79 struct nvmem_cell_info *info, void *buf); 80
81 const size_t nvmem_dev_size(struct nvmem_device *nvmem);
82 const char *nvmem_dev_name(struct nvmem_device *nvmem); 83

On 10.01.23 11:54, Rafał Miłecki wrote:
From: Rafał Miłecki rafal@milecki.pl
This is required by layouts that need to read whole NVMEM space. It applies to NVMEM devices without hardcoded layout (like U-Boot environment data block).
Signed-off-by: Rafał Miłecki rafal@milecki.pl
drivers/nvmem/core.c | 13 +++++++++++++ include/linux/nvmem-consumer.h | 1 + 2 files changed, 14 insertions(+)
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c index 1f05f0a50d86..81743ae8793b 100644 --- a/drivers/nvmem/core.c +++ b/drivers/nvmem/core.c @@ -2062,6 +2062,19 @@ void nvmem_del_cell_lookups(struct nvmem_cell_lookup *entries, size_t nentries) } EXPORT_SYMBOL_GPL(nvmem_del_cell_lookups);
+/**
- nvmem_dev_size() - Get the size of a given nvmem device.
- @nvmem: nvmem device.
- Return: size of the nvmem device.
- */
+const size_t nvmem_dev_size(struct nvmem_device *nvmem)
The const here is quite unusual. You can make the parameter a const struct nvmem_device though.
+{
- return nvmem->size;
+} +EXPORT_SYMBOL_GPL(nvmem_dev_size);
/**
- nvmem_dev_name() - Get the name of a given nvmem device.
diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h index fa030d93b768..d88294ddf562 100644 --- a/include/linux/nvmem-consumer.h +++ b/include/linux/nvmem-consumer.h @@ -78,6 +78,7 @@ ssize_t nvmem_device_cell_read(struct nvmem_device *nvmem, int nvmem_device_cell_write(struct nvmem_device *nvmem, struct nvmem_cell_info *info, void *buf);
+const size_t nvmem_dev_size(struct nvmem_device *nvmem); const char *nvmem_dev_name(struct nvmem_device *nvmem);
void nvmem_add_cell_lookups(struct nvmem_cell_lookup *entries,

Hi Rafał,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20230110] [also build test WARNING on v6.2-rc7] [cannot apply to robh/for-next shawnguo/for-next mtd/mtd/next mtd/mtd/fixes linus/master v6.2-rc3 v6.2-rc2 v6.2-rc1] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Rafa-Mi-ecki/nvmem-core-allow... patch link: https://lore.kernel.org/r/20230110105425.13188-1-zajec5%40gmail.com patch subject: [PATCH 1/6] nvmem: core: add nvmem_dev_size() helper config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230211/202302112138.XOdXy4yF-lkp@i...) compiler: m68k-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/4d5cc61f8d02a82344468f172a852f... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Rafa-Mi-ecki/nvmem-core-allow-read_post_process-callbacks-to-adjust-data-length/20230110-185915 git checkout 4d5cc61f8d02a82344468f172a852ffc56cf0d5c # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/nvmem/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com | Link: https://lore.kernel.org/oe-kbuild-all/202302112138.XOdXy4yF-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/nvmem/core.c:16:
include/linux/nvmem-consumer.h:81:1: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
81 | const size_t nvmem_dev_size(struct nvmem_device *nvmem); | ^~~~~
drivers/nvmem/core.c:2070:1: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
2070 | const size_t nvmem_dev_size(struct nvmem_device *nvmem) | ^~~~~ -- In file included from drivers/nvmem/brcm_nvram.c:10:
include/linux/nvmem-consumer.h:81:1: warning: type qualifiers ignored on function return type [-Wignored-qualifiers]
81 | const size_t nvmem_dev_size(struct nvmem_device *nvmem); | ^~~~~
vim +81 include/linux/nvmem-consumer.h
49 50 /* Cell based interface */ 51 struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *id); 52 struct nvmem_cell *devm_nvmem_cell_get(struct device *dev, const char *id); 53 void nvmem_cell_put(struct nvmem_cell *cell); 54 void devm_nvmem_cell_put(struct device *dev, struct nvmem_cell *cell); 55 void *nvmem_cell_read(struct nvmem_cell *cell, size_t *len); 56 int nvmem_cell_write(struct nvmem_cell *cell, void *buf, size_t len); 57 int nvmem_cell_read_u8(struct device *dev, const char *cell_id, u8 *val); 58 int nvmem_cell_read_u16(struct device *dev, const char *cell_id, u16 *val); 59 int nvmem_cell_read_u32(struct device *dev, const char *cell_id, u32 *val); 60 int nvmem_cell_read_u64(struct device *dev, const char *cell_id, u64 *val); 61 int nvmem_cell_read_variable_le_u32(struct device *dev, const char *cell_id, 62 u32 *val); 63 int nvmem_cell_read_variable_le_u64(struct device *dev, const char *cell_id, 64 u64 *val); 65 66 /* direct nvmem device read/write interface */ 67 struct nvmem_device *nvmem_device_get(struct device *dev, const char *name); 68 struct nvmem_device *devm_nvmem_device_get(struct device *dev, 69 const char *name); 70 void nvmem_device_put(struct nvmem_device *nvmem); 71 void devm_nvmem_device_put(struct device *dev, struct nvmem_device *nvmem); 72 int nvmem_device_read(struct nvmem_device *nvmem, unsigned int offset, 73 size_t bytes, void *buf); 74 int nvmem_device_write(struct nvmem_device *nvmem, unsigned int offset, 75 size_t bytes, void *buf); 76 ssize_t nvmem_device_cell_read(struct nvmem_device *nvmem, 77 struct nvmem_cell_info *info, void *buf); 78 int nvmem_device_cell_write(struct nvmem_device *nvmem, 79 struct nvmem_cell_info *info, void *buf); 80
81 const size_t nvmem_dev_size(struct nvmem_device *nvmem);
82 const char *nvmem_dev_name(struct nvmem_device *nvmem); 83
participants (4)
-
Ahmad Fatoum
-
kernel test robot
-
Michael Walle
-
Rafał Miłecki