Sunny,    thanks for your review and PSB my remarks.

________________________________
From: Sunny Wang <sunny.w...@arm.com>
Sent: Monday, July 12, 2021 4:03 PM
To: Vikas Singh <vikas.si...@puresoftware.com>; devel@edk2.groups.io 
<devel@edk2.groups.io>; Meenakshi Aggarwal (meenakshi.aggar...@nxp.com) 
<meenakshi.aggar...@nxp.com>; l...@nuviainc.com <l...@nuviainc.com>
Cc: Sami Mujawar <sami.muja...@arm.com>; l...@nuviainc.com <l...@nuviainc.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 Dwivedi 
<kuldip.dwiv...@puresoftware.com>; Ard Biesheuvel <ard.biesheu...@arm.com>; 
vikas.si...@nxp.com <vikas.si...@nxp.com>; White Weng <white.w...@nxp.com>; Ran 
Wang <ran.wan...@nxp.com>; Sunny Wang <sunny.w...@arm.com>; Joe Byrne 
<joseph.by...@nxp.com>
Subject: RE: [PATCH V2 0/4] Enable Dynamic ACPI for LS1046AFRWY

Hi Vikas,

Thanks for working on this.

As our offline discussion with NXP, our goal is to make the Tiano edk2-platform 
and NXP LSDK opensource 
https://source.codeaurora.org/external/qoriq/qoriq-components/edk2-platforms in 
sync. Now the main problem is that some folders' names and locations have been 
changed to be different from NXP LSDK opensource in previous commits, which 
causes difficulty in doing synchronization between Tiano edk2-platform and NXP 
LSDK opensource and also causes LSDK user's confusion.  I'm fine with keeping 
some changes that are needed for cleanup purposes or fixing build issues. 
However, I think we can still avoid some folder-renaming or folder-moving 
changes. For avoiding them, could you check my questions/comments below?

1. Why do we need to have ConfigurationManagerPkg.dec? Can we remove this? 
After removing it, we can rename the ConfigurationManagerPkg folder back to 
ConfigurationManager to be consistent with other platforms (JunoPkg).
[[Vikas]] ConfigurationManagerPkg folder should not be renamed back to 
ConfigurationManager because of the hierarchy and placement of this folder as a 
common generic for all the platform pkgs.
This is already been discussed with leif and moreover in case of JunoPkg the CM 
is private to JunoPkg not visible to other ARM platform pkgs. Here in our case 
CM serves all the NXP platforms.

2. Can we move \Platform\NXP\LX2160aRdbPkg\Include\Platform.h to the same 
location as LSDK (\Platform\NXP\LX2160aRdbPkg\AcpiTables\)?
[[Vikas]] This involves extra effort/rework.

3. Can we move \Platform\NXP\LS1046aFrwyPkg\Include\Platform.h t h to the same 
location as LSDK (\Platform\NXP\ LS1046aFrwyPkg\AcpiTables\)?
[[Vikas]] This involves extra effort/rework.

4. Can we add \Silicon\NXP\LS1046A\Library\SocFixupLib\ for patch 2/4 (adding 
SocGetSvr() function)? Furthermore, can we just add the whole 
Silicon\NXP\LS1046A\Library\SocFixupLib\ from NXP LSDK?
[[Vikas]] This involves extra effort/rework.

Add few more NXP guys.



Hi Leif and Meenakshi,

Can we just push the latest LSDK to the Tiano edk2-platform in one patch set? 
Then, If there is anything that needs to be cleaned up like the Coding style 
issue, we can create an issue in Bugzilla for it. What do you guys think?


Best Regards,
Sunny Wang

-----Original Message-----
From: Vikas Singh <vikas.si...@puresoftware.com>
Sent: Friday, June 18, 2021 11:28 PM
To: devel@edk2.groups.io
Cc: Sami Mujawar <sami.muja...@arm.com>; l...@nuviainc.com; Meenakshi Aggarwal 
(meenakshi.aggar...@nxp.com) <meenakshi.aggar...@nxp.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; Sunny Wang 
<sunny.w...@arm.com>
Subject: [PATCH V2 0/4] Enable Dynamic ACPI for LS1046AFRWY

This patch series basically aims to extend the Dynamic ACPI
framework towards NXP's LS1046AFRWY platform.

In continuation to https://edk2.groups.io/g/devel/message/71709

The change set in the series is in below order -

(1)Introducing a new platform specific macro "PLAT_SOC_NAME"
This macro will be consumed by Configuration Manager(CM).
Platforms who extends CM services for themselves must notify
their SoC details to CM using this macro only.
Additionally also update the lx2160ardb platform header with
PLAT_SOC_NAME, this will be consumed by CM.

(2)Introduced a function to get SoC's System Version Register(SVR)
This function will fetch SVR details for LS1046A SoC based platforms.
In current patch series, this function will be used by LS1046aFrwyPkg.

(3)Extending Configuration Manager (CM) and its services to leverage
the Dynamic ACPI support for NXP's LS1046aFrwy platform.

(4)Introduced an OEM specific firmware acpi table generator
Also add Dsdt.asl as a place holder having only platform's clock
related dsdt properties for now and will accommodate other IP specific
dsdt tables(acpi properties) for LS1046AFRWY in future patch series.

Vikas Singh (4):
  Platform/NXP: Make SoC version log in ConfigurationManager generic
  Silicon/NXP: Add support of SVR handling for LS1046A SoC
  NXP/LS1046aFrwyPkg: Enable ConfigurationManager on LS1046AFRWY
  Platform/NXP/LS1046aFrwyPkg: Add OEM specific DSDT generator

 
Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManager.c
  |  11 +-
 Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Clk.asl                     
      |  60 ++++++++
 Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Dsdt.asl                    
      |  15 ++
 Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib.inf          
      |  39 +++++
 
Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib/RawDsdtGenerator.c
 | 138 +++++++++++++++++
 Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiLib.h                
      |  23 +++
 Platform/NXP/LS1046aFrwyPkg/Include/Platform.h                                 
      | 156 ++++++++++++++++++++
 Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc                                 
      |  29 ++++
 Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf                                 
      |  13 ++
 Platform/NXP/LX2160aRdbPkg/Include/Platform.h                                  
      |   8 +-
 Silicon/NXP/LS1046A/LS1046A.dsc.inc                                            
      |  11 ++
 Silicon/NXP/LS1046A/Library/SocLib/SocLib.c                                    
      |  16 ++
 Silicon/NXP/LX2160A/LX2160A.dsc.inc                                            
      |   3 +-
 13 files changed, 508 insertions(+), 14 deletions(-)
 create mode 100644 Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Clk.asl
 create mode 100644 Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/Dsdt/Dsdt.asl
 create mode 100644 
Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib.inf
 create mode 100644 
Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiDsdtLib/RawDsdtGenerator.c
 create mode 100644 
Platform/NXP/LS1046aFrwyPkg/AcpiTablesInclude/PlatformAcpiLib.h
 create mode 100644 Platform/NXP/LS1046aFrwyPkg/Include/Platform.h

--
2.25.1

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78172): https://edk2.groups.io/g/devel/message/78172
Mute This Topic: https://groups.io/mt/83630879/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to