From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR05-AM6-obe.outbound.protection.outlook.com (EUR05-AM6-obe.outbound.protection.outlook.com [40.107.22.81]) by mx.groups.io with SMTP id smtpd.web12.7053.1587651390461540980 for ; Thu, 23 Apr 2020 07:16:31 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nxp.com header.s=selector2 header.b=koA0buwH; spf=pass (domain: nxp.com, ip: 40.107.22.81, mailfrom: pankaj.bansal@nxp.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=d90MC8bCbMDvvliOtL5zp5ygBLnGC97HqPtpM93mry8rjB1cqov6LVUU4LDJSXB1ySy7OxPkQGger+JAuFuGLMOWF8pUXRcT/LmG/nSejGJqB+5lAvyzPWSIysQEJ32HMo6o2rnmy0CqJ+6L0Pii8UZSHnG9LRmZowcPaFGopzIbXOBJlpnvZpq5e2VceHKBRwCaD9CjGmheo8sDwiaZcsdtz1IQeNKtCUEU+GaLSD/33bUEtinQsD5yUbeX35vhcMHmQfqM/55Kiys/r9XUrumrX9eYecfd1yTeNM3TJDmpJ6+VFDyQvm4/MJseVR90YaSJ3CflO0m6gkmTBAFSYw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=5YGK7s0/o8chXvIlOK2nj70C0sn3rDaSr/Z9rF8QPao=; b=KCOWM5cLT9qSbaL6v1eEicTJMKoLEMuXP982wSYxZ1Po9e/3Z/vtoTLlgTvnjhRTA/wD7m+RgBfy5lMr3AS4XF7HEkH1wO7TJ0bvSvRI3Z6P6i/4U7vHYiNvDeIzM1rN8V5nYk7KCr7ALZ98k9PBMA5WPvL4HFYIR7wL3ZohJVcaOUbSKeyMGVZJW2l9SoP24G/R4nFAkhEBJIG35Ck44S0IlGgH+ZFB40oVrvqQmRQ9hbS+gSRUbEYk9n8UORSiPydkw5zXWU2oUB+eYzqQldcEuox+qCNYmggHpJKW24LMBqf3oeWz768lFUdSjkhkFit6D9LL6bfmEQ1zDvFXQQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com; dkim=pass header.d=nxp.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=5YGK7s0/o8chXvIlOK2nj70C0sn3rDaSr/Z9rF8QPao=; b=koA0buwHRyx3DkL1elBODjpI7TAL5CIXGK6t4QQ4wssYAqttYLwU5MdyORO+xXVVT0g28pQUjtoEn4C6KhwAqPZfS9n2FlwIErKYtlxC6mx+2BFeXwUMm5KKKgNrzcUk1jlzXYjNXsotfJMiwBS8/FzxY/f2k3gLk+JeCYpTcww= Received: from VI1PR04MB5933.eurprd04.prod.outlook.com (2603:10a6:803:ec::16) by VI1SPR01MB0354.eurprd04.prod.outlook.com (2603:10a6:803:a3::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2937.13; Thu, 23 Apr 2020 14:16:28 +0000 Received: from VI1PR04MB5933.eurprd04.prod.outlook.com ([fe80::45c4:8846:5327:9513]) by VI1PR04MB5933.eurprd04.prod.outlook.com ([fe80::45c4:8846:5327:9513%7]) with mapi id 15.20.2921.032; Thu, 23 Apr 2020 14:16:28 +0000 From: "Pankaj Bansal" To: Leif Lindholm , "devel@edk2.groups.io" CC: Meenakshi Aggarwal Subject: Re: [PATCH edk2-platforms 1/1] Silicon/NXP: rework IoAccessLib to use single function pointer struct Thread-Topic: [PATCH edk2-platforms 1/1] Silicon/NXP: rework IoAccessLib to use single function pointer struct Thread-Index: AQHWGVdB+KIihHt0+0yXeRnDx5JRn6iGwJ1Q Date: Thu, 23 Apr 2020 14:16:28 +0000 Message-ID: References: <20200423100916.13064-1-leif@nuviainc.com> In-Reply-To: <20200423100916.13064-1-leif@nuviainc.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=pankaj.bansal@nxp.com; x-originating-ip: [49.36.135.81] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: f7216af7-3e76-4d0b-74c8-08d7e790e9fc x-ms-traffictypediagnostic: VI1SPR01MB0354:|VI1SPR01MB0354: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 03827AF76E x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR04MB5933.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(6029001)(4636009)(396003)(376002)(136003)(366004)(39860400002)(346002)(71200400001)(26005)(76116006)(66556008)(66476007)(66446008)(9686003)(52536014)(55016002)(5660300002)(66946007)(478600001)(64756008)(2906002)(186003)(316002)(33656002)(53546011)(6506007)(8676002)(86362001)(44832011)(81156014)(19627235002)(110136005)(7696005)(8936002)(4326008);DIR:OUT;SFP:1101; received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: efgFXEMvmOOh5JWlwWrj2ao9J4+IhRD6zY3uWMwdrLqnEQWEGkjZ1H+NAiepjE9IoXGrAYXGtMopkdbT1Av0dP/AoVY0kRMYfqp3CIoiZdc1KGFXJqStRDgP80ZbPIxO/ggfwIvzUhwEQfKQYyJl/M73rY9WAQqLkc3bMNdXBda4Yk6osrZxScMWKCrb4Dirc2kQ66IOJT3h/7oJY/U8+fblNhiwPmAJb//sgjxPptGpC4iqk2tb9Cd22eQ8jOoGbOe+rTolSeS/xi/bJ/cFPN2rXV+lB0TfZeq/ARXquPn5LfyV8lCQHaeI0dUEUGG2846pdV23jyeLjJ/JnZWiNPvwPNEVjYIWDASkxgQ3zxY0XkbcNF1geQ6L70CtecBZHgHKf+VN19/1KEH9DSG/+wkBFb1Gvzxf75LM9WjKFynVoV3wqEecd7cTkqLB2+KB x-ms-exchange-antispam-messagedata: rStj7GsmZL01kKQnwNybvBDptPv+iLS0+oJg9DDEtHvI+sZQnFj24cqFRUxUb73CdjyFpURfVLJiHYicXVXZGyGJ0cN8gMFrY8ND9r273OfGHN61hNOh5VISPQrH+bW/df80ikLFcMAxKTvr8fYm4A== MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: f7216af7-3e76-4d0b-74c8-08d7e790e9fc X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Apr 2020 14:16:28.2008 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: SIidEJE+G3VpRRKgc8wq9nuKLak5NcD9ytuV80Y36gj0P/bPH3rri1PonoOQKhECoU9RVPAnSJmw7ObPFjnD6A== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1SPR01MB0354 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Leif Lindholm > Sent: Thursday, April 23, 2020 3:39 PM > To: devel@edk2.groups.io > Cc: Meenakshi Aggarwal ; Pankaj Bansal > > Subject: [PATCH edk2-platforms 1/1] Silicon/NXP: rework IoAccessLib to us= e > single function pointer struct >=20 > 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. >=20 > 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. >=20 > Cc: Meenakshi Aggarwal > Cc: Pankaj Bansal > Signed-off-by: Leif Lindholm > --- >=20 > 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. >=20 Reviewed-by: Pankaj Bansal Tested-by: Pankaj Bansal > Silicon/NXP/Include/Library/IoAccessLib.h | 52 ++---------- > Silicon/NXP/Library/IoAccessLib/IoAccessLib.c | 82 ++++--------------- > 2 files changed, 22 insertions(+), 112 deletions(-) >=20 > 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; >=20 > /** > - Function to return pointer to 16 bit Mmio operations. > + Function to return pointer to Mmio operations. >=20 > @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. >=20 > **/ > -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 > ); >=20 > 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)); > } >=20 > -STATIC MMIO_OPERATIONS_16 SwappingFunctions16 =3D { > +STATIC MMIO_OPERATIONS SwappingFunctions =3D { > SwapMmioRead16, > SwapMmioWrite16, > SwapMmioOr16, > SwapMmioAnd16, > SwapMmioAndThenOr16, > -}; > - > -STATIC MMIO_OPERATIONS_16 NonSwappingFunctions16 =3D { > - MmioRead16, > - MmioWrite16, > - MmioOr16, > - MmioAnd16, > - MmioAndThenOr16, > -}; > - > -STATIC MMIO_OPERATIONS_32 SwappingFunctions32 =3D { > SwapMmioRead32, > SwapMmioWrite32, > SwapMmioOr32, > SwapMmioAnd32, > SwapMmioAndThenOr32, > -}; > - > -STATIC MMIO_OPERATIONS_32 NonSwappingFunctions32 =3D { > - MmioRead32, > - MmioWrite32, > - MmioOr32, > - MmioAnd32, > - MmioAndThenOr32, > -}; > - > -STATIC MMIO_OPERATIONS_64 SwappingFunctions64 =3D { > SwapMmioRead64, > SwapMmioWrite64, > SwapMmioOr64, > @@ -341,7 +319,17 @@ STATIC MMIO_OPERATIONS_64 SwappingFunctions64 > =3D { > SwapMmioAndThenOr64, > }; >=20 > -STATIC MMIO_OPERATIONS_64 NonSwappingFunctions64 =3D { > +STATIC MMIO_OPERATIONS NonSwappingFunctions =3D { > + MmioRead16, > + MmioWrite16, > + MmioOr16, > + MmioAnd16, > + MmioAndThenOr16, > + MmioRead32, > + MmioWrite32, > + MmioOr32, > + MmioAnd32, > + MmioAndThenOr32, > MmioRead64, > MmioWrite64, > MmioOr64, > @@ -350,7 +338,7 @@ STATIC MMIO_OPERATIONS_64 > NonSwappingFunctions64 =3D { > }; >=20 > /** > - Function to return pointer to 16 bit Mmio operations. > + Function to return pointer to Mmio operations. >=20 > @param Swap Flag to tell if Swap is needed or not > on Mmio Operations. > @@ -358,47 +346,11 @@ STATIC MMIO_OPERATIONS_64 > NonSwappingFunctions64 =3D { > @return Pointer to Mmio Operations. >=20 > **/ > -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