From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) by mx.groups.io with SMTP id smtpd.web09.945.1623706119719145908 for ; Mon, 14 Jun 2021 14:28:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=KltAdHIG; spf=pass (domain: nuviainc.com, ip: 209.85.128.49, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f49.google.com with SMTP id u5-20020a7bc0450000b02901480e40338bso571136wmc.1 for ; Mon, 14 Jun 2021 14:28:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=nWJA93Rfsz40hkuPhMecEAIPOX40lQV6OZ9NadmY7Uw=; b=KltAdHIG81X29jWPhDE6BBfxznJzn3zebfNjujMWC4peeX6Caf2kQpw6/N3cA2AwNE Cpl+bPkNS+KzYt/HjmQCnuFhHZPBLoI+LR+BdNMVktFruqu3F9sNsNzgVtVpIEEsmEwu YQalbKRIpwrCC3+/luga5srFs1hu9Ck6w/mBu5buy+DchBaJoGoVhPhz2XfVGE0pGdJi 3yScbydJgrn2UjdMkJ43C14SFbwnZgPiqrKWsEiHh1zpaPZeXb7nA/Fr8/mnnL/0Ghy7 N4H5HGlquXJqwygDow0QhSLeoB6jGoVsjI0iEhmdrrDkjQRbHkubmAqpvTjgE2bZkRtJ XoOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=nWJA93Rfsz40hkuPhMecEAIPOX40lQV6OZ9NadmY7Uw=; b=tf5EaNTpN1vVHVd/JjIcrPix7IrO8Gn1mXaLhJoom3TDyiN3sxQ8FZnIlDTVn/A6ER 3RIi5+PgxaLDNaabeBcLHNdv3rUrs41bKDCGET4eHCQ7URfJcld+ZYa/CvijBxh/zGz6 e/Mm0LVABKjCSlgftTOijVvC1iY3BBcLnjtNFyEyWSB2gBvvPXk3emGZuG4R2292d0o3 S4jGGpCkK2jd75eW/AXmCpCEII85wHAlPnDb8wZ9PMnqTpwxixvlcetPZk3VUo61V1tM 0oeY4e/8f+marcfZKW0ATv8nZtR4W7W4+/2Uu6ZP2SoKoVWIwpOCzpENan79V7dzfNFa ZKvQ== X-Gm-Message-State: AOAM5330IY6dA9KrFCfFXJSk3kBkqqq0d8DT2E7kEdyUiRiUqKu4rqWe c7Hb1y9Ez5Tja2paSPFxrKPCFw== X-Google-Smtp-Source: ABdhPJyH7G4rFf6v2Xw8maM8aATHULa2PkNqQaLJ3cqxCGHEpsq/x74cWEjuIvRVuC53EXZsSrh+Kw== X-Received: by 2002:a1c:4b0d:: with SMTP id y13mr18462168wma.179.1623706118059; Mon, 14 Jun 2021 14:28:38 -0700 (PDT) Return-Path: Received: from leviathan (cpc1-cmbg19-2-0-cust915.5-4.cable.virginm.net. [82.27.183.148]) by smtp.gmail.com with ESMTPSA id l3sm14182980wmh.2.2021.06.14.14.28.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Jun 2021 14:28:37 -0700 (PDT) Date: Mon, 14 Jun 2021 22:28:35 +0100 From: "Leif Lindholm" To: Vikas Singh 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 Message-ID: <20210614212835.grfvqk6w2ejwlsib@leviathan> References: <20210611155200.15535-1-vikas.singh@puresoftware.com> <20210611155200.15535-4-vikas.singh@puresoftware.com> MIME-Version: 1.0 In-Reply-To: <20210611155200.15535-4-vikas.singh@puresoftware.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > --- > 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 { > + > + 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 { > + > + *_*_*_PLATFORM_FLAGS = -I$(WORKSPACE)/Platform/NXP/LS1046aFrwyPkg/Include > + } > +!endif > + > ## > -- > 2.25.1 >