Hi Ahmad,

On 26-01-15, Ahmad Fatoum wrote:
> Hi,
> 
> On 11/10/25 9:34 PM, Marco Felsch wrote:
> > Add support to handover the BL32 and BL33 entrypoints via the TF-A
> > struct::bl_params in arg0. This eliminates the requirement to share the
> > different load addresses between multiple binaries to lower the BSP
> > integration effort.
> >
> > In addition to the entriespoints, this commit also adds the support to
> > pass the builtin barebox DTB to OP-TEE if enabled.
> >
> > Signed-off-by: Marco Felsch <[email protected]>
> > ---
> 
> 
> > +config ARCH_IMX_ATF_PASS_BL_PARAMS
> > +   bool "Pass BL3x bl_params as arg0 to TF-A"
> > +   depends on ARCH_IMX_ATF
> > +   select ARM_ATF
> > +   select ARCH_HAS_EARLY_FDT_SUPPORT
> > +   help
> > +     Enable this option if you are using an upstream TF-A that uses
> > +     the struct::bl_params to handover all required BL32 and BL33
> > +     information required to start the BL32 and BL33 image.
> > +
> > +     Since upstream TF-A v2.12 all i.MX8M support this feature except for
> > +     the i.MX8MQ.
> I think the help text is misleading, because you still need to enable
> the PBL_EARLY_FDT_LOAD option for this to actually work.
> 
> What you want instead is for the SoC symbols for i.MX8M[MNQ] to select
> ARCH_HAS_EARLY_FDT_SUPPORT and then this symbol here would select
> PBL_EARLY_FDT_LOAD.

I agree with the ARCH_HAS_EARLY_FDT_SUPPORT selection. For actual making
use of the "BL3x bl_params" we should make it a runtime selectable
variable. Since there may be boards with an old OP-TEE not supporting
this and newer ones. To honor multiconfig board builds, I would rather
make it a runtime-variable, like I said PBL_EARLY_FDT_LOAD.

> PBL_EARLY_FDT_LOAD itself would lose its prompt, but you probably will
> want to keep a
> 
>   prompt "Include Early FDT loading support" if COMPILE_TEST
> 
> So it has coverage from the static analyzers running for sandbox.
> 
> > +   if (!IS_ENABLED(CONFIG_ARCH_IMX_ATF_PASS_BL_PARAMS) || cpu_is_mx8mq()) {
> 
> Ok, good thing this is guarded, so I think that answers my questions
> regarding the LIBFDT dependency.
> 
> > +           ret = pbl_load_fdt(fdt, buf, bufsz);
> 
> Did you consider replacing the uncompressed DT in the handoff data if we
> already spent the time to decompress it here?

I saw the duplicated decompress step too :/ But I dedicded to skip this
part for further improvements. It's woth a comment for sure!

> > +           if (!ret) {
> > +                   unsigned long mem_base = MX8M_DDR_CSD1_BASE_ADDR;
> > +                   unsigned long mem_sz;
> > +
> > +                   if (cpu_is_mx8mn())
> > +                           mem_sz = imx8m_ddrc_sdram_size(16);
> > +                   else
> > +                           mem_sz = imx8m_ddrc_sdram_size(32);
> > +
> > +                   fdt = buf;
> > +                   ret = fdt_fixup_mem(fdt, &mem_base, &mem_sz, 1);
> > +                   if (ret) {
> > +                           pr_warn("Failed to fixup FDT memory node, 
> > continue without\n");
> > +                           fdt = NULL;
> > +                   }
> 
> Ok, that answers my question about the memory being inaccurate.
> 
> Keep in mind that all this runs without MMU enabled, so it will be
> slower. A FIXME comment may be apt to alert to this fact.

Sure! It's worth a comment that these ops are slower :/

Regards,
  Marco


> 
> Thanks,
> Ahmad
> 
> 
> -- 
> 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 |
> 
> 

-- 
#gernperDu 
#CallMeByMyFirstName

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

Reply via email to