Re: [edk2-devel] [edk2-platforms][PATCH 1/5] Silicon/Bcm283x: Clean up Bcm2836.h header

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

On 2019.11.27 14:47, Philippe Mathieu-Daudé wrote:
> On 11/27/19 1:37 PM, Pete Batard wrote:
>> Add missing RNG registers,
> 
> This is one change.
> 
>> prefer reusing shorter define's
>> instead of PCDs and clean up spacing.
> 
> This is another change.

All these changes belong to a "cleanup" category, which is what, 
globally, this patch is all about. Of course, I expect people to argue 
that adding trivial missing data does not count as cleaning up, which 
I'd disagree with, but then again, it's not my repo.

> Why suddenly go back to fixed PERIPHERAL block base address?

Earlier version was:

#define BCM2836_WDOG_BASE_ADDRESS    0x3f100000

which we replaced with:

#define BCM2836_WDOG_BASE_ADDRESS   (FixedPcdGet64 
(PcdBcm283xRegistersAddress) + BCM2836_WDOG_OFFSET)

But considering that we have "BCM2836_SOC_REGISTERS" that now equates 
                             "(FixedPcdGet64 
(PcdBcm283xRegistersAddress))" we can write the exact same thing as 
above, only shorter, which we do.

I personally find this more understandable for newcomers who'll be 
looking at the code and easier to maintain. But I'm not the maintainer, 
so I'll go with whatever suits you.

Regards,

/Pete

> 
>>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
>> ---
>>   Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h | 23 
>> ++++++++++++--------
>>   1 file changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git 
>> a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h 
>> b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
>> index 72c8e9dc4b14..744c7ac3b9f4 100644
>> --- a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
>> +++ b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836.h
>> @@ -24,8 +24,7 @@
>>   /* watchdog constants */
>>   #define BCM2836_WDOG_OFFSET                                 0x00100000
>> -#define BCM2836_WDOG_BASE_ADDRESS                           
>> (FixedPcdGet64 (PcdBcm283xRegistersAddress) \
>> -                                                            + 
>> BCM2836_WDOG_OFFSET)
>> +#define BCM2836_WDOG_BASE_ADDRESS                           
>> (BCM2836_SOC_REGISTERS + BCM2836_WDOG_OFFSET)
>>   #define BCM2836_WDOG_PASSWORD                               0x5a000000
>>   #define BCM2836_WDOG_RSTC_OFFSET                            0x0000001c
>>   #define BCM2836_WDOG_WDOG_OFFSET                            0x00000024
>> @@ -34,8 +33,7 @@
>>   /* mailbox interface constants */
>>   #define BCM2836_MBOX_OFFSET                                 0x0000b880
>> -#define BCM2836_MBOX_BASE_ADDRESS                           
>> (FixedPcdGet64 (PcdBcm283xRegistersAddress) \
>> -                                                            + 
>> BCM2836_MBOX_OFFSET)
>> +#define BCM2836_MBOX_BASE_ADDRESS                           
>> (BCM2836_SOC_REGISTERS + BCM2836_MBOX_OFFSET)
>>   #define BCM2836_MBOX_READ_OFFSET                            0x00000000
>>   #define BCM2836_MBOX_STATUS_OFFSET                          0x00000018
>>   #define BCM2836_MBOX_CONFIG_OFFSET                          0x0000001c
>> @@ -51,12 +49,19 @@
>>   #define BCM2836_INTC_TIMER_PENDING_OFFSET                   0x00000060
>>   /* random number generator */
>> -#define RNG_BASE_ADDRESS   (BCM2836_SOC_REGISTERS + 0x00104000)
>> +#define BCM2836_RNG_OFFSET                                  0x00104000
>> +#define RNG_BASE_ADDRESS                                    
>> (BCM2836_SOC_REGISTERS + BCM2836_RNG_OFFSET)
>> -#define RNG_CTRL           (RNG_BASE_ADDRESS + 0x0)
>> -#define RNG_STATUS         (RNG_BASE_ADDRESS + 0x4)
>> -#define RNG_DATA           (RNG_BASE_ADDRESS + 0x8)
>> +#define RNG_CTRL                                            
>> (RNG_BASE_ADDRESS + 0x0)
>> +#define RNG_STATUS                                          
>> (RNG_BASE_ADDRESS + 0x4)
>> +#define RNG_DATA                                            
>> (RNG_BASE_ADDRESS + 0x8)
>> +#define RNG_BIT_COUNT                                       
>> (RNG_BASE_ADDRESS + 0xc)
>> +#define RNG_BIT_COUNT_THRESHOLD                             
>> (RNG_BASE_ADDRESS + 0x10)
>> +#define RNG_INT_STATUS                                      
>> (RNG_BASE_ADDRESS + 0x18)
>> +#define RNG_INT_ENABLE                                      
>> (RNG_BASE_ADDRESS + 0x1c)
>> +#define RNG_FIFO_DATA                                       
>> (RNG_BASE_ADDRESS + 0x20)
>> +#define RNG_FIFO_COUNT                                      
>> (RNG_BASE_ADDRESS + 0x24)
>> -#define RNG_CTRL_ENABLE    0x1
>> +#define RNG_CTRL_ENABLE                                     0x1
>>   #endif /*__BCM2836_H__ */
>>
> 
> 
> 
>