On Tue, 10 Jun 2025 07:38:54 +0200
Lukas Schmid <[email protected]> wrote:

Hi Lukas,

> Boards using MAC addresses stored in EEPROM via the device tree's
> `nvmem-cells` mechanism may already have a valid MAC loaded by the
> device model. However, setup_environment() currently ignores this
> and generates a fallback address from the SoC SID if no environment
> variable is set.
> 
> This leads to a mismatch warning during boot when the kernel or U-Boot
> detects the MAC from nvmem and compares it to the environment.
> 
> This patch checks whether the corresponding ethernet device node
> has a `nvmem-cells` property and skips generating a fallback MAC
> if it does, thus avoiding redundant or incorrect ethaddr setup.

So while this seems like a pragmatic solution, I feel it's not the
right one: This "peeking into the DT to skip something" sounds
backwards. You have probably investigated this more thoroughly, but how
does this work here? Did the generic code already parse the MAC address
and put it somewhere, and our code here was about to overwrite it? Or
will the generic code later fill the gap, if the sunxi code didn't set a
MAC address based on the SID?

I feel like the sunxi code is to blame here for doing it at the wrong
time or using its own method instead of coordinating with the generic
code, so I'd rather fix this properly than papering over it here.

I will help you find a better solution, but if you can dump here what
you learned, I'd be grateful, as I am too lazy atm to chase down the
code ;-)

Cheers,
Andre
 
> This improves compatibility with nvmem-based MAC provisioning and
> aligns U-Boot behavior with Linux.
> 
> Signed-off-by: Lukas Schmid <[email protected]>
> ---
>  board/sunxi/board.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/board/sunxi/board.c b/board/sunxi/board.c
> index ac9cefc6..41b85c66 100644
> --- a/board/sunxi/board.c
> +++ b/board/sunxi/board.c
> @@ -774,10 +774,23 @@ static void setup_environment(const void *fdt)
>               return;
>  
>       for (i = 0; i < 4; i++) {
> +             const char *alias;
> +             int nodeoffset;
> +             int len;
> +             const fdt32_t *prop;
> +
>               sprintf(ethaddr, "ethernet%d", i);
> -             if (!fdt_get_alias(fdt, ethaddr))
> +             alias = fdt_get_alias(fdt, ethaddr);
> +             if (!alias)
>                       continue;
>  
> +             nodeoffset = fdt_path_offset(fdt, alias);
> +             if (nodeoffset >= 0) {
> +                     prop = fdt_getprop(fdt, nodeoffset, "nvmem-cells", 
> &len);
> +                     if (prop && len > 0)
> +                             continue;
> +             }
> +
>               if (i == 0)
>                       strcpy(ethaddr, "ethaddr");
>               else

Reply via email to