> Date: Fri, 09 Feb 2024 18:58:01 +0100 > From: Dragan Simic <dsi...@manjaro.org> > > Hello Quentin, > > Please, see my comments below. > > On 2024-02-09 10:50, Quentin Schulz wrote: > > From: Quentin Schulz <quentin.sch...@theobroma-systems.com> > > > > The functions aren't used anywhere else than in board.c, therefore, > > let's not expose them anymore at all. > > > > This merges misc.c and board.c together and removes the functions from > > the misc.h header file. > > The patches I'm going to send, which exclude the unneeded code > from the U-Boot images for the Pinebook Pro and the Pinebook Pro, > change the code that this patch moves around. > > Perhaps it would be better to have the move of the code and the > subsequent changes to it performed in a separate series, to make > checking and accepting the patches simpler as a group, and to > perhaps make it a bit more clear what actually happened to anyone > going through the repository history later. > > Thus, I was wondering would you, please, be fine with excluding > this patch from this series and letting me submit it, with proper > attribution tags, of course, as part of the series I'll submit > in the next couple of days?
Hello Dragan, I'm not covinced your patches are a good idea. Unless we're running into space constraints, or if there are noticable slowdowns in the boot process because of the extra code, I really don't see what the benefit of further differentiation between rk3399 boards would be. It just makes testing things harder. The smaller the number of special snow flakes, the better! > > Cc: Quentin Schulz <foss+ub...@0leil.net> > > Reviewed-by: Kever Yang <kever.y...@rock-chips.com> > > Signed-off-by: Quentin Schulz <quentin.sch...@theobroma-systems.com> > > --- > > arch/arm/include/asm/arch-rockchip/misc.h | 5 -- > > arch/arm/mach-rockchip/Makefile | 1 - > > arch/arm/mach-rockchip/board.c | 125 > > +++++++++++++++++++++++++++ > > arch/arm/mach-rockchip/misc.c | 135 > > ------------------------------ > > 4 files changed, 125 insertions(+), 141 deletions(-) > > > > diff --git a/arch/arm/include/asm/arch-rockchip/misc.h > > b/arch/arm/include/asm/arch-rockchip/misc.h > > index 4155af8c3b0..ef37ff1661a 100644 > > --- a/arch/arm/include/asm/arch-rockchip/misc.h > > +++ b/arch/arm/include/asm/arch-rockchip/misc.h > > @@ -6,9 +6,4 @@ > > * Rohan Garg <rohan.g...@collabora.com> > > */ > > > > -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_setup_macaddr(void); > > void rockchip_capsule_update_board_setup(void); > > diff --git a/arch/arm/mach-rockchip/Makefile > > b/arch/arm/mach-rockchip/Makefile > > index 1dc92066bbf..c07bdaee4c3 100644 > > --- a/arch/arm/mach-rockchip/Makefile > > +++ b/arch/arm/mach-rockchip/Makefile > > @@ -23,7 +23,6 @@ ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),) > > # meaning "turn it off". > > obj-y += boot_mode.o > > obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o > > -obj-$(CONFIG_MISC_INIT_R) += misc.o > > endif > > > > ifeq ($(CONFIG_TPL_BUILD),) > > diff --git a/arch/arm/mach-rockchip/board.c > > b/arch/arm/mach-rockchip/board.c > > index d5cb59c10fa..80b4514852f 100644 > > --- a/arch/arm/mach-rockchip/board.c > > +++ b/arch/arm/mach-rockchip/board.c > > @@ -1,20 +1,32 @@ > > // SPDX-License-Identifier: GPL-2.0+ > > /* > > * (C) Copyright 2019 Rockchip Electronics Co., Ltd. > > + * > > + * Copyright (C) 2019 Collabora Inc - https://www.collabora.com/ > > + * Rohan Garg <rohan.g...@collabora.com> > > + * > > + * Based on puma-rk3399.c: > > + * (C) Copyright 2017 Theobroma Systems Design und Consulting > > GmbH > > */ > > #include <common.h> > > #include <clk.h> > > #include <cpu_func.h> > > +#include <env.h> > > #include <dm.h> > > #include <efi_loader.h> > > #include <fastboot.h> > > +#include <hash.h> > > #include <init.h> > > #include <log.h> > > #include <mmc.h> > > +#include <dm/uclass-internal.h> > > +#include <misc.h> > > #include <part.h> > > #include <ram.h> > > #include <syscon.h> > > #include <uuid.h> > > +#include <u-boot/crc.h> > > +#include <u-boot/sha256.h> > > #include <asm/cache.h> > > #include <asm/io.h> > > #include <asm/arch-rockchip/boot_mode.h> > > @@ -297,6 +309,119 @@ int fastboot_set_reboot_flag(enum > > fastboot_reboot_reason reason) > > #endif > > > > #ifdef CONFIG_MISC_INIT_R > > +int rockchip_setup_macaddr(void) > > +{ > > +#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")) > > + return 0; > > + > > + if (!cpuid) { > > + debug("%s: could not retrieve 'cpuid#'\n", __func__); > > + return -1; > > + } > > + > > + ret = hash_block("sha256", (void *)cpuid, strlen(cpuid), hash, > > &size); > > + if (ret) { > > + debug("%s: failed to calculate SHA256\n", __func__); > > + return -1; > > + } > > + > > + /* Copy 6 bytes of the hash to base the MAC address on */ > > + memcpy(mac_addr, hash, 6); > > + > > + /* 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); > > +#endif > > + return 0; > > +} > > + > > +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) > > + struct udevice *dev; > > + int ret; > > + > > + /* retrieve the device */ > > +#if IS_ENABLED(CONFIG_ROCKCHIP_EFUSE) > > + ret = uclass_get_device_by_driver(UCLASS_MISC, > > + DM_DRIVER_GET(rockchip_efuse), &dev); > > +#elif IS_ENABLED(CONFIG_ROCKCHIP_OTP) > > + ret = uclass_get_device_by_driver(UCLASS_MISC, > > + DM_DRIVER_GET(rockchip_otp), &dev); > > +#endif > > + if (ret) { > > + debug("%s: could not find efuse device\n", __func__); > > + return -1; > > + } > > + > > + /* read the cpu_id range from the efuses */ > > + ret = misc_read(dev, cpuid_offset, cpuid, cpuid_length); > > + if (ret < 0) { > > + debug("%s: reading cpuid from the efuses failed\n", > > + __func__); > > + return -1; > > + } > > +#endif > > + return 0; > > +} > > + > > +int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length) > > +{ > > + u8 low[cpuid_length / 2], high[cpuid_length / 2]; > > + char cpuid_str[cpuid_length * 2 + 1]; > > + u64 serialno; > > + char serialno_str[17]; > > + const char *oldid; > > + int i; > > + > > + memset(cpuid_str, 0, sizeof(cpuid_str)); > > + for (i = 0; i < cpuid_length; i++) > > + sprintf(&cpuid_str[i * 2], "%02x", cpuid[i]); > > + > > + debug("cpuid: %s\n", cpuid_str); > > + > > + /* > > + * Mix the cpuid bytes using the same rules as in > > + * ${linux}/drivers/soc/rockchip/rockchip-cpuinfo.c > > + */ > > + for (i = 0; i < cpuid_length / 2; i++) { > > + low[i] = cpuid[1 + (i << 1)]; > > + high[i] = cpuid[i << 1]; > > + } > > + > > + serialno = crc32_no_comp(0, low, cpuid_length / 2); > > + serialno |= (u64)crc32_no_comp(serialno, high, cpuid_length / 2) << > > 32; > > + snprintf(serialno_str, sizeof(serialno_str), "%016llx", serialno); > > + > > + oldid = env_get("cpuid#"); > > + if (oldid && strcmp(oldid, cpuid_str) != 0) > > + printf("cpuid: value %s present in env does not match hardware > > %s\n", > > + oldid, cpuid_str); > > + > > + env_set("cpuid#", cpuid_str); > > + > > + /* Only generate serial# when none is set yet */ > > + if (!env_get("serial#")) > > + env_set("serial#", serialno_str); > > + > > + return 0; > > +} > > + > > __weak int rockchip_early_misc_init_r(void) > > { > > return 0; > > diff --git a/arch/arm/mach-rockchip/misc.c > > b/arch/arm/mach-rockchip/misc.c > > deleted file mode 100644 > > index 15397cff009..00000000000 > > --- a/arch/arm/mach-rockchip/misc.c > > +++ /dev/null > > @@ -1,135 +0,0 @@ > > -/* SPDX-License-Identifier: GPL-2.0+ */ > > -/* > > - * RK3399: Architecture common definitions > > - * > > - * Copyright (C) 2019 Collabora Inc - https://www.collabora.com/ > > - * Rohan Garg <rohan.g...@collabora.com> > > - * > > - * Based on puma-rk3399.c: > > - * (C) Copyright 2017 Theobroma Systems Design und Consulting > > GmbH > > - */ > > - > > -#include <common.h> > > -#include <env.h> > > -#include <dm.h> > > -#include <hash.h> > > -#include <log.h> > > -#include <dm/uclass-internal.h> > > -#include <misc.h> > > -#include <u-boot/crc.h> > > -#include <u-boot/sha256.h> > > - > > -#include <asm/arch-rockchip/misc.h> > > - > > -int rockchip_setup_macaddr(void) > > -{ > > -#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")) > > - return 0; > > - > > - if (!cpuid) { > > - debug("%s: could not retrieve 'cpuid#'\n", __func__); > > - return -1; > > - } > > - > > - ret = hash_block("sha256", (void *)cpuid, strlen(cpuid), hash, > > &size); > > - if (ret) { > > - debug("%s: failed to calculate SHA256\n", __func__); > > - return -1; > > - } > > - > > - /* Copy 6 bytes of the hash to base the MAC address on */ > > - memcpy(mac_addr, hash, 6); > > - > > - /* 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); > > -#endif > > - return 0; > > -} > > - > > -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) > > - struct udevice *dev; > > - int ret; > > - > > - /* retrieve the device */ > > -#if IS_ENABLED(CONFIG_ROCKCHIP_EFUSE) > > - ret = uclass_get_device_by_driver(UCLASS_MISC, > > - DM_DRIVER_GET(rockchip_efuse), &dev); > > -#elif IS_ENABLED(CONFIG_ROCKCHIP_OTP) > > - ret = uclass_get_device_by_driver(UCLASS_MISC, > > - DM_DRIVER_GET(rockchip_otp), &dev); > > -#endif > > - if (ret) { > > - debug("%s: could not find efuse device\n", __func__); > > - return -1; > > - } > > - > > - /* read the cpu_id range from the efuses */ > > - ret = misc_read(dev, cpuid_offset, cpuid, cpuid_length); > > - if (ret < 0) { > > - debug("%s: reading cpuid from the efuses failed\n", > > - __func__); > > - return -1; > > - } > > -#endif > > - return 0; > > -} > > - > > -int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length) > > -{ > > - u8 low[cpuid_length / 2], high[cpuid_length / 2]; > > - char cpuid_str[cpuid_length * 2 + 1]; > > - u64 serialno; > > - char serialno_str[17]; > > - const char *oldid; > > - int i; > > - > > - memset(cpuid_str, 0, sizeof(cpuid_str)); > > - for (i = 0; i < cpuid_length; i++) > > - sprintf(&cpuid_str[i * 2], "%02x", cpuid[i]); > > - > > - debug("cpuid: %s\n", cpuid_str); > > - > > - /* > > - * Mix the cpuid bytes using the same rules as in > > - * ${linux}/drivers/soc/rockchip/rockchip-cpuinfo.c > > - */ > > - for (i = 0; i < cpuid_length / 2; i++) { > > - low[i] = cpuid[1 + (i << 1)]; > > - high[i] = cpuid[i << 1]; > > - } > > - > > - serialno = crc32_no_comp(0, low, cpuid_length / 2); > > - serialno |= (u64)crc32_no_comp(serialno, high, cpuid_length / 2) << > > 32; > > - snprintf(serialno_str, sizeof(serialno_str), "%016llx", serialno); > > - > > - oldid = env_get("cpuid#"); > > - if (oldid && strcmp(oldid, cpuid_str) != 0) > > - printf("cpuid: value %s present in env does not match hardware > > %s\n", > > - oldid, cpuid_str); > > - > > - env_set("cpuid#", cpuid_str); > > - > > - /* Only generate serial# when none is set yet */ > > - if (!env_get("serial#")) > > - env_set("serial#", serialno_str); > > - > > - return 0; > > -} >
