Re: [edk2-platforms][PATCH 6/8] Platform/RPi: Replace Mailbox and Watchdog addresses with PCDs

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

Hi Pete, Samer,

On 11/14/19 5:07 PM, Pete Batard wrote:
> From: Samer El-Haj-Mahmoud <samer@elhajmahmoud.com>
> 
> Set BCM2836_WDOG_BASE_ADDRESS and BCM2836_MBOX_BASE_ADDRESS to
> using PCDs instead of hard-coded addresses. This is needed in
> preparation for adding Raspberry Pi 4 support.
> 
> Signed-off-by: Pete Batard <pete@akeo.ie>
> ---
>   Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf | 1 +
>   Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf       | 1 +
>   Platform/RaspberryPi/RPi3/RPi3.dsc                             | 3 +++
>   Silicon/Broadcom/Bcm283x/Bcm283x.dec                           | 2 ++
>   Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h    | 4 ++--
>   5 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
> index a3fc0fa49a3c..527cee8bacc1 100644
> --- a/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
> +++ b/Platform/RaspberryPi/Drivers/RpiFirmwareDxe/RpiFirmwareDxe.inf
> @@ -42,6 +42,7 @@ [Protocols]
>   
>   [FixedPcd]
>     gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
> +  gBcm283xTokenSpaceGuid.PcdMboxBaseAddress
>   
>   [Depex]
>     TRUE
> diff --git a/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf b/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
> index c0e2a75451c3..f627fca2e406 100644
> --- a/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
> +++ b/Platform/RaspberryPi/Library/PlatformLib/PlatformLib.inf
> @@ -58,6 +58,7 @@ [FixedPcd]
>     gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>     gBcm27xxTokenSpaceGuid.PcdBcm27xxRegistersAddress
>     gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
> +  gBcm283xTokenSpaceGuid.PcdMboxBaseAddress
>   
>   [Ppis]
>     gArmMpCoreInfoPpiGuid
> diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc
> index 4e5a9f0b05e6..88af37e6edaa 100644
> --- a/Platform/RaspberryPi/RPi3/RPi3.dsc
> +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc
> @@ -376,6 +376,9 @@ [PcdsFixedAtBuild.common]
>     # Device specific addresses
>     #
>     gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress|0x3f000000

The RegistersAddress 'base' changes in the 2711,

> +  gBcm283xTokenSpaceGuid.PcdMboxBaseAddress|0x3f00b880
> +  gBcm283xTokenSpaceGuid.PcdWdogBaseAddress|0x3f100000

but the BaseAddress 'offset' stay constant.

Why not use this approach instead?

#define BCM2836_WDOG_OFFSET          0x00100000
#define BCM2836_WDOG_BASE_ADDRESS \
     (FixedPcdGet64 (PcdBcm283xRegistersAddress) \
      + BCM2836_WDOG_OFFSET)

(and similarly with other devices).

>     ## NS16550 compatible UART
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x3f215040
>     gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
> diff --git a/Silicon/Broadcom/Bcm283x/Bcm283x.dec b/Silicon/Broadcom/Bcm283x/Bcm283x.dec
> index 5b839b00d286..fe1907cb8cf1 100644
> --- a/Silicon/Broadcom/Bcm283x/Bcm283x.dec
> +++ b/Silicon/Broadcom/Bcm283x/Bcm283x.dec
> @@ -21,3 +21,5 @@ [Guids]
>   
>   [PcdsFixedAtBuild.common]
>     gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress|0x0|UINT32|0x00000001
> +  gBcm283xTokenSpaceGuid.PcdMboxBaseAddress|0x0|UINT32|0x00000002
> +  gBcm283xTokenSpaceGuid.PcdWdogBaseAddress|0x0|UINT32|0x00000003
> diff --git a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
> index 8bd68c234bfd..6bc21f0ddcf8 100644
> --- a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
> +++ b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
> @@ -23,7 +23,7 @@
>   #define BCM2836_DMA_DEVICE_OFFSET                           0xc0000000
>   
>   /* watchdog constants */
> -#define BCM2836_WDOG_BASE_ADDRESS                           0x3f100000
> +#define BCM2836_WDOG_BASE_ADDRESS                           (FixedPcdGet64 (PcdWdogBaseAddress))
>   #define BCM2836_WDOG_PASSWORD                               0x5a000000
>   #define BCM2836_WDOG_RSTC_OFFSET                            0x0000001c
>   #define BCM2836_WDOG_WDOG_OFFSET                            0x00000024
> @@ -31,7 +31,7 @@
>   #define BCM2836_WDOG_RSTC_WRCFG_FULL_RESET                  0x00000020
>   
>   /* mailbox interface constants */
> -#define BCM2836_MBOX_BASE_ADDRESS                           0x3f00b880
> +#define BCM2836_MBOX_BASE_ADDRESS                           (FixedPcdGet64 (PcdMboxBaseAddress))
>   #define BCM2836_MBOX_READ_OFFSET                            0x00000000
>   #define BCM2836_MBOX_STATUS_OFFSET                          0x00000018
>   #define BCM2836_MBOX_CONFIG_OFFSET                          0x0000001c
>