Am 2021-11-16 22:14, schrieb Tom Rini:
On Mon, Nov 15, 2021 at 11:45:51PM +0100, Michael Walle wrote:

Nowadays, u-boot (when CONFIG_NET_RANDOM_ETHADDR is set) will set
enetaddr to a random value if not set and then pass the randomly
generated MAC address to linux.

First, for clarity I'm not nak'ing this.  I kind of would like to see a
slight reword as I think some things aren't 100% correct, even if the
"save random MAC to ethaddr environment variable" change goes in.  For
example, it's quite long standing that (dev|pdata)->enetaddr populates
"mac-address" and "local-mac-address" and it seems in some older cases
we only set the "local-mac-address" property.

fdt_fixup_memory() in common/fdt_support.c does a env_get(mac).

I'm not even sure, if there is a connection between what is fixed up
in the kernel DT and the corresponding device in u-boot if
CONFIG_DM_SEQ_ALIAS is not set.

This is bad for the following reasons:
 (1) it makes it impossible for linux to detect this error
 (2) linux won't trigger any fallback mechanism for the case where
     it didn't find any valid MAC address

This feels in some ways to be a limitation of the binding:
https://www.kernel.org/doc/Documentation/devicetree/bindings/net/ethernet-controller.yaml

Agreed. But it doesn't render my argument invalid ;)

And it reads like we really must be populating "mac-address" with that
random one and while providing a blank "local-mac-address" would be a
way to say we don't know the true device one, it seems that wouldn't be
used / noticed?

I guess it will just use the one populated in mac-address, i.e. the
random one. Btw, the binding says "last used" what if u-boot never
actually used that ethernet controller, should it then be empty or
populated with the random mac address.

 (3) a saveenv will store this randomly generated MAC address in the
     environment

Probably, the user will also be unaware that something is wrong. He will just get different MAC addresses on each reboot, asking himself why this
is the case.

As this board usually have a serial port, the user can just fix this by
setting the MAC address manually in the environment. Also disable the
netconsole just in case, because it cannot be guaranteed that it will
work in any case. After all, this was just a convenience option, because
the bootloader - right now - doesn't have the ability to read the MAC
address, which is stored in the OTP. But it is far more important to
have a clear view of whats wrong with a board and that means we can no
longer use this Kconfig option.

Signed-off-by: Michael Walle <mich...@walle.cc>
---
 configs/kontron_sl28_defconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/configs/kontron_sl28_defconfig b/configs/kontron_sl28_defconfig
index af907175f1..7fb6bdbe82 100644
--- a/configs/kontron_sl28_defconfig
+++ b/configs/kontron_sl28_defconfig
@@ -59,8 +59,6 @@ CONFIG_OF_LIST=""
 CONFIG_ENV_OVERWRITE=y
 CONFIG_ENV_IS_IN_SPI_FLASH=y
 CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
-CONFIG_NET_RANDOM_ETHADDR=y
-CONFIG_NETCONSOLE=y
 CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_SCSI_AHCI=y
 CONFIG_SATA_CEVA=y

Now, an alternate solution here would be to enable these two options
still and write an ft_board_setup() with:
... if ethaddr is in the locally administered pool then
        fdt_delprop(... "mac-address");
        fdt_delprop(... "local-mac-address");

Which would also trigger if a user sets a "locally administered"
mac himself. Even if unlikely, I'm opposed to such magic and
unexpected behavior. Sooner or later it will bite you.

And that should cause the kernel to fall through the cases to find out
where to get the real MAC from. I'm not super happy with this at first,
but I also don't see anything clever in the binding we can do.

If I'll need this one again, then I'll just add the random mac
generation in board_eth_init(), I think.

-michael

Reply via email to