Hi Quentin On Thu, 16 May 2024 at 16:13, Quentin Schulz <quentin.sch...@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.sch...@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