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. > 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 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? > (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"); 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. -- Tom
signature.asc
Description: PGP signature