Re: [edk2-platforms][PATCH 1/1] Silicon/Broadcom/Bcm283x: GpioLib enhancements

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

On 2020.07.02 13:03, Ard Biesheuvel wrote:
> On 6/29/20 8:39 PM, Pete Batard wrote:
>> * Add GpioPinSet () function to set a GPIO pin value
>> * Add GpioPinGet () function to read a GPIO pin value
>> * Add GpioSetPull () function to set pullup/down state of a GPIO pin
>>
>> GpioSetPull () supports both the legacy method used on Bcm283[5-7]
>> as well as the new method used on Bcm2711.
>>
>> Each of these calls was tested on Raspberry Pi 3 B+ as well as on a
>> Raspberry Pi 4 B.
>>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
> 
> Remind me again what the purpose of this patch is?

Feature completion, potential future improvements and providing 
additional options for testing (since one will now be able to use this 
driver to control LEDs or read switches).

It doesn't serve any *immediate* purpose.

However, I've been playing with it to simulate an SD card detect, which 
came handy for developing the other patch I sent, and I would assert 
having an established means to control GPIO signals is likely to help 
others as well.

Regards,

/Pete

> 
>> ---
>>   Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836Gpio.h | 44 
>> ++++++++-
>>   Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h              | 17 
>> ++++
>>   Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c              | 93 
>> ++++++++++++++++++++
>>   Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf            |  1 +
>>   4 files changed, 152 insertions(+), 3 deletions(-)
>>
>> diff --git 
>> a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836Gpio.h 
>> b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836Gpio.h
>> index e65cc5c3bbb4..8ad81776b43a 100644
>> --- a/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836Gpio.h
>> +++ b/Silicon/Broadcom/Bcm283x/Include/IndustryStandard/Bcm2836Gpio.h
>> @@ -1,5 +1,6 @@
>>   /** @file
>>    *
>> + *  Copyright (c) 2020, Pete Batard <pete@akeo.ie>
>>    *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
>>    *  Copyright (c) Microsoft Corporation. All rights reserved.
>>    *
>> @@ -23,12 +24,45 @@
>>   #define GPIO_GPFSEL4       (GPIO_BASE_ADDRESS + 0x10)
>>   #define GPIO_GPFSEL5       (GPIO_BASE_ADDRESS + 0x14)
>> -#define GPIO_GPCLR0        (GPIO_BASE_ADDRESS + 0x28)
>> -#define GPIO_GPCLR1        (GPIO_BASE_ADDRESS + 0x2C)
>> -
>>   #define GPIO_GPSET0        (GPIO_BASE_ADDRESS + 0x1C)
>>   #define GPIO_GPSET1        (GPIO_BASE_ADDRESS + 0x20)
>> +#define GPIO_GPCLR0        (GPIO_BASE_ADDRESS + 0x28)
>> +#define GPIO_GPCLR1        (GPIO_BASE_ADDRESS + 0x2C)
>> +
>> +#define GPIO_GPLEV0        (GPIO_BASE_ADDRESS + 0x34)
>> +#define GPIO_GPLEV1        (GPIO_BASE_ADDRESS + 0x38)
>> +
>> +#define GPIO_GPEDS0        (GPIO_BASE_ADDRESS + 0x40)
>> +#define GPIO_GPEDS1        (GPIO_BASE_ADDRESS + 0x44)
>> +
>> +#define GPIO_GPREN0        (GPIO_BASE_ADDRESS + 0x4C)
>> +#define GPIO_GPREN1        (GPIO_BASE_ADDRESS + 0x50)
>> +
>> +#define GPIO_GPFEN0        (GPIO_BASE_ADDRESS + 0x58)
>> +#define GPIO_GPFEN1        (GPIO_BASE_ADDRESS + 0x5C)
>> +
>> +#define GPIO_GPHEN0        (GPIO_BASE_ADDRESS + 0x64)
>> +#define GPIO_GPHEN1        (GPIO_BASE_ADDRESS + 0x68)
>> +
>> +#define GPIO_GPLEN0        (GPIO_BASE_ADDRESS + 0x70)
>> +#define GPIO_GPLEN1        (GPIO_BASE_ADDRESS + 0x74)
>> +
>> +#define GPIO_GPAREN0       (GPIO_BASE_ADDRESS + 0x7C)
>> +#define GPIO_GPAREN1       (GPIO_BASE_ADDRESS + 0x80)
>> +
>> +#define GPIO_GPAFEN0       (GPIO_BASE_ADDRESS + 0x88)
>> +#define GPIO_GPAFEN1       (GPIO_BASE_ADDRESS + 0x8C)
>> +
>> +#define GPIO_GPPUD         (GPIO_BASE_ADDRESS + 0x94)
>> +#define GPIO_GPPUDCLK0     (GPIO_BASE_ADDRESS + 0x98)
>> +#define GPIO_GPPUDCLK1     (GPIO_BASE_ADDRESS + 0x9C)
>> +
>> +#define GPIO_GPPUPPDN0     (GPIO_BASE_ADDRESS + 0xE4)
>> +#define GPIO_GPPUPPDN1     (GPIO_BASE_ADDRESS + 0xE8)
>> +#define GPIO_GPPUPPDN2     (GPIO_BASE_ADDRESS + 0xEC)
>> +#define GPIO_GPPUPPDN3     (GPIO_BASE_ADDRESS + 0xF0)
>> +
>>   #define GPIO_FSEL_INPUT    0x0
>>   #define GPIO_FSEL_OUTPUT   0x1
>>   #define GPIO_FSEL_ALT0     0x4
>> @@ -44,4 +78,8 @@
>>   #define GPIO_PINS          54
>> +#define GPIO_PULL_NONE     0x00
>> +#define GPIO_PULL_DOWN     0x01
>> +#define GPIO_PULL_UP       0x02
>> +
>>   #endif /* __BCM2836_GPIO_H__ */
>> diff --git a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h 
>> b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
>> index 014c6b07a29f..75c2c8be51aa 100644
>> --- a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
>> +++ b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
>> @@ -24,4 +24,21 @@ GpioPinFuncGet (
>>     IN  UINTN Pin
>>     );
>> +VOID
>> +GpioPinSet (
>> +  IN  UINTN Pin,
>> +  IN  UINTN Val
>> +  );
>> +
>> +UINTN
>> +GpioPinGet (
>> +  IN  UINTN Pin
>> +  );
>> +
>> +VOID
>> +GpioSetPull (
>> +  IN  UINTN   Pin,
>> +  IN  UINTN   Pud
>> +);
>> +
>>   #endif /* __GPIO_LIB__ */
>> diff --git a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c 
>> b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
>> index 542b6e8f6b2f..a4b4af59ebb1 100644
>> --- a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
>> +++ b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
>> @@ -2,6 +2,7 @@
>>    *
>>    *  GPIO manipulation.
>>    *
>> + *  Copyright (c) 2020, Pete Batard <pete@akeo.ie>
>>    *  Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
>>    *
>>    *  SPDX-License-Identifier: BSD-2-Clause-Patent
>> @@ -13,6 +14,7 @@
>>   #include <Library/DebugLib.h>
>>   #include <Library/IoLib.h>
>>   #include <Library/GpioLib.h>
>> +#include <Library/TimerLib.h>
>>   #include <IndustryStandard/Bcm2836.h>
>>   #include <IndustryStandard/Bcm2836Gpio.h>
>> @@ -81,3 +83,94 @@ GpioPinFuncGet (
>>     Val &= GPIO_FSEL_MASK;
>>     return Val;
>>   }
>> +
>> +VOID
>> +GpioPinSet (
>> +  IN  UINTN Pin,
>> +  IN  UINTN Val
>> +  )
>> +{
>> +  EFI_PHYSICAL_ADDRESS Reg;
>> +  UINTN RegIndex;
>> +  UINTN SelIndex;
>> +
>> +  ASSERT (Pin < GPIO_PINS);
>> +
>> +  RegIndex = Pin / 32;
>> +  SelIndex = Pin % 32;
>> +
>> +  //
>> +  // Different base addresses are used for clear and set
>> +  //
>> +  Reg = (Val == 0) ? GPIO_GPCLR0 : GPIO_GPSET0;
>> +  Reg += RegIndex * sizeof (UINT32);
>> +  MmioWrite32 (Reg, 1 << SelIndex);
>> +}
>> +
>> +UINTN
>> +GpioPinGet (
>> +  IN  UINTN Pin
>> +  )
>> +{
>> +  EFI_PHYSICAL_ADDRESS Reg;
>> +  UINTN RegIndex;
>> +  UINTN SelIndex;
>> +  UINT32 Val;
>> +
>> +  ASSERT (Pin < GPIO_PINS);
>> +
>> +  RegIndex = Pin / 32;
>> +  SelIndex = Pin % 32;
>> +
>> +  Reg = RegIndex * sizeof (UINT32) + GPIO_GPLEV0;
>> +  Val = MmioRead32 (Reg) >> SelIndex;
>> +  return Val & 1;
>> +}
>> +
>> +VOID
>> +GpioSetPull (
>> +  IN  UINTN Pin,
>> +  IN  UINTN Pud
>> +)
>> +{
>> +  EFI_PHYSICAL_ADDRESS Reg;
>> +  UINTN RegIndex;
>> +  UINTN SelIndex;
>> +  UINT32 Val;
>> +
>> +  ASSERT (Pin < GPIO_PINS);
>> +  ASSERT (Pud <= GPIO_PULL_UP);
>> +
>> +  //
>> +  // Check if we should use the legacy or newer method of
>> +  // enabling pullup/pulldown by querying GPPUPPDN3, which
>> +  // is unused on Bcm2835/2836/2837 and returns 'gpio'.
>> +  //
>> +  if (MmioRead32 (GPIO_GPPUPPDN3) == 0x6770696f) {
>> +    RegIndex = Pin / 32;
>> +    SelIndex = Pin % 32;
>> +
>> +    MmioWrite32 (GPIO_GPPUD, Pud);
>> +    MicroSecondDelay (5);
>> +
>> +    Reg = RegIndex * sizeof (UINT32) + GPIO_GPPUDCLK0;
>> +    MmioWrite32 (Reg, 1 << SelIndex);
>> +    MicroSecondDelay (5);
>> +
>> +    MmioWrite32 (GPIO_GPPUD, 0);
>> +    MmioWrite32 (Reg, 0);
>> +  } else {
>> +    //
>> +    // The newer method uses inverted values for up/down.
>> +    //
>> +    if (Pud != 0)
>> +      Pud ^= 0x3;
>> +    RegIndex = Pin / 16;
>> +    SelIndex = (Pin % 16) * 2;
>> +    Reg = RegIndex * sizeof (UINT32) + GPIO_GPPUPPDN0;
>> +    Val = MmioRead32 (Reg);
>> +    Val &= ~(0x3 << SelIndex);
>> +    Val |= Pud << SelIndex;
>> +    MmioWrite32 (Reg, Val);
>> +  }
>> +}
>> diff --git a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf 
>> b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf
>> index ff1b5af6db6e..a0356306d83c 100644
>> --- a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf
>> +++ b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.inf
>> @@ -30,6 +30,7 @@ [LibraryClasses]
>>     BaseLib
>>     DebugLib
>>     IoLib
>> +  TimerLib
>>   [FixedPcd]
>>     gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress
>>
>