| Message ID | 20231218133722.16150-4-zajec5@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] dt-bindings: nvmem: layouts: add U-Boot environment variables layout | expand |
On 18.12.2023 15:21, Miquel Raynal wrote: > Hi Rafał, > > zajec5@gmail.com wrote on Mon, 18 Dec 2023 14:37:22 +0100: > >> From: Rafał Miłecki <rafal@milecki.pl> >> >> This patch moves all generic (NVMEM devices independent) code from NVMEM >> device driver to NVMEM layout driver. Then it adds a simple NVMEM layout >> code on top of it. >> >> Thanks to proper layout it's possible to support U-Boot env data stored >> on any kind of NVMEM device. >> >> For backward compatibility with old DT bindings we need to keep old >> NVMEM device driver functional. To avoid code duplication a parsing >> function is exported and reused in it. >> >> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >> --- > > I have a couple of comments about the original driver which gets > copy-pasted in the new layout driver, maybe you could clean these > (the memory leak should be fixed before the migration so it can be > backported easily, the others are just style so it can be done after, I > don't mind). > > ... > >> +int u_boot_env_parse(struct device *dev, struct nvmem_device *nvmem, >> + 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; >> + uint8_t *buf; >> + int bytes; >> + int err; >> + >> + dev_size = nvmem_dev_size(nvmem); >> + >> + buf = kcalloc(1, dev_size, GFP_KERNEL); > > Out of curiosity, why kcalloc(1,...) rather than kzalloc() ? I used kcalloc() initially as I didn't need buffer to be zeroed. I see that memory-allocation.rst however says: > And, to be on the safe side it's best to use routines that set memory to zero, like kzalloc(). It's probably close to zero cost to zero that buffer so it could be kzalloc(). >> + if (!buf) { >> + err = -ENOMEM; >> + goto err_out; > > We could directly return ENOMEM here I guess. > >> + } >> + >> + bytes = nvmem_device_read(nvmem, 0, dev_size, buf); >> + if (bytes < 0) >> + return bytes; >> + else if (bytes != dev_size) >> + return -EIO; > > Don't we need to free buf in the above cases? > >> + 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)); > > Looks a bit convoluted, any chances we can use intermediate variables > to help decipher this? > >> + 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_cells(dev, nvmem, buf, data_offset, data_len); >> + if (err) >> + dev_err(dev, "Failed to add cells: %d\n", err); > > Please drop this error message, the only reason for which the function > call would fail is apparently an ENOMEM case. > >> + >> +err_kfree: >> + kfree(buf); >> +err_out: >> + return err; >> +} >> +EXPORT_SYMBOL_GPL(u_boot_env_parse); >> + >> +static int u_boot_env_add_cells(struct device *dev, struct nvmem_device *nvmem) >> +{ >> + const struct of_device_id *match; >> + struct device_node *layout_np; >> + enum u_boot_env_format format; >> + >> + layout_np = of_nvmem_layout_get_container(nvmem); >> + if (!layout_np) >> + return -ENOENT; >> + >> + match = of_match_node(u_boot_env_of_match_table, layout_np); >> + if (!match) >> + return -ENOENT; >> + >> + format = (uintptr_t)match->data; > > In the core there is currently an unused helper called > nvmem_layout_get_match_data() which does that. I think the original > intent of this function was to be used in this driver, so depending on > your preference, can you please either use it or remove it? The problem is that nvmem_layout_get_match_data() uses: layout->dev.driver It doesn't work with layouts driver (since refactoring?) as driver is NULL. That results in NULL pointer dereference when trying to reach of_match_table. That is why I used u_boot_env_of_match_table directly. If you know how to fix nvmem_layout_get_match_data() that would be great. Do we need driver_register() somewhere in NVMEM core? >> + >> + of_node_put(layout_np); >> + >> + return u_boot_env_parse(dev, nvmem, format); >> +} > > Thanks, > Miquèl
On 19.12.2023 10:55, Rafał Miłecki wrote: > On 19.12.2023 08:55, Miquel Raynal wrote: >> Hi Rafał, >> >> zajec5@gmail.com wrote on Mon, 18 Dec 2023 23:10:20 +0100: >> >>> On 18.12.2023 15:21, Miquel Raynal wrote: >>>> Hi Rafał, >>>> >>>> zajec5@gmail.com wrote on Mon, 18 Dec 2023 14:37:22 +0100: >>>>> From: Rafał Miłecki <rafal@milecki.pl> >>>>> >>>>> This patch moves all generic (NVMEM devices independent) code from NVMEM >>>>> device driver to NVMEM layout driver. Then it adds a simple NVMEM layout >>>>> code on top of it. >>>>> >>>>> Thanks to proper layout it's possible to support U-Boot env data stored >>>>> on any kind of NVMEM device. >>>>> >>>>> For backward compatibility with old DT bindings we need to keep old >>>>> NVMEM device driver functional. To avoid code duplication a parsing >>>>> function is exported and reused in it. >>>>> >>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >>>>> --- >>>> >>>> I have a couple of comments about the original driver which gets >>>> copy-pasted in the new layout driver, maybe you could clean these >>>> (the memory leak should be fixed before the migration so it can be >>>> backported easily, the others are just style so it can be done after, I >>>> don't mind). >>>> >>>> ... >>>>> +int u_boot_env_parse(struct device *dev, struct nvmem_device *nvmem, >>>>> + 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; >>>>> + uint8_t *buf; >>>>> + int bytes; >>>>> + int err; >>>>> + >>>>> + dev_size = nvmem_dev_size(nvmem); >>>>> + >>>>> + buf = kcalloc(1, dev_size, GFP_KERNEL); >>>> >>>> Out of curiosity, why kcalloc(1,...) rather than kzalloc() ? >>> >>> I used kcalloc() initially as I didn't need buffer to be zeroed. >> >> I think kcalloc() initializes the memory to zero. >> https://elixir.bootlin.com/linux/latest/source/include/linux/slab.h#L659 >> >> If you don't need it you can switch to kmalloc() instead, I don't mind, >> but kcalloc() is meant to be used with arrays, I don't see the point of >> using kcalloc() in this case. >> >>> >>> I see that memory-allocation.rst however says: >>> > And, to be on the safe side it's best to use routines that set memory to zero, like kzalloc(). >>> >>> It's probably close to zero cost to zero that buffer so it could be kzalloc(). >>> >>> >>>>> + if (!buf) { >>>>> + err = -ENOMEM; >>>>> + goto err_out; >>>> >>>> We could directly return ENOMEM here I guess. >>>>> + } >>>>> + >>>>> + bytes = nvmem_device_read(nvmem, 0, dev_size, buf); >>>>> + if (bytes < 0) >>>>> + return bytes; >>>>> + else if (bytes != dev_size) >>>>> + return -EIO; >>>> >>>> Don't we need to free buf in the above cases? >>>>> + 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)); >>>> >>>> Looks a bit convoluted, any chances we can use intermediate variables >>>> to help decipher this? >>>>> + 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_cells(dev, nvmem, buf, data_offset, data_len); >>>>> + if (err) >>>>> + dev_err(dev, "Failed to add cells: %d\n", err); >>>> >>>> Please drop this error message, the only reason for which the function >>>> call would fail is apparently an ENOMEM case. >>>>> + >>>>> +err_kfree: >>>>> + kfree(buf); >>>>> +err_out: >>>>> + return err; >>>>> +} >>>>> +EXPORT_SYMBOL_GPL(u_boot_env_parse); >>>>> + >>>>> +static int u_boot_env_add_cells(struct device *dev, struct nvmem_device *nvmem) >>>>> +{ >>>>> + const struct of_device_id *match; >>>>> + struct device_node *layout_np; >>>>> + enum u_boot_env_format format; >>>>> + >>>>> + layout_np = of_nvmem_layout_get_container(nvmem); >>>>> + if (!layout_np) >>>>> + return -ENOENT; >>>>> + >>>>> + match = of_match_node(u_boot_env_of_match_table, layout_np); >>>>> + if (!match) >>>>> + return -ENOENT; >>>>> + >>>>> + format = (uintptr_t)match->data; >>>> >>>> In the core there is currently an unused helper called >>>> nvmem_layout_get_match_data() which does that. I think the original >>>> intent of this function was to be used in this driver, so depending on >>>> your preference, can you please either use it or remove it? >>> >>> The problem is that nvmem_layout_get_match_data() uses: >>> layout->dev.driver >> >> I'm surprised .driver is unset. Well anyway, please either fix the core >> helper and use it or drop the core helper, because we have no user for >> it otherwise? > > I believe it's because of a very minimalistic "nvmem_bus_type" bus > implementation. Scratch that, I was looking at "nvmem_bus_type" instead of "nvmem_layout_bus_type". I'll see if I can debug that. > From a quick look it seems that default expected FORWARD-trace is: > driver_register() > bus_add_driver() > driver_attach() > __driver_attach() > driver_probe_device() > __driver_probe_device() > really_probe() > > It's really_probe() that seems to set dev->driver pointer. > > >>> It doesn't work with layouts driver (since refactoring?) as driver is >>> NULL. That results in NULL pointer dereference when trying to reach >>> of_match_table. >>> >>> That is why I used u_boot_env_of_match_table directly. >>> >>> If you know how to fix nvmem_layout_get_match_data() that would be >>> great. Do we need driver_register() somewhere in NVMEM core? >
diff --git a/MAINTAINERS b/MAINTAINERS index b589218605b4..1f7e6d74cd51 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -22282,6 +22282,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/layouts/u-boot-env.c F: drivers/nvmem/u-boot-env.c UACCE ACCELERATOR FRAMEWORK diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig index 283134498fbc..d2c384f58028 100644 --- a/drivers/nvmem/Kconfig +++ b/drivers/nvmem/Kconfig @@ -363,8 +363,7 @@ config NVMEM_SUNXI_SID config NVMEM_U_BOOT_ENV tristate "U-Boot environment variables support" depends on OF && MTD - select CRC32 - select GENERIC_NET_UTILS + 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 diff --git a/drivers/nvmem/layouts/Kconfig b/drivers/nvmem/layouts/Kconfig index 9c6e672fc350..5e586dfebe47 100644 --- a/drivers/nvmem/layouts/Kconfig +++ b/drivers/nvmem/layouts/Kconfig @@ -26,6 +26,17 @@ config NVMEM_LAYOUT_ONIE_TLV If unsure, say N. +config NVMEM_LAYOUT_U_BOOT_ENV + tristate "U-Boot environment variables layout" + 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 + as NVMEM cells so they can be referenced by other drivers. + + If unsure, say N. + endmenu endif 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..1787cfaf45f8 --- /dev/null +++ b/drivers/nvmem/layouts/u-boot-env.c @@ -0,0 +1,212 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2022 - 2023 Rafał Miłecki <rafal@milecki.pl> + */ + +#include <linux/crc32.h> +#include <linux/etherdevice.h> +#include <linux/export.h> +#include <linux/if_ether.h> +#include <linux/nvmem-consumer.h> +#include <linux/nvmem-provider.h> +#include <linux/of.h> +#include <linux/slab.h> + +#include "u-boot-env.h" + +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; + DECLARE_FLEX_ARRAY(uint8_t, data); +} __packed; + +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 int u_boot_env_read_post_process_ethaddr(void *context, const char *id, int index, + unsigned int offset, void *buf, size_t bytes) +{ + u8 mac[ETH_ALEN]; + + if (bytes != 3 * ETH_ALEN - 1) + return -EINVAL; + + if (!mac_pton(buf, mac)) + return -EINVAL; + + if (index) + eth_addr_add(mac, index); + + ether_addr_copy(buf, mac); + + return 0; +} + +static int u_boot_env_parse_cells(struct device *dev, struct nvmem_device *nvmem, uint8_t *buf, + size_t data_offset, size_t data_len) +{ + char *data = buf + data_offset; + char *var, *value, *eq; + + 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) + return -ENOMEM; + info.offset = data_offset + value - data; + info.bytes = strlen(value); + info.np = of_get_child_by_name(dev->of_node, info.name); + if (!strcmp(var, "ethaddr")) { + info.raw_len = strlen(value); + info.bytes = ETH_ALEN; + info.read_post_process = u_boot_env_read_post_process_ethaddr; + } + + nvmem_add_one_cell(nvmem, &info); + } + + return 0; +} + +int u_boot_env_parse(struct device *dev, struct nvmem_device *nvmem, + 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; + uint8_t *buf; + int bytes; + int err; + + 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) + return bytes; + else if (bytes != dev_size) + return -EIO; + + 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_cells(dev, nvmem, 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; +} +EXPORT_SYMBOL_GPL(u_boot_env_parse); + +static int u_boot_env_add_cells(struct device *dev, struct nvmem_device *nvmem) +{ + const struct of_device_id *match; + struct device_node *layout_np; + enum u_boot_env_format format; + + layout_np = of_nvmem_layout_get_container(nvmem); + if (!layout_np) + return -ENOENT; + + match = of_match_node(u_boot_env_of_match_table, layout_np); + if (!match) + return -ENOENT; + + format = (uintptr_t)match->data; + + of_node_put(layout_np); + + return u_boot_env_parse(dev, nvmem, format); +} + +static int u_boot_env_probe(struct nvmem_layout *layout) +{ + layout->add_cells = u_boot_env_add_cells; + + return nvmem_layout_register(layout); +} + +static void u_boot_env_remove(struct nvmem_layout *layout) +{ + nvmem_layout_unregister(layout); +} + +static struct nvmem_layout_driver u_boot_env_layout = { + .driver = { + .owner = THIS_MODULE, + .name = "u-boot-env-layout", + .of_match_table = u_boot_env_of_match_table, + }, + .probe = u_boot_env_probe, + .remove = u_boot_env_remove, +}; +module_nvmem_layout_driver(u_boot_env_layout); + +MODULE_AUTHOR("Rafał Miłecki"); +MODULE_LICENSE("GPL"); +MODULE_DEVICE_TABLE(of, u_boot_env_of_match_table); diff --git a/drivers/nvmem/layouts/u-boot-env.h b/drivers/nvmem/layouts/u-boot-env.h new file mode 100644 index 000000000000..dd5f280ac047 --- /dev/null +++ b/drivers/nvmem/layouts/u-boot-env.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _LINUX_NVMEM_LAYOUTS_U_BOOT_ENV_H +#define _LINUX_NVMEM_LAYOUTS_U_BOOT_ENV_H + +enum u_boot_env_format { + U_BOOT_FORMAT_SINGLE, + U_BOOT_FORMAT_REDUNDANT, + U_BOOT_FORMAT_BROADCOM, +}; + +int u_boot_env_parse(struct device *dev, struct nvmem_device *nvmem, + enum u_boot_env_format format); + +#endif /* ifndef _LINUX_NVMEM_LAYOUTS_U_BOOT_ENV_H */ diff --git a/drivers/nvmem/u-boot-env.c b/drivers/nvmem/u-boot-env.c index ab8c9bf63d99..386b2b255c30 100644 --- a/drivers/nvmem/u-boot-env.c +++ b/drivers/nvmem/u-boot-env.c @@ -3,23 +3,15 @@ * Copyright (C) 2022 Rafał Miłecki <rafal@milecki.pl> */ -#include <linux/crc32.h> -#include <linux/etherdevice.h> -#include <linux/if_ether.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.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, -}; +#include "layouts/u-boot-env.h" struct u_boot_env { struct device *dev; @@ -29,24 +21,6 @@ struct u_boot_env { struct mtd_info *mtd; }; -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; - DECLARE_FLEX_ARRAY(uint8_t, data); -} __packed; - static int u_boot_env_read(void *context, unsigned int offset, void *val, size_t bytes) { @@ -69,131 +43,6 @@ static int u_boot_env_read(void *context, unsigned int offset, void *val, return 0; } -static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, int index, - unsigned int offset, void *buf, size_t bytes) -{ - u8 mac[ETH_ALEN]; - - if (bytes != 3 * ETH_ALEN - 1) - return -EINVAL; - - if (!mac_pton(buf, mac)) - return -EINVAL; - - if (index) - eth_addr_add(mac, index); - - ether_addr_copy(buf, mac); - - 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 nvmem_device *nvmem = priv->nvmem; - struct device *dev = priv->dev; - char *data = buf + data_offset; - char *var, *value, *eq; - - 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) - return -ENOMEM; - info.offset = data_offset + value - data; - info.bytes = strlen(value); - info.np = of_get_child_by_name(dev->of_node, info.name); - if (!strcmp(var, "ethaddr")) { - info.raw_len = strlen(value); - info.bytes = ETH_ALEN; - info.read_post_process = u_boot_env_read_post_process_ethaddr; - } - - nvmem_add_one_cell(nvmem, &info); - } - - return 0; -} - -static int u_boot_env_parse(struct u_boot_env *priv) -{ - struct nvmem_device *nvmem = priv->nvmem; - 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; - size_t dev_size; - uint32_t crc32; - uint32_t calc; - uint8_t *buf; - int bytes; - int err; - - 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) - return bytes; - else if (bytes != dev_size) - return -EIO; - - 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 = 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_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 = { @@ -225,7 +74,7 @@ static int u_boot_env_probe(struct platform_device *pdev) if (IS_ERR(priv->nvmem)) return PTR_ERR(priv->nvmem); - return u_boot_env_parse(priv); + return u_boot_env_parse(dev, priv->nvmem, priv->format); } static const struct of_device_id u_boot_env_of_match_table[] = {