From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web10.1792.1589271172320318524 for ; Tue, 12 May 2020 01:12:52 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id EE1891FB; Tue, 12 May 2020 01:12:51 -0700 (PDT) Received: from [192.168.1.81] (unknown [10.37.8.255]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7DD5D3F305; Tue, 12 May 2020 01:12:50 -0700 (PDT) Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi: add Gpio output set/clear functions to GpioLib To: Andrei Warkentin , Andrei Warkentin , "devel@edk2.groups.io" Cc: "leif@nuviainc.com" , "pete@akeo.ie" , "philmd@redhat.com" References: <20200510213650.12829-1-andrey.warkentin@gmail.com> <720059bd-3069-f39b-070a-0c4e0a4e0c8c@arm.com> From: "Ard Biesheuvel" Message-ID: Date: Tue, 12 May 2020 10:12:47 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 5/11/20 7:02 PM, Andrei Warkentin wrote: > I was going to have a patch to support power-off using an alternate=20 > mechanism (not PSCI, but via GPIO for the=20 > https://raspberrypiwiki.com/index.php/X735=A0hat. >=20 > Ultimately, I decided against complicating UEFI - an OS isn't required= =20 > to use UEFI RT today over PSCI (OpenBSD didn't), the memory map becomes= =20 > more complicated (to cover the RT MMIO for GPIO block). The right place= =20 > for this functionality appears to be TF-A, so that's why I just have the= = =20 > GpioLib change. >=20 > So in the near future I don't anticipate any changes from me that would= =20 > use this GpioLib functionality, yet it is tested (at the least GpioSet)= =20 > and would be useful to others to have it. Shall I bail on it entirely? >=20 If we can't use it and won't be able to test it, I don't see the point=20 in keeping it tbh. > ------------------------------------------------------------------------ > *From:* devel@edk2.groups.io on behalf of Ard=20 > Biesheuvel via groups.io > *Sent:* Sunday, May 10, 2020 4:38 PM > *To:* Andrei Warkentin ;=20 > devel@edk2.groups.io > *Cc:* leif@nuviainc.com ; pete@akeo.ie=20 > ; philmd@redhat.com > *Subject:* Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi: add Gpio=20 > 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). >>=20 >> Signed-off-by: Andrei Warkentin >=20 > Can you add this patch to the series that actually introduces a user for > this new functionality? >=20 > Also, please don't hide unrelated changes in your patches like this. >=20 >> --- >>=A0=A0 Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h | 10 +++++ >>=A0=A0 Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c | 42 +++++++++= ++++++++++- >>=A0=A0 2 files changed, 50 insertions(+), 2 deletions(-) >>=20 >> diff --git a/Silicon/Broadcom/Bcm283x/Include/Library/GpioLib.h b/Silic= on/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 ( >>=A0=A0=A0=A0 IN=A0 UINTN Pin >>=A0=A0=A0=A0 ); >> =20 >> +VOID >> +GpioSet ( >> +=A0 IN=A0 UINTN Pin >> +=A0 ); >> + >> +VOID >> +GpioClear ( >> +=A0 IN=A0 UINTN Pin >> +=A0 ); >> + >>=A0=A0 #endif /* __GPIO_LIB__ */ >> diff --git a/Silicon/Broadcom/Bcm283x/Library/GpioLib/GpioLib.c b/Silic= on/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 @@ >> =20 >>=A0=A0 STATIC >>=A0=A0 VOID >> -GpioFSELModify ( >> +GpioFSelModify ( >>=A0=A0=A0=A0 IN=A0 UINTN RegIndex, >>=A0=A0=A0=A0 IN=A0 UINT32 ModifyMask, >>=A0=A0=A0=A0 IN=A0 UINT32 FunctionMask >> @@ -38,6 +38,44 @@ GpioFSELModify ( >>=A0=A0=A0=A0 MmioWrite32 (Reg, Val); >>=A0=A0 } >> =20 >> +VOID >> +GpioSet ( >> +=A0 IN=A0 UINTN Pin >> +=A0 ) >> +{ >> +=A0 UINT32 Val; >> +=A0 EFI_PHYSICAL_ADDRESS Reg; >> +=A0 UINT8 RegIndex =3D Pin / 32; >> +=A0 UINT8 SelIndex =3D Pin % 32; >> + >> +=A0 Reg =3D RegIndex * sizeof (UINT32) + GPIO_GPSET0; >> + >> +=A0 ASSERT (Reg <=3D GPIO_GPSET1); >> + >> +=A0 Val =3D MmioRead32 (Reg); >> +=A0 Val |=3D 1 << SelIndex; >> +=A0 MmioWrite32 (Reg, Val); >> +} >> + >> +VOID >> +GpioClear ( >> +=A0 IN=A0 UINTN Pin >> +=A0 ) >> +{ >> +=A0 UINT32 Val; >> +=A0 EFI_PHYSICAL_ADDRESS Reg; >> +=A0 UINT8 RegIndex =3D Pin / 32; >> +=A0 UINT8 SelIndex =3D Pin % 32; >> + >> +=A0 Reg =3D RegIndex * sizeof (UINT32) + GPIO_GPCLR0; >> + >> +=A0 ASSERT (Reg <=3D GPIO_GPCLR1); >> + >> +=A0 Val =3D MmioRead32 (Reg); >> +=A0 Val |=3D 1 << SelIndex; >> +=A0 MmioWrite32 (Reg, Val); >> +} >> + >>=A0=A0 VOID >>=A0=A0 GpioPinFuncSet ( >>=A0=A0=A0=A0 IN=A0 UINTN Pin, >> @@ -57,7 +95,7 @@ GpioPinFuncSet ( >> =20 >>=A0=A0=A0=A0 ModifyMask =3D GPIO_FSEL_MASK << (SelIndex * GPIO_FSEL_BITS= _PER_PIN); >>=A0=A0=A0=A0 FunctionMask =3D Function << (SelIndex * GPIO_FSEL_BITS_PER= _PIN); >> -=A0 GpioFSELModify (RegIndex, ModifyMask, FunctionMask); >> +=A0 GpioFSelModify (RegIndex, ModifyMask, FunctionMask); >>=A0=A0 } >> =20 >>=A0=A0 UINTN >>=20 >=20 >=20 >=20 >=20