From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web11.9861.1591360591059844252 for ; Fri, 05 Jun 2020 05:36:31 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=d+GgWB7W; spf=pass (domain: nuviainc.com, ip: 209.85.221.66, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f66.google.com with SMTP id e1so9610716wrt.5 for ; Fri, 05 Jun 2020 05:36:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=9MOjYjwsigRSp9gIn46yB08MYDkJSpY0ZdZ//83qGtw=; b=d+GgWB7WOIeCVxfYHweBlcdFtHMjY+qzvIEGkFo6EM8x6OJ787SJkuKKFrqtu0vnN9 8sunryuzdz3pBzyRSz5VN4R5ZMLWeNunWTAcF7KNxdHAwjOXKd4hVXDFpBAdztcx7br8 gHjXcc0G93en1Q6jP/OOy+Eh57Sp99jI6LdmyrRW0QXsQ1ZYfBiCOPWx9ngYdaErk2hv apVStRJiIZibLvmpX/ZT6YkvNn+F1BCfatRlhOmmepLNP/tHxBFKlUklRXE541uzj5nQ Q51Ij8peytOhWFbBx7+vOwge2gv/bvgeoEUwOXUEPYvsLBRtTmvTyLzqsdV0wsKcZeIC 0Y6A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=9MOjYjwsigRSp9gIn46yB08MYDkJSpY0ZdZ//83qGtw=; b=VbakebknOOcDqZ0ZZ+xKRSyiDl1Ehg0WdPd6va9A/jyysYPhRSFOALgerELNRulRY4 w73/3Z6BpTrbUaecBoomOwcmzFbR2QFZDCWsrTnS4qyXEOYKNwgrRo/k9zzFPO6LYZNo qVCQTSus2R74fnsapFMjrO3mjd1Ve++cUruom0YncezCSvLYPZI8Eruu8h1CwcRYQo5W CpdAVEgJUfpT15UT4mp9qkjTmKHt3fhpm0BRj07mKELeJ535nWDiRkSTls6cIht+6kU6 uI76IKoCEkbOCvRUGnxloN7r7ClqltuzB7yHxleP0lvAOSak8p8D9DRSpQ+f8NntTES8 J+eA== X-Gm-Message-State: AOAM533JWC3noISaoxctzclvOYc5oVi6v46cXyEelwpXyF9fCwHqNlwu G3tw+x5Dld49Oc8I4WNXmgM4AA== X-Google-Smtp-Source: ABdhPJxQ7SearmTBTdPvl4rRN9vYRq0+H9P4ShooMk3VIXQoY5fWvozqaNhqxAGufMGMC4IxO1TxOQ== X-Received: by 2002:adf:fc81:: with SMTP id g1mr9674048wrr.156.1591360589585; Fri, 05 Jun 2020 05:36:29 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id k16sm11960436wrp.66.2020.06.05.05.36.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 05 Jun 2020 05:36:28 -0700 (PDT) Date: Fri, 5 Jun 2020 13:36:26 +0100 From: "Leif Lindholm" To: Wasim Khan Cc: devel@edk2.groups.io, meenakshi.aggarwal@nxp.com, V.Sethi@nxp.com, ard.biesheuvel@arm.com, Wasim Khan Subject: Re: [PATCH edk2-platforms 2/3] Silicon/NXP: LS1043A: Add Serdes Support Message-ID: <20200605123626.GE28566@vanye> References: <1591039658-18541-1-git-send-email-wasim.khan@oss.nxp.com> <1591039658-18541-3-git-send-email-wasim.khan@oss.nxp.com> MIME-Version: 1.0 In-Reply-To: <1591039658-18541-3-git-send-email-wasim.khan@oss.nxp.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 Insert alphabetically sorted. > + > +[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 = 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[] = { No data definitions in header files please. Also, global variables should have 'm' or 'g' prefix. > + /* 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[] = { No data definitions in header files please. Also, global variables should have 'm' or 'g' prefix. 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 intended to mean "Table", and here it is supposed to mean "Tables"? The reader is not supposed to have to figure that out. > + 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 callback function. > + > + @param SerDesLaneProbeCallback Pointer Callback function to be called 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; SerdesProtocol (or SerDesProtocol, depending on which variant you choose). > + LS1043A_DEVICE_CONFIG *Dcfg; DeviceConfig > + > + Dcfg = (LS1043A_DEVICE_CONFIG *)LS1043A_DCFG_ADDRESS; > + SrdsProt = DcfgRead32 ((UINTN)&Dcfg->RcwSr[4]) & RCWSR_SRDS1_PRTCL_MASK; > + SrdsProt >>= RCWSR_SRDS1_PRTCL_SHIFT; RCWSR is not a generally understood abbreviation. Please either add a glossary section to the start of the file, or expand the abbreviation at every point of use. > + > + 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 board. > + > + @param SerDesPrtclMap Pointer to Serdes protocl map. > + > +**/ > +VOID > +GetSerdesProtocolMaps ( > + OUT UINT64 *SerDesPrtclMap > + ) > +{ > + UINT32 SrdsProt; > + LS1043A_DEVICE_CONFIG *Dcfg; Both comments for previous function apply here too. / Leif > + EFI_STATUS Status; > + > + *SerDesPrtclMap = 0x0; > + Dcfg = (LS1043A_DEVICE_CONFIG *)LS1043A_DCFG_ADDRESS; > + SrdsProt = DcfgRead32 ((UINTN)&Dcfg->RcwSr[4]) & RCWSR_SRDS1_PRTCL_MASK; > + SrdsProt >>= RCWSR_SRDS1_PRTCL_SHIFT; > + > + Status = GetSerDesMap (SRDS_1, SrdsProt, FixedPcdGet8 (PcdSerdesLanes), > + SERDES_PRTCL_COUNT, SerDesConfigTbl[SRDS_1], > + SerDesPrtclMap); > + > + if (Status != EFI_SUCCESS) { > + DEBUG ((DEBUG_ERROR, "%a: failed for Serdes1 \n",__FUNCTION__)); > + *SerDesPrtclMap = 0x0; > + } > +} > -- > 2.7.4 >