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

[Tom Rini]

On Thu, Aug 24, 2023 at 11:22:47AM -0500, Nishanth Menon wrote:
> 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 at 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.

It "works" in that the generated environment will be something like:
foo=a
foo=b

But when we import the at run time we set "foo" to "a" and then override
"foo" to be "b".  Which works, but isn't super clear at the outset, so
it's best to not do that.

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

OK, thanks.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230824/79c0bea3/attachment.sig>