[PATCH v2 0/3] Introduce NET_PREFER_ROM_MAC_ADDR config

[PATCH v2 0/3] Introduce NET_PREFER_ROM_MAC_ADDR config

[Detlev Casanova]

When there is a MAC address mismatch between ROM and environment, u-boot
chooses the one from the environment. This config allows to prefer the
MAC address from the ROM instead of the environment.

See the first commit message for details.

Changes since v1:
 - Simplify implementation with a NET_PREFER_ROM_MAC_ADDR config

Detlev Casanova (3):
  net: eth-uclass: Introduce NET_PREFER_ROM_MAC_ADDR
  configs/rock5b: Set NET_PREFER_ROM_MAC_ADDR to y
  net: eth-uclass: Add driver source possibility

 configs/rock5b-rk3588_defconfig |  1 +
 net/Kconfig                     |  7 +++++++
 net/eth-uclass.c                | 17 +++++++++++++----
 3 files changed, 21 insertions(+), 4 deletions(-)

-- 
2.43.2

[PATCH v2 1/3] net: eth-uclass: Introduce NET_PREFER_ROM_MAC_ADDR

[Detlev Casanova]

On some boards, a MAC address is set based on the CPU ID or other
information. This is usually done in the misc_init_r() function.

This becomes a problem for net devices that are probed after the call to
misc_init_r(), for example, when the ethernet is on a PCI port, which
needs to be enumerated.

In this case, misc_init_r() will set the ethaddr variable, then, when
the ethernet device is probed, if it has a ROM address, u-boot will warn
about a MAC address mismatch and use the misc_init_r() address instead
of the one in ROM.

The operating system later will most likely use the ROM MAC address,
which can be confusing.

To avoid that, this commit introduces NET_PREFER_ROM_MAC_ADDR that can
be set for boards that have such an interface.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 net/Kconfig      | 7 +++++++
 net/eth-uclass.c | 9 +++++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/net/Kconfig b/net/Kconfig
index 5dff6336293..1797c2cea35 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -227,6 +227,13 @@ config SERVERIP
 	string "Value of the default 'serverip' value in the environment"
 	depends on USE_SERVERIP
 
+config NET_PREFER_ROM_MAC_ADDR
+	bool "Prefer using HW MAC address if environment address differs"
+	default n
+	help
+	  In case of a MAC address mismatch between the environment and the HW,
+	  prefer using the HW address.
+
 config PROT_TCP
 	bool "TCP stack"
 	help
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 3d0ec91dfa4..682de3ec7bd 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -584,8 +584,13 @@ static int eth_post_probe(struct udevice *dev)
 			       env_enetaddr);
 		}
 
