[PATCH v2 2/3] rockchip: rk3588: Implement checkboard() to print SoC variant

[Jagan Teki]

On Fri, 8 Nov 2024 at 09:10, Kever Yang <kever.yang at rock-chips.com> 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 <jonas at kwiboo.se>
> >>> ---
> >>> 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.