Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi: add Gpio output set/clear functions to GpioLib

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 5/11/20 7:02 PM, Andrei Warkentin wrote:
> I was going to have a patch to support power-off using an alternate 
> mechanism (not PSCI, but via GPIO for the 
> https://raspberrypiwiki.com/index.php/X735 hat.
> 
> Ultimately, I decided against complicating UEFI - an OS isn't required 
> to use UEFI RT today over PSCI (OpenBSD didn't), the memory map becomes 
> more complicated (to cover the RT MMIO for GPIO block). The right place 
> for this functionality appears to be TF-A, so that's why I just have the 
> GpioLib change.
> 
> So in the near future I don't anticipate any changes from me that would 
> use this GpioLib functionality, yet it is tested (at the least GpioSet) 
> and would be useful to others to have it. Shall I bail on it entirely?
> 

If we can't use it and won't be able to test it, I don't see the point 
in keeping it tbh.


> ------------------------------------------------------------------------
> *From:* devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Ard 
> Biesheuvel via groups.io <ard.biesheuvel=arm.com@groups.io>
> *Sent:* Sunday, May 10, 2020 4:38 PM
> *To:* Andrei Warkentin <andrey.warkentin@gmail.com>; 
> devel@edk2.groups.io <devel@edk2.groups.io>
> *Cc:* leif@nuviainc.com <leif@nuviainc.com>; pete@akeo.ie 
> <pete@akeo.ie>; philmd@redhat.com <philmd@redhat.com>
> *Subject:* Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi: add Gpio 
> output set/clear functions to GpioLib
> On 5/10/20 11:36 PM, Andrei Warkentin wrote:
>> GpioSet and GpioClear. Using hw of course (not mailbox).
>> 
>> Signed-off-by: Andrei Warkentin <andrey.warkentin@gmail.com>
> 
> Can you add this patch to the series that actually introduces a user for
> this new functionality?
> 
> Also, please don't hide unrelated changes in your patches like this.
> 
>> ---
>>   Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h | 10 +++++
>>   Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c | 42 +++++++++++++++++++-
>>   2 files changed, 50 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
>> index 014c6b07..10c9cdfb 100644
>> --- a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
>> +++ b/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h
>> @@ -24,4 +24,14 @@ GpioPinFuncGet (
>>     IN  UINTN Pin
>>     );
>>   
>> +VOID
>> +GpioSet (
>> +  IN  UINTN Pin
>> +  );
>> +
>> +VOID
>> +GpioClear (
>> +  IN  UINTN Pin
>> +  );
>> +
>>   #endif /* __GPIO_LIB__ */
>> diff --git a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
>> index 542b6e8f..716b05be 100644
>> --- a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
>> +++ b/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c
>> @@ -18,7 +18,7 @@
>>   
>>   STATIC
>>   VOID
>> -GpioFSELModify (
>> +GpioFSelModify (
>>     IN  UINTN RegIndex,
>>     IN  UINT32 ModifyMask,
>>     IN  UINT32 FunctionMask
>> @@ -38,6 +38,44 @@ GpioFSELModify (
>>     MmioWrite32 (Reg, Val);
>>   }
>>   
>> +VOID
>> +GpioSet (
>> +  IN  UINTN Pin
>> +  )
>> +{
>> +  UINT32 Val;
>> +  EFI_PHYSICAL_ADDRESS Reg;
>> +  UINT8 RegIndex = Pin / 32;
>> +  UINT8 SelIndex = Pin % 32;
>> +
>> +  Reg = RegIndex * sizeof (UINT32) + GPIO_GPSET0;
>> +
>> +  ASSERT (Reg <= GPIO_GPSET1);
>> +
>> +  Val = MmioRead32 (Reg);
>> +  Val |= 1 << SelIndex;
>> +  MmioWrite32 (Reg, Val);
>> +}
>> +
>> +VOID
>> +GpioClear (
>> +  IN  UINTN Pin
>> +  )
>> +{
>> +  UINT32 Val;
>> +  EFI_PHYSICAL_ADDRESS Reg;
>> +  UINT8 RegIndex = Pin / 32;
>> +  UINT8 SelIndex = Pin % 32;
>> +
>> +  Reg = RegIndex * sizeof (UINT32) + GPIO_GPCLR0;
>> +
>> +  ASSERT (Reg <= GPIO_GPCLR1);
>> +
>> +  Val = MmioRead32 (Reg);
>> +  Val |= 1 << SelIndex;
>> +  MmioWrite32 (Reg, Val);
>> +}
>> +
>>   VOID
>>   GpioPinFuncSet (
>>     IN  UINTN Pin,
>> @@ -57,7 +95,7 @@ GpioPinFuncSet (
>>   
>>     ModifyMask = GPIO_FSEL_MASK << (SelIndex * GPIO_FSEL_BITS_PER_PIN);
>>     FunctionMask = Function << (SelIndex * GPIO_FSEL_BITS_PER_PIN);
>> -  GpioFSELModify (RegIndex, ModifyMask, FunctionMask);
>> +  GpioFSelModify (RegIndex, ModifyMask, FunctionMask);
>>   }
>>   
>>   UINTN
>> 
> 
> 
> 
>