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

["Pete Batard" <pete@akeo.ie>]

Hi Philippe,

On 2019.11.18 11:13, Philippe Mathieu-Daudé wrote:
> 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).

I guess we might have been initially worried that some of the offsets 
might not be the same between Bcm2386 and Bcm2711 (which I agree is 
unlikely... but you never know what a SoC manufacturer might do).

I have however confirmed that the offsets are the same for the registers 
we use. I will therefore send a v2 with the change you suggest.

Regards,

/Pete

> 
>>     ## 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
>>
>