Re: [PATCH V5 08/17] include: env: ti: ti_common: Add a generic findfdt

[Nishanth Menon <nm@ti.com>]

On 11:14-20230824, Tom Rini wrote:
> On Wed, Aug 23, 2023 at 10:10:52PM -0500, Nishanth Menon wrote:
> > ti_mmc bootmethod uses a findfdt routine that is expected to be
> > implemented by all platforms.
> > 
> > Define a default findfdt based on configured DEFAULT_DEVICE_TREE option
> > for u-boot. This saves duplication across multiple boards and handles
> > architecture folder location changes centrally.
> > 
> > TI ARMV7 platforms will need to override default_device_tree_subarch
> > in the env file to point to the appropriate platform. Note: default
> > "omap" is used to cater to "most common" default.
> > 
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> > New patch (part of bootstd transition)
> > 
> >  include/env/ti/ti_common.env | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/include/env/ti/ti_common.env b/include/env/ti/ti_common.env
> > index e87a41a6590b..322f849a5560 100644
> > --- a/include/env/ti/ti_common.env
> > +++ b/include/env/ti/ti_common.env
> > @@ -32,3 +32,15 @@ bootcmd_ti_mmc=
> >  	else;
> >  		run get_kern_${boot}; run get_fdt_${boot}; run get_overlay_${boot}; run run_kern;
> >  	fi;
> > +default_device_tree=CONFIG_DEFAULT_DEVICE_TREE
> > +default_device_tree_arch=ti
> > +#ifdef CONFIG_ARM64
> > +findfdt=
> > +	setenv name_fdt ${default_device_tree_arch}/${default_device_tree}.dtb;
> > +	setenv fdtfile ${name_fdt}
> > +#else
> > +default_device_tree_subarch=omap
> > +findfdt=
> > +	setenv name_fdt ${default_device_tree_arch}/${default_device_tree_subarch}/${default_device_tree}.dtb;
> > +	setenv fdtfile ${name_fdt}
> > +#endif
> 
> How does this override work in practice?  I fear that it's relying on
> the (not exactly documented) mechanic of "last one set generally work".

I was depending on that -> since we work with #includes anyways - I had
assumed it implicitly.

> So perhaps we don't want this in ti_common.env but instead
> include/env/ti/default_findfdt.env and then #include that, and then
> platforms that need different just don't #include this.

Sure, I can do that in the next spin. Will wait a couple of days for any
further feedback to fixup (without broadening scope unless mandatory).


-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D