Hi Leif,
Please find my response inline marked [SAMI].
Regards,
Sami Mujawar
-----Original Message-----
From: Leif Lindholm <l...@nuviainc.com>
Sent: 15 February 2021 01:18 PM
To: Vikas Singh <vikas.si...@puresoftware.com>; Sami Mujawar
<sami.muja...@arm.com>
Cc: devel@edk2.groups.io; Meenakshi Aggarwal (meenakshi.aggar...@nxp.com)
<meenakshi.aggar...@nxp.com>; Paul Yang <paul.y...@arm.com>; Augustine Philips
<augustine.phil...@arm.com>; Samer El-Haj-Mahmoud
<samer.el-haj-mahm...@arm.com>; V Sethi (v.se...@nxp.com) <v.se...@nxp.com>;
arokia.samy <arokia.s...@puresoftware.com>; kuldip.dwiv...@puresoftware.com;
Ard Biesheuvel <ard.biesheu...@arm.com>; vikas.si...@nxp.com
Subject: Re: [PATCH v2 0/2] Dynamic ACPI framework for fsl layerscape platforms
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.
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?
[SAMI] We will get a patch to fix this soon.
[/SAMI]
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
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71678): https://edk2.groups.io/g/devel/message/71678
Mute This Topic: https://groups.io/mt/80152008/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
