diff mbox series

[v2] rpi: Copy eth MAC address from fw DT to loaded DT

Message ID 20240319215836.48178-1-martin@wetterwald.eu
State Rejected, archived
Delegated to: Peter Robinson
Headers show
Series [v2] rpi: Copy eth MAC address from fw DT to loaded DT | expand

Commit Message

Martin Wetterwald March 19, 2024, 9:58 p.m. UTC
Raspberry Pi B models before model 4 don't have an EEPROM nor an OTP to
store the permanent factory MAC address of the NIC.
So the firmware that runs initially computes the factory MAC address of
the board and patches the DTB to give that information to the next
stage.
The MAC is put in the standard property `local-mac-address` which is
inserted in the `ethernet0` node of the firmware-provided FDT.

Here is the algo used by Linux to determine the MAC address (applies for
all models with smsc95xx). It stops as soon as we find a valid MAC.
1) Look at the FDT (mac-address, local-mac-address, address)
2) Try to fetch MAC from EEPROM (always fails on those Raspberry Pis)
[ 3) Check module parameter smsc95xx.macaddr ]
4) Generate a random MAC if we didn't find anything

Note that step 3) only applies in the Raspberry Pi fork of the Linux
kernel. The upstream kernel doesn't have that step, as that module
variable doesn't exist.

When CONFIG_MISC_INIT_R=y, U-Boot requests directly the MAC from the
running firmware in the GPU through the Raspberry Pi Mailbox. It then
stores it in ${usbethaddr} environment variable.
In U-Boot, the MAC is then often given to Linux like this:

> setenv bootargs [...] smsc95xx.macaddr="${usbethaddr}" [...]

This works in the Raspberry Pi fork of the kernel, because that module
parameter exists. But it doesn't work in the upstream kernel.
With the upstream kernel: if we make U-Boot forward the firmware-patched
FDT directly to the kernel, the MAC will be correct, because
`local-mac-addr` will be present. But if we configure U-Boot to give a
fresh FDT to the kernel, the MAC will be randomly generated because the
`local-mac-addr` property set by the firmware was not copied by U-Boot
to the loaded FDT.

This patch extends commit 6d0642494993 ("rpi: Copy properties from
firmware dtb to the loaded dtb") by making U-Boot copy the
`local-mac-address` property from the firmware FDT to the loaded FDT.
It makes it then possible to use the upstream kernel and to give it a
fresh FDT (not the firmware-provided one) without having the kernel
generate a random MAC address.

Note that this is only possible if CONFIG_OF_BOARD_SETUP=y and
ft_board_setup() is called.

Cc: Matthias Brugger <mbrugger@suse.com>
Cc: Peter Robinson <pbrobinson@gmail.com>
Cc: Antoine Mazeas <antoine@karthanis.net>
Signed-off-by: Martin Wetterwald <martin@wetterwald.eu>
---

Changes in v2:
- Clarify the commit message by pointing out that smsc95xx.macaddr
  module param only exists in the Raspberry Pi fork of the Linux kernel
  and not upstream
- Make the intent of the patch clearer in the commit message (booting
  upstream kernel + fresh FDT without having a random-generated MAC)

 board/raspberrypi/rpi/rpi.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Martin Wetterwald March 19, 2024, 10:23 p.m. UTC | #1
I reworked the commit message because I noticed the upstream Linux kernel has a
difference with the Raspberry Pi fork of the kernel regarding the algorithm
used to determine the MAC address of the smsc95xx.
There is no smsc95xx.macaddr in the upstream kernel, and using the upstream
kernel is actually the real use case for the patch.

I'm now wondering whether some users are using the module parameter
smsc95xx.macaddr of the Raspberry Pi fork of the kernel to try to change the
MAC, and put one which is not the factory default?
Does this use case exists? If this is the case, the current algorithm in the
forked kernel would mean that, the FDT-provided MAC would have priority over
anything set in the module parameter.
This is already the case anyway when people give to the forked kernel the
firmware-provided FDT (smsc95xx.macaddr is ignored). So this patch aligns
behaviors.
But we should be aware that applying this patch would change the behavior in
the following situation (all conditions need to be true):
- Forked kernel is used
- Fresh FDT (not firmware one) is provided to the kernel
- Custom (not factory) MAC was set in smsc95xx.macaddr parameter

Martin
Peter Robinson March 20, 2024, 12:24 p.m. UTC | #2
Hi Martin,

> I reworked the commit message because I noticed the upstream Linux kernel has a
> difference with the Raspberry Pi fork of the kernel regarding the algorithm
> used to determine the MAC address of the smsc95xx.

So looking at this on an original 3B because that's what I had booted
for other testing.

> There is no smsc95xx.macaddr in the upstream kernel, and using the upstream
> kernel is actually the real use case for the patch.

In most cases in device tree this is dealt with aliases [1] to
ethernet0, 1 etc. In the U-Boot env I have two env set, ethaddr and
usbethaddr both to the same MAC address with b8:27:eb prefix which is
assigned to RPi foundation [2]. When I boot Linux I get that MAC
address on the interface as expected.

> I'm now wondering whether some users are using the module parameter
> smsc95xx.macaddr of the Raspberry Pi fork of the kernel to try to change the
> MAC, and put one which is not the factory default?

I would have thought that the recent RPi kernels would also fall back
to the upstream variation too if their fork option wasn't available.

> Does this use case exists? If this is the case, the current algorithm in the
> forked kernel would mean that, the FDT-provided MAC would have priority over
> anything set in the module parameter.
> This is already the case anyway when people give to the forked kernel the
> firmware-provided FDT (smsc95xx.macaddr is ignored). So this patch aligns
> behaviors.
> But we should be aware that applying this patch would change the behavior in
> the following situation (all conditions need to be true):
> - Forked kernel is used
> - Fresh FDT (not firmware one) is provided to the kernel
> - Custom (not factory) MAC was set in smsc95xx.macaddr parameter

Can we take a step back, I'm still unsure what you're trying to
resolve here. For me with the upstream U-Boot and the upstream kernel
I get the vendor MAC address as I would expect. It should work on any
kernel/DT combo as long as the ether ethernet alias is present in the
DT.

Peter

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/broadcom/bcm283x-rpi-smsc9514.dtsi#n3
[2] https://maclookup.app/search/result?mac=b8:27:eb:
diff mbox series

Patch

diff --git a/board/raspberrypi/rpi/rpi.c b/board/raspberrypi/rpi/rpi.c
index 2851ebc985..b36a893047 100644
--- a/board/raspberrypi/rpi/rpi.c
+++ b/board/raspberrypi/rpi/rpi.c
@@ -566,6 +566,9 @@  void  update_fdt_from_fw(void *fdt, void *fw_fdt)
 
 	/* address of the PHY device as provided by the firmware  */
 	copy_property(fdt, fw_fdt, "ethernet0/mdio@e14/ethernet-phy@1", "reg");
+
+	/* MAC address of the NIC as provided by the firmware */
+	copy_property(fdt, fw_fdt, "ethernet0", "local-mac-address");
 }
 
 int ft_board_setup(void *blob, struct bd_info *bd)