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.web11.1504.1589146734444152343
 for <devel@edk2.groups.io>;
 Sun, 10 May 2020 14:38:54 -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 21029D6E;
	Sun, 10 May 2020 14:38:54 -0700 (PDT)
Received: from [192.168.1.81] (unknown [172.31.20.19])
	by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id CDA003F71E;
	Sun, 10 May 2020 14:38:52 -0700 (PDT)
Subject: Re: [edk2-platforms][PATCH 1/1] RPi: add Gpio output set/clear functions to GpioLib
To: Andrei Warkentin <andrey.warkentin@gmail.com>, devel@edk2.groups.io
Cc: leif@nuviainc.com, pete@akeo.ie, philmd@redhat.com
References: <20200510213650.12829-1-andrey.warkentin@gmail.com>
From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
Message-ID: <720059bd-3069-f39b-070a-0c4e0a4e0c8c@arm.com>
Date: Sun, 10 May 2020 23:38:50 +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: <20200510213650.12829-1-andrey.warkentin@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit

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
>