drivers/nvmem/layouts/u-boot-env.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
Manufacturers use various names for MAC addresses in u-boot.
Stuff like:
eth1addr
RADIOADDR0
RADIOADDR1
wmac1
wmac2
wanaddr
wlanmac
lanmac
wanmac
and many others. Don't restruct to just ethaddr in order to handle the
various names.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/nvmem/layouts/u-boot-env.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/nvmem/layouts/u-boot-env.c b/drivers/nvmem/layouts/u-boot-env.c
index f27f387bb52a..afa196be4161 100644
--- a/drivers/nvmem/layouts/u-boot-env.c
+++ b/drivers/nvmem/layouts/u-boot-env.c
@@ -75,11 +75,9 @@ static int u_boot_env_parse_cells(struct device *dev, struct nvmem_device *nvmem
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;
- }
+ 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);
}
--
2.54.0
Hi Rosen,
> --- a/drivers/nvmem/layouts/u-boot-env.c
> +++ b/drivers/nvmem/layouts/u-boot-env.c
> @@ -75,11 +75,9 @@ static int u_boot_env_parse_cells(struct device *dev, struct nvmem_device *nvmem
> 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;
> - }
> + info.raw_len = strlen(value);
> + info.bytes = ETH_ALEN;
How do you know this is a MAC address?
Thanks,
Miquèl
On 29/04/2026 at 09:39:53 +02, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> Hi Rosen,
>
>> --- a/drivers/nvmem/layouts/u-boot-env.c
>> +++ b/drivers/nvmem/layouts/u-boot-env.c
>> @@ -75,11 +75,9 @@ static int u_boot_env_parse_cells(struct device *dev, struct nvmem_device *nvmem
>> 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;
>> - }
>> + info.raw_len = strlen(value);
>> + info.bytes = ETH_ALEN;
>
> How do you know this is a MAC address?
You are overwriting info.bytes above. I am sorry but this change has
only been tested with your very own settings and does not look like
something we could accept.
You cannot attach the "process ethaddr" subprocess to any node, that
would not make any sense.
I guess we can extend a bit the condition with thinks like "eth%daddr",
but that must remain limited.
Thanks,
Miquèl
On Wed, Apr 29, 2026 at 12:44 AM Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
>
> On 29/04/2026 at 09:39:53 +02, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> > Hi Rosen,
> >
> >> --- a/drivers/nvmem/layouts/u-boot-env.c
> >> +++ b/drivers/nvmem/layouts/u-boot-env.c
> >> @@ -75,11 +75,9 @@ static int u_boot_env_parse_cells(struct device *dev, struct nvmem_device *nvmem
> >> 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;
> >> - }
> >> + info.raw_len = strlen(value);
> >> + info.bytes = ETH_ALEN;
> >
> > How do you know this is a MAC address?
>
> You are overwriting info.bytes above. I am sorry but this change has
> only been tested with your very own settings and does not look like
> something we could accept.
>
> You cannot attach the "process ethaddr" subprocess to any node, that
> would not make any sense.
>
> I guess we can extend a bit the condition with thinks like "eth%daddr",
> but that must remain limited.
how about a static array of the names and strcmp over that?
>
> Thanks,
> Miquèl
On 29/04/2026 at 14:41:36 -07, Rosen Penev <rosenp@gmail.com> wrote:
> On Wed, Apr 29, 2026 at 12:44 AM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
>>
>> On 29/04/2026 at 09:39:53 +02, Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>>
>> > Hi Rosen,
>> >
>> >> --- a/drivers/nvmem/layouts/u-boot-env.c
>> >> +++ b/drivers/nvmem/layouts/u-boot-env.c
>> >> @@ -75,11 +75,9 @@ static int u_boot_env_parse_cells(struct device *dev, struct nvmem_device *nvmem
>> >> 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;
>> >> - }
>> >> + info.raw_len = strlen(value);
>> >> + info.bytes = ETH_ALEN;
>> >
>> > How do you know this is a MAC address?
>>
>> You are overwriting info.bytes above. I am sorry but this change has
>> only been tested with your very own settings and does not look like
>> something we could accept.
>>
>> You cannot attach the "process ethaddr" subprocess to any node, that
>> would not make any sense.
>>
>> I guess we can extend a bit the condition with thinks like "eth%daddr",
>> but that must remain limited.
> how about a static array of the names and strcmp over that?
You can check for eth on the first offsets, and then addr starting
from offset 4, allowing up to 10 ethernet interfaces to by dynamically
assigned, that should be enough. If not enough, you can also try at
offset 5, which would bring that up to 100 possible interfaces. I belive
you can directly give the strings to strcmp, no need for a global
declaration.
Thanks,
Miquèl
© 2016 - 2026 Red Hat, Inc.