Re: [PATCH edk2-platforms v3 15/24] Silicon: NXP: Remove direct calls to SwapMmio* APIs

["Leif Lindholm" <leif@nuviainc.com>]

Actually...

On Wed, Apr 15, 2020 at 17:43:33 +0530, Pankaj Bansal wrote:
> From: Pankaj Bansal <pankaj.bansal@nxp.com>
> 
> The SwapMmio** APIs are supposed to be called indirectly via
> GetMmioOperations** APIs.
> Therefore, remove the SwapMmio** APIs from IoAccessLib.h and make
> these APIs STATIC to IoAccessLib.c, so that no accidental call can
> be made to these.
> 
> Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
> ---
> 
> Notes:
>     - New commit
> 
>  Silicon/NXP/Include/Library/IoAccessLib.h     | 236 +-------------------
>  Silicon/NXP/Library/IoAccessLib/IoAccessLib.c |  17 +-
>  Silicon/NXP/Library/SocLib/Chassis.c          |  10 +-
>  3 files changed, 22 insertions(+), 241 deletions(-)
> 

> diff --git a/Silicon/NXP/Library/SocLib/Chassis.c b/Silicon/NXP/Library/SocLib/Chassis.c
> index 90677f0f36ca..d7bc55c9d275 100644
> --- a/Silicon/NXP/Library/SocLib/Chassis.c
> +++ b/Silicon/NXP/Library/SocLib/Chassis.c
> @@ -30,11 +30,11 @@ GurRead (
>    IN  UINTN     Address
>    )
>  {
> -  if (FixedPcdGetBool (PcdGurBigEndian)) {
> -    return SwapMmioRead32 (Address);
> -  } else {
> -    return MmioRead32 (Address);
> -  }
> +  MMIO_OPERATIONS_32  *GurOps;
> +
> +  GurOps = GetMmioOperations32 (FixedPcdGetBool (PcdGurBigEndian));
> +
> +  return GurOps->Read32 (Address);

This bit didn't jump out at me, but when I looked at 16/24 I nearly
choked on my coffee.

When the IoAccessLib was being reviewed, I requested that the
16/32/64-bit accessors be merged
(https://edk2.groups.io/g/devel/message/34403). They ended up not
being, and I didn't notice so pushed the result anyway.

I have sent out a patch for review/test to do this, but it will need
to go in before this patch - and it *will* require changes to this
one:
  MMIO_OPERATIONS_32 -> MMIO_OPERATIONS
  GetMmioOperations32 -> GetMmioOperations

If those changes are done, there is no need for me to re-review this
one.

Regards,

Leif

>  }
>  
>  /*
> -- 
> 2.17.1
>