[PATCH] nvmem: layouts: u-boot-env: remove ethaddr check

Rosen Penev posted 1 patch 1 month, 2 weeks ago
drivers/nvmem/layouts/u-boot-env.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
[PATCH] nvmem: layouts: u-boot-env: remove ethaddr check
Posted by Rosen Penev 1 month, 2 weeks ago
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
Re: [PATCH] nvmem: layouts: u-boot-env: remove ethaddr check
Posted by Miquel Raynal 1 month, 2 weeks ago
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
Re: [PATCH] nvmem: layouts: u-boot-env: remove ethaddr check
Posted by Miquel Raynal 1 month, 2 weeks ago
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
Re: [PATCH] nvmem: layouts: u-boot-env: remove ethaddr check
Posted by Rosen Penev 1 month, 2 weeks ago
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
Re: [PATCH] nvmem: layouts: u-boot-env: remove ethaddr check
Posted by Miquel Raynal 1 month, 1 week ago
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