On June 21, 2024 thus sayeth Dhruva Gole:
> Bryan,
> 
> On Jun 19, 2024 at 16:44:48 -0500, Bryan Brattlof wrote:
> > On June 18, 2024 thus sayeth Dhruva Gole:
> > > Add support for packaging the TIFS Stub as it's required for basic Low
> > > Power Modes like Deep Sleep.
> > > 
> > > Acked-by: Neha Malcom Francis <n-fran...@ti.com>
> > > Signed-off-by: Dhruva Gole <d-g...@ti.com>
> > > ---
> > > 
> > > No changes from v1, just picked Neha's ack and rebased on master again.
> > > Link to v1:
> > > https://lore.kernel.org/u-boot/20240612062351.3690091-1-d-g...@ti.com/
> > > 
> > >  arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi | 33 +++++++++++++++++++-
> > >  1 file changed, 32 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi 
> > > b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> > > index fb2032068d1c..5e2248a4a668 100644
> > > --- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> > > +++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> > > @@ -69,6 +69,23 @@
> > >           };
> > >   };
> > >  
> > > + tifsstub-gp {
> > > +         filename = "tifsstub.bin_gp";
> > > +         ti-secure-rom {
> > > +                 content = <&tifsstub_gp>;
> > > +                 core = "secure";
> > > +                 load = <0x60000>;
> > > +                 sw-rev = <CONFIG_K3_X509_SWRV>;
> > > +                 keyfile = "ti-degenerate-key.pem";
> > > +                 tifsstub;
> > > +         };
> > > +         tifsstub_gp: tifsstub-gp.bin {
> > > +                 filename = "ti-sysfw/ti-fs-stub-firmware-am62x-gp.bin";
> > > +                 type = "blob-ext";
> > > +                 optional;
> > > +         };
> > > + };
> > > +
> > >   ti-spl_unsigned {
> > >           filename = "tispl.bin_unsigned";
> > >           pad-byte = <0xff>;
> > > @@ -105,6 +122,19 @@
> > >                                   };
> > >                           };
> > >  
> > > +                         tifsstub-gp {
> > > +                                 description = "tifsstub";
> > > +                                 type = "firmware";
> > > +                                 arch = "arm32";
> > > +                                 compression = "none";
> > > +                                 os = "tifsstub-gp";
> > > +                                 load = <0x9dc00000>;
> > > +                                 entry = <0x9dc00000>;
> > 
> > Is this stub position independent? Or is this address compiled in at 
> > build time? We have some variants of 62x that don't have these addresses 
> > backed by DRAM.
> 
> The stub is not position independent and is indeed fixed at build time.
> Can you talk more about these am62x variants that don't have the address
> backed by DRAM? What would be the suggested address range in those
> cases?
> 

We've always had a 'soft' limit of 512MB so you're correct this isn't an 
issue today and I haven't seen any 'u-boot shall support' language 
internally however there have been a few questions about minimum DDR 
capacities we can support. Some have asked about 256MB.

(This 512MB 'soft' limit is most likely why this stub and OP-TEE is 
where it is today)

>
> For BeaglePlay atleast I don't see an issue. So I don't think we'd have
> issues with this patch in particular?
>

No you're correct however knowing that this address location is compiled 
into other firmware means we would need to have separate firmware builds 
if we wanted this stub in another location and it's awkwardly in the 
middle of DRAM and not at the bottom like I would prefer. 

So while yes this is a board/ level decision because it's also dealing 
with mach-k3/ level firmware this is an unfortunate thing we're 
hard-coding. This is essentially hard-coding a 512MB minimum limit for 
the entire AM62x SoC family.

My question was trying to determine if we should/could add this address 
to Kconfig so we can pass in the address to tispl.bin here and somehow 
get this into board-cfg.yaml. (This will be some work for us)

All of that to say, I'm okay with this change however I wish there was a 
'there be dragons' trailer I could add to this

~Bryan

Reply via email to