Re: [PATCH 6/8] nvmem: transformations: ethernet address offset support
From: Rafał Miłecki <zajec5@gmail.com>
Date: 2022-01-25 12:14:43
Also in:
linux-arm-kernel, linux-devicetree, lkml
On 28.12.2021 15:25, Michael Walle wrote:
quoted hunk ↗ jump to hunk
An nvmem cell might just contain a base MAC address. To generate a address of a specific interface, add a transformation to add an offset to this base address. Add a generic implementation and the first user of it, namely the sl28 vpd storage. Signed-off-by: Michael Walle <redacted> --- drivers/nvmem/transformations.c | 45 +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)diff --git a/drivers/nvmem/transformations.c b/drivers/nvmem/transformations.c index 61642a9feefb..15cd26da1f83 100644 --- a/drivers/nvmem/transformations.c +++ b/drivers/nvmem/transformations.c@@ -12,7 +12,52 @@ struct nvmem_transformations { nvmem_cell_post_process_t pp; }; +/** + * nvmem_transform_mac_address_offset() - Add an offset to a mac address cell + * + * A simple transformation which treats the index argument as an offset and add + * it to a mac address. This is useful, if the nvmem cell stores a base + * ethernet address. + * + * @index: nvmem cell index + * @data: nvmem data + * @bytes: length of the data + * + * Return: 0 or negative error code on failure. + */ +static int nvmem_transform_mac_address_offset(int index, unsigned int offset, + void *data, size_t bytes) +{ + if (bytes != ETH_ALEN) + return -EINVAL; + + if (index < 0) + return -EINVAL; + + if (!is_valid_ether_addr(data)) + return -EINVAL; + + eth_addr_add(data, index); + + return 0; +} + +static int nvmem_kontron_sl28_vpd_pp(void *priv, const char *id, int index, + unsigned int offset, void *data, + size_t bytes) +{ + if (!id) + return 0; + + if (!strcmp(id, "mac-address")) + return nvmem_transform_mac_address_offset(index, offset, data, + bytes); + + return 0; +} + static const struct nvmem_transformations nvmem_transformations[] = { + { .compatible = "kontron,sl28-vpd", .pp = nvmem_kontron_sl28_vpd_pp }, {} };
I think it's a rather bad solution that won't scale well at all.
You'll end up with a lot of NVMEM device specific strings and code in a
NVMEM core.
You'll have a lot of duplicated code (many device specific functions
calling e.g. nvmem_transform_mac_address_offset()).
I think it also ignores fact that one NVMEM device can be reused in
multiple platforms / device models using different (e.g. vendor / device
specific) cells.
What if we have:
1. Foo company using "kontron,sl28-vpd" with NVMEM cells:
a. "mac-address"
b. "mac-address-2"
c. "mac-address-3"
2. Bar company using "kontron,sl28-vpd" with NVMEM cell:
a. "mac-address"
In the first case you don't want any transformation.
If you consider using transformations for ASCII formats too then it
causes another conflict issue. Consider two devices:
1. Foo company device with BIN format of MAC
2. Bar company device with ASCII format of MAC
Both may use exactly the same binding:
partition@0 {
compatible = "nvmem-cells";
reg = <0x0 0x100000>;
label = "bootloader";
#address-cells = <1>;
#size-cells = <1>;
mac-address@100 {
reg = <0x100 0x6>;
};
};
how are you going to handle them with proposed implementation? You can't
support both if you share "nvmem-cells" compatible string.
I think that what can solve those problems is assing "compatible" to
NVMEM cells.
Let me think about details of that possible solution.