From: "Leif Lindholm" <leif@nuviainc.com>
To: Vikas Singh <vikas.singh@puresoftware.com>
Cc: devel@edk2.groups.io, sami.mujawar@arm.com,
meenakshi.aggarwal@nxp.com, samer.el-haj-mahmoud@arm.com,
v.sethi@nxp.com, arokia.samy@puresoftware.com,
kuldip.dwivedi@puresoftware.com, ard.biesheuvel@arm.com,
vikas.singh@nxp.com, Sunny.Wang@arm.com
Subject: Re: [PATCH V1 3/4] Platform/NXP/LS1046aFrwyPkg: Extend Dynamic ACPI support
Date: Mon, 14 Jun 2021 22:28:35 +0100 [thread overview]
Message-ID: <20210614212835.grfvqk6w2ejwlsib@leviathan> (raw)
In-Reply-To: <20210611155200.15535-4-vikas.singh@puresoftware.com>
On Fri, Jun 11, 2021 at 21:21:59 +0530, Vikas Singh wrote:
> This patch set extends Configuration Manager (CM) and
> its services to leverage the Dynamic ACPI support for
> NXP's LS1046aFrwy platform.
This patch does not touch ConfigurationManager.
Please describe what this patch does.
My guess is it's along the lines of:
This set enables use of the ConfigurationManager framework for the
LS1046aFrwy platform.
> Refer-https://edk2.groups.io/g/devel/message/71710
That is a 1326-line patch set.
What is this reference supposed to tell me?
> Signed-off-by: Vikas Singh <vikas.singh@puresoftware.com>
> ---
> Platform/NXP/LS1046aFrwyPkg/Include/Platform.h | 155 ++++++++++++++++++++
> Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc | 28 ++++
> Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf | 13 ++
> Silicon/NXP/LS1046A/LS1046A.dsc.inc | 10 ++
> 4 files changed, 206 insertions(+)
>
> diff --git a/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h b/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h
> new file mode 100644
> index 0000000000..19e879ec6d
> --- /dev/null
> +++ b/Platform/NXP/LS1046aFrwyPkg/Include/Platform.h
> @@ -0,0 +1,155 @@
> +/** @file
> + * Platform headers
> + *
> + * Copyright 2021 NXP
> + * Copyright 2021 Puresoftware Ltd
> + *
> + * SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> +**/
> +
> +
> +#ifndef LS1046AFRWY_PLATFORM_H
> +#define LS1046AFRWY_PLATFORM_H
> +
> +#define EFI_ACPI_ARM_OEM_REVISION 0x00000000
> +
> +// Soc defines
> +#define PLAT_SOC_NAME "LS1046AFRWY"
> +#define SVR_SOC_VER(svr) (((svr) >> 8) & 0xFFFFFE)
> +#define SVR_MAJOR(svr) (((svr) >> 4) & 0xf)
> +#define SVR_MINOR(svr) (((svr) >> 0) & 0xf)
We already have three identical copies of these three macros:
Platform/NXP/LX2160aRdbPkg/Include/Platform.h
Silicon/NXP/Chassis2/Include/Chassis.h
Silicon/NXP/Chassis3V2/Include/Chassis.h
Could they be defined once, in a single common header, rather than
adding a fourth one?
> +
> +// Gic
> +#define GIC_VERSION 2
> +#define GICD_BASE 0x1410000
> +#define GICC_BASE 0x142f000
> +#define GICH_BASE 0x1440000
> +#define GICV_BASE 0x1460000
> +
> +// UART
> +#define UART0_BASE 0x21C0500
> +#define UART0_IT 86
These (GIC and UART) definitions duplicate things already described in
Silicon/NXP/LS1046A/LS1046A.dsc.inc
/
Leif
> +#define UART0_LENGTH 0x100
> +#define SPCR_FLOW_CONTROL_NONE 0
> +
> +// Timer
> +#define TIMER_BLOCK_COUNT 1
> +#define TIMER_FRAME_COUNT 4
> +#define TIMER_WATCHDOG_COUNT 1
> +#define TIMER_BASE_ADDRESS 0x23E0000 // a.k.a CNTControlBase
> +#define TIMER_READ_BASE_ADDRESS 0x23F0000 // a.k.a CNTReadBase
> +#define TIMER_SEC_IT 29
> +#define TIMER_NON_SEC_IT 30
> +#define TIMER_VIRT_IT 27
> +#define TIMER_HYP_IT 26
> +#define TIMER_FRAME0_IT 78
> +#define TIMER_FRAME1_IT 79
> +#define TIMER_FRAME2_IT 92
> +
> +// Mcfg
> +#define LS1046A_PCI_SEG0_CONFIG_BASE 0x4000000000
> +#define LS1046A_PCI_SEG0 0x0
> +#define LS1046A_PCI_SEG_BUSNUM_MIN 0x0
> +#define LS1046A_PCI_SEG_BUSNUM_MAX 0xff
> +#define LS1046A_PCI_SEG1_CONFIG_BASE 0x4800000000
> +#define LS1046A_PCI_SEG2_CONFIG_BASE 0x5000000000
> +#define LS1046A_PCI_SEG1 0x1
> +#define LS1046A_PCI_SEG2 0x2
> +
> +// Platform specific info needed by Configuration Manager
> +
> +#define CFG_MGR_TABLE_ID SIGNATURE_64 ('L','S','1','0','4','6',' ',' ')
> +
> +// Specify the OEM defined tables
> +#define OEM_ACPI_TABLES 0
> +
> +#define PLAT_PCI_SEG0 LS1046A_PCI_SEG0
> +#define PLAT_PCI_SEG1_CONFIG_BASE LS1046A_PCI_SEG1_CONFIG_BASE
> +#define PLAT_PCI_SEG1 LS1046A_PCI_SEG1
> +#define PLAT_PCI_SEG_BUSNUM_MIN LS1046A_PCI_SEG_BUSNUM_MIN
> +#define PLAT_PCI_SEG_BUSNUM_MAX LS1046A_PCI_SEG_BUSNUM_MAX
> +#define PLAT_PCI_SEG2_CONFIG_BASE LS1046A_PCI_SEG2_CONFIG_BASE
> +#define PLAT_PCI_SEG2 LS1046A_PCI_SEG2
> +
> +#define PLAT_GIC_VERSION GIC_VERSION
> +#define PLAT_GICD_BASE GICD_BASE
> +#define PLAT_GICI_BASE GICI_BASE
> +#define PLAT_GICR_BASE GICR_BASE
> +#define PLAT_GICR_LEN GICR_LEN
> +#define PLAT_GICC_BASE GICC_BASE
> +#define PLAT_GICH_BASE GICH_BASE
> +#define PLAT_GICV_BASE GICV_BASE
> +
> +#define PLAT_CPU_COUNT 4
> +#define PLAT_GTBLOCK_COUNT 0
> +#define PLAT_GTFRAME_COUNT 0
> +#define PLAT_PCI_CONFG_COUNT 2
> +
> +#define PLAT_WATCHDOG_COUNT 0
> +#define PLAT_GIC_REDISTRIBUTOR_COUNT 0
> +#define PLAT_GIC_ITS_COUNT 0
> +
> +/* GIC CPU Interface information
> + GIC_ENTRY (CPUInterfaceNumber, Mpidr, PmuIrq, VGicIrq, EnergyEfficiency)
> + */
> +#define PLAT_GIC_CPU_INTERFACE { \
> + GICC_ENTRY (0, GET_MPID (0, 0), 138, 0x19, 0), \
> + GICC_ENTRY (1, GET_MPID (0, 1), 139, 0x19, 0), \
> + GICC_ENTRY (2, GET_MPID (0, 2), 127, 0x19, 0), \
> + GICC_ENTRY (3, GET_MPID (0, 3), 129, 0x19, 0), \
> +}
> +
> +#define PLAT_WATCHDOG_INFO \
> + { \
> + } \
> +
> +#define PLAT_TIMER_BLOCK_INFO \
> + { \
> + } \
> +
> +#define PLAT_TIMER_FRAME_INFO \
> + { \
> + } \
> +
> +#define PLAT_GIC_DISTRIBUTOR_INFO \
> + { \
> + PLAT_GICD_BASE, /* UINT64 PhysicalBaseAddress */ \
> + 0, /* UINT32 SystemVectorBase */ \
> + PLAT_GIC_VERSION /* UINT8 GicVersion */ \
> + } \
> +
> +#define PLAT_GIC_REDISTRIBUTOR_INFO \
> + { \
> + } \
> +
> +#define PLAT_GIC_ITS_INFO \
> + { \
> + } \
> +
> +#define PLAT_MCFG_INFO \
> + { \
> + { \
> + PLAT_PCI_SEG1_CONFIG_BASE, \
> + PLAT_PCI_SEG1, \
> + PLAT_PCI_SEG_BUSNUM_MIN, \
> + PLAT_PCI_SEG_BUSNUM_MAX, \
> + }, \
> + { \
> + PLAT_PCI_SEG2_CONFIG_BASE, \
> + PLAT_PCI_SEG2, \
> + PLAT_PCI_SEG_BUSNUM_MIN, \
> + PLAT_PCI_SEG_BUSNUM_MAX, \
> + } \
> + } \
> +
> +#define PLAT_SPCR_INFO \
> + { \
> + UART0_BASE, \
> + UART0_IT, \
> + 115200, \
> + 0, \
> + EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERFACE_TYPE_16550 \
> + } \
> +
> +#endif // LS1046AFRWY_PLATFORM_H
> diff --git a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
> index 67cf15cbe4..20111e6037 100755
> --- a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
> +++ b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.dsc
> @@ -3,6 +3,7 @@
> # LS1046AFRWY Board package.
> #
> # Copyright 2019-2020 NXP
> +# Copyright 2021 Puresoftware Ltd
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -22,10 +23,18 @@
> OUTPUT_DIRECTORY = Build/LS1046aFrwyPkg
> FLASH_DEFINITION = Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
>
> + # This flag controls the dynamic acpi generation
> + #
> + DEFINE DYNAMIC_ACPI_ENABLE = TRUE
> +
> !include Silicon/NXP/NxpQoriqLs.dsc.inc
> !include MdePkg/MdeLibs.dsc.inc
> !include Silicon/NXP/LS1046A/LS1046A.dsc.inc
>
> +!if $(DYNAMIC_ACPI_ENABLE) == TRUE
> + !include DynamicTablesPkg/DynamicTables.dsc.inc
> +!endif
> +
> [LibraryClasses.common]
> ArmPlatformLib|Platform/NXP/LS1046aFrwyPkg/Library/ArmPlatformLib/ArmPlatformLib.inf
> RealTimeClockLib|EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.inf
> @@ -46,4 +55,23 @@
>
> Silicon/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
>
> + #
> + # Dynamic Table Factory
> + !if $(DYNAMIC_ACPI_ENABLE) == TRUE
> + DynamicTablesPkg/Drivers/DynamicTableFactoryDxe/DynamicTableFactoryDxe.inf {
> + <LibraryClasses>
> + NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiFadtLibArm/AcpiFadtLibArm.inf
> + NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiGtdtLibArm/AcpiGtdtLibArm.inf
> + NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMadtLibArm/AcpiMadtLibArm.inf
> + NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiMcfgLibArm/AcpiMcfgLibArm.inf
> + NULL|DynamicTablesPkg/Library/Acpi/Arm/AcpiSpcrLibArm/AcpiSpcrLibArm.inf
> + }
> + !endif
> +
> + #
> + # Acpi Support
> + #
> + MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
> + MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> +
> ##
> diff --git a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
> index 34c4e5a025..f3cac033bc 100755
> --- a/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
> +++ b/Platform/NXP/LS1046aFrwyPkg/LS1046aFrwyPkg.fdf
> @@ -3,6 +3,7 @@
> # FLASH layout file for LS1046a board.
> #
> # Copyright 2019-2020 NXP
> +# Copyright 2021 Puresoftware Ltd
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -99,6 +100,18 @@ READ_LOCK_STATUS = TRUE
> INF MdeModulePkg/Universal/Metronome/Metronome.inf
> INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>
> +
> + #
> + # Acpi Support
> + #
> + INF MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf
> + INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
> +
> + !if $(DYNAMIC_ACPI_ENABLE) == TRUE
> + INF Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> + !include DynamicTablesPkg/DynamicTables.fdf.inc
> + !endif
> +
> #
> # Multiple Console IO support
> #
> diff --git a/Silicon/NXP/LS1046A/LS1046A.dsc.inc b/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> index 7004533ed5..98f999edfd 100644
> --- a/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> +++ b/Silicon/NXP/LS1046A/LS1046A.dsc.inc
> @@ -2,6 +2,7 @@
> # LS1046A Soc package.
> #
> # Copyright 2017-2020 NXP
> +# Copyright 2021 Puresoftware Ltd
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -48,4 +49,13 @@
> [Components.common]
> MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>
> +#
> +# Configuration Manager
> +!if $(DYNAMIC_ACPI_ENABLE) == TRUE
> + Platform/NXP/ConfigurationManagerPkg/ConfigurationManagerDxe/ConfigurationManagerDxe.inf {
> + <BuildOptions>
> + *_*_*_PLATFORM_FLAGS = -I$(WORKSPACE)/Platform/NXP/LS1046aFrwyPkg/Include
> + }
> +!endif
> +
> ##
> --
> 2.25.1
>
next prev parent reply other threads:[~2021-06-14 21:28 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-11 15:51 [PATCH V1 0/4] Enable Dynamic ACPI for LS1046AFRWY Vikas Singh
2021-06-11 15:51 ` [PATCH V1 1/4] Platform/NXP: Add generic log in CM to print SoC version Vikas Singh
2021-06-14 20:58 ` Leif Lindholm
2021-06-18 14:16 ` Vikas Singh
2021-06-11 15:51 ` [PATCH V1 2/4] Silicon/NXP: Add support of SVR handling for LS1046FRWY Vikas Singh
2021-06-14 20:59 ` Leif Lindholm
2021-06-18 14:27 ` Vikas Singh
2021-06-11 15:51 ` [PATCH V1 3/4] Platform/NXP/LS1046aFrwyPkg: Extend Dynamic ACPI support Vikas Singh
2021-06-14 21:28 ` Leif Lindholm [this message]
2021-06-18 14:54 ` Vikas Singh
2021-06-11 15:52 ` [PATCH V1 4/4] Platform/NXP/LS1046aFrwyPkg: Add OEM specific DSDT generator Vikas Singh
2021-06-14 21:37 ` Leif Lindholm
2021-06-18 14:56 ` Vikas Singh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210614212835.grfvqk6w2ejwlsib@leviathan \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox