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

["Pankaj Bansal" <pankaj.bansal@nxp.com>]

> On Fri, May 29, 2020 at 05:21:24 +0000, Pankaj Bansal (OSS) wrote:
> > > > ---
> > >
> a/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > > > +++
> > >
> b/Platform/NXP/LX2160aRdbPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> > > > @@ -19,9 +19,12 @@
> > > >    ArmPkg/ArmPkg.dec
> > > >    ArmPlatformPkg/ArmPlatformPkg.dec
> > > >    Silicon/NXP/NxpQoriqLs.dec
> > > > +  Silicon/NXP/Chassis3V2/Chassis3V2.dec
> > > > +  Silicon/NXP/LX2160A/LX2160A.dec
> > >
> > > Please insert alphabetically sorted.
> >
> > 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"

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.

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