From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by mx.groups.io with SMTP id smtpd.web12.32117.1591629178765388484 for ; Mon, 08 Jun 2020 08:12:59 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=Xff3yV7t; spf=pass (domain: nuviainc.com, ip: 209.85.221.65, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f65.google.com with SMTP id x14so17817762wrp.2 for ; Mon, 08 Jun 2020 08:12:58 -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=y8Z6UTpetCSMWRQKzVqcKCp24Z6CKea8ExnolsUajoo=; b=Xff3yV7tOXxpnKC5GB/pUI0XegKZFf8+IIPk6zrEFL129fG8qcBPuf4/w/JZ1Zfyku DRYlrP64RPeZFvwOO8IN5181Cq9ACng0crO2bcY9ueua504FWyya2vY2Mjgj4BvRkwWJ uVW9AubBRVeVjJaP92JZhYVddxxyGxig+5Txyh7+NgxDI9ytCqpN7F8SIFytZgEsgbJw j3wrCjoKSqKVFuJ+DB0PkEdPCtLn40o61tJ6O9P0anRm9Kh9GX8HLaUesl2MnMTS+8P6 odetvKlTirVOInNQnepYgAbElZu0wv5Pn+kkPwdtlpgLpZgVd8CsExOSEgDNkMcjcrub PdaQ== 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=y8Z6UTpetCSMWRQKzVqcKCp24Z6CKea8ExnolsUajoo=; b=SYjUYO5361G2oWh7T3a/3jhpROB8SMK/gOAlgrFeJoU1pQGgz1MBfK6rLIVs1QsxUn KoOqUlMKz7HiN7WUfyZNi046i4LhtpdTTPLleVbbSznErzYMiiTXnooNUUfaThI/pA6t PIBFAOta6NgjNW3MdNl5myui4pHaZDZtkKRqgaCREA0IlgkES+7RC03azWEYVKg3B1Fc IbdTKdCruygIDHTGwOUVTqAU6qwdh/lF44Cz2wK6IimJm4wyroUYigtGxwyfaVQBL5h1 OsfaTonWqezeJc+jcrLrpFM1bMaUii0tTsRDMHghpOisIIlK8+XvdnN01iu8b6BLe1l3 TSuw== X-Gm-Message-State: AOAM531FGfLKHxPw7bR4o0qBM/qfPnXtQ/QW0Pwa4/buckwH5gwCHF3L 8HZ8Bx0nAxW8L7VqjWeMZvxCpQ== X-Google-Smtp-Source: ABdhPJwond4evN3YpBV0NUrog99uHy8UHC3OhVCKhNhM46BtwtlO7lBJPYlglz+4uaY3xXX38cxirw== X-Received: by 2002:adf:c391:: with SMTP id p17mr22877855wrf.243.1591629177315; Mon, 08 Jun 2020 08:12:57 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id k26sm23134612wmi.27.2020.06.08.08.12.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 08 Jun 2020 08:12:56 -0700 (PDT) Date: Mon, 8 Jun 2020 16:12:54 +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 v2 1/3] Silicon/NXP/Library: Implement SerDesHelperLib Message-ID: <20200608151254.GP28566@vanye> References: <1591535750-15743-1-git-send-email-wasim.khan@oss.nxp.com> <1591535750-15743-2-git-send-email-wasim.khan@oss.nxp.com> MIME-Version: 1.0 In-Reply-To: <1591535750-15743-2-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 Sun, Jun 07, 2020 at 18:45:48 +0530, Wasim Khan wrote: > From: Wasim Khan > > Implement SerDesHelperLib to provide helper functions which > can be used for SoC specific SerDes configuration. > > Signed-off-by: Wasim Khan > --- > > Notes: > Changes in V2: > - Addressed review comments for structure, variable and function names > - Using BIT0 instead of 0x1u > > Silicon/NXP/NxpQoriqLs.dec | 1 + > Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf | 28 ++++ > Silicon/NXP/Include/Library/SerDesHelperLib.h | 64 ++++++++ > Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c | 164 ++++++++++++++++++++ > 4 files changed, 257 insertions(+) > > diff --git a/Silicon/NXP/NxpQoriqLs.dec b/Silicon/NXP/NxpQoriqLs.dec > index d4d3057af509..d09a1ae194be 100644 > --- a/Silicon/NXP/NxpQoriqLs.dec > +++ b/Silicon/NXP/NxpQoriqLs.dec > @@ -35,6 +35,7 @@ [PcdsFixedAtBuild.common] > gNxpQoriqLsTokenSpaceGuid.PcdNumPciController|0|UINT32|0x00000501 > gNxpQoriqLsTokenSpaceGuid.PcdPcieLutBase|0x0|UINT32|0x00000502 > gNxpQoriqLsTokenSpaceGuid.PcdPcieLutDbg|0x0|UINT32|0x00000503 > + gNxpQoriqLsTokenSpaceGuid.PcdSerDesLanes|0x0|UINT8|0x00000504 > > [PcdsDynamic.common] > gNxpQoriqLsTokenSpaceGuid.PcdPciCfgShiftEnable|FALSE|BOOLEAN|0x00000600 > diff --git a/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf > new file mode 100644 > index 000000000000..7a781620e449 > --- /dev/null > +++ b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.inf > @@ -0,0 +1,28 @@ > +## @file > +# > +# Copyright 2020 NXP > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +## > + > +[Defines] > + INF_VERSION = 0x0001001A > + BASE_NAME = SerDesHelperLib > + FILE_GUID = 2930e932-a700-41e8-80f9-f1a2dedd2c4f > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = SerDesHelperLib > + > +[Packages] > + MdePkg/MdePkg.dec > + Silicon/NXP/NxpQoriqLs.dec > + > +[LibraryClasses] > + DebugLib > + PcdLib > + > +[Sources.common] > + SerDesHelperLib.c > + > +[FixedPcd] > + gNxpQoriqLsTokenSpaceGuid.PcdSerDesLanes > diff --git a/Silicon/NXP/Include/Library/SerDesHelperLib.h b/Silicon/NXP/Include/Library/SerDesHelperLib.h > new file mode 100644 > index 000000000000..377f020e0b3a > --- /dev/null > +++ b/Silicon/NXP/Include/Library/SerDesHelperLib.h > @@ -0,0 +1,64 @@ > +/** SerDesHelperLib.h > + The Header file for SerDesHelperLib > + > + Copyright 2020 NXP > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > +**/ > + > +#ifndef SERDES_HELPER_LIB_H > +#define SERDES_HELPER_LIB_H > + > +#include > +#include > + > +typedef struct { > + UINT16 Protocol; > + UINT8 SerDesLane[FixedPcdGet8 (PcdSerDesLanes)]; > +} SERDES_CONFIG; > + > +typedef enum { > + SERDES_1 = 0, > + SERDES_2, > + SERDES_3, > + SERDES_MAX > +} SERDES_NUMBER; > + > +UINT32 > +GetSerDesProtocol ( > + IN INTN SerDes, > + IN INTN SerDesProtocol, > + IN INTN Lane, > + IN UINT32 SerDesMaxProtocol, > + IN SERDES_CONFIG *Config > + ); > + > +EFI_STATUS > +IsSerDesProtocolValid ( > + IN INTN SerDes, > + IN UINT32 SerDesProtocol, > + IN UINT8 SerDesNumLanes, > + IN SERDES_CONFIG *Config > + ); > + > +EFI_STATUS > +GetSerDesMap ( > + IN UINT32 SerDes, > + IN UINT32 SerDesProtocol, > + IN UINT8 SerDesNumLanes, > + IN UINT32 SerDesMaxProtocol, > + IN SERDES_CONFIG *Config, > + OUT UINT64 *SerDesProtocolMap > + ); > + > +VOID > +SerDesInstanceProbeLanes ( > + IN UINT32 SerDes, > + IN UINT32 SerDesProtocol, > + IN UINT8 SerDesNumLanes, > + IN UINT32 SerDesMaxProtocol, > + IN SERDES_CONFIG *Config, > + IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback, > + IN VOID *Arg > + ); > +#endif > diff --git a/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c > new file mode 100644 > index 000000000000..52c27a0fd49d > --- /dev/null > +++ b/Silicon/NXP/Library/SerDesHelperLib/SerDesHelperLib.c > @@ -0,0 +1,164 @@ > +/** SerDes.c > + Provides SoC specific SerDes interface > + > + Copyright 2020 NXP > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > +**/ > + > +#include > +#include > + > +/** > + Function to get SerDes Lane protocol corresponding to > + SerDes protocol. > + > + @param SerDes SerDes number. > + @param SerDesProtocol SerDes protocol number. > + @param Lane SerDes Lane number. > + @param SerDesMaxProtocol Max SerDes protocol number. > + @param Config SerDes Configuration. > + > + @return SerDes Lane protocol. > + > +**/ > +UINT32 > +GetSerDesProtocol ( > + IN INTN SerDes, > + IN INTN SerDesProtocol, > + IN INTN Lane, > + IN UINT32 SerDesMaxProtocol, > + IN SERDES_CONFIG *Config > + ) > +{ > + while (Config->Protocol) { > + if (Config->Protocol == SerDesProtocol) { > + return Config->SerDesLane[Lane]; > + } > + Config++; > + } > + > + return SerDesMaxProtocol; > +} > + > +/** > + Function to validate input SerDes protocol. > + > + @param SerDes SerDes number. > + @param SerDesProtocol SerDes protocol number. > + @param SerDesNumLanes Number of SerDes Lanes. > + @param Config SerDes Configuration. > + > + @return EFI_NOT_FOUND SerDes Protocol not a valid protocol. > + @return EFI_SUCCESS SerDes Protocol is a valid protocol. > + > +**/ > +EFI_STATUS > +IsSerDesProtocolValid ( > + IN INTN SerDes, > + IN UINT32 SerDesProtocol, > + IN UINT8 SerDesNumLanes, > + IN SERDES_CONFIG *Config > + ) > +{ > + UINT8 Count; > + > + while (Config->Protocol) { > + if (Config->Protocol == SerDesProtocol) { > + DEBUG ((DEBUG_INFO, "Protocol: %x Matched with the one in Table\n", SerDesProtocol)); > + break; > + } > + Config++; > + } > + > + if (!Config->Protocol) { > + return EFI_NOT_FOUND; > + } > + > + for (Count = 0; Count < SerDesNumLanes; Count++) { > + if (Config->SerDesLane[Count] != 0) { > + return EFI_SUCCESS; > + } > + } > + > + return EFI_NOT_FOUND; > +} > + > +/** > + Get Lane protocol on provided SerDes Lane and execute callback function. > + > + @param SerDes SerDes number. > + @param SerDesProtocol SerDes protocol number. > + @param SerDesNumLanes Number of SerDes Lanes. > + @param SerDesMaxProtocol Max SerDes protocol number. > + @param Config SerDes Configuration. > + @param SerDesLaneProbeCallback Pointer Callback function to be called for Lane protocol > + @param Arg Pointer to Arguments to be passed to callback function. > +**/ > +VOID > +SerDesInstanceProbeLanes ( > + IN UINT32 SerDes, > + IN UINT32 SerDesProtocol, > + IN UINT8 SerDesNumLanes, > + IN UINT32 SerDesMaxProtocol, > + IN SERDES_CONFIG *Config, > + IN SERDES_PROBE_LANES_CALLBACK SerDesLaneProbeCallback, > + IN VOID *Arg > + ) > +{ > + INT8 Lane; > + UINT32 LaneProtocol; > + > + // Invoke callback for all lanes in the SerDes instance: > + for (Lane = 0; Lane < SerDesNumLanes; Lane++) { > + LaneProtocol = GetSerDesProtocol (SerDes, SerDesProtocol, Lane, SerDesMaxProtocol, Config); > + ASSERT (LaneProtocol < SerDesMaxProtocol); > + if (LaneProtocol != 0x0) { > + SerDesLaneProbeCallback (LaneProtocol, Arg); > + } > + } > +} > + > +/** > + Function to fill SerDes map information. > + > + @param SerDes SerDes number. > + @param SerDesProtocol SerDes protocol number. > + @param SerDesNumLanes Number of SerDes Lanes. > + @param SerDesMaxProtocol Max SerDes protocol number. > + @param Config SerDes Configuration. > + @param SerDesProtocolMap Output SerDes protocol map of enabled devices. > + > +**/ > +EFI_STATUS > +GetSerDesMap ( > + IN UINT32 SerDes, > + IN UINT32 SerDesProtocol, > + IN UINT8 SerDesNumLanes, > + IN UINT32 SerDesMaxProtocol, > + IN SERDES_CONFIG *Config, > + OUT UINT64 *SerDesProtocolMap > + ) > +{ > + INTN Lane; > + EFI_STATUS Status; > + UINT32 LanePrtcl; Still some Prtcl left in this function. Other than that I'm happy with this. With that fixed: Reviewed-by: Leif Lindholm If that's the only comment I have on v2, I can fix that up before committing. If I ask for a v3 for 2 or 3/3, could you please include this update? > + > + Status = IsSerDesProtocolValid (SerDes, SerDesProtocol, SerDesNumLanes, Config); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: SERDES%d[PRTCL] = 0x%x is not valid, Status = %r \n", > + __FUNCTION__, SerDes + 1, SerDesProtocol, Status)); > + return Status; > + } > + > + for (Lane = 0; Lane < SerDesNumLanes; Lane++) { > + LanePrtcl = GetSerDesProtocol (SerDes, SerDesProtocol, Lane, SerDesMaxProtocol, Config); > + if (LanePrtcl >= SerDesMaxProtocol) { > + DEBUG ((DEBUG_ERROR, "Unknown SerDes lane protocol %d\n", LanePrtcl)); > + return EFI_NO_MAPPING; > + } > + *SerDesProtocolMap |= (BIT0 << (LanePrtcl)); > + } > + > + return EFI_SUCCESS; > +} > -- > 2.7.4 >