Hi Quentin
On Thu, 16 May 2024 at 16:13, Quentin Schulz quentin.schulz@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@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_CPUINFOThis 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-i... 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/Makef...
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-ro... should probably be ifdef'ed
which means...
https://elixir.bootlin.com/u-boot/latest/source/board/firefly/roc-pc-rk3399/... 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