From: "Sami Mujawar" <sami.mujawar@arm.com>
To: Rebecca Cran <rebecca@bsdio.com>,
devel@edk2.groups.io, Ard Biesheuvel <ardb+tianocore@kernel.org>,
Thomas Abraham <thomas.abraham@arm.com>, nd <nd@arm.com>
Subject: Re: [PATCH edk2-platforms 2/3] Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM addr
Date: Thu, 17 Mar 2022 09:54:17 +0000 [thread overview]
Message-ID: <27a3e61f-c708-5710-f4d7-7475d73b0e20@arm.com> (raw)
In-Reply-To: <20220305041955.20918-3-rebecca@bsdio.com>
Hi Rebecca,
I have one minor suggestion marked inline as [SAMI]. Otherwise these
changes look good to me.
With that changed,
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
On 05/03/2022 04:19 AM, Rebecca Cran wrote:
> Instead of using a custom Pcd for the ECAM address
> (gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress),
> use the Pcd from MdePkg.
>
> Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
> ---
> Platform/ARM/JunoPkg/ArmJuno.dec | 4 ++--
> Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf | 2 +-
> Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf | 2 +-
> Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf | 2 +-
> Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf | 2 +-
> Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h | 2 +-
> Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c | 2 +-
> Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c | 2 +-
> Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c | 4 ++--
> 9 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/Platform/ARM/JunoPkg/ArmJuno.dec b/Platform/ARM/JunoPkg/ArmJuno.dec
> index 37ea6857366f..b6437d6fe98c 100644
> --- a/Platform/ARM/JunoPkg/ArmJuno.dec
> +++ b/Platform/ARM/JunoPkg/ArmJuno.dec
> @@ -34,8 +34,8 @@ [PcdsFeatureFlag.common]
> [PcdsFixedAtBuild.common]
> gArmJunoTokenSpaceGuid.PcdPcieControlBaseAddress|0x7FF20000|UINT64|0x0000000B
> gArmJunoTokenSpaceGuid.PcdPcieRootPortBaseAddress|0x7FF30000|UINT64|0x0000000C
> - gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress|0x40000000|UINT64|0x00000011
> - gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize|0x10000000|UINT64|0x00000012
> + gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize|0x10000000|UINT64|0x00000011
> + gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceLimit|0x4FFFFFFF|UINT64|0x00000012
>
> gArmJunoTokenSpaceGuid.PcdSynopsysUsbOhciBaseAddress|0x7FFB0000|UINT32|0x00000004
> gArmJunoTokenSpaceGuid.PcdSynopsysUsbEhciBaseAddress|0x7FFC0000|UINT32|0x00000005
> diff --git a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> index 00be2c435bd6..7ca134d6674b 100644
> --- a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> +++ b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManagerDxe.inf
> @@ -46,7 +46,7 @@ [Protocols]
>
> [FixedPcd]
> # PCI Root complex specific PCDs
> - gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
> + gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
> gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
>
> ## PL011 Serial Debug UART
> diff --git a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
> index d016967c3c37..c35984c172e1 100644
> --- a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
> +++ b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.inf
> @@ -67,7 +67,7 @@ [FixedPcd]
> gArmJunoTokenSpaceGuid.PcdJunoFdtDevicePath
>
> # PCI Root complex specific PCDs
> - gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
> + gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
> gArmTokenSpaceGuid.PcdPciBusMin
> gArmTokenSpaceGuid.PcdPciBusMax
>
> diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> index 145663c2fa28..fb80f10a9409 100644
> --- a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> +++ b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> @@ -45,7 +45,7 @@ [FixedPcd]
> gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
>
> gArmJunoTokenSpaceGuid.PcdPcieControlBaseAddress
> - gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
> + gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
> gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
>
> # Framebuffer Memory
> diff --git a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf
> index f448803fda7d..784618ffa013 100644
> --- a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf
> +++ b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/JunoPciHostBridgeLib.inf
> @@ -62,7 +62,7 @@ [FixedPcd]
>
> gArmJunoTokenSpaceGuid.PcdPcieControlBaseAddress
> gArmJunoTokenSpaceGuid.PcdPcieRootPortBaseAddress
> - gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceBaseAddress
> + gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
> gArmJunoTokenSpaceGuid.PcdPciConfigurationSpaceSize
>
> [Protocols]
> diff --git a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h
> index 420bdda7534b..78889c1b1196 100644
> --- a/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h
> +++ b/Platform/ARM/JunoPkg/Library/JunoPciHostBridgeLib/XPressRich3.h
> @@ -13,7 +13,7 @@
> #include <Protocol/CpuIo2.h>
> #include <Library/PcdLib.h>
>
> -#define PCI_ECAM_BASE FixedPcdGet64 (PcdPciConfigurationSpaceBaseAddress)
> +#define PCI_ECAM_BASE FixedPcdGet64 (PcdPciExpressBaseAddress)
> #define PCI_ECAM_SIZE FixedPcdGet64 (PcdPciConfigurationSpaceSize)
> #define PCI_IO_BASE FixedPcdGet64 (PcdPciIoTranslation)
> #define PCI_IO_SIZE FixedPcdGet64 (PcdPciIoSize)
> diff --git a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> index 80a98a10d869..3420b6eba66a 100644
> --- a/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> +++ b/Platform/ARM/JunoPkg/ConfigurationManager/ConfigurationManagerDxe/ConfigurationManager.c
> @@ -230,7 +230,7 @@ EDKII_PLATFORM_REPOSITORY_INFO ArmJunoPlatformRepositoryInfo = {
> // PCI Configuration Space Info
> {
> // The physical base address for the PCI segment
> - FixedPcdGet64 (PcdPciConfigurationSpaceBaseAddress),
> + FixedPcdGet64 (gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress),
[SAMI] I think this should just be FixedPcdGet64
(PcdPciExpressBaseAddress).
> // The PCI segment group number
> 0,
> // The start bus number
> diff --git a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c
> index aaa493a9284b..870dfa5f7ef3 100644
> --- a/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c
> +++ b/Platform/ARM/JunoPkg/Drivers/ArmJunoDxe/AcpiTables.c
> @@ -30,7 +30,7 @@ MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ACCESS_TABLE mAcpiMcfgTable = {
> ),
> 0, // Reserved
> }, {
> - FixedPcdGet32 (PcdPciConfigurationSpaceBaseAddress),
> + FixedPcdGet32 (PcdPciExpressBaseAddress),
> 0, // PciSegmentGroupNumber
> FixedPcdGet32 (PcdPciBusMin),
> FixedPcdGet32 (PcdPciBusMax),
> diff --git a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> index 990a1664e496..8ac2e3273a4a 100644
> --- a/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> +++ b/Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> @@ -118,8 +118,8 @@ ArmPlatformGetVirtualMemoryMap (
> //
> // PCI Configuration Space
> //
> - VirtualMemoryTable[++Index].PhysicalBase = PcdGet64 (PcdPciConfigurationSpaceBaseAddress);
> - VirtualMemoryTable[Index].VirtualBase = PcdGet64 (PcdPciConfigurationSpaceBaseAddress);
> + VirtualMemoryTable[++Index].PhysicalBase = PcdGet64 (PcdPciExpressBaseAddress);
> + VirtualMemoryTable[Index].VirtualBase = PcdGet64 (PcdPciExpressBaseAddress);
> VirtualMemoryTable[Index].Length = PcdGet64 (PcdPciConfigurationSpaceSize);
> VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>
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.
next prev parent reply other threads:[~2022-03-17 9:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-05 4:19 [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area Rebecca Cran
2022-03-05 4:19 ` [PATCH edk2-platforms 1/3] Platform/ARM/JunoPkg: Convert AcpiSsdtRootPci.asl from tabs to spaces Rebecca Cran
2022-03-16 15:40 ` Sami Mujawar
2022-03-05 4:19 ` [PATCH edk2-platforms 2/3] Platform/ARM/JunoPkg: Use MdePkg PcdPciExpressBaseAddress for ECAM addr Rebecca Cran
2022-03-17 9:54 ` Sami Mujawar [this message]
2022-03-19 19:56 ` Rebecca Cran
2022-03-05 4:19 ` [PATCH edk2-platforms 3/3] Platform/ARM/JunoPkg: Reserve the ECAM area in ACPI with RES0 device Rebecca Cran
2022-03-17 9:55 ` Sami Mujawar
2022-03-19 19:56 ` Rebecca Cran
2022-03-15 2:44 ` [PATCH edk2-platforms 0/3] JunoPkg: Fix AcpiSsdtRootPci.asl to use spaces and reserve ECAM area Rebecca Cran
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=27a3e61f-c708-5710-f4d7-7475d73b0e20@arm.com \
--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