-		/* Override the ROM MAC address */
-		memcpy(pdata->enetaddr, env_enetaddr, ARP_HLEN);
+		if (!IS_ENABLED(CONFIG_NET_PREFER_ROM_MAC_ADDR)) {
+			/* Override the ROM MAC address */
+			printf("Using address in environment\n");
+			memcpy(pdata->enetaddr, env_enetaddr, ARP_HLEN);
+		} else {
+			printf("Using address in %s\n", source);
+		}
 	} else if (is_valid_ethaddr(pdata->enetaddr)) {
 		eth_env_set_enetaddr_by_index("eth", dev_seq(dev),
 					      pdata->enetaddr);
-- 
2.43.2

Re: [PATCH v2 1/3] net: eth-uclass: Introduce NET_PREFER_ROM_MAC_ADDR

[Marek Vasut]

On 4/22/24 3:56 PM, Detlev Casanova wrote:
> On some boards, a MAC address is set based on the CPU ID or other
> information. This is usually done in the misc_init_r() function.
> 
> This becomes a problem for net devices that are probed after the call to
> misc_init_r(), for example, when the ethernet is on a PCI port, which
> needs to be enumerated.
> 
> In this case, misc_init_r() will set the ethaddr variable, then, when
> the ethernet device is probed, if it has a ROM address, u-boot will warn
> about a MAC address mismatch and use the misc_init_r() address instead
> of the one in ROM.
> 
> The operating system later will most likely use the ROM MAC address,
> which can be confusing.
> 
> To avoid that, this commit introduces NET_PREFER_ROM_MAC_ADDR that can
> be set for boards that have such an interface.
> 
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>

Won't the system pick ROM MAC if $ethaddr is not set ?

Re: [PATCH v2 1/3] net: eth-uclass: Introduce NET_PREFER_ROM_MAC_ADDR

[Detlev Casanova]

[-- Attachment #1: Type: text/plain, Size: 1204 bytes --]

On Monday, April 22, 2024 3:47:21 P.M. EDT Marek Vasut wrote:
> On 4/22/24 3:56 PM, Detlev Casanova wrote:
> > On some boards, a MAC address is set based on the CPU ID or other
> > information. This is usually done in the misc_init_r() function.
> > 
> > This becomes a problem for net devices that are probed after the call to
> > misc_init_r(), for example, when the ethernet is on a PCI port, which
> > needs to be enumerated.
> > 
> > In this case, misc_init_r() will set the ethaddr variable, then, when
> > the ethernet device is probed, if it has a ROM address, u-boot will warn
> > about a MAC address mismatch and use the misc_init_r() address instead
> > of the one in ROM.
> > 
> > The operating system later will most likely use the ROM MAC address,
> > which can be confusing.
> > 
> > To avoid that, this commit introduces NET_PREFER_ROM_MAC_ADDR that can
> > be set for boards that have such an interface.
> > 
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> 
> Won't the system pick ROM MAC if $ethaddr is not set ?

Yes, and in the case of rockchip, misc_init_r() will set an $ethaddr based on 
the cpuid, which makes the eth driver use that instead of the ROM one.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v2 1/3] net: eth-uclass: Introduce NET_PREFER_ROM_MAC_ADDR

[Marek Vasut]

On 4/23/24 3:35 PM, Detlev Casanova wrote:
> On Monday, April 22, 2024 3:47:21 P.M. EDT Marek Vasut wrote:
>> On 4/22/24 3:56 PM, Detlev Casanova wrote:
>>> On some boards, a MAC address is set based on the CPU ID or other
>>> information. This is usually done in the misc_init_r() function.
>>>
>>> This becomes a problem for net devices that are probed after the call to
>>> misc_init_r(), for example, when the ethernet is on a PCI port, which
>>> needs to be enumerated.
>>>
>>> In this case, misc_init_r() will set the ethaddr variable, then, when
>>> the ethernet device is probed, if it has a ROM address, u-boot will warn
>>> about a MAC address mismatch and use the misc_init_r() address instead
>>> of the one in ROM.
>>>
>>> The operating system later will most likely use the ROM MAC address,
>>> which can be confusing.
>>>
>>> To avoid that, this commit introduces NET_PREFER_ROM_MAC_ADDR that can
>>> be set for boards that have such an interface.
>>>
>>> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
>>
>> Won't the system pick ROM MAC if $ethaddr is not set ?
> 
> Yes, and in the case of rockchip, misc_init_r() will set an $ethaddr based on
> the cpuid, which makes the eth driver use that instead of the ROM one.

Shouldn't the rockchip misc_init_r be fixed then ?

[PATCH v2 2/3] configs/rock5b: Set NET_PREFER_ROM_MAC_ADDR to y

[Detlev Casanova]

---
 configs/rock5b-rk3588_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
index 58c7c44fb4f..4cd4d369561 100644
--- a/configs/rock5b-rk3588_defconfig
+++ b/configs/rock5b-rk3588_defconfig
@@ -106,3 +106,4 @@ CONFIG_USB_GADGET_PRODUCT_NUM=0x350b
 CONFIG_USB_GADGET_DOWNLOAD=y
 CONFIG_USB_FUNCTION_ROCKUSB=y
 CONFIG_ERRNO_STR=y
+CONFIG_NET_PREFER_ROM_MAC_ADDR=y
-- 
2.43.2

Re: [PATCH v2 2/3] configs/rock5b: Set NET_PREFER_ROM_MAC_ADDR to y

[Marek Vasut]

On 4/22/24 3:56 PM, Detlev Casanova wrote:
> ---
>   configs/rock5b-rk3588_defconfig | 1 +
>   1 file changed, 1 insertion(+)

This patch seems to be missing commit message and SoB line

[PATCH v2 3/3] net: eth-uclass: Add driver source possibility

[Detlev Casanova]

Some net driver, like rtl8169, can set/get the MAC address from the
registers and store it in pdata->enetaddr.

When that happens, if there is a mismatch with the environment MAC
address, u-boot will show that the MAC address source is DT. This patch
ensures that the shown source is "driver" instead to avoid confusion.

Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
 net/eth-uclass.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 682de3ec7bd..3caf03eaef6 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -559,9 +559,13 @@ static int eth_post_probe(struct udevice *dev)
 	priv->state = ETH_STATE_INIT;
 	priv->running = false;
 
+	/* Check if the driver has already set a valid MAC address */
+	if (is_valid_ethaddr(pdata->enetaddr)) {
+		source = "driver";
+	}
 	/* Check if the device has a valid MAC address in device tree */
-	if (!eth_dev_get_mac_address(dev, pdata->enetaddr) ||
-	    !is_valid_ethaddr(pdata->enetaddr)) {
+	else if (!eth_dev_get_mac_address(dev, pdata->enetaddr) ||
+		 !is_valid_ethaddr(pdata->enetaddr)) {
 		/* Check if the device has a MAC address in ROM */
 		if (eth_get_ops(dev)->read_rom_hwaddr) {
 			int ret;
-- 
2.43.2