Re: [PATCH edk2-platforms v2 8/9] Platform/NXP: Add LX2160ARDB Platform

["Leif Lindholm" <leif@nuviainc.com>]

On Fri, May 29, 2020 at 12:25:43 +0000, Pankaj Bansal (OSS) wrote:
> > > I intentionally kept these in this order. because Soc depends on Chassis for any
> > include files
> > > Chassis depends on NxpQoriqLs.dec for any include files.
> > 
> > OK, I see the logic behind that, but as it has no functional meaning I
> > don't think it outweighs the benefit of alphabetical sorting.
> > 
> 
> I referred to the inf specifications for this:
> https://edk2-docs.gitbook.io/edk-ii-inf-specification/3_edk_ii_inf_file_format/37_-packages-_sections
> 
> "Packages must be listed in the order that may be required for
> specifying include path statements for a compiler.
> For example, the MdePkg/MdePkg.dec file must be listed before the
> MdeModulePkg/MdeModulePkg.dec file"

I see that. (And sigh.) Clearly, that needs to be fixed. I have cc:d
you on the patch to do that.

I am sure that made sense when that text was originally written, which
was presumably the previous millenium. But this is akin to "optimising"
the order in which header files is included.

> By design the Chassis package files can include generic NXP package
> files. not the other way around.
> same goes for SOC package files. SOC package files can include
> Chassis Package file but not the other way around
> and we can extend this further. any platform package files can
> include SOC package files
> 
> > I *do* tend to say "alphabetically, unless there is a better logical
> > order", but by that I usually mean grouping related Pcd dependency
> > declarations.
> 
> as I mentioned above, there *is* a logical order to packages.

And as I said "I see the logic behind that".
I'm saying readability trumps that logic.

> > 
> > This is my only remaining niggle with this set - are you OK with me
> > folding in
> > 
> > --- a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > +++
> > b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > @@ -18,9 +18,9 @@ [Packages]
> >    MdeModulePkg/MdeModulePkg.dec
> >    ArmPkg/ArmPkg.dec
> >    ArmPlatformPkg/ArmPlatformPkg.dec
> > -  Silicon/NXP/NxpQoriqLs.dec
> >    Silicon/NXP/Chassis3V2/Chassis3V2.dec
> >    Silicon/NXP/LX2160A/LX2160A.dec
> > +  Silicon/NXP/NxpQoriqLs.dec
> > 
> > to v3 9/10 and pushing the set?
> > 
> > /
> >     Leif