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

["Pankaj Bansal" <pankaj.bansal@nxp.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.
> 

hmm. I think it was written to maintain the dependency between include files.
But I guess if we follow edk2 c guideline we can avoid this problem.

https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/53_include_files#5-3-4-include-files-may-include-only-those-headers-that-it-directly-depends-upon


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

Ok. it makes sense.

> 
> > >
> > > 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?

Ok. can you please sort all the dec files under [Packages] based on names before pushing ?
I think this file also needs alphabetical sorting:
https://github.com/tianocore/edk2/blob/master/ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf#L16

> > >
> > > /
> > >     Leif