public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: samer.el-haj-mahmoud@arm.com
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"leif.lindholm@linaro.org" <leif.lindholm@linaro.org>,
	Pete Batard <pete@akeo.ie>
Cc: "ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>,
	"philmd@redhat.com" <philmd@redhat.com>,
	Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH 2/8] Platform/RPi: Replace Bcm283x SoC base register address with a PCD
Date: Mon, 18 Nov 2019 17:19:32 +0000	[thread overview]
Message-ID: <VE1PR08MB483046C2B4BB3F635506414D904D0@VE1PR08MB4830.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <20191118164853.GQ7323@bivouac.eciton.net>

> I don't actually have a strong opinion here, but would appreciate an explicit statement that the code is being submitted
> as copyright ARM with author @elhajmahmoud.com and not @arm.com?

Leif, I use @elhajmahmoud.com for Github. But the code is Arm copyrighted (since I am doing this for work). Please let me know if you have any concerns .

Thanks for your review!

--Samer

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Leif Lindholm via Groups.Io
Sent: Monday, November 18, 2019 10:49 AM
To: Pete Batard <pete@akeo.ie>
Cc: devel@edk2.groups.io; ard.biesheuvel@linaro.org; philmd@redhat.com; Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH 2/8] Platform/RPi: Replace Bcm283x SoC base register address with a PCD

On Thu, Nov 14, 2019 at 04:07:34PM +0000, Pete Batard wrote:
> From: Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>
>
> Define BCM2836_SOC_REGISTERS from PcdBcm283xRegistersAddress. This is
> needed in preparation for adding Raspberry Pi 4 support, since the two
> Pi's have a different base addresses for the Bcm283x specific registers.

Minor style comments below, would be nice if you could fold into any
v2 coming based on Phil's comments. (Although from my reading the discussion, most of these intances may in fact fall out in a v2.)

Also adding Samer to cc on this reply so he sees the feedback.

> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
>  Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.inf | 2 ++
>  Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf               | 2 ++
>  Platform/RaspberryPi/Drivers/DwUsbHostDxe/DwUsbHostDxe.inf         | 4 ++++
>  Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf     | 5 ++++-
>  Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.inf               | 2 ++
>  Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf           | 3 ++-
>  Platform/RaspberryPi/RPi3/RPi3.dsc                                 | 6 +++++-
>  Silicon/Broadcom/Bcm283x/Bcm283x.dec                               | 7 +++++++
>  Silicon/Broadcom/Bcm283x/Drivers/InterruptDxe/InterruptDxe.inf     | 4 +++-
>  Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.inf                 | 5 +++++
>  Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h        | 3 ++-
>  Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf               | 4 ++++
>  12 files changed, 42 insertions(+), 5 deletions(-)
>
> diff --git
> a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.inf
> b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.inf
> index 3f0d7b6b9e9d..034c8c449f00 100644
> ---
> a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.i
> +++ nf
> @@ -1,5 +1,6 @@
>  #/** @file
>  #
> +#  Copyright (c) 2019, ARM Limited. All rights reserved.

I don't actually have a strong opinion here, but would appreciate an explicit statement that the code is being submitted as copyright ARM with author @elhajmahmoud.com and not @arm.com?

>  #  Copyright (c) 2017, Andrei Warkentin <andrey.warkentin@gmail.com>
> #  Copyright (c) Microsoft Corporation. All rights reserved.
>  #
> @@ -42,6 +43,7 @@ [Protocols]
>
>  [Pcd]
>    gRaspberryPiTokenSpaceGuid.PcdSdIsArasan
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress

When inserting new items like this, please do so alphabetically.

>
>  [Depex]
>    gRaspberryPiFirmwareProtocolGuid AND
> gRaspberryPiConfigAppliedProtocolGuid
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> index 28fc2682b585..4f4fdef4e003 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf
> @@ -1,5 +1,6 @@
>  #/** @file
>  #
> +#  Copyright (c) 2019, ARM Limited. All rights reserved.
>  #  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> #  #  SPDX-License-Identifier: BSD-2-Clause-Patent @@ -66,6 +67,7 @@
> [Pcd]
>    gRaspberryPiTokenSpaceGuid.PcdDebugShowUEFIExit
>    gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes
>    gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress

