| Message ID | 20250515131246.784206-2-dario.binacchi@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Dario, On 5/15/25 3:12 PM, Dario Binacchi wrote: > Commit 2a00d73d081a ("spl: mmc: Try to clean up raw-mode options") breaks > the boot of the BSH SMM S2 board. As stated in the commit itself, "Some > boards use this value even though MMC is not enabled in SPL, for example > imx8mn_bsh_smm_s2". The same commit makes SPL_SYS_MMCSD_RAW_MODE depend > on SPL_DM_MMC || SPL_MMC. With SPL_SYS_MMCSD_RAW_MODE unset, it is not > possible to enable SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR, thus breaking > the board's boot process. > > The patch once again allows SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR to be > selected also for boards that do not use MMC, in this case on the i.MX8M > platform. > Why would you use SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR if you don't have any MMC? Shouldn't we rather fix the incorrect usage of this variable to be something else (or better guarded?). Cheers, Quentin To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
On Thu, May 15, 2025 at 03:26:18PM +0200, Quentin Schulz wrote: > Hi Dario, > > On 5/15/25 3:12 PM, Dario Binacchi wrote: > > Commit 2a00d73d081a ("spl: mmc: Try to clean up raw-mode options") breaks > > the boot of the BSH SMM S2 board. As stated in the commit itself, "Some > > boards use this value even though MMC is not enabled in SPL, for example > > imx8mn_bsh_smm_s2". The same commit makes SPL_SYS_MMCSD_RAW_MODE depend > > on SPL_DM_MMC || SPL_MMC. With SPL_SYS_MMCSD_RAW_MODE unset, it is not > > possible to enable SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR, thus breaking > > the board's boot process. > > > > The patch once again allows SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR to be > > selected also for boards that do not use MMC, in this case on the i.MX8M > > platform. > > > > Why would you use SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR if you don't have any > MMC? Shouldn't we rather fix the incorrect usage of this variable to be > something else (or better guarded?). It appears that arch/arm/mach-imx/spl_imx_romapi.c::spl_romapi_get_uboot_base() will use the value so that we (apparently) go ROM->U-Boot, or at least ROM->some series of prior stage loaders->ROM->U-Boot. I'm not sure I love overloading the value like this, but I can't say it's wrong. Perhaps the help needs to be updated too?
On 5/15/25 4:30 PM, Tom Rini wrote: > On Thu, May 15, 2025 at 03:26:18PM +0200, Quentin Schulz wrote: >> Hi Dario, >> >> On 5/15/25 3:12 PM, Dario Binacchi wrote: >>> Commit 2a00d73d081a ("spl: mmc: Try to clean up raw-mode options") breaks >>> the boot of the BSH SMM S2 board. As stated in the commit itself, "Some >>> boards use this value even though MMC is not enabled in SPL, for example >>> imx8mn_bsh_smm_s2". The same commit makes SPL_SYS_MMCSD_RAW_MODE depend >>> on SPL_DM_MMC || SPL_MMC. With SPL_SYS_MMCSD_RAW_MODE unset, it is not >>> possible to enable SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR, thus breaking >>> the board's boot process. >>> >>> The patch once again allows SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR to be >>> selected also for boards that do not use MMC, in this case on the i.MX8M >>> platform. >>> >> >> Why would you use SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR if you don't have any >> MMC? Shouldn't we rather fix the incorrect usage of this variable to be >> something else (or better guarded?). > > It appears that arch/arm/mach-imx/spl_imx_romapi.c::spl_romapi_get_uboot_base() > will use the value so that we (apparently) go ROM->U-Boot, or at least > ROM->some series of prior stage loaders->ROM->U-Boot. I'm not sure I > love overloading the value like this, but I can't say it's wrong. > Perhaps the help needs to be updated too? > Isn't it rather U-Boot SPL asking the BootROM to load the image for it instead of doing it by itself? Some MMC is technically present then, just that because the BootROM handles it and not U-Boot, we need to specify the offset in the MMC even though we only return that value to the BootROM. If that is correct, then I'm not sure we want to use SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR considering it selects SPL_LOAD_BLOCK which I would assume to be useless if the BootROM is doing all the work for the SPL? Cheers, Quentin To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
diff --git a/common/spl/Kconfig b/common/spl/Kconfig index aa3a85eea54d..77bb21e74cd6 100644 --- a/common/spl/Kconfig +++ b/common/spl/Kconfig @@ -542,7 +542,7 @@ config SPL_SYS_MMCSD_RAW_MODE help Support booting from an MMC without a filesystem. -if SPL_SYS_MMCSD_RAW_MODE +if SPL_SYS_MMCSD_RAW_MODE || ARCH_IMX8M choice prompt "Method for locating next phase of boot (e.g. U-Boot)"
Commit 2a00d73d081a ("spl: mmc: Try to clean up raw-mode options") breaks the boot of the BSH SMM S2 board. As stated in the commit itself, "Some boards use this value even though MMC is not enabled in SPL, for example imx8mn_bsh_smm_s2". The same commit makes SPL_SYS_MMCSD_RAW_MODE depend on SPL_DM_MMC || SPL_MMC. With SPL_SYS_MMCSD_RAW_MODE unset, it is not possible to enable SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR, thus breaking the board's boot process. The patch once again allows SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR to be selected also for boards that do not use MMC, in this case on the i.MX8M platform. Fixes: 2a00d73d081a ("spl: mmc: Try to clean up raw-mode options") Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com> --- common/spl/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)