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

[Quentin Schulz]

Hi Jonas,

On 11/7/24 3:29 PM, 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.
> 

But I think that doing this also build the assert() in?

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

This defines DEBUG which then defines _DEBUG to 1 which enables assert().

Not sure how to selectively increase the log level for a specific 
drivers/class though but the docs[1] say #define LOG_DEBUG. And if I 
trust my ability to read C, it'll end up also enabling assert()s...

I think those should not be enabled with the same constant but that's 
where we stand today.

[1] 
https://docs.u-boot.org/en/latest/develop/logging.html#temporary-logging-within-a-single-file

Cheers,
Quentin