Hi Micha,
On 6/12/25 5:59 PM, MichaIng wrote:
[You don't often get email from [email protected]. Learn why this is
important at https://aka.ms/LearnAboutSenderIdentification ]
Hi Quentin,
thanks for the review!
> You could set the eth2addr in your board file if you wanted instead of
> setting it for everybody.
We provide OS images for everyone, and our users complained about the
changing MAC addresses. I agree with them that generally the exception
is that MAC addresses are static by default, and randomising them being
an optional privacy feature when connecting to untrusted networks, like
a WiFi hotspot with a mobile phone. Setting eth2addr in boot
script/environment would require a random MAC address generation our end
on first boot or so on each system, to rule out collisions. Since U-Boot
I didn't mean to suggest to have this as part of the boot script. Board
file (for me) is any file in the board/ directory which applies to a
board. See board/theobroma-systems/ringneck_px30 for example.
There you could replace misc_init_r entirely, or define your own
rockchip_early_misc_init_r (like I do in
board/theobroma-systems/ringneck_px30/ringneck-px30.c for example) or
add a rockchip_late_misc_init_r callback after the ethernet addresses if
you want?
That's what I was suggesting essentially.
has the feature implemented already, it just makes sense to make use of
it and extend it for the affected Rockchip builds, also to have the
somewhat nicely aligned with the last digit deferring only. So we
currently patch our sources.
That's fair. The point is I'm not sure it makes sense to have X ethXaddr
variables with X being the highest possible amount of Ethernet devices
one can find on a Rockchip device.
> What's the number when we'll decide this is too many? I have a board
> with 5 Ethernet ports for example :)
Good question. In Germany we say "Aller guten Dinge sind drei" (All good
things come in threes). Jokes aside: not sure whether there are Rockchip
SoC boards which provide more Ethernet NICs, but I have 3 with 3 ones
(in one case provided by an official expansion board), all RK3588 with
two onboard PCIe adapters. There is another PCIe bus, but commonly used
for an M.2 socket, not sure whether it could theoretically provide a 4th
Ethernet adapter. I don't know the background about why the Ethernet
driver/firmware is not able to read/assign the hardware MAC, or why
there is no hardware MAC, as I understand that this is an IEEE
requirement. So what U-Boot does seems a workaround, and I agree that,
if there is no generic approach, it has limits.
> I don't know how far we want to go to avoid defining unused ethXaddr
> variables in the environment? We could use of_alias_get_highest_id for
> example and generate enough environment variables based on that info?
This sounds reasonable. But are ethXaddr variables not generated before
boot scripts are called, when the number of alias IDs cannot be known
yet, and MAC addresses assigned afterwards, when the OS device tree was
loaded? When generating the variables afterwards, all good, but then
I assume you should have a Device Tree in U-Boot, on Rockchip it's
typically available in U-Boot SPL already (except on some TPL-less or
SPL-extremly-size-limited configurations) so you can do that based on
the aliases found in the U-Boot device tree.
Then you can also do additional fdt fixups on the kernel DT before
booting the kernel if that is something you're worried about (U-Boot DT
being different from kernel DT for example). There you could read the
aliases node and generate a mac address for all controllers that do not
already have one in the environment for example.
You likely need both, first one for consistent network support (MAC
address) in U-Boot and the second one in case the kernel DT differs from
the U-Boot DT (in terms of amount of Ethernet controllers for example).
E.g. you could have an overlay for an adapter board that has multiple
Ethernet ports each with their own MAC address. You don't necessarily
need those enabled in U-Boot so the base DTB would be enough, but you
for sure want all of them enabled in the kernel).
Cheers,
Quentin
