public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
To: Aditya Angadi <aditya.angadi@arm.com>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>,
	 Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>,
	Leif Lindholm <leif@nuviainc.com>
Subject: Re: [edk2-platforms][PATCH v3 1/9] Platform/ARM/SgiPkg: create individual
Date: Tue, 31 Mar 2020 12:33:46 +0200	[thread overview]
Message-ID: <CAKv+Gu9zXdhbLnwG_93gQuy39-yNfnQJY-LaapuM9n2vRA3+=g@mail.gmail.com> (raw)
In-Reply-To: <20200325105252.13905-2-aditya.angadi@arm.com>

Hello Aditya,

Could you please use a meaningful subject line for this patch?

On Wed, 25 Mar 2020 at 11:53, Aditya Angadi <aditya.angadi@arm.com> wrote:
>
> From: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
>
> In preparation for adding support for Reference Design (RD) platforms
> that have different base addresses for GIC distributor or redistributor,
> create individual platform description files for all SGI/RD platforms
> and move GIC related base addresses from the common SGI/RD platform
> description file to individual platform description files.
> The existing platform description is then included by individual
> platform description files.
>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Aditya Angadi <aditya.angadi@arm.com>
> ---
>  Platform/ARM/SgiPkg/Include/SgiPlatform.h                |  7 +---
>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf  |  3 +-
>  Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c |  8 ++---
>  Platform/ARM/SgiPkg/RdE1Edge.dsc                         | 37 ++++++++++++++++++++
>  Platform/ARM/SgiPkg/RdN1Edge.dsc                         | 37 ++++++++++++++++++++
>  Platform/ARM/SgiPkg/Sgi575.dsc                           | 37 ++++++++++++++++++++
>  Platform/ARM/SgiPkg/SgiPlatform.dec                      |  5 ++-
>  Platform/ARM/SgiPkg/SgiPlatform.dsc                      | 25 ++-----------
>  8 files changed, 124 insertions(+), 35 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> index e36a412155ff..d87fb2b5409f 100644
> --- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> +++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> @@ -1,6 +1,6 @@
>  /** @file
>  *
> -*  Copyright (c) 2018, ARM Limited. All rights reserved.
> +*  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -45,11 +45,6 @@
>  #define SGI_SUBSYS_GENERIC_WDOG_BASE              0x2A440000
>  #define SGI_SUBSYS_GENERIC_WDOG_SZ                SIZE_128KB
>
> -// Sub System Peripherals - GIC
> -#define SGI_SUBSYS_GENERIC_GIC_BASE               0x30000000
> -#define SGI_SUBSYS_GENERIC_GICR_BASE              0x300C0000
> -#define SGI_SUBSYS_GENERIC_GIC_SZ                 SIZE_1MB
> -
>  // Expansion AXI - Platform Peripherals - HDLCD1
>  #define SGI_EXP_PLAT_PERIPH_HDLCD1_BASE           0x7FF60000
>  #define SGI_EXP_PLAT_PERIPH_HDLCD1_SZ             SIZE_64KB
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> index 3db70e900d61..a918afef5fba 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
> @@ -1,5 +1,5 @@
>  #
> -#  Copyright (c) 2018, ARM Limited. All rights reserved.
> +#  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -42,6 +42,7 @@ [FixedPcd]
>
>    gArmSgiTokenSpaceGuid.PcdDramBlock2Base
>    gArmSgiTokenSpaceGuid.PcdDramBlock2Size
> +  gArmSgiTokenSpaceGuid.PcdGicSize
>
>    gArmTokenSpaceGuid.PcdSystemMemoryBase
>    gArmTokenSpaceGuid.PcdSystemMemorySize
> diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
> index 845aeaf4dd49..8d0ad4ec9c84 100644
> --- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
> +++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
> @@ -1,6 +1,6 @@
>  /** @file
>  *
> -*  Copyright (c) 2018, ARM Limited. All rights reserved.
> +*  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -93,9 +93,9 @@ ArmPlatformGetVirtualMemoryMap (
>    VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>
>    // Sub System Peripherals - GIC-600
> -  VirtualMemoryTable[++Index].PhysicalBase  = SGI_SUBSYS_GENERIC_GIC_BASE;
> -  VirtualMemoryTable[Index].VirtualBase     = SGI_SUBSYS_GENERIC_GIC_BASE;
> -  VirtualMemoryTable[Index].Length          = SGI_SUBSYS_GENERIC_GIC_SZ;
> +  VirtualMemoryTable[++Index].PhysicalBase  = FixedPcdGet64(PcdGicDistributorBase);
> +  VirtualMemoryTable[Index].VirtualBase     = FixedPcdGet64(PcdGicDistributorBase);
> +  VirtualMemoryTable[Index].Length          = FixedPcdGet64(PcdGicSize);

Please use a space before '('

>    VirtualMemoryTable[Index].Attributes      = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>
>    // Expansion AXI - Platform Peripherals - HDLCD1
> diff --git a/Platform/ARM/SgiPkg/RdE1Edge.dsc b/Platform/ARM/SgiPkg/RdE1Edge.dsc
> new file mode 100644
> index 000000000000..082cbb0157f7
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/RdE1Edge.dsc
> @@ -0,0 +1,37 @@
> +#
> +#  Copyright (c) 2020, ARM Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +
> +################################################################################
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +################################################################################
> +[Defines]
> +  PLATFORM_NAME                  = ArmSgi
> +  PLATFORM_GUID                  = c834de39-c5b0-458b-8ea3-882427179b8a
> +  PLATFORM_VERSION               = 0.1
> +  DSC_SPECIFICATION              = 0x0001001B
> +  OUTPUT_DIRECTORY               = Build/$(PLATFORM_NAME)
> +  SUPPORTED_ARCHITECTURES        = AARCH64|ARM
> +  BUILD_TARGETS                  = NOOPT|DEBUG|RELEASE
> +  SKUID_IDENTIFIER               = DEFAULT
> +  FLASH_DEFINITION               = Platform/ARM/SgiPkg/SgiPlatform.fdf

If you are going to split these into separate DSCs, please do this
first in a separate patch, and update the PLATFORM_NAME accordingly.

If there is no need to use separate FDFs than you can keep using a
shared one. If there is, please do the split in the same patch.

> +  BUILD_NUMBER                   = 1
> +
> +# include common definitions from SgiPlatform.dsc
> +!include Platform/ARM/SgiPkg/SgiPlatform.dsc
> +

Please rename that file to use a .dsc.inc extension to make it clear
this is an include file

> +################################################################################
> +#
> +# Pcd Section - list of all EDK II PCD Entries defined by this Platform
> +#
> +################################################################################
> +
> +[PcdsFixedAtBuild.common]
> +  # GIC Base Addresses
> +  gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000
> +  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000
> +  gArmSgiTokenSpaceGuid.PcdGicSize|0x100000
> diff --git a/Platform/ARM/SgiPkg/RdN1Edge.dsc b/Platform/ARM/SgiPkg/RdN1Edge.dsc
> new file mode 100644
> index 000000000000..6774990ad6f6
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/RdN1Edge.dsc
> @@ -0,0 +1,37 @@
> +#
> +#  Copyright (c) 2020, ARM Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +
> +################################################################################
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +################################################################################
> +[Defines]
> +  PLATFORM_NAME                  = ArmSgi
> +  PLATFORM_GUID                  = dbc75915-03df-4640-8f3d-3d3abf7c119b
> +  PLATFORM_VERSION               = 0.1
> +  DSC_SPECIFICATION              = 0x0001001B
> +  OUTPUT_DIRECTORY               = Build/$(PLATFORM_NAME)
> +  SUPPORTED_ARCHITECTURES        = AARCH64|ARM
> +  BUILD_TARGETS                  = NOOPT|DEBUG|RELEASE
> +  SKUID_IDENTIFIER               = DEFAULT
> +  FLASH_DEFINITION               = Platform/ARM/SgiPkg/SgiPlatform.fdf
> +  BUILD_NUMBER                   = 1
> +
> +# include common definitions from SgiPlatform.dsc
> +!include Platform/ARM/SgiPkg/SgiPlatform.dsc
> +
> +################################################################################
> +#
> +# Pcd Section - list of all EDK II PCD Entries defined by this Platform
> +#
> +################################################################################
> +
> +[PcdsFixedAtBuild.common]
> +  # GIC Base Addresses
> +  gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000
> +  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000
> +  gArmSgiTokenSpaceGuid.PcdGicSize|0x100000
> diff --git a/Platform/ARM/SgiPkg/Sgi575.dsc b/Platform/ARM/SgiPkg/Sgi575.dsc
> new file mode 100644
> index 000000000000..3c1904c2da91
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Sgi575.dsc
> @@ -0,0 +1,37 @@
> +#
> +#  Copyright (c) 2020, ARM Limited. All rights reserved.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +
> +################################################################################
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +################################################################################
> +[Defines]
> +  PLATFORM_NAME                  = ArmSgi
> +  PLATFORM_GUID                  = 3a6b2eae-0275-4b6e-a5d1-bd2ba1ce1fae
> +  PLATFORM_VERSION               = 0.1
> +  DSC_SPECIFICATION              = 0x0001001B
> +  OUTPUT_DIRECTORY               = Build/$(PLATFORM_NAME)
> +  SUPPORTED_ARCHITECTURES        = AARCH64|ARM
> +  BUILD_TARGETS                  = NOOPT|DEBUG|RELEASE
> +  SKUID_IDENTIFIER               = DEFAULT
> +  FLASH_DEFINITION               = Platform/ARM/SgiPkg/SgiPlatform.fdf
> +  BUILD_NUMBER                   = 1
> +
> +# include common definitions from SgiPlatform.dsc
> +!include Platform/ARM/SgiPkg/SgiPlatform.dsc
> +
> +################################################################################
> +#
> +# Pcd Section - list of all EDK II PCD Entries defined by this Platform
> +#
> +################################################################################
> +
> +[PcdsFixedAtBuild.common]
> +  # GIC Base Addresses
> +  gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000
> +  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000
> +  gArmSgiTokenSpaceGuid.PcdGicSize|0x100000
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
> index 9d70ec677776..4ac3dec91e3d 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dec
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
> @@ -1,5 +1,5 @@
>  #
> -#  Copyright (c) 2018, ARM Limited. All rights reserved.
> +#  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -49,5 +49,8 @@ [PcdsFixedAtBuild]
>    gArmSgiTokenSpaceGuid.PcdVirtioNetSize|0x00000000|UINT32|0x00000008
>    gArmSgiTokenSpaceGuid.PcdVirtioNetInterrupt|0x00000000|UINT32|0x00000009
>
> +  # GIC
> +  gArmSgiTokenSpaceGuid.PcdGicSize|0|UINT64|0x0000000A

Wouldn't a UINT32 be sufficient to describe the size of a MMIO block?

> +
>  [Ppis]
>    gNtFwConfigDtInfoPpiGuid     = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> index 5226c5751e98..4e1fcefb1442 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> @@ -1,26 +1,9 @@
>  #
> -#  Copyright (c) 2018, ARM Limited. All rights reserved.
> +#  Copyright (c) 2018-2020, ARM Limited. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
>
> -################################################################################
> -#
> -# Defines Section - statements that will be processed to create a Makefile.
> -#
> -################################################################################
> -[Defines]
> -  PLATFORM_NAME                  = ArmSgi
> -  PLATFORM_GUID                  = 3a6b2eae-0275-4b6e-a5d1-bd2ba1ce1fae
> -  PLATFORM_VERSION               = 0.1
> -  DSC_SPECIFICATION              = 0x0001001B
> -  OUTPUT_DIRECTORY               = Build/$(PLATFORM_NAME)
> -  SUPPORTED_ARCHITECTURES        = AARCH64|ARM
> -  BUILD_TARGETS                  = NOOPT|DEBUG|RELEASE
> -  SKUID_IDENTIFIER               = DEFAULT
> -  FLASH_DEFINITION               = Platform/ARM/SgiPkg/SgiPlatform.fdf
> -  BUILD_NUMBER                   = 1
> -
>  !include Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
>
>  [BuildOptions]
> @@ -93,7 +76,7 @@ [LibraryClasses.common.UEFI_DRIVER, LibraryClasses.common.UEFI_APPLICATION, Libr
>
>  ################################################################################
>  #
> -# Pcd Section - list of all EDK II PCD Entries defined by this Platform
> +# Pcd Section - list of all EDK II PCD Entries common to all SGI/RD platforms
>  #
>  ################################################################################
>
> @@ -126,10 +109,6 @@ [PcdsFixedAtBuild.common]
>    gArmTokenSpaceGuid.PcdSystemMemoryBase|0x80000000
>    gArmTokenSpaceGuid.PcdSystemMemorySize|0x7F000000
>
> -  # GIC Base Addresses
> -  gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000
> -  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000
> -
>    #
>    # PCIe
>    #
> --
> 2.17.1
>

  reply	other threads:[~2020-03-31 10:34 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25 10:52 [edk2-platforms][PATCH v3 0/8] Add platform support for RD-Daniel Aditya Angadi
2020-03-25 10:52 ` [edk2-platforms][PATCH v3 1/9] Platform/ARM/SgiPkg: create individual Aditya Angadi
2020-03-31 10:33   ` Ard Biesheuvel [this message]
2020-03-31 10:42     ` Ard Biesheuvel
2020-04-13 12:31       ` [edk2-devel] " Aditya Angadi
2020-03-25 10:52 ` [edk2-platforms][PATCH v3 2/9] Platform/ARM/SgiPkg: move the GIC Aditya Angadi
2020-03-31 10:35   ` Ard Biesheuvel
2020-03-25 10:52 ` [edk2-platforms][PATCH v3 3/9] Platform/ARM/SgiPkg: move common Aditya Angadi
2020-03-31 10:36   ` Ard Biesheuvel
2020-03-25 10:52 ` [edk2-platforms][PATCH v3 4/9] Platform/ARM/SgiPkg: remove Aditya Angadi
2020-03-31 10:37   ` Ard Biesheuvel
2020-03-25 10:52 ` [edk2-platforms][PATCH v3 5/9] Platform/ARM/SgiPkg: add ACPI tables Aditya Angadi
2020-03-31 10:40   ` Ard Biesheuvel
2020-03-25 10:52 ` [edk2-platforms][PATCH v3 6/9] Platform/ARM/Sgi: add initial support Aditya Angadi
2020-03-31 10:39   ` Ard Biesheuvel
2020-03-25 10:52 ` [edk2-platforms][PATCH v3 7/9] Platform/ARM/SgiPkg: add ACPI tables Aditya Angadi
2020-03-31 10:45   ` Ard Biesheuvel
2020-03-25 10:52 ` [edk2-platforms][PATCH v3 8/9] Platform/ARM/Sgi: add initial support Aditya Angadi
2020-03-31 10:46   ` Ard Biesheuvel
2020-03-25 10:52 ` [edk2-platforms][PATCH v3 9/9] Maintainers.txt: Update Arm platform Aditya Angadi
2020-03-31 10:48   ` Ard Biesheuvel
2020-03-29 16:41 ` [edk2-devel] [edk2-platforms][PATCH v3 0/8] Add platform support for RD-Daniel Thomas Abraham

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='CAKv+Gu9zXdhbLnwG_93gQuy39-yNfnQJY-LaapuM9n2vRA3+=g@mail.gmail.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