Re: [PATCH] include: configs: Change dtb names in fitImage to match oe-core

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 4097 bytes --]

On Fri, Mar 31, 2023 at 01:52:44PM -0500, Andrew Davis wrote:
> On 3/31/23 1:36 PM, Tom Rini wrote:
> > On Fri, Mar 31, 2023 at 01:23:23PM -0500, Andrew Davis wrote:
> > > On 3/31/23 1:11 PM, Tom Rini wrote:
> > > > On Fri, Mar 31, 2023 at 12:19:14PM -0500, Andrew Davis wrote:
> > > > > On 3/31/23 12:04 PM, Nishanth Menon wrote:
> > > > > > On 16:06-20230330, Ryan Eatmon wrote:
> > > > > > > The oe-core class for assembling the fitImage includes the vendor
> > > > > > > sub-directory (with the / changed to _) in the config sections of
> > > > > > > the fitImage.  Our env var settings for chosing which section to
> > > > > > > boot from needs to be updated to agree with the fitImage.
> > > > > > > 
> > > > > > > Signed-off-by: Ryan Eatmon <reatmon@ti.com>
> > > > > > > ---
> > > > > > >     include/configs/am64x_evm.h  | 4 ++--
> > > > > > >     include/configs/am65x_evm.h  | 2 +-
> > > > > > >     include/configs/j721e_evm.h  | 6 +++---
> > > > > > >     include/configs/j721s2_evm.h | 6 +++---
> > > > > > >     4 files changed, 9 insertions(+), 9 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/include/configs/am64x_evm.h b/include/configs/am64x_evm.h
> > > > > > > index 26a7f2521e..456a44730c 100644
> > > > > > > --- a/include/configs/am64x_evm.h
> > > > > > > +++ b/include/configs/am64x_evm.h
> > > > > > > @@ -26,9 +26,9 @@
> > > > > > >     #define EXTRA_ENV_AM642_BOARD_SETTINGS					\
> > > > > > >     	"findfdt="							\
> > > > > > >     		"if test $board_name = am64x_gpevm; then " \
> > > > > > > -			"setenv fdtfile k3-am642-evm.dtb; fi; " \
> > > > > > > +			"setenv fdtfile ti_k3-am642-evm.dtb; fi; " \
> > > > > > >     		"if test $board_name = am64x_skevm; then " \
> > > > > > > -			"setenv fdtfile k3-am642-sk.dtb; fi;" \
> > > > > > > +			"setenv fdtfile ti_k3-am642-sk.dtb; fi;" \
> > > > > > 
> > > > > > 
> > > > > > Kernel.org builds k3-am642-sk.dtb
> > > > > > 
> > > > > 
> > > > > Nope, kernel.org build ti/k3-am642-sk.dtb
> > > > > 
> > > > > > Why would we want to go and fix it with non-standard?
> > > > > > 
> > > > > 
> > > > > The dtbs need to have their dir prefixed or there could be collisions
> > > > 
> > > > The key here is we're talking about what's used in FIT images, and not
> > > > just loaded directly. All of the cases where fdtfile is used on arm64
> > > > platforms in tree today use "/" because it's loading a literal. In this
> > > > case, why again is OE mangling the name, and what are we using it for
> > > > again here? In other words, why can't it be treated as "/" to match the
> > > > load a file case? And if it _must_ be mangled, must we still use fdtfile
> > > > and not something else?
> > > > 
> > > 
> > > So to match other plats, we should:
> > > 
> > > setenv fdtfile ti/k3-am642-sk.dtb
> > > 
> > > That way loading works in the non-FIT case (and we just need to stop moving
> > > all the DTBs out of the ti/ to the root dir (which is probably broken on our
> > > part anyway..))
> > 
> > Yes, this sounds right for the non-FIT case to match other platforms /
> > etc.
> > 
> > > Then when we use "fdtfile" in the FIT case, we just mangle ("/"->"_") it
> > > at that point only before use. That work for everyone?
> > 
> > Well, what _is_ the FIT case and how does it go exactly? I still want to
> > know why we're mangling things and come up with something that'll work
> > for when someone says "I've got an imx8mm and I'm using a FIT image from
> > OE and this doesn't work!".
> > 
> 
> So what happens is when OE is packaging a dtb file into the FIT image
> it names the node based on the dtb filename. Node names can't have
> / so it is turned into _ [0]. We select our FIT config using the
> "fdtfile" env var so we don't duplicate the above board_name to fdt
> logic. Result is fdtfile needs mangled when used to select a config
> node from OE made FIT image.

OK, so the next step would be to use setexpr + regex to mangle fdtfile
to get the node name to use, so you don't have to duplicate the logic.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]