On Thu, Apr 23, 2020 at 14:16:28 +0000, Pankaj Bansal wrote:
>
>
> > -----Original Message-----
> > From: Leif Lindholm <l...@nuviainc.com>
> > Sent: Thursday, April 23, 2020 3:39 PM
> > To: devel@edk2.groups.io
> > Cc: Meenakshi Aggarwal <meenakshi.aggar...@nxp.com>; Pankaj Bansal
> > <pankaj.ban...@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.aggar...@nxp.com>
> > Cc: Pankaj Bansal <pankaj.ban...@nxp.com>
> > Signed-off-by: Leif Lindholm <l...@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.ban...@nxp.com>
> Tested-by: Pankaj Bansal <pankaj.ban...@nxp.com>
Thanks!
Pushed as ce617e442c70.
/
Leif
> > 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 (#58053): https://edk2.groups.io/g/devel/message/58053
Mute This Topic: https://groups.io/mt/73215273/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
