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

[Quentin Schulz]

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 <jo...@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 with #define LOG_CATEGORY LOGC_ARCH

at the top?

+               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.

I tested on multiple RK3588 Jaguar, and the commercial (RK3588) grade 
ones all showed RK3588 v0.

The one industrial grade (RK3588J) showed RK3588J v1.

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?

FWIW, the Linux kernel now exposes different nvmem devices for each cell 
and since we have a cell defined for the three OTP ranges you're using 
here, maybe it'd make sense to use that instead of hardcoding the 
offsets and size in here? Not a blocker for this though as I don't think 
it brings THAT much value to those fixed NVMEM layouts.

Cheers,
Quentin