Also here.

>
>  [FeaturePcd]
>
> diff --git
> a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DwUsbHostDxe.inf
> b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DwUsbHostDxe.inf
> index e880c2fb0261..8817f20622d6 100644
> --- a/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DwUsbHostDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/DwUsbHostDxe/DwUsbHostDxe.inf
> @@ -1,5 +1,6 @@
>  #/** @file
>  #
> +#  Copyright (c) 2019, ARM Limited. All rights reserved.
>  #  Copyright (c) 2017-2018, Andrei Warkentin
> <andrey.warkentin@gmail.com>  #  Copyright (c) 2015-2016, Linaro Limited. All rights reserved.
>  #
> @@ -51,5 +52,8 @@ [Protocols]
>    gEfiUsb2HcProtocolGuid
>    gRaspberryPiFirmwareProtocolGuid
>
> +[FixedPcd]
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
> +
>  [Depex]
>    gRaspberryPiFirmwareProtocolGuid
> diff --git
> a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
> b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
> index 87bca98fec28..a3fc0fa49a3c 100644
> --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
> @@ -1,5 +1,5 @@
>  #/** @file
> -#
> +#  Copyright (c) 2019, ARM Limited. All rights reserved.
>  #  Copyright (c) 2017-2018, Andrei Warkentin
> <andrey.warkentin@gmail.com>  #  Copyright (c) 2016, Linaro, Ltd. All rights reserved.
>  #
> @@ -40,5 +40,8 @@ [LibraryClasses]
>  [Protocols]
>    gRaspberryPiFirmwareProtocolGuid    ## PRODUCES
>
> +[FixedPcd]
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
> +
>  [Depex]
>    TRUE
> diff --git a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.inf
> b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.inf
> index 7386ff251864..b99f197bb007 100644
> --- a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.inf
> @@ -1,5 +1,6 @@
>  #/** @file
>  #
> +#  Copyright (c) 2019, ARM Limited. All rights reserved.
>  #  Copyright (c) 2017, Andrei Warkentin <andrey.warkentin@gmail.com>
> #  Copyright (c) Microsoft Corporation. All rights reserved.
>  #
> @@ -44,6 +45,7 @@ [Protocols]
>
>  [Pcd]
>    gRaspberryPiTokenSpaceGuid.PcdSdIsArasan
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress

Also here.

