On Fri, 8 Nov 2024 at 09:10, Kever Yang <[email protected]> wrote: > > Hi Jonas, > > On 2024/11/7 22:29, Jonas Karlman wrote: > > Hi Quentin, > > > > On 2024-11-07 13:01, Quentin Schulz wrote: > >> Hi Jonas, > >> > >> On 11/2/24 9:37 PM, Jonas Karlman wrote: > >>> Implement checkboard() to print current SoC model used by a board, > >>> e.g. one of: > >>> > >>> SoC: RK3582 v1 > >>> SoC: RK3588 v0 > >>> SoC: RK3588 v1 > >>> SoC: RK3588S v0 > >>> SoC: RK3588S v1 > >>> SoC: RK3588S2 v1 > >>> > >>> when U-Boot proper is running. > >>> > >>> U-Boot 2025.01-rc1 (Nov 02 2024 - 20:19:01 +0000) > >>> > >>> Model: Generic RK3588S/RK3588 > >>> SoC: RK3588S2 v1 > >>> DRAM: 8 GiB > >>> > >>> Information about the SoC model, variant and version is read from OTP. > >>> > >>> Also update rk3588s-u-boot.dtsi to include OTP in U-Boot pre-reloc phase, > >>> where checkboard() is called. > >>> > >>> Signed-off-by: Jonas Karlman <[email protected]> > >>> --- > >>> v2: > >>> - Update commit message > >>> - Update code comments > >>> - Drop generic-rk3588_defconfig change > >>> --- > >>> arch/arm/dts/rk3588s-u-boot.dtsi | 4 ++ > >>> arch/arm/mach-rockchip/rk3588/rk3588.c | 58 ++++++++++++++++++++++++++ > >>> 2 files changed, 62 insertions(+) > >>> > >>> diff --git a/arch/arm/dts/rk3588s-u-boot.dtsi > >>> b/arch/arm/dts/rk3588s-u-boot.dtsi > >>> index 09d8b311cec5..8880d162b11c 100644 > >>> --- a/arch/arm/dts/rk3588s-u-boot.dtsi > >>> +++ b/arch/arm/dts/rk3588s-u-boot.dtsi > >>> @@ -69,6 +69,10 @@ > >>> bootph-all; > >>> }; > >>> > >>> +&otp { > >>> + bootph-some-ram; > >>> +}; > >>> + > >>> &pcfg_pull_down { > >>> bootph-all; > >>> }; > >>> diff --git a/arch/arm/mach-rockchip/rk3588/rk3588.c > >>> b/arch/arm/mach-rockchip/rk3588/rk3588.c > >>> index e2dac2a5b806..f9da7a6f1477 100644 > >>> --- a/arch/arm/mach-rockchip/rk3588/rk3588.c > >>> +++ b/arch/arm/mach-rockchip/rk3588/rk3588.c > >>> @@ -4,6 +4,8 @@ > >>> * Copyright (c) 2022 Edgeble AI Technologies Pvt. Ltd. > >>> */ > >>> > >>> +#include <dm.h> > >>> +#include <misc.h> > >>> #include <spl.h> > >>> #include <asm/armv8/mmu.h> > >>> #include <asm/arch-rockchip/bootrom.h> > >>> @@ -178,3 +180,59 @@ int arch_cpu_init(void) > >>> return 0; > >>> } > >>> #endif > >>> + > >>> +#define RK3588_OTP_CPU_CODE_OFFSET 0x02 > >>> +#define RK3588_OTP_SPECIFICATION_OFFSET 0x06 > >>> +#define RK3588_OTP_CPU_VERSION_OFFSET 0x1c > >>> + > >>> +int checkboard(void) > >>> +{ > >>> + u8 cpu_code[2], specification, package, cpu_version; > >>> + struct udevice *dev; > >>> + char suffix[3]; > >>> + int ret; > >>> + > >>> + ret = uclass_get_device_by_driver(UCLASS_MISC, > >>> + DM_DRIVER_GET(rockchip_otp), &dev); > >>> + if (ret) { > >>> + debug("%s: could not find otp device, ret=%d\n", __func__, > >>> ret); > >> We should probably start using log_debug instead? > > I guess we should?, not sure why there is two different and what the > > difference is. Have typically only ever used debug() or printf(), so I > > just went with what was used in the code copied from board.c. > > > >> I guess with #define LOG_CATEGORY LOGC_ARCH > >> > >> at the top? > > I can give it a try, what/how is the log category used for? > > > > My simple debug workflow is just to add a '#define LOG_DEBUG' at top of > > a file when I want some more debug info. > > > >>> + return 0; > >>> + } > >>> + > >>> + /* cpu-code: SoC model, e.g. 0x35 0x82 or 0x35 0x88 */ > >>> + ret = misc_read(dev, RK3588_OTP_CPU_CODE_OFFSET, cpu_code, 2); > >> This will fail if CONFIG_MISC=n which is neither selected nor implied > >> for RK3588. > > Good catch, did not test that scenario, will fix in a v3. > > > >> I tested on multiple RK3588 Jaguar, and the commercial (RK3588) grade > >> ones all showed RK3588 v0. > >> > >> The one industrial grade (RK3588J) showed RK3588J v1. > > Thanks for confirming this works on a J-variant. > > > >> I'm really not sure what this version number is for. If even Kever > >> doesn't know what it means, I am not sure it makes sense to print it? > > For rk356x there is some special handling for cpu-version <> 0 in vendor > > code, and DT describe a cpu-version nvmem cell, so thought it could be > > useful debug information. However it may just cause more confusion since > > there is no clear information on what the different cpu-version mean. > > > > Will drop the cpu-version information in a v3. > > Yes, this cpu-version does not help users, I think it may be one of the > silicon version( > > The process may change in foundry), package version(change in package > factory),
Apart from package number chip versions like RK3588, RK3588J and RK3588M would be much beneficial when we boot multi-dtb from SPL and load the respective silicon dtb in u-boot proper. So, chip numbers, temperature (if possible) are useful like imx chips supported so far. Thanks, Jagan.
