Re: [PATCH v2 0/2] Dynamic ACPI framework for fsl layerscape platforms

["Vikas Singh" <vikas.singh@puresoftware.com>]

On Mon, Feb 15, 2021 at 6:48 PM Leif Lindholm <leif@nuviainc.com> wrote:
>
> Hi Vikas,
>
> Apologies for very slow review.
>
> However, this set doesn't build for me.
> It fails with
>
> In file included from /work/git/edk2-platforms/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c:26:
> /work/git/edk2-platforms/Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.h:17:10: fatal error: Platform.h: No such file or directory
>  #include <Platform.h>
>            ^~~~~~~~~~~~
>
> Since the location of ConfigurationManagerPkg suggests the intent to
> use this for additional platforms in the future, this cannot be
> resolved by referencing LX2160aRdbPkg.dec from
> ConfigurationManagerDxe.inf.
>
> I would suggest doing like the ARM VExpressPkg and add:
> [BuildOptions]
>   GCC:*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/Platform/NXP/LX2160aRdbPkg/Include
> to LX2160aRdbPkg.dsc.
>

Thank you leif for your review comments and suggesting the way out.
I will do the necessary changes (suggested by you) and drop the build
script idea at this moment.

I will share the updated V3 series asap.

Thnx!!
VS

> Also, I'm seeing a build failure in edk2 DynamicTablesPkg when
> building with NOOPT:
> /work/git/edk2/DynamicTablesPkg/Library/Common/AmlLib/NameSpace/AmlNameSpace.c: In function ‘AmlEnumeratePathCallback’:
> /work/git/edk2/DynamicTablesPkg/Library/Common/AmlLib/NameSpace/AmlNameSpace.c:1081:31: error: variable ‘CurrNodePath’ set but not used [-Werror=unused-but-set-variable]
>    CHAR8                     * CurrNodePath;
>                                ^~~~~~~~~~~~
> /work/git/edk2/DynamicTablesPkg/Library/Common/AmlLib/NameSpace/AmlNameSpace.c:1078:31: error: variable ‘SearchedPath’ set but not used [-Werror=unused-but-set-variable]
>    CHAR8                     * SearchedPath;
>                                ^~~~~~~~~~~~
>
> Sami - can you have a look at that?
>
> I'll follow up with a couple of minor comments on the patches.
>
> Best Regards,
>
> Leif
>
> On Wed, Jan 27, 2021 at 12:14:57 +0530, Vikas Singh wrote:
> > This patch series sets the foundation of Dynamic ACPI framework for all
> > fsl layerscape platforms. In order to achieve:
> >   - Configurable firmware builds.
> >   - Unify firmware build for similar platforms.
> >   - Minimize/eliminate human induced errors.
> >   - Validate and generate firmware that complies with relevant specifications.
> > this change set will introduce following changes in below defined order under
> > edk2-platforms/NXP for LX2160ARDB platform.
> >
> > (1) Introduced edk2-platforms/NXP/ConfigurationManager
> > It creates the platform repositories dynamically during build time
> > and initializes with platform specific information and serves all requestes
> > coming from OEM/standard firmware table generators. Configuration Manager(CM)
> > will be common for all fsl platforms.
> >
> > (2) Introduced edk2-platforms/NXP/LX2160ARDB/Include/Platform.h
> > It has all the declarations and the definitions specified for the platforms.
> > These definitions will be inturn consumed by Configuration Manager.
> > Additionally the placement of this header under "Include" dir will make these
> > macro's availale to other translation units of the platform built.
> >
> > (3) Introduced edk2-platforms/NXP/LX2160ARDB/AcpiTablesInclude
> > This is a placeholder for - OEM specific firmware acpi table generators.
> > This also includes IP specific - DSDT/SSDT generators for the OEM's platform.
> > Currently Dsdt.asl is a place holder having only platform's clock related dsdt
> > properties for LX2160ARDB but it is intended to extend this "Dsdt.asl" to hold
> > other table as well in next patch series.
> >
> > (4) Introduced a new "DYNAMIC_ACPI_ENABLE" flag to control DACPI framework.
> > Currently this flag is used for LX2160ARDB and by default it is enabled.
> > This flag can also extend to other fsl layerscape platforms in future.
> > Changes can be referred under:
> >   - LX2160ARDb.dsc
> >   - LX2160ARDB.fdf
> >
> > (5) Introduced a "build.sh" script under edk2-platforms/NXP/ to automate builds.
> > And also mandatorily need to support Dynamic ACPI framework for all fsl platforms.
> > It exports build environment variables and also invokes "Env.sh".
> >
> > Vikas Singh (2):
> >   Platform/NXP: Add Dynamic Acpi for layerscape platforms
> >   Platform/NXP: Add OEM specific DSDT generator
> >
> >  .../ConfigurationManagerDxe/ConfigurationManager.c | 843 +++++++++++++++++++++
> >  .../ConfigurationManagerDxe/ConfigurationManager.h | 153 ++++
> >  .../ConfigurationManagerDxe.inf                    |  52 ++
> >  .../ConfigurationManagerPkg.dec                    |  23 +
> >  .../Include/PlatformAcpiTableGenerator.h           |  20 +
> >  Platform/NXP/Env.sh                                |  67 ++
> >  .../LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Clk.asl   |  40 +
> >  .../LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Dsdt.asl  |  15 +
> >  .../AcpiTablesInclude/PlatformAcpiDsdtLib.inf      |  39 +
> >  .../PlatformAcpiDsdtLib/RawDsdtGenerator.c         | 146 ++++
> >  .../AcpiTablesInclude/PlatformAcpiLib.h            |  24 +
> >  Platform/NXP/LX2160aRdbPkg/Include/Platform.h      | 246 ++++++
> >  Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dec       |   4 +
> >  Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.dsc       |  29 +
> >  Platform/NXP/LX2160aRdbPkg/LX2160aRdbPkg.fdf       |  12 +
> >  Platform/NXP/Readme.md                             |  35 +-
> >  Platform/NXP/build.sh                              | 115 +++
> >  Silicon/NXP/LX2160A/LX2160A.dsc.inc                |   9 +
> >  18 files changed, 1871 insertions(+), 1 deletion(-)
> >  create mode 100644 Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
> >  create mode 100644 Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.h
> >  create mode 100644 Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> >  create mode 100644 Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerPkg.dec
> >  create mode 100644 Platform/NXP/ConfigurationManagerPkg/Include/PlatformAcpiTableGenerator.h
> >  create mode 100755 Platform/NXP/Env.sh
> >  create mode 100644 Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Clk.asl
> >  create mode 100644 Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/Dsdt/Dsdt.asl
> >  create mode 100644 Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/PlatformAcpiDsdtLib.inf
> >  create mode 100644 Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/PlatformAcpiDsdtLib/RawDsdtGenerator.c
> >  create mode 100644 Platform/NXP/LX2160aRdbPkg/AcpiTablesInclude/PlatformAcpiLib.h
> >  create mode 100644 Platform/NXP/LX2160aRdbPkg/Include/Platform.h
> >  create mode 100755 Platform/NXP/build.sh
> >
> > --
> > 2.7.4
> >