>
>  [Depex]
>    gRaspberryPiFirmwareProtocolGuid AND
> gRaspberryPiConfigAppliedProtocolGuid
> diff --git a/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
> b/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
> index ed986034b957..85462febdd8d 100644
> --- a/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
> +++ b/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
> @@ -2,7 +2,7 @@
>  #
>  #  Copyright (c) 2017-2018, Andrei Warkentin
> <andrey.warkentin@gmail.com>  #  Copyright (c) 2014-2016, Linaro Limited. All rights reserved.
> -#  Copyright (c) 2011-2014, ARM Limited. All rights reserved.
> +#  Copyright (c) 2011-2019, ARM Limited. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent  # @@ -54,6 +54,7 @@
> [FixedPcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress

Also here.

>
>  [Ppis]
>    gArmMpCoreInfoPpiGuid
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index a0365c5cf606..4e5a9f0b05e6 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -1,6 +1,6 @@
>  # @file
>  #
> -#  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> +#  Copyright (c) 2011 - 2019, ARM Limited. All rights reserved.
>  #  Copyright (c) 2014, Linaro Limited. All rights reserved.
>  #  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
>  #  Copyright (c) 2017 - 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> @@ -372,6 +372,10 @@ [PcdsFixedAtBuild.common]
>    gArmTokenSpaceGuid.PcdSystemMemoryBase|0x00400000
>    gArmTokenSpaceGuid.PcdSystemMemorySize|0x3FC00000
>
> +  #
> +  # Device specific addresses
> +  #
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress|0x3f000000
>    ## NS16550 compatible UART
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x3f215040
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
> diff --git a/Silicon/Broadcom/Bcm283x/Bcm283x.dec b/Silicon/Broadcom/Bcm283x/Bcm283x.dec
> index ec62ff27fbb3..5b839b00d286 100644
> --- a/Silicon/Broadcom/Bcm283x/Bcm283x.dec
> +++ b/Silicon/Broadcom/Bcm283x/Bcm283x.dec
> @@ -1,5 +1,6 @@
>  ## @file
>  #
> +#  Copyright (c) 2019, ARM Limited. All rights reserved.
>  #  Copyright (c) 2019, Pete Batard <pete@akeo.ie>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -14,3 +15,9 @@ [Defines]
>
>  [Includes]
>    Include
> +
> +[Guids]
> +  gBcm283xTokenSpaceGuid = {0x82f36a92, 0xfb7e, 0x43a1, {0xb9, 0x9e, 0x49, 0x13, 0x3f, 0xc7, 0xa4, 0x2e}}
> +
> +[PcdsFixedAtBuild.common]
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress|0x0|UINT32|0x00000001
> diff --git a/Silicon/Broadcom/Bcm283x/Drivers/InterruptDxe/InterruptDxe.inf b/Silicon/Broadcom/Bcm283x/Drivers/InterruptDxe/InterruptDxe.inf
> index cdce11a51e14..6c58df5c3285 100644
> --- a/Silicon/Broadcom/Bcm283x/Drivers/InterruptDxe/InterruptDxe.inf
> +++ b/Silicon/Broadcom/Bcm283x/Drivers/InterruptDxe/InterruptDxe.inf
> @@ -1,5 +1,5 @@
>  #/** @file
> -#
> +#  Copyright (c) 2019, ARM Limited. All rights reserved.
>  #  Copyright (c) 2017, Andrei Warkentin <andrey.warkentin@gmail.com>
>  #  Copyright (c) 2016 Linaro, Ltd. All rights reserved.
>  #
> @@ -30,6 +30,7 @@ [LibraryClasses]
>    UefiBootServicesTableLib
>    UefiLib
>    UefiDriverEntryPoint
> +  PcdLib

Also here.

>
>  [Protocols]
>    gHardwareInterruptProtocolGuid  ## PRODUCES
> @@ -37,6 +38,7 @@ [Protocols]
>
>  [FixedPcd]
>    gEmbeddedTokenSpaceGuid.PcdInterruptBaseAddress
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress

Also here.

>
>  [Depex]
>    TRUE
> diff --git a/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.inf b/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.inf
> index cb1695bd2dfc..4481d71aaff0 100644
> --- a/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.inf
> +++ b/Silicon/Broadcom/Bcm283x/Drivers/RngDxe/RngDxe.inf
> @@ -1,5 +1,6 @@
>  #/** @file
>  #
> +#  Copyright (c) 2019, ARM Limited. All rights reserved.
>  #  Copyright (c) 2019 Linaro, Ltd. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -28,6 +29,7 @@ [LibraryClasses]
>    IoLib
>    UefiBootServicesTableLib
>    UefiDriverEntryPoint
> +  PcdLib

Also here.

/
    Leif

>
>  [Protocols]
>    gEfiRngProtocolGuid              ## PRODUCES
> @@ -35,5 +37,8 @@ [Protocols]
>  [Guids]
>    gEfiRngAlgorithmRaw
>
> +[FixedPcd]
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
> +
>  [Depex]
>    TRUE
> diff --git a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
> index 4007301228be..8bd68c234bfd 100644
> --- a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
> +++ b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
> @@ -1,5 +1,6 @@
>  /** @file
>   *
> + *  Copyright (c) 2019, ARM Limited. All rights reserved.
>   *  Copyright (c) 2017, Andrei Warkentin <andrey.warkentin@gmail.com>
>   *  Copyright (c) 2016, Linaro Limited. All rights reserved.
>   *
> @@ -13,7 +14,7 @@
>  /*
>   * Both "core" and SoC perpherals (1M each).
>   */
> -#define BCM2836_SOC_REGISTERS                               0x3f000000
> +#define BCM2836_SOC_REGISTERS                               (FixedPcdGet64 (PcdBcm283xRegistersAddress))
>  #define BCM2836_SOC_REGISTER_LENGTH                         0x02000000
>
>  /*
> diff --git a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf
> index 50da4eb771f3..ff1b5af6db6e 100644
> --- a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf
> +++ b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf
> @@ -2,6 +2,7 @@
>  #
>  #  Manipulate GPIOs.
>  #
> +#  Copyright (c) 2019, ARM Limited. All rights reserved.
>  #  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -30,4 +31,7 @@ [LibraryClasses]
>    DebugLib
>    IoLib
>
> +[FixedPcd]
> +  gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
> +
>  [Guids]
> --
> 2.21.0.windows.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.

  reply	other threads:[~2019-11-18 17:19 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-14 16:07 [edk2-platforms][PATCH 0/8] Platform/RPi: Early Raspberry Pi 4 groundwork Pete Batard
2019-11-14 16:07 ` [edk2-platforms][PATCH 1/8] Platform/RPi: Add model family detection Pete Batard
2019-11-14 16:36   ` [edk2-devel] " Michael Brown
2019-11-14 16:55     ` Pete Batard
2019-11-18 17:51   ` Leif Lindholm
2019-11-18 17:58     ` Pete Batard
2019-11-18 18:05       ` Leif Lindholm
2019-11-18 18:32         ` Pete Batard
2019-11-19 15:07           ` Ard Biesheuvel
2019-11-19 16:30             ` [edk2-devel] " Pete Batard
2019-11-20 10:27               ` Leif Lindholm
2019-11-20 21:50                 ` Pete Batard
2019-11-21  8:55                   ` Laszlo Ersek
2019-11-21  9:04                     ` Laszlo Ersek
2019-11-21 20:02                       ` Pete Batard
2019-11-14 16:07 ` [edk2-platforms][PATCH 2/8] Platform/RPi: Replace Bcm283x SoC base register address with a PCD Pete Batard
2019-11-18 16:48   ` Leif Lindholm
2019-11-18 17:19     ` samer.el-haj-mahmoud [this message]
2019-11-18 17:26       ` [edk2-devel] " Leif Lindholm
2019-11-14 16:07 ` [edk2-platforms][PATCH 3/8] Silicon/Broadcom: Add Bcm2711 header Pete Batard
2019-11-18 16:50   ` Leif Lindholm
2019-11-14 16:07 ` [edk2-platforms][PATCH 4/8] Platform/RPi: Read more variables from VideoCore during early init Pete Batard
2019-11-18 17:11   ` Leif Lindholm
2019-11-14 16:07 ` [edk2-platforms][PATCH 5/8] Platform/RPi: Clean up and improve early memory init Pete Batard
2019-11-18 17:20   ` Leif Lindholm
2019-11-18 17:34     ` Pete Batard
2019-11-18 17:38       ` Leif Lindholm
2019-11-18 17:40         ` Pete Batard
2019-11-14 16:07 ` [edk2-platforms][PATCH 6/8] Platform/RPi: Replace Mailbox and Watchdog addresses with PCDs Pete Batard
2019-11-18 11:13   ` Philippe Mathieu-Daudé
2019-11-18 13:32     ` Pete Batard
2019-11-14 16:07 ` [edk2-platforms][PATCH 7/8] Platform/RPi: Replace MMCHS1BASE define with a PCD Pete Batard
2019-11-14 16:07 ` [edk2-platforms][PATCH 8/8] Platform/RPi: Replace DW2_USB_BASE_ADDRESS " Pete Batard

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=VE1PR08MB483046C2B4BB3F635506414D904D0@VE1PR08MB4830.eurprd08.prod.outlook.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