[edk2-devel] [PATCH edk2-platforms 1/1] Silicon/NXP: rework IoAccessLib to use single function pointer struct
Pankaj Bansal
pankaj.bansal at nxp.com
Thu Apr 23 14:16:28 UTC 2020
> -----Original Message-----
> From: Leif Lindholm <leif at nuviainc.com>
> Sent: Thursday, April 23, 2020 3:39 PM
> To: devel at edk2.groups.io
> Cc: Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Pankaj Bansal
> <pankaj.bansal at nxp.com>
> Subject: [PATCH edk2-platforms 1/1] Silicon/NXP: rework IoAccessLib to use
> single function pointer struct
>
> For whatever reason, the function pointer structs (and their retrieval
> functions) were split up on data size boundary - separate for 16-bit,
> 32-bit, and 64-bit.
>
> This makes the already tedious boilerplate require to deal with these
> hardware quirks even worse, so unify them into a single function pointer
> struct and retrieval function.
>
> Cc: Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>
> Cc: Pankaj Bansal <pankaj.bansal at nxp.com>
> Signed-off-by: Leif Lindholm <leif at nuviainc.com>
> ---
>
> Meenakshi, Pankaj - I have no hardware to verify that this patch works,
> but it builds cleanlyi and isn't exactly complex.
> I will need R-b from at least one of you, and ideally Tested-by as well.
>
Reviewed-by: Pankaj Bansal <pankaj.bansal at nxp.com>
Tested-by: Pankaj Bansal <pankaj.bansal at nxp.com>
> Silicon/NXP/Include/Library/IoAccessLib.h | 52 ++----------
> Silicon/NXP/Library/IoAccessLib/IoAccessLib.c | 82 ++++---------------
> 2 files changed, 22 insertions(+), 112 deletions(-)
>
> diff --git a/Silicon/NXP/Include/Library/IoAccessLib.h
> b/Silicon/NXP/Include/Library/IoAccessLib.h
> index 0b708d544fa7..0f5b19dcf149 100644
> --- a/Silicon/NXP/Include/Library/IoAccessLib.h
> +++ b/Silicon/NXP/Include/Library/IoAccessLib.h
> @@ -15,40 +15,26 @@
> /// Structure to have pointer to R/W
> /// Mmio operations for 16 bits.
> ///
> -typedef struct _MMIO_OPERATIONS_16 {
> +typedef struct _MMIO_OPERATIONS {
> UINT16 (*Read16) (UINTN Address);
> UINT16 (*Write16) (UINTN Address, UINT16 Value);
> UINT16 (*Or16) (UINTN Address, UINT16 OrData);
> UINT16 (*And16) (UINTN Address, UINT16 AndData);
> UINT16 (*AndThenOr16) (UINTN Address, UINT16 AndData, UINT16 OrData);
> -} MMIO_OPERATIONS_16;
> -
> -///
> -/// Structure to have pointer to R/W
> -/// Mmio operations for 32 bits.
> -///
> -typedef struct _MMIO_OPERATIONS_32 {
> UINT32 (*Read32) (UINTN Address);
> UINT32 (*Write32) (UINTN Address, UINT32 Value);
> UINT32 (*Or32) (UINTN Address, UINT32 OrData);
> UINT32 (*And32) (UINTN Address, UINT32 AndData);
> UINT32 (*AndThenOr32) (UINTN Address, UINT32 AndData, UINT32 OrData);
> -} MMIO_OPERATIONS_32;
> -
> -///
> -/// Structure to have pointer to R/W
> -/// Mmio operations for 64 bits.
> -///
> -typedef struct _MMIO_OPERATIONS_64 {
> UINT64 (*Read64) (UINTN Address);
> UINT64 (*Write64) (UINTN Address, UINT64 Value);
> UINT64 (*Or64) (UINTN Address, UINT64 OrData);
> UINT64 (*And64) (UINTN Address, UINT64 AndData);
> UINT64 (*AndThenOr64) (UINTN Address, UINT64 AndData, UINT64 OrData);
> -} MMIO_OPERATIONS_64;
> +} MMIO_OPERATIONS;
>
> /**
> - Function to return pointer to 16 bit Mmio operations.
> + Function to return pointer to Mmio operations.
>
> @param Swap Flag to tell if Swap is needed or not
> on Mmio Operations.
> @@ -56,36 +42,8 @@ typedef struct _MMIO_OPERATIONS_64 {
> @return Pointer to Mmio Operations.
>
> **/
> -MMIO_OPERATIONS_16 *
> -GetMmioOperations16 (
> - IN BOOLEAN Swap
> - );
> -
> -/**
> - Function to return pointer to 32 bit Mmio operations.
> -
> - @param Swap Flag to tell if Swap is needed or not
> - on Mmio Operations.
> -
> - @return Pointer to Mmio Operations.
> -
> -**/
> -MMIO_OPERATIONS_32 *
> -GetMmioOperations32 (
> - IN BOOLEAN Swap
> - );
> -
> -/**
> - Function to return pointer to 64 bit Mmio operations.
> -
> - @param Swap Flag to tell if Swap is needed or not
> - on Mmio Operations.
> -
> - @return Pointer to Mmio Operations.
> -
> -**/
> -MMIO_OPERATIONS_64 *
> -GetMmioOperations64 (
> +MMIO_OPERATIONS *
> +GetMmioOperations (
> IN BOOLEAN Swap
> );
>
> diff --git a/Silicon/NXP/Library/IoAccessLib/IoAccessLib.c
> b/Silicon/NXP/Library/IoAccessLib/IoAccessLib.c
> index 6ed83d019a6e..33039afda40f 100644
> --- a/Silicon/NXP/Library/IoAccessLib/IoAccessLib.c
> +++ b/Silicon/NXP/Library/IoAccessLib/IoAccessLib.c
> @@ -301,39 +301,17 @@ SwapMmioAnd64 (
> return MmioAnd64 (Address, SwapBytes64 (AndData));
> }
>
> -STATIC MMIO_OPERATIONS_16 SwappingFunctions16 = {
> +STATIC MMIO_OPERATIONS SwappingFunctions = {
> SwapMmioRead16,
> SwapMmioWrite16,
> SwapMmioOr16,
> SwapMmioAnd16,
> SwapMmioAndThenOr16,
> -};
> -
> -STATIC MMIO_OPERATIONS_16 NonSwappingFunctions16 = {
> - MmioRead16,
> - MmioWrite16,
> - MmioOr16,
> - MmioAnd16,
> - MmioAndThenOr16,
> -};
> -
> -STATIC MMIO_OPERATIONS_32 SwappingFunctions32 = {
> SwapMmioRead32,
> SwapMmioWrite32,
> SwapMmioOr32,
> SwapMmioAnd32,
> SwapMmioAndThenOr32,
> -};
> -
> -STATIC MMIO_OPERATIONS_32 NonSwappingFunctions32 = {
> - MmioRead32,
> - MmioWrite32,
> - MmioOr32,
> - MmioAnd32,
> - MmioAndThenOr32,
> -};
> -
> -STATIC MMIO_OPERATIONS_64 SwappingFunctions64 = {
> SwapMmioRead64,
> SwapMmioWrite64,
> SwapMmioOr64,
> @@ -341,7 +319,17 @@ STATIC MMIO_OPERATIONS_64 SwappingFunctions64
> = {
> SwapMmioAndThenOr64,
> };
>
> -STATIC MMIO_OPERATIONS_64 NonSwappingFunctions64 = {
> +STATIC MMIO_OPERATIONS NonSwappingFunctions = {
> + MmioRead16,
> + MmioWrite16,
> + MmioOr16,
> + MmioAnd16,
> + MmioAndThenOr16,
> + MmioRead32,
> + MmioWrite32,
> + MmioOr32,
> + MmioAnd32,
> + MmioAndThenOr32,
> MmioRead64,
> MmioWrite64,
> MmioOr64,
> @@ -350,7 +338,7 @@ STATIC MMIO_OPERATIONS_64
> NonSwappingFunctions64 = {
> };
>
> /**
> - Function to return pointer to 16 bit Mmio operations.
> + Function to return pointer to Mmio operations.
>
> @param Swap Flag to tell if Swap is needed or not
> on Mmio Operations.
> @@ -358,47 +346,11 @@ STATIC MMIO_OPERATIONS_64
> NonSwappingFunctions64 = {
> @return Pointer to Mmio Operations.
>
> **/
> -MMIO_OPERATIONS_16 *
> -GetMmioOperations16 (BOOLEAN Swap) {
> +MMIO_OPERATIONS *
> +GetMmioOperations (BOOLEAN Swap) {
> if (Swap) {
> - return &SwappingFunctions16;
> + return &SwappingFunctions;
> } else {
> - return &NonSwappingFunctions16;
> - }
> -}
> -
> -/**
> - Function to return pointer to 32 bit Mmio operations.
> -
> - @param Swap Flag to tell if Swap is needed or not
> - on Mmio Operations.
> -
> - @return Pointer to Mmio Operations.
> -
> -**/
> -MMIO_OPERATIONS_32 *
> -GetMmioOperations32 (BOOLEAN Swap) {
> - if (Swap) {
> - return &SwappingFunctions32;
> - } else {
> - return &NonSwappingFunctions32;
> - }
> -}
> -
> -/**
> - Function to return pointer to 64 bit Mmio operations.
> -
> - @param Swap Flag to tell if Swap is needed or not
> - on Mmio Operations.
> -
> - @return Pointer to Mmio Operations.
> -
> -**/
> -MMIO_OPERATIONS_64 *
> -GetMmioOperations64 (BOOLEAN Swap) {
> - if (Swap) {
> - return &SwappingFunctions64;
> - } else {
> - return &NonSwappingFunctions64;
> + return &NonSwappingFunctions;
> }
> }
> --
> 2.20.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#57970): https://edk2.groups.io/g/devel/message/57970
Mute This Topic: https://groups.io/mt/73215273/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list