[PATCH v1 9/9] rockchip: RK3588: Enable display cpuinfo support on all boards

[Quentin Schulz]

Hi Anand,

On 5/16/24 12:12 PM, Anand Moon wrote:
> Hi Quentin
> 
> On Thu, 16 May 2024 at 14:52, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>>
>> Hi Anand,
>>
>> This is patch 9/9 but somehow I didn't receive any other patch, nor did
>> the mailing list? c.f.
>> https://lists.denx.de/pipermail/u-boot/2024-May/thread.html and
>> https://lore.kernel.org/u-boot/. Are you registered on the ML?
>>
> 
> Thanks for your  review comments.
> 
> Something went wrong with git sendmail,
> Your message have not reached my email client (gmail)
> 

A mail server rejected the mail to edgeble.ai domain (both you and 
Jagan) /me shrugs.

>> On 5/16/24 10:59 AM, Anand Moon wrote:
>>> Imply DISPLAY_CPUINFO Kconfig options to support on all RK3588s and
>>> RK3588 boards, Its used to determine the reset cause of the board.
>>>
>>> Cc: Jagan Teki <jagan at edgeble.ai>
>>> Signed-off-by: Anand Moon <anand at edgeble.ai>
>>> ---
>>>    arch/arm/mach-rockchip/Kconfig           | 1 +
>>>    configs/coolpi-4b-rk3588s_defconfig      | 1 -
>>>    configs/coolpi-cm5-evb-rk3588_defconfig  | 1 -
>>>    configs/evb-rk3588_defconfig             | 1 -
>>>    configs/generic-rk3588_defconfig         | 1 -
>>>    configs/jaguar-rk3588_defconfig          | 1 -
>>>    configs/nanopc-t6-rk3588_defconfig       | 1 -
>>>    configs/neu6a-io-rk3588_defconfig        | 1 -
>>>    configs/neu6b-io-rk3588_defconfig        | 1 -
>>>    configs/orangepi-5-plus-rk3588_defconfig | 1 -
>>>    configs/orangepi-5-rk3588s_defconfig     | 1 -
>>>    configs/quartzpro64-rk3588_defconfig     | 1 -
>>>    configs/rock5a-rk3588s_defconfig         | 1 -
>>>    configs/rock5b-rk3588_defconfig          | 1 -
>>>    configs/toybrick-rk3588_defconfig        | 1 -
>>>    configs/turing-rk1-rk3588_defconfig      | 1 -
>>>    16 files changed, 1 insertion(+), 15 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
>>> index 2e9c71138e..1b5cc34f99 100644
>>> --- a/arch/arm/mach-rockchip/Kconfig
>>> +++ b/arch/arm/mach-rockchip/Kconfig
>>> @@ -366,6 +366,7 @@ config ROCKCHIP_RK3588
>>>        imply SCMI_FIRMWARE
>>>        imply SPL_ATF_NO_PLATFORM_PARAM if SPL_ATF
>>>        imply SPL_MMC_HS200_SUPPORT if SPL_MMC && MMC_HS200_SUPPORT
>>> +     imply DISPLAY_CPUINFO
>>
>> This is unnecessary, it's already defaulting to y if building for ARM
>> boards: https://elixir.bootlin.com/u-boot/latest/source/common/Kconfig#L596
>>
> See below...
>> I also don't think this is SO useful that we need to enable it on all
>> rk3588 boards? But also, doesn't hurt, so... whatever I guess :) ?
>>
>> While looking at the code, I think we can remove the ifdef in
>> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/mach-rockchip/cpu-info.c#L47
>> because this file is anyway only compiled when CONFIG_DISPLAY_CPUINFO is
>> set, c.f.
> 
> Oops I missed this changes, my bad
> I will dop my changes over here.
> 
>> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/mach-rockchip/Makefile#L30
>>
> On Rockchip SoC CONFIG_DISPLAY_CPUINFO is been disable on most of the
> configs files.
> 
> -# CONFIG_DISPLAY_CPUINFO is not set
> 
> My changes are related to determine the reset cause of the board and
> display the results.
> its only enable on selected SoC hence I have to used this logic.
> 

It's enabled for all Aarch64/Aarch32 SoCs by default. People explicitly 
disabled them in their own defconfig, either because the first person 
who added a board based on rk3588 didn't know or didn't care and 
everybody just copied it as a base, or because they don't care about 
it/don't want it.

In any case, you only need to change the defconfigs, nothing else.

> U-Boot 2024.07-rc2-00397-g0370324feb-dirty (May 16 2024 - 13:11:14 +0530)
> 
> SoC: Rockchip rk3568
> Reset cause: POR         <-----------
> Model: Radxa ROCK3 Model A
> DRAM:  8 GiB (effective 7.7 GiB)
> PMIC:  RK8090 (on=0x40, off=0x00)
> Core:  344 devices, 31 uclasses,
>  >> which also means...
>>
>> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/include/asm/arch-rockchip/cru.h#L35
>> should probably be ifdef'ed
>>
>> which means...
>>
>> https://elixir.bootlin.com/u-boot/latest/source/board/firefly/roc-pc-rk3399/roc-pc-rk3399.c#L64
>> should probably also be ifdef'ed (but the config is enabled already
>> (well... it wouldn't compile otherwsie), so I guess this is fine?).
> 
> This code changes will not affect this feature by default its enable
> on RK3399 boards.
> 

Yes, but if you disable it, it won't compile anymore. (I'm not asking 
you to fix anything I've reported here though).

Cheers,
Quentin