Hello Quentin,
Please see my comments below.
On 2024-01-23 15:49, Quentin Schulz wrote:
From: Quentin Schulz quentin.schulz@theobroma-systems.com
Only setup_iodomain() differs from the original misc_init_r from Rockchip mach code, so let's use rockchip_early_misc_init_r instead of reimplementing the whole misc_init_r from Rockchip.
Your patches from this series are fine, but the trouble is that we end up executing rockchip_setup_macaddr() for the devices that don't use the RK3339's built-in Gigabit Ethernet interface, which includes the Pinebook Pro and the Pinephone Pro.
We should add more ifdef guards to rockchip_setup_macaddr(), to prevent the execution of its body for devices such as the ones listed above, which eliminates the unneeded code from the resulting U-Boot images, making them a bit smaller, and saves some CPU cycles and a bit of time on boot. It also prevents the unneeded "ethaddr" and "eth1addr" variables from being added to the environment.
The patch below should do the trick, which also performs a few small code cleanups for additional file-level consistency:
diff --git a/arch/arm/mach-rockchip/misc.c b/arch/arm/mach-rockchip/misc.c index 7d03f0c2b679..ed5bdab5a648 100644 --- a/arch/arm/mach-rockchip/misc.c +++ b/arch/arm/mach-rockchip/misc.c @@ -23,7 +23,8 @@
int rockchip_setup_macaddr(void) { -#if CONFIG_IS_ENABLED(HASH) && CONFIG_IS_ENABLED(SHA256) +#if CONFIG_IS_ENABLED(HASH) && CONFIG_IS_ENABLED(SHA256) && \ + CONFIG_IS_ENABLED(GMAC_ROCKCHIP) int ret; const char *cpuid = env_get("cpuid#"); u8 hash[SHA256_SUM_LEN]; @@ -64,15 +65,15 @@ int rockchip_cpuid_from_efuse(const u32 cpuid_offset, const u32 cpuid_length, u8 *cpuid) { -#if IS_ENABLED(CONFIG_ROCKCHIP_EFUSE) || IS_ENABLED(CONFIG_ROCKCHIP_OTP) +#if CONFIG_IS_ENABLED(ROCKCHIP_EFUSE) || CONFIG_IS_ENABLED(ROCKCHIP_OTP) struct udevice *dev; int ret;
/* retrieve the device */ -#if IS_ENABLED(CONFIG_ROCKCHIP_EFUSE) +#if CONFIG_IS_ENABLED(ROCKCHIP_EFUSE) ret = uclass_get_device_by_driver(UCLASS_MISC, DM_DRIVER_GET(rockchip_efuse), &dev); -#elif IS_ENABLED(CONFIG_ROCKCHIP_OTP) +#elif CONFIG_IS_ENABLED(ROCKCHIP_OTP) ret = uclass_get_device_by_driver(UCLASS_MISC, DM_DRIVER_GET(rockchip_otp), &dev); #endif
Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com
board/pine64/rockpro64_rk3399/rockpro64-rk3399.c | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-)
diff --git a/board/pine64/rockpro64_rk3399/rockpro64-rk3399.c b/board/pine64/rockpro64_rk3399/rockpro64-rk3399.c index d79084614f1..d0a694ead1d 100644 --- a/board/pine64/rockpro64_rk3399/rockpro64-rk3399.c +++ b/board/pine64/rockpro64_rk3399/rockpro64-rk3399.c @@ -11,7 +11,6 @@ #include <asm/arch-rockchip/clock.h> #include <asm/arch-rockchip/grf_rk3399.h> #include <asm/arch-rockchip/hardware.h> -#include <asm/arch-rockchip/misc.h>
#define GRF_IO_VSEL_BT565_SHIFT 0 #define PMUGRF_CON0_VSEL_SHIFT 8 @@ -31,26 +30,11 @@ static void setup_iodomain(void) rk_setreg(&pmugrf->soc_con0, 1 << PMUGRF_CON0_VSEL_SHIFT); }
-int misc_init_r(void) +int rockchip_early_misc_init_r(void) {
const u32 cpuid_offset = 0x7;
const u32 cpuid_length = 0x10;
u8 cpuid[cpuid_length];
int ret;
setup_iodomain();
ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid);
if (ret)
return ret;ret = rockchip_cpuid_set(cpuid, cpuid_length);
if (ret)
return ret;ret = rockchip_setup_macaddr();
return ret;
- return 0;
}
#endif