From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR02-VE1-obe.outbound.protection.outlook.com (EUR02-VE1-obe.outbound.protection.outlook.com [40.107.2.57]) by mx.groups.io with SMTP id smtpd.web10.5753.1591528473831452341 for ; Sun, 07 Jun 2020 04:14:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nxp.com header.s=selector2 header.b=ieCsx0RO; spf=pass (domain: nxp.com, ip: 40.107.2.57, mailfrom: wasim.khan@nxp.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=YbRgoZU5cGHnVBAFfWgXGs7i108jD/WEvY2MYrLOjaZsjbXHycv4rJ7Dcz9vTKhAvnzDaGwZJ1j+kXxB3uVOE49tXgGXWqV4MNodipH1hlpxj/zNHMx2xqyXbHIsA9dpVOJeAGED7+ivROBLCckSlW627HSczW3MmRCzD+ai6+GyNN+noREU9D2KbIwRvG9TeOsaU+/s8Ct4JmlATVCW1qLZ1PnzBMVBsjQk2hG0JvOfRpZrDe9winSza36Z6uFUtGjC42v7vfoqzR8xmqAd9kDGTnHaqtnAZ6lACMFMHEt1IwLhiUnIorbQhuZGHs9z5ftI94OhTpX4ip/7gTr9mg== 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=2EhXecHGbXRWWlURfDzSoymrvA0UIei3m1NM7C3Aq6U=; b=AgDvelPd5bntfCSnSOFtWUCzFM0BRLGW/NvC2+pwLDw8mp95XiDqOrT04LlqqdQGGQv7pu8vhh8dZrbcQ+yG92Z/aDn8ISML3iI1r38AE7ekZdmVFyAitWVRuUE8ascvrcD3kTaILpsVUjJRUMNNBBKq6jQ5PMvFK1j/kqqlkxoBULa9fHToFH2P7bdN0CVuWRp5r46d9UJFwlkUY8qz6Z2EEv0KzO/SvgtoNwdP6n5j2b2x2gVRqBcyHQLedcVquVJvxtxWMAHBQxlG2psuNyWMRyzI8l2xnjlzDwU9S3mRqpcw5EAFrmnV2+/rB1fR+myc0krR2rgrwOtdkEeF9g== 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=2EhXecHGbXRWWlURfDzSoymrvA0UIei3m1NM7C3Aq6U=; b=ieCsx0ROl8v5k5KZHqJMqREqhoCcG8psOzR+jwHw4hjAiPuVpGeNzPEamc9QHF4k5jiv2svxzu/ZyKrfZDFL3wAWwyqFal5nBAiGpyQYgHxNufH1yuZZifzQWa9dd8azgZGcRuK6SW6NlgTv0JHAY3qwBnyC5ZSSkc1tkDqvdjQ= Received: from VE1PR04MB6702.eurprd04.prod.outlook.com (2603:10a6:803:123::13) by VE1PR04MB6734.eurprd04.prod.outlook.com (2603:10a6:803:121::33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3066.18; Sun, 7 Jun 2020 11:14:30 +0000 Received: from VE1PR04MB6702.eurprd04.prod.outlook.com ([fe80::81c4:97a6:7592:f225]) by VE1PR04MB6702.eurprd04.prod.outlook.com ([fe80::81c4:97a6:7592:f225%7]) with mapi id 15.20.3066.023; Sun, 7 Jun 2020 11:14:30 +0000 From: "Wasim Khan" To: Leif Lindholm , "Wasim Khan (OSS)" CC: "devel@edk2.groups.io" , Meenakshi Aggarwal , Varun Sethi , "ard.biesheuvel@arm.com" Subject: Re: [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes Support Thread-Topic: [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes Support Thread-Index: AQHWOErGsqkx7F6B60OfhtnN+W+MsajJ+0UAgAMNjCA= Date: Sun, 7 Jun 2020 11:14:30 +0000 Message-ID: References: <1591039658-18541-1-git-send-email-wasim.khan@oss.nxp.com> <1591039658-18541-3-git-send-email-wasim.khan@oss.nxp.com> <20200605123626.GE28566@vanye> In-Reply-To: <20200605123626.GE28566@vanye> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: nuviainc.com; dkim=none (message not signed) header.d=none;nuviainc.com; dmarc=none action=none header.from=nxp.com; x-originating-ip: [157.47.233.236] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 747c9e2c-6344-4565-9348-08d80ad3f310 x-ms-traffictypediagnostic: VE1PR04MB6734: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 04270EF89C x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: K/2klmwDId7X6UjxySq/fWX7MJ6lfQpdgeexgcTuyV52WF/U1k5DN67NCn+DG9YOS+I5+jFkEJZuvpMDb8Oe9HuPMuvQL5vjf2+dyE9maolXXll/1sYVzoUaa3tvZjThvMo2FxGiZUwgKiGioTin9HNyhc0/koL/2uDuLJ9sFXmNjofDkib2QHCWH4EZAu1H7U8tB/DVKJX6px/6a3EX3DQWdB4w49hqKFSSmJ2UEnc9y1Nvg0TD99MbZMY/Jlo9LOvsEBbydMn0ofhYWzUSexkmyFIM7+8Y3/ITZN++UtMGaQMHHp7/fQUn+unh+nGdo1m1g9e+JUycLrv5EXDhfg== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VE1PR04MB6702.eurprd04.prod.outlook.com;PTR:;CAT:NONE;SFTY:;SFS:(4636009)(396003)(39850400004)(346002)(366004)(376002)(136003)(19627235002)(9686003)(8676002)(316002)(55016002)(54906003)(33656002)(86362001)(7696005)(2906002)(83380400001)(6506007)(110136005)(478600001)(44832011)(71200400001)(186003)(53546011)(66556008)(64756008)(66446008)(66476007)(8936002)(66946007)(26005)(76116006)(4326008)(5660300002)(52536014);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata: YCdyzMGLkFlBXOT/5mW9jW3lMIYXtLdCTSEcpbx6VwgbofpUWELS+M00iSVIL7Bs0ccH7Jatdgx5QaPADB7cUQxuPfecTtWCZru7Bk6EyQfnUgCyi2koRJ/XkPJSZgd90DW11p9nDG27XwaI/GM9W53PTZ6O2T6BfvBLRryZ+p4n/U1o9qrjOL8zs50PJZtuIonzmnp0zBIMvCTxYEy5cQPfjZYn/z7gOr7vyo+zhw2M8PAF31RtOf8IskWfrbPqQqCUcMHFFE4XIU3rpRWMPTc70yaQDCRhcaAPdhtSdTcEX+SaO6webrGqbXQI6aJ9zv7tgYpU6T4wv8F2y81Gf9mbubpWKbCMxG0cT3GakWlSmyEDtK86JdlrBLZeR7IRCP9rIETxvbQodMAKrpz6PMTffH/h7QP0/K374gRpW8alN7yU+c9Mjkdu8iHUl1jWXr+qrlgXTTKWlfK9yDV6G302Qd6dtxrEhUfu9m0S4LcBN3KFEkbHhy7DRLSUhYc/ MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 747c9e2c-6344-4565-9348-08d80ad3f310 X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jun 2020 11:14:30.4142 (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: s7Ek3PtScUQyHZbt3fhcfkYWZ0rHXv23Z1/B8WWOvINPasWG9CAVH/gW91m79T1nhaPzKu9aSsbR0e+A1aYjtw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB6734 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Leif Lindholm > Sent: Friday, June 5, 2020 6:06 PM > To: Wasim Khan (OSS) > Cc: devel@edk2.groups.io; Meenakshi Aggarwal > ; Varun Sethi ; > ard.biesheuvel@arm.com; Wasim Khan > Subject: Re: [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes > Support >=20 > On Tue, Jun 02, 2020 at 00:57:37 +0530, Wasim Khan wrote: > > From: Wasim Khan > > > > Based on serdes protocol value in reset configuration word (RCW) > > different IP blocks gets enabled in HW. > > Add SoC specific serdes configuration for LS1043A, which can be used > > by different IPs to know the enabled interfaces and perform the > > required initialization. > > > > Signed-off-by: Wasim Khan > > --- > > Silicon/NXP/LS1043A/LS1043A.dsc.inc | 2 + > > Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf | 6 ++ > > Silicon/NXP/Include/Library/SerDes.h | 28 ++++++++ > > Silicon/NXP/LS1043A/Include/SocSerDes.h | 68 ++++++++++++++++++ > > Silicon/NXP/LS1043A/Library/SocLib/SerDes.c | 75 +++++++++++++++++++= + > > 5 files changed, 179 insertions(+) > > > > diff --git a/Silicon/NXP/LS1043A/LS1043A.dsc.inc > > b/Silicon/NXP/LS1043A/LS1043A.dsc.inc > > index e023bfbc7c04..b598b5790b65 100644 > > --- a/Silicon/NXP/LS1043A/LS1043A.dsc.inc > > +++ b/Silicon/NXP/LS1043A/LS1043A.dsc.inc > > @@ -12,6 +12,7 @@ > > [LibraryClasses.common] > > SocLib|Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf > > SerialPortLib|Silicon/NXP/Library/DUartPortLib/DUartPortLib.inf > > + > > + SerDesHelperLib|Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib. > > + inf > > > > > > > ################################################################# > ##### > > ########## > > # > > @@ -37,4 +38,5 @@ [PcdsFixedAtBuild.common] > > gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|3 > > gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x10000 > > gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x7FC > > + gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes|0x4 > > ## > > diff --git a/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf > > b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf > > index 3d0f988e1c67..4731bea8b92e 100644 > > --- a/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf > > +++ b/Silicon/NXP/LS1043A/Library/SocLib/SocLib.inf > > @@ -22,6 +22,12 @@ [Packages] > > [LibraryClasses] > > ChassisLib > > DebugLib > > + PcdLib > > + SerDesHelperLib > > > > [Sources.common] > > SocLib.c > > + SerDes.c >=20 > Insert alphabetically sorted. >=20 > > + > > +[FixedPcd] > > + gNxpQoriqLsTokenSpaceGuid.PcdSerdesLanes > > diff --git a/Silicon/NXP/Include/Library/SerDes.h > > b/Silicon/NXP/Include/Library/SerDes.h > > new file mode 100644 > > index 000000000000..d846589401c6 > > --- /dev/null > > +++ b/Silicon/NXP/Include/Library/SerDes.h > > @@ -0,0 +1,28 @@ > > +/** SerDes.h > > + Header file for SoC specific Serdes routines > > + > > + Copyright 2017-2020 NXP > > + > > + SPDX-License-Identifier: BSD-2-Clause-Patent **/ > > + > > +#ifndef SERDES_H > > +#define SERDES_H > > + > > +VOID > > +GetSerdesProtocolMaps ( > > + OUT UINT64 *SerDesPrtclMap > > + ); > > + > > +typedef VOID > > +(*SERDES_PROBE_LANES_CALLBACK) ( > > + IN UINT32 LaneProtocol, > > + IN VOID *Arg > > + ); > > + > > +VOID > > +SerDesProbeLanes ( > > + IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback, > > + IN VOID *Arg > > + ); > > +#endif > > diff --git a/Silicon/NXP/LS1043A/Include/SocSerDes.h > > b/Silicon/NXP/LS1043A/Include/SocSerDes.h > > new file mode 100644 > > index 000000000000..5bf93cb32df6 > > --- /dev/null > > +++ b/Silicon/NXP/LS1043A/Include/SocSerDes.h > > @@ -0,0 +1,68 @@ > > +/** @file > > + SoC Specific header file for Serdes mapping > > + > > + Copyright 2017-2020 NXP > > + > > + SPDX-License-Identifier: BSD-2-Clause-Patent > > + > > +**/ > > + > > +#ifndef SOC_SERDES_H > > +#define SOC_SERDES_H > > + > > +typedef enum { > > + NONE =3D 0, > > + PCIE1, > > + PCIE2, > > + PCIE3, > > + SATA, > > + SGMII_FM1_DTSEC1, > > + SGMII_FM1_DTSEC2, > > + SGMII_FM1_DTSEC5, > > + SGMII_FM1_DTSEC6, > > + SGMII_FM1_DTSEC9, > > + SGMII_FM1_DTSEC10, > > + QSGMII_FM1_A, > > + XFI_FM1_MAC9, > > + XFI_FM1_MAC10, > > + SGMII_2500_FM1_DTSEC2, > > + SGMII_2500_FM1_DTSEC5, > > + SGMII_2500_FM1_DTSEC9, > > + SGMII_2500_FM1_DTSEC10, > > + SERDES_PRTCL_COUNT > > +} SERDES_PROTOCOL; > > + > > +SERDES_CONFIG SerDes1ConfigTbl[] =3D { >=20 > No data definitions in header files please. > Also, global variables should have 'm' or 'g' prefix. >=20 > > + /* SerDes 1 */ > > + {0x1555, {XFI_FM1_MAC9, PCIE1, PCIE2, PCIE3 } }, > > + {0x2555, {SGMII_2500_FM1_DTSEC9, PCIE1, PCIE2, PCIE3 } }, > > + {0x4555, {QSGMII_FM1_A, PCIE1, PCIE2, PCIE3 } }, > > + {0x4558, {QSGMII_FM1_A, PCIE1, PCIE2, SATA } }, > > + {0x1355, {XFI_FM1_MAC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } }, > > + {0x2355, {SGMII_2500_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } > > +}, > > + {0x3335, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, SGMII_FM1_DTSEC5, > > +PCIE3 } }, > > + {0x3355, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, PCIE3 } }, > > + {0x3358, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, PCIE2, SATA } }, > > + {0x3555, {SGMII_FM1_DTSEC9, PCIE1, PCIE2, PCIE3 } }, > > + {0x3558, {SGMII_FM1_DTSEC9, PCIE1, PCIE2, SATA } }, > > + {0x7000, {PCIE1, PCIE1, PCIE1, PCIE1 } }, > > + {0x9998, {PCIE1, PCIE2, PCIE3, SATA } }, > > + {0x6058, {PCIE1, PCIE1, PCIE2, SATA } }, > > + {0x1455, {XFI_FM1_MAC9, QSGMII_FM1_A, PCIE2, PCIE3 } }, > > + {0x2455, {SGMII_2500_FM1_DTSEC9, QSGMII_FM1_A, PCIE2, PCIE3 } }, > > + {0x2255, {SGMII_2500_FM1_DTSEC9, SGMII_2500_FM1_DTSEC2, PCIE2, > > +PCIE3 } }, > > + {0x3333, {SGMII_FM1_DTSEC9, SGMII_FM1_DTSEC2, SGMII_FM1_DTSEC5, > > +SGMII_FM1_DTSEC6 } }, > > + {0x1460, {XFI_FM1_MAC9, QSGMII_FM1_A, PCIE3, PCIE3 } }, > > + {0x2460, {SGMII_2500_FM1_DTSEC9, QSGMII_FM1_A, PCIE3, PCIE3 } }, > > + {0x3460, {SGMII_FM1_DTSEC9, QSGMII_FM1_A, PCIE3, PCIE3 } }, > > + {0x3455, {SGMII_FM1_DTSEC9, QSGMII_FM1_A, PCIE2, PCIE3 } }, > > + {0x9960, {PCIE1, PCIE2, PCIE3, PCIE3 } }, > > + {0x2233, {SGMII_2500_FM1_DTSEC9, SGMII_FM1_DTSEC2, > > +SGMII_FM1_DTSEC5, SGMII_FM1_DTSEC6 } }, > > + {0x2533, {SGMII_2500_FM1_DTSEC9, PCIE1, SGMII_FM1_DTSEC5, > > +SGMII_FM1_DTSEC6 } }, > > + {} > > +}; > > + > > +SERDES_CONFIG *SerDesConfigTbl[] =3D { >=20 > No data definitions in header files please. > Also, global variables should have 'm' or 'g' prefix. >=20 > Finally, consider naming. I usually let the abbreviation "Tbl" slip, but = in this case > it is inapproproate. I assume for the previous data definition it is inte= nded to > mean "Table", and here it is supposed to mean "Tables"? The reader is not > supposed to have to figure that out. >=20 > > + SerDes1ConfigTbl > > +}; > > +#endif > > diff --git a/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c > > b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c > > new file mode 100644 > > index 000000000000..c27367621168 > > --- /dev/null > > +++ b/Silicon/NXP/LS1043A/Library/SocLib/SerDes.c > > @@ -0,0 +1,75 @@ > > +/** SerDes.c > > + Provides SoC specific serdes interface > > + > > + Copyright 2017-2020 NXP > > + > > + SPDX-License-Identifier: BSD-2-Clause-Patent **/ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#define RCWSR_SRDS1_PRTCL_MASK 0xffff0000 > > +#define RCWSR_SRDS1_PRTCL_SHIFT 16 > > + > > +/** > > + Probe all serdes lanes for lane protocol and execute provided callba= ck > function. > > + > > + @param SerDesLaneProbeCallback Pointer Callback function to be call= ed > for Lane protocol > > + @param Arg Pointer to Arguments to be passed to= callback > function. > > + > > +**/ > > +VOID > > +SerDesProbeLanes ( > > + IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback, > > + IN VOID *Arg > > + ) > > +{ > > + UINT32 SrdsProt; >=20 > SerdesProtocol (or SerDesProtocol, depending on which variant you choose)= . >=20 > > + LS1043A_DEVICE_CONFIG *Dcfg; >=20 > DeviceConfig >=20 > > + > > + Dcfg =3D (LS1043A_DEVICE_CONFIG *)LS1043A_DCFG_ADDRESS; SrdsProt = =3D > > + DcfgRead32 ((UINTN)&Dcfg->RcwSr[4]) & RCWSR_SRDS1_PRTCL_MASK; > > + SrdsProt >>=3D RCWSR_SRDS1_PRTCL_SHIFT; >=20 > RCWSR is not a generally understood abbreviation. Please either add a glo= ssary > section to the start of the file, or expand the abbreviation at every poi= nt of use. >=20 > > + > > + SerDesInstanceProbeLanes (SRDS_1, SrdsProt, > > + FixedPcdGet8 (PcdSerdesLanes), > > + SERDES_PRTCL_COUNT, > > + SerDesConfigTbl[SRDS_1], > > + SerDesLaneProbeCallback, > > + Arg); } > > + > > +/** > > + Function to return Serdes protocol map for all serdes available on b= oard. > > + > > + @param SerDesPrtclMap Pointer to Serdes protocl map. > > + > > +**/ > > +VOID > > +GetSerdesProtocolMaps ( > > + OUT UINT64 *SerDesPrtclMap > > + ) > > +{ > > + UINT32 SrdsProt; > > + LS1043A_DEVICE_CONFIG *Dcfg; >=20 > Both comments for previous function apply here too. >=20 > / > Leif Thank you Leif for the review. I am addressing your review comments. >=20 > > + EFI_STATUS Status; > > + > > + *SerDesPrtclMap =3D 0x0; > > + Dcfg =3D (LS1043A_DEVICE_CONFIG *)LS1043A_DCFG_ADDRESS; SrdsProt = =3D > > + DcfgRead32 ((UINTN)&Dcfg->RcwSr[4]) & RCWSR_SRDS1_PRTCL_MASK; > > + SrdsProt >>=3D RCWSR_SRDS1_PRTCL_SHIFT; > > + > > + Status =3D GetSerDesMap (SRDS_1, SrdsProt, FixedPcdGet8 (PcdSerdesLa= nes), > > + SERDES_PRTCL_COUNT, SerDesConfigTbl[SRDS_1], > > + SerDesPrtclMap); > > + > > + if (Status !=3D EFI_SUCCESS) { > > + DEBUG ((DEBUG_ERROR, "%a: failed for Serdes1 \n",__FUNCTION__)); > > + *SerDesPrtclMap =3D 0x0; > > + } > > +} > > -- > > 2.7.4 > >