* [PATCH 0/3] net: Add a CONFIG_NET_BOARD_ETHADDR
@ 2024-03-14 14:43 Detlev Casanova
2024-03-14 14:43 ` [PATCH 1/3] " Detlev Casanova
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Detlev Casanova @ 2024-03-14 14:43 UTC (permalink / raw)
To: u-boot
Cc: Joe Hershberger, Ramon Fried, Tom Rini, Philipp Tomsich,
Kever Yang, Marek Vasut, Jonas Karlman, Simon Glass,
Fabio Estevam, Detlev Casanova
Basically, the MAC address generation needs to use a board function in
its decision algorithm so that there is no MAC mismatch between ROM and
environment depending on the order of function calling.
See the first commit message for details.
Detlev Casanova (3):
net: Add a CONFIG_NET_BOARD_ETHADDR
rockchip: Add a board_gen_ethaddr() function
net: eth-uclass: Add driver source possibility
arch/arm/Kconfig | 1 +
arch/arm/include/asm/arch-rockchip/misc.h | 1 +
arch/arm/mach-rockchip/board.c | 30 ++++++++++++++++++-----
arch/arm/mach-rockchip/misc.c | 22 ++++++++++++-----
net/Kconfig | 7 ++++++
net/eth-uclass.c | 25 ++++++++++++++++---
6 files changed, 70 insertions(+), 16 deletions(-)
--
2.43.2
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] net: Add a CONFIG_NET_BOARD_ETHADDR
2024-03-14 14:43 [PATCH 0/3] net: Add a CONFIG_NET_BOARD_ETHADDR Detlev Casanova
@ 2024-03-14 14:43 ` Detlev Casanova
2024-03-14 17:36 ` Marek Vasut
2024-03-14 14:43 ` [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function Detlev Casanova
2024-03-14 14:43 ` [PATCH 3/3] net: eth-uclass: Add driver source possibility Detlev Casanova
2 siblings, 1 reply; 8+ messages in thread
From: Detlev Casanova @ 2024-03-14 14:43 UTC (permalink / raw)
To: u-boot
Cc: Joe Hershberger, Ramon Fried, Tom Rini, Philipp Tomsich,
Kever Yang, Marek Vasut, Jonas Karlman, Simon Glass,
Fabio Estevam, 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 a CONFIG_NET_BOARD_ETHADDR that
allows board files to implement a function to set an ethaddr in the
environment, that will only be called when necessary. The logic is now:
- If there is there an ethaddr env var, use it.
- If not, if there is a DT MAC address, use it.
- If not, if there is a ROM MAC address, use it.
- If not, if CONFIG_NET_BOARD_ETHADDR, call board_gen_ethaddr() and use
it.
- If not, if CONFIG_NET_RANDOM_ETHADDR, generate random MAC
- If not, fail with No valid MAC address found
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
net/Kconfig | 7 +++++++
net/eth-uclass.c | 17 +++++++++++++++--
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/net/Kconfig b/net/Kconfig
index 5dff6336293..6dd333ddb9e 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -54,6 +54,13 @@ config NET_RANDOM_ETHADDR
generated. It will be saved to the appropriate environment variable,
too.
+config NET_BOARD_ETHADDR
+ bool "Board specific ethaddr if unset"
+ help
+ Allow a board function to set a specific Ethernet address that can
+ be, e.g., based on the CPU ID. If set, this will be tried before
+ setting a random address (if set).
+
config NETCONSOLE
bool "NetConsole support"
help
diff --git a/net/eth-uclass.c b/net/eth-uclass.c
index 3d0ec91dfa4..f194df8512a 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -56,6 +56,12 @@ __weak int board_interface_eth_init(struct udevice *dev,
return 0;
}
+/* board-specific MAC Address generation. */
+__weak int board_gen_ethaddr(int dev_num, u8 *mac_addr)
+{
+ return 0;
+}
+
static struct eth_uclass_priv *eth_get_uclass_priv(void)
{
struct uclass *uc;
@@ -563,13 +569,20 @@ static int eth_post_probe(struct udevice *dev)
if (!eth_dev_get_mac_address(dev, pdata->enetaddr) ||
!is_valid_ethaddr(pdata->enetaddr)) {
/* Check if the device has a MAC address in ROM */
+ int ret = -1;
if (eth_get_ops(dev)->read_rom_hwaddr) {
- int ret;
-
ret = eth_get_ops(dev)->read_rom_hwaddr(dev);
if (!ret)
source = "ROM";
}
+ if (IS_ENABLED(CONFIG_NET_BOARD_ETHADDR) && ret) {
+ board_gen_ethaddr(dev_seq(dev), pdata->enetaddr);
+
+ if (!is_zero_ethaddr(pdata->enetaddr) &&
+ is_valid_ethaddr(pdata->enetaddr)) {
+ source = "board";
+ }
+ }
}
eth_env_get_enetaddr_by_index("eth", dev_seq(dev), env_enetaddr);
--
2.43.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function
2024-03-14 14:43 [PATCH 0/3] net: Add a CONFIG_NET_BOARD_ETHADDR Detlev Casanova
2024-03-14 14:43 ` [PATCH 1/3] " Detlev Casanova
@ 2024-03-14 14:43 ` Detlev Casanova
2024-03-14 17:37 ` Marek Vasut
2024-03-14 18:38 ` Jonas Karlman
2024-03-14 14:43 ` [PATCH 3/3] net: eth-uclass: Add driver source possibility Detlev Casanova
2 siblings, 2 replies; 8+ messages in thread
From: Detlev Casanova @ 2024-03-14 14:43 UTC (permalink / raw)
To: u-boot
Cc: Joe Hershberger, Ramon Fried, Tom Rini, Philipp Tomsich,
Kever Yang, Marek Vasut, Jonas Karlman, Simon Glass,
Fabio Estevam, Detlev Casanova
Set the MAC address based on the CPU ID only if the ethernet device has
no ROM or DT address set.
Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
---
arch/arm/Kconfig | 1 +
arch/arm/include/asm/arch-rockchip/misc.h | 1 +
arch/arm/mach-rockchip/board.c | 30 ++++++++++++++++++-----
arch/arm/mach-rockchip/misc.c | 22 ++++++++++++-----
4 files changed, 42 insertions(+), 12 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 01d6556c42b..21b41675ef6 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -2003,6 +2003,7 @@ config ARCH_ROCKCHIP
select DM_SPI_FLASH
select DM_USB_GADGET if USB_DWC3_GADGET
select ENABLE_ARM_SOC_BOOT0_HOOK
+ select NET_BOARD_ETHADDR
select OF_CONTROL
select MTD
select SPI
diff --git a/arch/arm/include/asm/arch-rockchip/misc.h b/arch/arm/include/asm/arch-rockchip/misc.h
index 4155af8c3b0..6e972de6279 100644
--- a/arch/arm/include/asm/arch-rockchip/misc.h
+++ b/arch/arm/include/asm/arch-rockchip/misc.h
@@ -10,5 +10,6 @@ int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
const u32 cpuid_length,
u8 *cpuid);
int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length);
+int rockchip_gen_macaddr(int dev_num, u8 *mac_addr);
int rockchip_setup_macaddr(void);
void rockchip_capsule_update_board_setup(void);
diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index 2620530e03f..283d3b9ed3a 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -296,8 +296,8 @@ int fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
}
#endif
-#ifdef CONFIG_MISC_INIT_R
-__weak int misc_init_r(void)
+#if IS_ENABLED(CONFIG_MISC_INIT_R) || IS_ENABLED(CONFIG_NET_BOARD_ETHADDR)
+static int set_cpuid(void)
{
const u32 cpuid_offset = CFG_CPUID_OFFSET;
const u32 cpuid_length = 0x10;
@@ -309,10 +309,6 @@ __weak int misc_init_r(void)
return ret;
ret = rockchip_cpuid_set(cpuid, cpuid_length);
- if (ret)
- return ret;
-
- ret = rockchip_setup_macaddr();
return ret;
}
@@ -349,3 +345,25 @@ __weak int board_rng_seed(struct abuf *buf)
return 0;
}
#endif
+
+#if IS_ENABLED(CONFIG_MISC_INIT_R)
+__weak int misc_init_r(void)
+{
+ return set_cpuid();
+}
+#endif
+
+int board_gen_ethaddr(int dev_num, u8 *mac_addr)
+{
+ if (!IS_ENABLED(CONFIG_NET_BOARD_ETHADDR))
+ return 0;
+
+ if (!env_get("cpuid#")) {
+ int err = set_cpuid();
+
+ if (err)
+ return err;
+ }
+
+ return rockchip_gen_macaddr(dev_num, mac_addr);
+}
diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c
index 7d03f0c2b67..9c7b04ee5a8 100644
--- a/arch/arm/mach-rockchip/misc.c
+++ b/arch/arm/mach-rockchip/misc.c
@@ -21,14 +21,13 @@
#include <asm/arch-rockchip/misc.h>
-int rockchip_setup_macaddr(void)
+int rockchip_gen_macaddr(int dev_num, u8 *mac_addr)
{
#if CONFIG_IS_ENABLED(HASH) && CONFIG_IS_ENABLED(SHA256)
int ret;
const char *cpuid = env_get("cpuid#");
u8 hash[SHA256_SUM_LEN];
int size = sizeof(hash);
- u8 mac_addr[6];
/* Only generate a MAC address, if none is set in the environment */
if (env_get("ethaddr"))
@@ -51,15 +50,26 @@ int rockchip_setup_macaddr(void)
/* Make this a valid MAC address and set it */
mac_addr[0] &= 0xfe; /* clear multicast bit */
mac_addr[0] |= 0x02; /* set local assignment bit (IEEE802) */
- eth_env_set_enetaddr("ethaddr", mac_addr);
- /* Make a valid MAC address for ethernet1 */
- mac_addr[5] ^= 0x01;
- eth_env_set_enetaddr("eth1addr", mac_addr);
+ /* Make a valid MAC address for the given device number */
+ mac_addr[5] ^= dev_num;
#endif
return 0;
}
+int rockchip_setup_macaddr(void)
+{
+ u8 mac_addr[6];
+
+ if (rockchip_gen_macaddr(0, mac_addr) == 0)
+ eth_env_set_enetaddr("ethaddr", mac_addr);
+
+ if (rockchip_gen_macaddr(1, mac_addr) == 0)
+ eth_env_set_enetaddr("eth1addr", mac_addr);
+
+ return 0;
+}
+
int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
const u32 cpuid_length,
u8 *cpuid)
--
2.43.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] net: eth-uclass: Add driver source possibility
2024-03-14 14:43 [PATCH 0/3] net: Add a CONFIG_NET_BOARD_ETHADDR Detlev Casanova
2024-03-14 14:43 ` [PATCH 1/3] " Detlev Casanova
2024-03-14 14:43 ` [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function Detlev Casanova
@ 2024-03-14 14:43 ` Detlev Casanova
2 siblings, 0 replies; 8+ messages in thread
From: Detlev Casanova @ 2024-03-14 14:43 UTC (permalink / raw)
To: u-boot
Cc: Joe Hershberger, Ramon Fried, Tom Rini, Philipp Tomsich,
Kever Yang, Marek Vasut, Jonas Karlman, Simon Glass,
Fabio Estevam, 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 f194df8512a..6e521955fa5 100644
--- a/net/eth-uclass.c
+++ b/net/eth-uclass.c
@@ -565,9 +565,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 */
int ret = -1;
if (eth_get_ops(dev)->read_rom_hwaddr) {
--
2.43.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] net: Add a CONFIG_NET_BOARD_ETHADDR
2024-03-14 14:43 ` [PATCH 1/3] " Detlev Casanova
@ 2024-03-14 17:36 ` Marek Vasut
0 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2024-03-14 17:36 UTC (permalink / raw)
To: Detlev Casanova, u-boot
Cc: Joe Hershberger, Ramon Fried, Tom Rini, Philipp Tomsich,
Kever Yang, Marek Vasut, Jonas Karlman, Simon Glass,
Fabio Estevam
On 3/14/24 3:43 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 a CONFIG_NET_BOARD_ETHADDR that
> allows board files to implement a function to set an ethaddr in the
> environment, that will only be called when necessary. The logic is now:
>
> - If there is there an ethaddr env var, use it.
> - If not, if there is a DT MAC address, use it.
> - If not, if there is a ROM MAC address, use it.
> - If not, if CONFIG_NET_BOARD_ETHADDR, call board_gen_ethaddr() and use
> it.
> - If not, if CONFIG_NET_RANDOM_ETHADDR, generate random MAC
> - If not, fail with No valid MAC address found
>
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
> net/Kconfig | 7 +++++++
> net/eth-uclass.c | 17 +++++++++++++++--
> 2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/net/Kconfig b/net/Kconfig
> index 5dff6336293..6dd333ddb9e 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -54,6 +54,13 @@ config NET_RANDOM_ETHADDR
> generated. It will be saved to the appropriate environment variable,
> too.
>
> +config NET_BOARD_ETHADDR
> + bool "Board specific ethaddr if unset"
> + help
> + Allow a board function to set a specific Ethernet address that can
> + be, e.g., based on the CPU ID. If set, this will be tried before
> + setting a random address (if set).
> +
> config NETCONSOLE
> bool "NetConsole support"
> help
> diff --git a/net/eth-uclass.c b/net/eth-uclass.c
> index 3d0ec91dfa4..f194df8512a 100644
> --- a/net/eth-uclass.c
> +++ b/net/eth-uclass.c
> @@ -56,6 +56,12 @@ __weak int board_interface_eth_init(struct udevice *dev,
> return 0;
> }
>
> +/* board-specific MAC Address generation. */
> +__weak int board_gen_ethaddr(int dev_num, u8 *mac_addr)
> +{
> + return 0;
> +}
> +
> static struct eth_uclass_priv *eth_get_uclass_priv(void)
> {
> struct uclass *uc;
> @@ -563,13 +569,20 @@ static int eth_post_probe(struct udevice *dev)
> if (!eth_dev_get_mac_address(dev, pdata->enetaddr) ||
> !is_valid_ethaddr(pdata->enetaddr)) {
> /* Check if the device has a MAC address in ROM */
> + int ret = -1;
> if (eth_get_ops(dev)->read_rom_hwaddr) {
> - int ret;
> -
> ret = eth_get_ops(dev)->read_rom_hwaddr(dev);
> if (!ret)
> source = "ROM";
> }
> + if (IS_ENABLED(CONFIG_NET_BOARD_ETHADDR) && ret) {
> + board_gen_ethaddr(dev_seq(dev), pdata->enetaddr);
You do need to pass in the udevice *dev directly, otherwise this DM
uclass patch would behave like non-DM code.
I'd personally just create a __weak board_gen_ethaddr() function here
and let boards override it, without new Kconfig symbol.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function
2024-03-14 14:43 ` [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function Detlev Casanova
@ 2024-03-14 17:37 ` Marek Vasut
2024-03-14 18:38 ` Jonas Karlman
1 sibling, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2024-03-14 17:37 UTC (permalink / raw)
To: Detlev Casanova, u-boot
Cc: Joe Hershberger, Ramon Fried, Tom Rini, Philipp Tomsich,
Kever Yang, Marek Vasut, Jonas Karlman, Simon Glass,
Fabio Estevam
On 3/14/24 3:43 PM, Detlev Casanova wrote:
> Set the MAC address based on the CPU ID only if the ethernet device has
> no ROM or DT address set.
>
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
More of a general design question -- how much of this can be done in MAC
driver specific .read_rom_hwaddr callback ?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function
2024-03-14 14:43 ` [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function Detlev Casanova
2024-03-14 17:37 ` Marek Vasut
@ 2024-03-14 18:38 ` Jonas Karlman
2024-04-02 15:38 ` Detlev Casanova
1 sibling, 1 reply; 8+ messages in thread
From: Jonas Karlman @ 2024-03-14 18:38 UTC (permalink / raw)
To: Detlev Casanova
Cc: Joe Hershberger, Ramon Fried, Tom Rini, Philipp Tomsich,
Kever Yang, Marek Vasut, Simon Glass, Fabio Estevam, u-boot
Hi Detlev,
On 2024-03-14 15:43, Detlev Casanova wrote:
> Set the MAC address based on the CPU ID only if the ethernet device has
> no ROM or DT address set.
This patch changes behavior and once again require CONFIG_NET to fixup
the device tree with local-mac-address for the ethernet0/1 alias nodes.
I.e. reverts one of the intentions with the commit 628fb0683b65
("rockchip: misc: Set eth1addr mac address"), fixup of ethaddr in DT
should not depend on enabled and working ethernet in U-Boot.
Maybe just having a Kconfig to control if ENV ethaddr should overwrite
ROM ethaddr could as easily solve your issue?
Please also note that misc.c is merging into board.c in another pending
series, see custodian u-boot-rockchip/for-next branch.
>
> Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> ---
> arch/arm/Kconfig | 1 +
> arch/arm/include/asm/arch-rockchip/misc.h | 1 +
> arch/arm/mach-rockchip/board.c | 30 ++++++++++++++++++-----
> arch/arm/mach-rockchip/misc.c | 22 ++++++++++++-----
> 4 files changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 01d6556c42b..21b41675ef6 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -2003,6 +2003,7 @@ config ARCH_ROCKCHIP
> select DM_SPI_FLASH
> select DM_USB_GADGET if USB_DWC3_GADGET
> select ENABLE_ARM_SOC_BOOT0_HOOK
> + select NET_BOARD_ETHADDR
You are selecting here, so why the need for IS_ENABLED checks below?
> select OF_CONTROL
> select MTD
> select SPI
> diff --git a/arch/arm/include/asm/arch-rockchip/misc.h b/arch/arm/include/asm/arch-rockchip/misc.h
> index 4155af8c3b0..6e972de6279 100644
> --- a/arch/arm/include/asm/arch-rockchip/misc.h
> +++ b/arch/arm/include/asm/arch-rockchip/misc.h
> @@ -10,5 +10,6 @@ int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
> const u32 cpuid_length,
> u8 *cpuid);
> int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length);
> +int rockchip_gen_macaddr(int dev_num, u8 *mac_addr);
> int rockchip_setup_macaddr(void);
> void rockchip_capsule_update_board_setup(void);
> diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
> index 2620530e03f..283d3b9ed3a 100644
> --- a/arch/arm/mach-rockchip/board.c
> +++ b/arch/arm/mach-rockchip/board.c
> @@ -296,8 +296,8 @@ int fastboot_set_reboot_flag(enum fastboot_reboot_reason reason)
> }
> #endif
>
> -#ifdef CONFIG_MISC_INIT_R
> -__weak int misc_init_r(void)
> +#if IS_ENABLED(CONFIG_MISC_INIT_R) || IS_ENABLED(CONFIG_NET_BOARD_ETHADDR)
NET_BOARD_ETHADDR is selected so this #if will always be true.
> +static int set_cpuid(void)
> {
> const u32 cpuid_offset = CFG_CPUID_OFFSET;
> const u32 cpuid_length = 0x10;
> @@ -309,10 +309,6 @@ __weak int misc_init_r(void)
> return ret;
>
> ret = rockchip_cpuid_set(cpuid, cpuid_length);
> - if (ret)
> - return ret;
> -
> - ret = rockchip_setup_macaddr();
>
> return ret;
> }
> @@ -349,3 +345,25 @@ __weak int board_rng_seed(struct abuf *buf)
> return 0;
> }
> #endif
> +
> +#if IS_ENABLED(CONFIG_MISC_INIT_R)
> +__weak int misc_init_r(void)
> +{
> + return set_cpuid();
> +}
> +#endif
> +
> +int board_gen_ethaddr(int dev_num, u8 *mac_addr)
> +{
> + if (!IS_ENABLED(CONFIG_NET_BOARD_ETHADDR))
> + return 0;
NET_BOARD_ETHADDR is selected so this if return 0 is dead code?
Regards,
Jonas
> +
> + if (!env_get("cpuid#")) {
> + int err = set_cpuid();
> +
> + if (err)
> + return err;
> + }
> +
> + return rockchip_gen_macaddr(dev_num, mac_addr);
> +}
> diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c
> index 7d03f0c2b67..9c7b04ee5a8 100644
> --- a/arch/arm/mach-rockchip/misc.c
> +++ b/arch/arm/mach-rockchip/misc.c
> @@ -21,14 +21,13 @@
>
> #include <asm/arch-rockchip/misc.h>
>
> -int rockchip_setup_macaddr(void)
> +int rockchip_gen_macaddr(int dev_num, u8 *mac_addr)
> {
> #if CONFIG_IS_ENABLED(HASH) && CONFIG_IS_ENABLED(SHA256)
> int ret;
> const char *cpuid = env_get("cpuid#");
> u8 hash[SHA256_SUM_LEN];
> int size = sizeof(hash);
> - u8 mac_addr[6];
>
> /* Only generate a MAC address, if none is set in the environment */
> if (env_get("ethaddr"))
> @@ -51,15 +50,26 @@ int rockchip_setup_macaddr(void)
> /* Make this a valid MAC address and set it */
> mac_addr[0] &= 0xfe; /* clear multicast bit */
> mac_addr[0] |= 0x02; /* set local assignment bit (IEEE802) */
> - eth_env_set_enetaddr("ethaddr", mac_addr);
>
> - /* Make a valid MAC address for ethernet1 */
> - mac_addr[5] ^= 0x01;
> - eth_env_set_enetaddr("eth1addr", mac_addr);
> + /* Make a valid MAC address for the given device number */
> + mac_addr[5] ^= dev_num;
> #endif
> return 0;
> }
>
> +int rockchip_setup_macaddr(void)
> +{
> + u8 mac_addr[6];
> +
> + if (rockchip_gen_macaddr(0, mac_addr) == 0)
> + eth_env_set_enetaddr("ethaddr", mac_addr);
> +
> + if (rockchip_gen_macaddr(1, mac_addr) == 0)
> + eth_env_set_enetaddr("eth1addr", mac_addr);
> +
> + return 0;
> +}
> +
> int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
> const u32 cpuid_length,
> u8 *cpuid)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function
2024-03-14 18:38 ` Jonas Karlman
@ 2024-04-02 15:38 ` Detlev Casanova
0 siblings, 0 replies; 8+ messages in thread
From: Detlev Casanova @ 2024-04-02 15:38 UTC (permalink / raw)
To: Jonas Karlman
Cc: Joe Hershberger, Ramon Fried, Tom Rini, Philipp Tomsich,
Kever Yang, Marek Vasut, Simon Glass, Fabio Estevam, u-boot
[-- Attachment #1: Type: text/plain, Size: 3563 bytes --]
Hi Jonas,
On Thursday, March 14, 2024 2:38:30 P.M. EDT Jonas Karlman wrote:
> Hi Detlev,
>
> On 2024-03-14 15:43, Detlev Casanova wrote:
> > Set the MAC address based on the CPU ID only if the ethernet device has
> > no ROM or DT address set.
>
> This patch changes behavior and once again require CONFIG_NET to fixup
> the device tree with local-mac-address for the ethernet0/1 alias nodes.
>
> I.e. reverts one of the intentions with the commit 628fb0683b65
> ("rockchip: misc: Set eth1addr mac address"), fixup of ethaddr in DT
> should not depend on enabled and working ethernet in U-Boot.
I see the intention, that makes sense.
> Maybe just having a Kconfig to control if ENV ethaddr should overwrite
> ROM ethaddr could as easily solve your issue?
If that is ok for everybody, it would indeed be enough for this issue and keep
the eth driver less complex.
> Please also note that misc.c is merging into board.c in another pending
> series, see custodian u-boot-rockchip/for-next branch.
>
> > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com>
> > ---
> >
> > arch/arm/Kconfig | 1 +
> > arch/arm/include/asm/arch-rockchip/misc.h | 1 +
> > arch/arm/mach-rockchip/board.c | 30 ++++++++++++++++++-----
> > arch/arm/mach-rockchip/misc.c | 22 ++++++++++++-----
> > 4 files changed, 42 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index 01d6556c42b..21b41675ef6 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -2003,6 +2003,7 @@ config ARCH_ROCKCHIP
> >
> > select DM_SPI_FLASH
> > select DM_USB_GADGET if USB_DWC3_GADGET
> > select ENABLE_ARM_SOC_BOOT0_HOOK
> >
> > + select NET_BOARD_ETHADDR
>
> You are selecting here, so why the need for IS_ENABLED checks below?
>
> > select OF_CONTROL
> > select MTD
> > select SPI
> >
> > diff --git a/arch/arm/include/asm/arch-rockchip/misc.h
> > b/arch/arm/include/asm/arch-rockchip/misc.h index
> > 4155af8c3b0..6e972de6279 100644
> > --- a/arch/arm/include/asm/arch-rockchip/misc.h
> > +++ b/arch/arm/include/asm/arch-rockchip/misc.h
> > @@ -10,5 +10,6 @@ int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
> >
> > const u32 cpuid_length,
> > u8 *cpuid);
> >
> > int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length);
> >
> > +int rockchip_gen_macaddr(int dev_num, u8 *mac_addr);
> >
> > int rockchip_setup_macaddr(void);
> > void rockchip_capsule_update_board_setup(void);
> >
> > diff --git a/arch/arm/mach-rockchip/board.c
> > b/arch/arm/mach-rockchip/board.c index 2620530e03f..283d3b9ed3a 100644
> > --- a/arch/arm/mach-rockchip/board.c
> > +++ b/arch/arm/mach-rockchip/board.c
> > @@ -296,8 +296,8 @@ int fastboot_set_reboot_flag(enum
> > fastboot_reboot_reason reason)>
> > }
> > #endif
> >
> > -#ifdef CONFIG_MISC_INIT_R
> > -__weak int misc_init_r(void)
> > +#if IS_ENABLED(CONFIG_MISC_INIT_R) ||
> > IS_ENABLED(CONFIG_NET_BOARD_ETHADDR)
>
> NET_BOARD_ETHADDR is selected so this #if will always be true.
>
> > +static int set_cpuid(void)
> >
> > {
> >
> > const u32 cpuid_offset = CFG_CPUID_OFFSET;
> > const u32 cpuid_length = 0x10;
> >
> > @@ -309,10 +309,6 @@ __weak int misc_init_r(void)
> >
> > return ret;
> >
> > ret = rockchip_cpuid_set(cpuid, cpuid_length);
> >
> > - if (ret)
> > - return ret;
> > -
> > - ret = rockchip_setup_macaddr();
> >
> > return ret;
> >
> > }
> >
> > @@ -349,3 +345,25 @@ __weak int board_rng_seed(struct abuf *buf)
> >
> > return 0;
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-04-02 15:38 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-14 14:43 [PATCH 0/3] net: Add a CONFIG_NET_BOARD_ETHADDR Detlev Casanova
2024-03-14 14:43 ` [PATCH 1/3] " Detlev Casanova
2024-03-14 17:36 ` Marek Vasut
2024-03-14 14:43 ` [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function Detlev Casanova
2024-03-14 17:37 ` Marek Vasut
2024-03-14 18:38 ` Jonas Karlman
2024-04-02 15:38 ` Detlev Casanova
2024-03-14 14:43 ` [PATCH 3/3] net: eth-uclass: Add driver source possibility Detlev Casanova
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox