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

[Anand Moon]

Hi Quentin

On Thu, 16 May 2024 at 16:13, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>
> 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.
>

Yeah, something went wrong. It's Gmail server or u-boot mail server blocked
I don't know the reason for this.

Remove me and Jagan (edgeble.ai) for now as of now.

> >> 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.
> >>>
> >>> 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.
>
My series focuses on determining the reset cause of the boards
Do we need to enable this feature ?

We are not compiling DISPLAY_CPUINFO for all for RK3568 and RK3588
But with the following changes enable this to be built for SPL_BUILD ( patch v5)
in this series. wow it is built for all SoC.

diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
index c07bdaee4c..6722e7c9ea 100644
--- a/arch/arm/mach-rockchip/Makefile
+++ b/arch/arm/mach-rockchip/Makefile
@@ -25,7 +25,7 @@ obj-y += boot_mode.o
 obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o
 endif

-ifeq ($(CONFIG_TPL_BUILD),)
+ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
 obj-$(CONFIG_DISPLAY_CPUINFO) += cpu-info.o
 endif

> 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).
>
Ok I will check this.
> Cheers,
> Quentin
Thanks
-Anand