diff mbox series

[5/5] riscv: select OF_HAS_PRIOR_STAGE by default if SBI is enabled

Message ID 20250227144734.61458-6-ziyao@disroot.org
State Superseded
Delegated to: Andes
Headers show
Series Simplifiy retrieving FDT from SBI in S-Mode | expand

Commit Message

Yao Zi Feb. 27, 2025, 2:47 p.m. UTC
Availability of RISC-V SBI service implies a prior stage exists. As SBI
firmware usually passes a FDT to the loaded program, let's select
OF_HAS_PRIOR_STAGE if SBI is enabled.

With previously added fallback version of board_fdt_blob_setup, S-Mode
RISC-V ports use the SBI-provided FDT by default. This covers the most
common usecase, where a SPL (probably the U-Boot one) selects proper
devicetree, loads SBI and U-Boot then invokes SBI with the devicetree.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/riscv/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Leo Liang March 6, 2025, 4:10 a.m. UTC | #1
On Thu, Feb 27, 2025 at 02:47:33PM +0000, Yao Zi wrote:
> Availability of RISC-V SBI service implies a prior stage exists. As SBI
> firmware usually passes a FDT to the loaded program, let's select
> OF_HAS_PRIOR_STAGE if SBI is enabled.
> 
> With previously added fallback version of board_fdt_blob_setup, S-Mode
> RISC-V ports use the SBI-provided FDT by default. This covers the most
> common usecase, where a SPL (probably the U-Boot one) selects proper
> devicetree, loads SBI and U-Boot then invokes SBI with the devicetree.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/riscv/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
Leo Liang March 6, 2025, 11:09 a.m. UTC | #2
On Thu, Feb 27, 2025 at 02:47:33PM +0000, Yao Zi wrote:
> Availability of RISC-V SBI service implies a prior stage exists. As SBI
> firmware usually passes a FDT to the loaded program, let's select
> OF_HAS_PRIOR_STAGE if SBI is enabled.
> 
> With previously added fallback version of board_fdt_blob_setup, S-Mode
> RISC-V ports use the SBI-provided FDT by default. This covers the most
> common usecase, where a SPL (probably the U-Boot one) selects proper
> devicetree, loads SBI and U-Boot then invokes SBI with the devicetree.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> ---
>  arch/riscv/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Hi Yao,

This patch would fail some of the CI tests.
I will take other patches from this patchset first,
and could you take a look at the CI tests issue ?
(https://source.denx.de/u-boot/custodians/u-boot-riscv/-/jobs/1049827)

Best regards,
Leo
Yao Zi March 6, 2025, 12:40 p.m. UTC | #3
On Thu, Mar 06, 2025 at 07:09:06PM +0800, Leo Liang wrote:
> On Thu, Feb 27, 2025 at 02:47:33PM +0000, Yao Zi wrote:
> > Availability of RISC-V SBI service implies a prior stage exists. As SBI
> > firmware usually passes a FDT to the loaded program, let's select
> > OF_HAS_PRIOR_STAGE if SBI is enabled.
> > 
> > With previously added fallback version of board_fdt_blob_setup, S-Mode
> > RISC-V ports use the SBI-provided FDT by default. This covers the most
> > common usecase, where a SPL (probably the U-Boot one) selects proper
> > devicetree, loads SBI and U-Boot then invokes SBI with the devicetree.
> > 
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> > ---
> >  arch/riscv/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Hi Yao,
> 
> This patch would fail some of the CI tests.
> I will take other patches from this patchset first,
> and could you take a look at the CI tests issue ?
> (https://source.denx.de/u-boot/custodians/u-boot-riscv/-/jobs/1049827)

Sure, will have a check later.

> Best regards,
> Leo

Thanks,
Yao Zi
Yao Zi March 7, 2025, 11:15 a.m. UTC | #4
On Thu, Mar 06, 2025 at 07:09:06PM +0800, Leo Liang wrote:
> On Thu, Feb 27, 2025 at 02:47:33PM +0000, Yao Zi wrote:
> > Availability of RISC-V SBI service implies a prior stage exists. As SBI
> > firmware usually passes a FDT to the loaded program, let's select
> > OF_HAS_PRIOR_STAGE if SBI is enabled.
> > 
> > With previously added fallback version of board_fdt_blob_setup, S-Mode
> > RISC-V ports use the SBI-provided FDT by default. This covers the most
> > common usecase, where a SPL (probably the U-Boot one) selects proper
> > devicetree, loads SBI and U-Boot then invokes SBI with the devicetree.
> > 
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> > ---
> >  arch/riscv/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Hi Yao,
> 
> This patch would fail some of the CI tests.
> I will take other patches from this patchset first,
> and could you take a look at the CI tests issue ?
> (https://source.denx.de/u-boot/custodians/u-boot-riscv/-/jobs/1049827)

After digging further with the failure, I found the series has two
problems,

- OF_HAS_PRIOR_STAGE is an option shared between XPL and proper U-Boot.
  The generic board_fdt_blob_setup() doesn't work in a XPL build,
  corresponding checks should be added.
- The default binman configuration for RISC-V platforms doesn't pack the
  FDTs into the fit image if OF_HAS_PRIOR_BOARD is enabled

Although it doesn't break anything to merge the first four patches for
now, I'd like to rework PATCH 1 and add a check against XPL build in the
new revision. Then we could merge a patch that is technically right and
avoid fixes tags.

Additionally, I found OF_BOARD is actually disabled on Unmatched and
Unleashed and the board_fdt_blob_setup() here is dead code in fact. Thus
the commit message of PATCH 4 should be fixed as well.

> Best regards,
> Leo

Best regards,
Yao Zi
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index b24623590f2..f7706788f92 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -410,6 +410,7 @@  config NR_CPUS
 config SBI
 	bool
 	default y if RISCV_SMODE || SPL_RISCV_SMODE
+	imply OF_HAS_PRIOR_STAGE
 
 choice
 	prompt "SBI support"