diff mbox series

[RFC,1/3] spl: Kconfig: allow to enable SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR for i.MX8M

Message ID 20250515131246.784206-2-dario.binacchi@amarulasolutions.com
State New
Headers
Series Restore imx8mn_bsh_smm_s2 properly booting |

Commit Message

Dario Binacchi May 15, 2025, 1:12 p.m. UTC 
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(-)

Comments

'Abel Vesa' via Amarula Linux May 15, 2025, 1:26 p.m. UTC | #1 
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.
Tom Rini May 15, 2025, 2:30 p.m. UTC | #2 
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?
'Abel Vesa' via Amarula Linux May 15, 2025, 2:52 p.m. UTC | #3 
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 mbox series

Patch

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)"