Re: [PATCH 2/3] rockchip: Add a board_gen_ethaddr() function

[Detlev Casanova <detlev.casanova@collabora.com>]

[-- 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 --]