Re: [PATCH 2/2] arch: arm: imx: detect secondary boot

On 2026-06-15 12:40, Fabian Pfitzner wrote:
> On Thu, 2026-05-21 at 12:27 +0000, Sascha Hauer wrote:
> > On 2026-05-20 10:33, Fabian Pfitzner wrote:
> > > Integrate support to detect secondary boot on imx machines.
> > > Copy the logic for detection from the uboot-imx project[1].
> > > 
> > > The information will be stored in the `bootsource_instance_index`
> > > variable.
> > > 
> > > [1]
> > > https://github.com/nxp-imx/uboot-imx/blob/99518e6b6f20cb6a2bf19115e355db9f58100af8/arch/arm/mach-imx/imx8m/soc.c#L792
> > > 
> > > Signed-off-by: Fabian Pfitzner <[email protected]>
> > > ---
> > >  arch/arm/mach-imx/boot.c | 52
> > > ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 52 insertions(+)
> > > 
> > > diff --git a/arch/arm/mach-imx/boot.c b/arch/arm/mach-imx/boot.c
> > > index
> > > 3fea22d05f3bfdf2fe9caa6767e1c7c5ea3ced00..a35a1acc2e71231236d3dd500
> > > 8523f6c66912000 100644
> > > --- a/arch/arm/mach-imx/boot.c
> > > +++ b/arch/arm/mach-imx/boot.c
> > > @@ -26,6 +26,8 @@
> > >  
> > >  #include <soc/fsl/fsl_udc.h>
> > >  
> > > +int imx8m_detect_secondary_image_boot(void);
> > 
> > It should be either static without forward declaration or exported
> > with
> > the declaration in a header file.
> > 
> > > +
> > >  static void
> > >  imx_boot_save_loc(void (*get_boot_source)(enum bootsource *, int
> > > *))
> > >  {
> > > @@ -35,6 +37,10 @@ imx_boot_save_loc(void (*get_boot_source)(enum
> > > bootsource *, int *))
> > >  	get_boot_source(&src, &instance);
> > >  
> > >  	bootsource_set(src, instance);
> > > +
> > > +	int index = imx8m_detect_secondary_image_boot();
> > 
> > You assume you are running on i.MX8M here which is not necessarily
> > the
> > case.
> > 
> > > +
> > > +	bootsource_set_instance_index(index);
> > >  }
> > >  
> > >  
> > > @@ -756,3 +762,49 @@ void imx8mn_get_boot_source(enum bootsource
> > > *src, int *instance)
> > >  
> > >  void imx8mn_boot_save_loc(void)
> > >  	__alias(imx8mp_boot_save_loc);
> > > +
> > > +int imx8m_detect_secondary_image_boot(void)
> > > +{
> > > +	u32 *rom_log_addr = (u32 *)0x9e0;
> > > +	u32 *rom_log;
> > > +	u8 event_id;
> > > +	int i, boot_secondary = 0;
> > > +
> > > +	/* If the ROM event log pointer is not valid. */
> > > +	if (*rom_log_addr < 0x900000 || *rom_log_addr >= 0xb00000
> > > ||
> > > +	    *rom_log_addr & 0x3)
> > > +		return -EINVAL;
> > 
> > We have imx8m_get_bootrom_log() which handles different cases like we
> > are in PBL/EL3 or in barebox proper. Why not use it here?
> 
> I tried using `imx8m_get_bootrom_log()` here, but it does not seem to
> work. The function returns an invalid address (0x8000) here.

imx8m_get_bootrom_log() is basically the same code as you open coded
here. Which path is used in imx8m_get_bootrom_log(), Are you in EL3?
is IN_PROPER true?

One difference is that in imx8m_get_bootrom_log() 0x9e0 is used as a
unsigned long * (64bit) whereas your code assumes it's u32, but I can't
see how this explains the 0x8000 value.

Sascha

--
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |