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

["Andrei Warkentin" <awarkentin@vmware.com>]

[-- Attachment #1: Type: text/plain, Size: 3936 bytes --]

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?

A
________________________________
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
>





[-- Attachment #2: Type: text/html, Size: 7010 bytes --]