public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Leif Lindholm <leif.lindholm@linaro.org>
To: Ming Huang <ming.huang@linaro.org>
Cc: linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org,
	graeme.gregory@linaro.org, ard.biesheuvel@linaro.org,
	guoheyi@huawei.com, wanghuiqiang@huawei.com,
	huangming23@huawei.com, zhangjinsong2@huawei.com,
	huangdaode@hisilicon.com, john.garry@huawei.com,
	xinliang.liu@linaro.org, Zhou You <zhouyou17@huawei.com>
Subject: Re: [PATCH edk2-platforms v1 01/38] Silicon/Hisilicon: Modify the MRC interface for other module
Date: Thu, 2 Aug 2018 15:42:42 +0100	[thread overview]
Message-ID: <20180802144242.2t3lfo34dol53vic@bivouac.eciton.net> (raw)
In-Reply-To: <20180724070922.63362-2-ming.huang@linaro.org>

On Tue, Jul 24, 2018 at 03:08:45PM +0800, Ming Huang wrote:
> This patch is to unify D0x. Add pGBL_INTERFACE struct define
> and remove useless interfece. Replace DMRC pGblData with pGblInterface;
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhou You <zhouyou17@huawei.com>
> Signed-off-by: Ming Huang <ming.huang@linaro.org>
> ---
>  Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c     |   4 +-
>  Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.c |  22 +-
>  Silicon/Hisilicon/Include/Library/HwMemInitLib.h                    | 351 ++++----------------

Please add an orderfile as described in
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-10
in order to make api/stuct changes are visible before changes to their
use.

You can set one permanently for your repository using
$ git config diff.orderFile <full path to orderfile>

>  3 files changed, 74 insertions(+), 303 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c b/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c
> index 7d06fccc2b..f5869841dc 100644
> --- a/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c
> +++ b/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c
> @@ -56,7 +56,7 @@ UpdateSrat (
>    UINT8               Skt = 0;
>    UINTN               Index = 0;
>    VOID                *HobList;
> -  GBL_DATA            *Gbl_Data;
> +  GBL_INTERFACE       *Gbl_Data;
>    UINTN               Base;
>    UINTN               Size;
>    UINT8               NodeId;
> @@ -69,7 +69,7 @@ UpdateSrat (
>    if (HobList == NULL) {
>      return EFI_UNSUPPORTED;
>    }
> -  Gbl_Data = (GBL_DATA*)GetNextGuidHob(&gHisiEfiMemoryMapGuid, HobList);
> +  Gbl_Data = (GBL_INTERFACE*)GetNextGuidHob(&gHisiEfiMemoryMapGuid, HobList);
>    if (Gbl_Data == NULL) {
>      DEBUG((DEBUG_ERROR, "Get next Guid HOb fail.\n"));
>      return EFI_NOT_FOUND;
> diff --git a/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.c b/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.c
> index da714c9e22..262b129419 100644
> --- a/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.c
> +++ b/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.c
> @@ -45,7 +45,7 @@ SmbiosGetManufacturer (
>  
>  VOID
>  SmbiosGetPartNumber (
> -  IN pGBL_DATA          pGblData,
> +  IN pGBL_INTERFACE     pGblData,
>    IN UINT8              Skt,
>    IN UINT8              Ch,
>    IN UINT8              Dimm,
> @@ -78,7 +78,7 @@ SmbiosGetPartNumber (
>  
>  VOID
>  SmbiosGetSerialNumber (
> -  IN pGBL_DATA          pGblData,
> +  IN pGBL_INTERFACE     pGblData,
>    IN UINT8              Skt,
>    IN UINT8              Ch,
>    IN UINT8              Dimm,
> @@ -96,7 +96,7 @@ SmbiosGetSerialNumber (
>  
>  BOOLEAN
>  IsDimmPresent (
> -  IN  pGBL_DATA          pGblData,
> +  IN  pGBL_INTERFACE     pGblData,
>    IN  UINT8              Skt,
>    IN  UINT8              Ch,
>    IN  UINT8              Dimm
> @@ -115,7 +115,7 @@ IsDimmPresent (
>  
>  UINT8
>  SmbiosGetMemoryType (
> -  IN  pGBL_DATA          pGblData,
> +  IN  pGBL_INTERFACE     pGblData,
>    IN  UINT8              Skt,
>    IN  UINT8              Ch,
>    IN  UINT8              Dimm
> @@ -146,7 +146,7 @@ SmbiosGetMemoryType (
>  
>  VOID
>  SmbiosGetTypeDetail (
> -  IN  pGBL_DATA             pGblData,
> +  IN  pGBL_INTERFACE        pGblData,
>    IN  UINT8                 Skt,
>    IN  UINT8                 Ch,
>    IN  UINT8                 Dimm,
> @@ -186,7 +186,7 @@ SmbiosGetTypeDetail (
>  
>  VOID
>  SmbiosGetDimmVoltageInfo (
> -  IN     pGBL_DATA             pGblData,
> +  IN     pGBL_INTERFACE        pGblData,
>    IN     UINT8                 Skt,
>    IN     UINT8                 Ch,
>    IN     UINT8                 Dimm,
> @@ -281,7 +281,7 @@ SmbiosGetPartitionWidth (
>  
>  EFI_STATUS
>  SmbiosAddType16Table (
> -  IN  pGBL_DATA          pGblData,
> +  IN  pGBL_INTERFACE     pGblData,
>    OUT EFI_SMBIOS_HANDLE  *MemArraySmbiosHandle
>    )
>  {
> @@ -345,7 +345,7 @@ SmbiosAddType16Table (
>  
>  EFI_STATUS
>  SmbiosAddType19Table (
> -  IN pGBL_DATA          pGblData,
> +  IN pGBL_INTERFACE     pGblData,
>    IN EFI_SMBIOS_HANDLE  MemArraySmbiosHandle
>    )
>  {
> @@ -397,7 +397,7 @@ SmbiosAddType19Table (
>  
>  EFI_STATUS
>  SmbiosAddType17Table (
> -  IN pGBL_DATA          pGblData,
> +  IN pGBL_INTERFACE     pGblData,
>    IN UINT8              Skt,
>    IN UINT8              Ch,
>    IN UINT8              Dimm,
> @@ -692,7 +692,7 @@ MemorySubClassEntryPoint(
>      EFI_STATUS                      Status;
>      EFI_SMBIOS_PROTOCOL             *Smbios;
>      EFI_HOB_GUID_TYPE               *GuidHob;
> -    pGBL_DATA                       pGblData;
> +    pGBL_INTERFACE                  pGblData;
>      EFI_SMBIOS_HANDLE               MemArraySmbiosHandle;
>      UINT8                           Skt, Ch, Dimm;
>  
> @@ -702,7 +702,7 @@ MemorySubClassEntryPoint(
>          DEBUG((EFI_D_ERROR, "Could not get MemoryMap Guid hob.  %r\n"));
>          return EFI_NOT_FOUND;
>      }
> -    pGblData = (pGBL_DATA) GET_GUID_HOB_DATA(GuidHob);
> +    pGblData = (pGBL_INTERFACE) GET_GUID_HOB_DATA(GuidHob);
>  
>      //
>      // Locate dependent protocols
> diff --git a/Silicon/Hisilicon/Include/Library/HwMemInitLib.h b/Silicon/Hisilicon/Include/Library/HwMemInitLib.h
> index 2663cad836..2be90d35c7 100644
> --- a/Silicon/Hisilicon/Include/Library/HwMemInitLib.h
> +++ b/Silicon/Hisilicon/Include/Library/HwMemInitLib.h
> @@ -50,48 +50,6 @@ typedef enum {
>      DDR_FREQ_MAX
>  } DDR_FREQUENCY_INDEX;
>  
> -typedef struct _DDR_FREQ_TCK
> -{
> -    UINT32      ddrFreq;
> -    UINT32      ddrCk;
> -}DDR_FREQ_TCK;
> -
> -typedef struct _GBL_CFG{
> -
> -
> -}GBL_CFG;
> -
> -typedef struct _GBL_VAR{
> -
> -
> -}GBL_VAR;
> -
> -typedef struct _GBL_NVDATA{
> -
> -
> -}GBL_NVDATA;
> -
> -typedef struct _GOBAL {
> -    const GBL_CFG Config;  // constant input data
> -    GBL_VAR       Variable;    // variable, volatile data
> -    GBL_NVDATA    NvData;  // variable, non-volatile data for S3, warm boot path
> -    UINT32        PreBootFailed;
> -}GOBAL, *PGOBAL;
> -
> -struct DDR_RANK {
> -    BOOLEAN     Status;
> -    UINT16      RttNom;
> -    UINT16      RttPark;
> -    UINT16      RttWr;
> -    UINT16      MR0;
> -    UINT16      MR1;
> -    UINT16      MR2;
> -    UINT16      MR3;
> -    UINT16      MR4;
> -    UINT16      MR5;
> -    UINT16      MR6[9];
> -};
> -
>  struct baseMargin {
>    INT16 n;
>    INT16 p;
> @@ -101,171 +59,7 @@ struct rankMargin {
>    struct baseMargin rank[MAX_CHANNEL][MAX_RANK_CH];
>  };
>  
> -typedef struct _DDR_DIMM{
> -    BOOLEAN     Status;
> -    UINT8       mapout;
> -    UINT8       DramType;           //Byte 2
> -    UINT8       ModuleType;         //Byte 3
> -    UINT8       ExtendModuleType;
> -    UINT8       SDRAMCapacity;      //Byte 4
> -    UINT8       BankNum;
> -    UINT8       BGNum;              //Byte 4 For DDR4
> -    UINT8       RowBits;            //Byte 5
> -    UINT8       ColBits;            //Byte 5
> -    UINT8       SpdVdd;             //Byte 6
> -    UINT8       DramWidth;          //Byte 7
> -    UINT8       RankNum;            //Byte 7
> -    UINT8       PrimaryBusWidth;    //Byte 8
> -    UINT8       ExtensionBusWidth;  //Byte 8
> -    UINT32      Mtb;
> -    UINT32      Ftb;
> -    UINT32      minTck;
> -    UINT8       MtbDividend;
> -    UINT8       MtbDivsor;
> -    UINT8       nCL;
> -    UINT32      nRCD;
> -    UINT32      nRP;
> -    UINT8       SPDftb;
> -    UINT8       SpdMinTCK;
> -    UINT8       SpdMinTCKFtb;
> -    UINT8       SpdMaxTCK;
> -    UINT8       SpdMinTCL;
> -    UINT8       SpdMinTCLFtb;
> -    UINT8       SpdMinTWR;
> -    UINT8       SpdMinTRCD;
> -    UINT8       SpdMinTRCDFtb;
> -    UINT8       SpdMinTRRD;
> -    UINT8       SpdMinTRRDL;
> -    UINT16      SpdMinTRAS;
> -    UINT16      SpdMinTRC;
> -    UINT16      SpdMinTRCFtb;
> -    UINT16      SpdMinTRFC;
> -    UINT8       SpdMinTWTR;
> -    UINT8       SpdMinTRTP;
> -    UINT8       SpdMinTAA;
> -    UINT8       SpdMinTAAFtb;
> -    UINT8       SpdMinTFAW;
> -    UINT8       SpdMinTRP;
> -    UINT8       SpdMinTRPFtb;
> -    UINT8       SpdMinTCCDL;
> -    UINT8       SpdMinTCCDLFtb;
> -    UINT8       SpdAddrMap;
> -    UINT8       SpdModuleAttr;
> -
> -    UINT8       SpdModPart[SPD_MODULE_PART];         // Module Part Number
> -    UINT8       SpdModPartDDR4[SPD_MODULE_PART_DDR4];     // Module Part Number DDR4
> -    UINT16      SpdMMfgId;              // Module Mfg Id from SPD
> -    UINT16      SpdRMId;              // Register Manufacturer Id
> -    UINT16      SpdMMDate;              // Module Manufacturing Date
> -    UINT32      SpdSerialNum;
> -    UINT16      DimmSize;
> -    UINT16      DimmSpeed;
> -    UINT32      RankSize;
> -    UINT8       SpdMirror; //Denote the dram address mapping is standard mode or mirrored mode
> -    struct DDR_RANK  Rank[MAX_RANK_DIMM];
> -}DDR_DIMM;
> -
> -typedef struct {
> -    UINT32      ddrcTiming0;
> -    UINT32      ddrcTiming1;
> -    UINT32      ddrcTiming2;
> -    UINT32      ddrcTiming3;
> -    UINT32      ddrcTiming4;
> -    UINT32      ddrcTiming5;
> -    UINT32      ddrcTiming6;
> -    UINT32      ddrcTiming7;
> -    UINT32      ddrcTiming8;
> -}DDRC_TIMING;
> -
> -typedef struct _MARGIN_RESULT{
> -    UINT32 OptimalDramVref[12];
> -    UINT32 optimalPhyVref[18];
> -}MARGIN_RESULT;
> -
> -typedef struct _DDR_Channel{
> -    BOOLEAN     Status;
> -    UINT8       CurrentDimmNum;
> -    UINT8       CurrentRankNum;
> -    UINT16      RankPresent;
> -    UINT8       DramType;
> -    UINT8       DramWidth;
> -    UINT8       ModuleType;
> -    UINT32      MemSize;
> -    UINT32      tck;
> -    UINT32      ratio;
> -    UINT32      CLSupport;
> -    UINT32      minTck;
> -    UINT32      taref;
> -    UINT32      nAA;
> -    UINT32      nAOND;
> -    UINT32      nCKE;
> -    UINT32      nCL;
> -    UINT32      nCCDL;
> -    UINT32      nCKSRX;
> -    UINT32      nCKSRE;
> -    UINT32      nCCDNSW;
> -    UINT32      nCCDNSR;
> -    UINT32      nFAW;
> -    UINT32      nMRD;
> -    UINT32      nMOD;
> -    UINT32      nRCD;
> -    UINT32      nRRD;
> -    UINT32      nRRDL;
> -    UINT32      nRAS;
> -    UINT32      nRC;
> -    UINT32      nRFC;
> -    UINT32      nRFCAB;
> -    UINT32      nRTP;
> -    UINT32      nRTW;
> -    UINT32      nRP;
> -    UINT32      nSRE;
> -    UINT32      nWL;
> -    UINT32      nWR;
> -    UINT32      nWTR;
> -    UINT32      nWTRL;
> -    UINT32      nXARD;
> -    UINT32      nZQPRD;
> -    UINT32      nZQINIT;
> -    UINT32      nZQCS;
> -    UINT8       cwl;  //tWL?
> -    UINT8       pl;     //parity latency
> -    UINT8       wr_pre_2t_en;
> -    UINT8       rd_pre_2t_en;
> -    UINT8       cmd_2t_en;
> -    UINT8       parity_en;
> -    UINT8       wr_dbi_en;
> -    UINT8       wr_dm_en;
> -    UINT8       ddr4_crc_en;
> -    UINT16      emrs0;
> -    UINT16      emrs1;
> -    UINT16      emrs1Wr;
> -    UINT16      emrs2;
> -    UINT16      emrs3;
> -    UINT16      emrs4;
> -    UINT16      emrs5;
> -    UINT16      emrs5Wr;
> -    UINT16      emrs6;
> -    UINT16      emrs7;
> -    UINT8       phy_rddata_set;
> -    UINT8       phyif_tim_rdcs;
> -    UINT8       phyif_tim_rden;
> -    UINT8       phyif_tim_wden;
> -    UINT8       phyif_tim_wdda;
> -    UINT8       phyif_tim_wdcs;
> -    UINT8       per_cs_training_en;
> -    UINT32      phyRdDataEnIeDly;
> -    UINT32      phyPadCalConfig;
> -    UINT32      phyDqsFallRiseDelay;
> -    UINT32      ddrcCfgDfiLat0;
> -    UINT32      ddrcCfgDfiLat1;
> -    UINT32      parityLatency;
> -    UINT32      dimm_parity_en;
> -    DDRC_TIMING ddrcTiming;
> -    DDR_DIMM    Dimm[MAX_DIMM];
> -    MARGIN_RESULT sMargin;
> -}DDR_CHANNEL;
> -
> -typedef struct _NVRAM_RANK{
> +typedef struct _NVRAM_RANK_DATA{

Space before {
(Applies throughout, but please don't fix up structs otherwise
untouched by this patch. Unless you do it as a separate patch
preceding this one.)

>      UINT16      MR0;
>      UINT16      MR1;
>      UINT16      MR2;
> @@ -273,15 +67,15 @@ typedef struct _NVRAM_RANK{
>      UINT16      MR4;
>      UINT16      MR5;
>      UINT16      MR6[9];
> -}NVRAM_RANK;
> +}NVRAM_RANK_DATA;

Similarly, space after {.
Also applies throughout.

>  
> -typedef struct _NVRAM_DIMM{
> -    NVRAM_RANK      Rank[MAX_RANK_DIMM];
> -}NVRAM_DIMM;
> +typedef struct _NVRAM_DIMM_DATA{
> +    NVRAM_RANK_DATA      Rank[MAX_RANK_DIMM];
> +}NVRAM_DIMM_DATA;
>  
>  
> -typedef struct _NVRAM_CHANNEL{
> -    NVRAM_DIMM      Dimm[MAX_DIMM];
> +typedef struct _NVRAM_CHANNEL_DATA{
> +    NVRAM_DIMM_DATA      Dimm[MAX_DIMM];
>      UINT32          DDRC_CFG_ECC;
>      UINT32          DDRC_CFG_WORKMODE;
>      UINT32          DDRC_CFG_WORKMODE1;
> @@ -325,94 +119,71 @@ typedef struct _NVRAM_CHANNEL{
>      UINT32          DDRC_CFG_DDRPHY;
>      UINT32          Config[24];
>      BOOLEAN         Status;
> -}NVRAM_CHANNEL;
> +}NVRAM_CHANNEL_DATA;
> +
> +typedef struct _NVRAM_DATA{
> +    UINT32              NvramCrc;
> +    NVRAM_CHANNEL_DATA  Channel[MAX_SOCKET][MAX_CHANNEL];
> +    UINT32              DdrFreqIdx;
>  
> -typedef struct _NVRAM{
> -    UINT32          NvramCrc;
> -    NVRAM_CHANNEL   Channel[MAX_SOCKET][MAX_CHANNEL];
> -    UINT32          DdrFreqIdx;
> +}NVRAM_DATA;
>  
> -}NVRAM;
> +struct DDR_RANK_DATA {
> +    BOOLEAN     Status;

Status is not a boolean thing.
A boolean is for "active" or "enabled" or something suchlike, so you
can write
  if (entry->enabled) {
.

Whereas
  if (entry->status) {
carries no meaning that can be discerned without fully reading through
all code touching that variable.

> +};
> +
> +typedef struct _DDR_DIMM_DATA {
> +    BOOLEAN         Status;

Status is not a boolean thing.

> +    UINT8           DramType;           //Byte 2
> +    UINT8           ModuleType;         //Byte 3
> +    UINT8           BankNum;            //Byte 4,??DDR4,?????BankGroup??Bank??

Some encoding issue in comment.

> +    UINT8           RowBits;            //Byte 5
> +    UINT8           ColBits;            //Byte 5
> +    UINT8           SpdVdd;             //Byte 6
> +    UINT8           RankNum;            //Byte 7
> +    UINT8           PrimaryBusWidth;    //Byte 8
> +    UINT8           ExtensionBusWidth;  //Byte 8
> +    UINT8           SpdModPart[SPD_MODULE_PART];                // Module Part Number
> +    UINT8           SpdModPartDDR4[SPD_MODULE_PART_DDR4];       // Module Part Number DDR4
> +    UINT16          SpdMMfgId;              // Module Mfg Id from SPD
> +    UINT32          SpdSerialNum;
> +    UINT32          RankSize;
> +    UINT16          DimmSize;
> +    UINT16          DimmSpeed;
> +    UINT16          SpdMMDate;
> +    struct DDR_RANK_DATA    Rank[MAX_RANK_DIMM];
> +}DDR_DIMM_DATA;
> +
> +typedef struct _DDR_CHANNEL_DATA {
> +    BOOLEAN         Status;
> +    DDR_DIMM_DATA   Dimm[MAX_DIMM];
> +    UINT8           CurrentDimmNum;
> +}DDR_CHANNEL_DATA;
>  
> -typedef struct _MEMORY{
> -    UINT8           Config0;
> -    UINT8           marginTest;
> -    UINT8           Config1[5];
> -    UINT8           ErrorBypass; //register of spd mirror mode
> -    UINT32          Config2;
> -}MEMORY;
> +typedef struct _MEMORY_DATA {
> +    UINT8           rascBypass;

RascBypass.

> +}MEMORY_DATA;
>  
> -typedef struct _NUMAINFO{
> +typedef struct _NUMAINFO_DATA {
>      UINT8           NodeId;
>      UINT64          Base;
>      UINT64          Length;
>      UINT32          ScclInterleaveEn;
> -}NUMAINFO;
> +}NUMAINFO_DATA;
>  
>  
> -typedef struct _GBL_DATA
> +typedef struct _GBL_DATA_INTERFACE
>  {
> -    DDR_CHANNEL Channel[MAX_SOCKET][MAX_CHANNEL];
> -    UINT8       DramType;
> -    UINT8       CurrentDimmNum;
> -    UINT8       CurrentRankNum;
> -    UINT8       MaxSPCNum;
> -    UINT32      Freq;
> -    UINT32      SpdTckMtb;
> -    UINT32      SpdTckFtb;
> -    UINT32      SpdTck;
> -    UINT32      Tck;
> -    UINT32      DdrFreqIdx;
> -    UINT32      DevParaFreqIdx; //Maximum frequency of DDR device
> -    UINT32      MemSize;
> -    UINT32      EccEn;
> -
> -    BOOLEAN     SetupExist;
> -    UINT8       warmReset;
> -    UINT8       needColdReset;
> -
> -    UINT8       cl;
> -    UINT8       cwl;
> -    UINT8       pl;
> -    UINT8       wr_pre_2t_en;
> -    UINT8       rd_pre_2t_en;
> -    UINT8       cmd_2t_en;
> -    UINT8       ddr4_parity_en;
> -    UINT8       wr_dbi_en;
> -    UINT8       wr_dm_en;
> -    UINT8       ddr4_crc_en;
> -    UINT16      emrs0;
> -    UINT16      emrs1;
> -    UINT16      emrs2;
> -    UINT16      emrs3;
> -    UINT16      emrs4;
> -    UINT16      emrs5;
> -    UINT16      emrs6;
> -    UINT16      emrs7;
> -    UINT8       phy_rddata_set;
> -    UINT8       phyif_tim_rdcs;
> -    UINT8       phyif_tim_rden;
> -    UINT8       phyif_tim_wden;
> -    UINT8       phyif_tim_wdda;
> -    UINT8       phyif_tim_wdcs;
> -    UINT8       dimm_trtr;
> -    UINT8       dimm_twtw;
> -    UINT8       rnk_trtr;
> -    UINT8       rnk_twtw;
> -    UINT8       rnk_trtw;
> -    UINT8       rnk_twtr;
> -    UINT8       per_cs_training_en;
> -    UINT8       scale;
> -    UINT8       ddrFreq;
> -    UINT8       debugNeed;
> -    UINT8       ddr3OdtEnable;
> -    double      fprd;
> -    BOOLEAN     chipIsEc;
> -    NVRAM       nvram;
> -    MEMORY      mem;
> -    NUMAINFO    NumaInfo[MAX_SOCKET][MAX_NUM_PER_TYPE];
> -
> -}GBL_DATA, *pGBL_DATA;
> +    DDR_CHANNEL_DATA        Channel[MAX_SOCKET][MAX_CHANNEL];
> +    UINT32                  DdrFreqIdx;
> +    UINT32                  Freq;
> +    UINT32                  EccEn;
> +    UINT32                  MemSize;
> +    BOOLEAN                 SetupExist;
> +    NVRAM_DATA              nvram;

A bit too short a name. (Yes, I know it the code being removed already
had this, but I was too stressed when I reviewed that.)

I think this should be NvRamInfo or NvRamData.

> +    MEMORY_DATA             mem;

As above, too short a name.
Something descriptive please.

> +    NUMAINFO_DATA           NumaInfo[MAX_SOCKET][MAX_NUM_PER_TYPE];
> +}GBL_INTERFACE, *pGBL_INTERFACE;

Please drop the *pGBL_INTERFACE typedef, it removes standardised
semantic information provided by C and reduces readability.

/
    Leif

>  
>  typedef union {
>      struct {
> -- 
> 2.17.0
> 


  reply	other threads:[~2018-08-02 14:42 UTC|newest]

Thread overview: 153+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-24  7:08 [PATCH edk2-platforms v1 00/38] Upload for D06 platform Ming Huang
2018-07-24  7:08 ` [PATCH edk2-platforms v1 01/38] Silicon/Hisilicon: Modify the MRC interface for other module Ming Huang
2018-08-02 14:42   ` Leif Lindholm [this message]
2018-08-05  3:35     ` Ming
2018-07-24  7:08 ` [PATCH edk2-platforms v1 02/38] Silicon/Hisilicon: Separate PlatformArch.h Ming Huang
2018-08-02 14:56   ` Leif Lindholm
2018-08-05 15:11     ` Ming
2018-07-24  7:08 ` [PATCH edk2-platforms v1 03/38] Hisilicon/D06: Add several basal file for D06 Ming Huang
2018-08-02 16:14   ` Leif Lindholm
2018-08-05 15:06     ` Ming
2018-08-06  3:01       ` Ming
2018-08-06  9:57         ` Leif Lindholm
2018-07-24  7:08 ` [PATCH edk2-platforms v1 04/38] Platform/Hisilicon/D06: Add M41T83RealTimeClockLib Ming Huang
2018-08-02 16:56   ` Leif Lindholm
2018-08-08  8:02     ` Ming
2018-08-08  9:12       ` Leif Lindholm
2018-07-24  7:08 ` [PATCH edk2-platforms v1 05/38] Platform/Hisilicon/D06: Add binary file for D06 Ming Huang
2018-08-02 17:05   ` Leif Lindholm
2018-08-07 15:04     ` Ming
2018-07-24  7:08 ` [PATCH edk2-platforms v1 06/38] Hisilicon/D06: Add OemMiscLibD06 Ming Huang
2018-08-02 17:22   ` Leif Lindholm
2018-08-08  3:49     ` Ming
2018-08-08  9:43       ` Leif Lindholm
2018-07-24  7:08 ` [PATCH edk2-platforms v1 07/38] Silicon/Hisilicon/D06: Wait for all disk ready Ming Huang
2018-08-02 17:36   ` Leif Lindholm
2018-08-08  9:02     ` Ming
2018-08-08  9:59       ` Leif Lindholm
2018-08-08 11:44         ` Ming
2018-08-08 12:53           ` Leif Lindholm
2018-08-10  1:44             ` Ming
2018-08-14 15:26               ` Leif Lindholm
2018-08-15  4:01                 ` Ming
2018-08-15 13:12                   ` Leif Lindholm
2018-07-24  7:08 ` [PATCH edk2-platforms v1 08/38] Silicon/Hisilicon/Acpi: Unify HisiAcipPlatformDxe Ming Huang
2018-08-02 17:39   ` Leif Lindholm
2018-07-24  7:08 ` [PATCH edk2-platforms v1 09/38] Hisilicon/D06: Add Debug Serial Port Init Driver Ming Huang
2018-08-02 18:10   ` Leif Lindholm
2018-08-08  7:37     ` Ming
2018-08-08 10:01       ` Leif Lindholm
2018-08-08 14:48         ` Ming
2018-07-24  7:08 ` [PATCH edk2-platforms v1 10/38] Hisilicon/D06: Add ACPI Tables for D06 Ming Huang
2018-08-02 18:13   ` Leif Lindholm
2018-08-02 18:24     ` Ard Biesheuvel
2018-07-24  7:08 ` [PATCH edk2-platforms v1 11/38] Hisilicon/D06: Add Hi1620OemConfigUiLib Ming Huang
2018-08-03 10:24   ` Leif Lindholm
2018-08-08 12:09     ` Ming
2018-08-11  6:35     ` Ming
2018-07-24  7:08 ` [PATCH edk2-platforms v1 12/38] Silicon/Hisilicon/D06: Stop watchdog Ming Huang
2018-08-03 10:28   ` Leif Lindholm
2018-08-08  9:49     ` Ming
2018-08-03 10:31   ` Ard Biesheuvel
2018-08-09 11:40     ` Ming
2018-08-09 11:53       ` Leif Lindholm
2018-07-24  7:08 ` [PATCH edk2-platforms v1 13/38] Silicon/Hisilicon/Acpi: Move some macro to PlatformArch.h Ming Huang
2018-08-03 10:37   ` Leif Lindholm
2018-08-08 12:22     ` Ming
2018-08-08 12:57       ` Leif Lindholm
2018-07-24  7:08 ` [PATCH edk2-platforms v1 14/38] Silicon/Hisilicon/D06: Fix I2C enable fail issue for D06 Ming Huang
2018-08-03 10:40   ` Leif Lindholm
2018-08-08 14:33     ` Ming
2018-07-24  7:08 ` [PATCH edk2-platforms v1 15/38] Silicon/Hisilicon/I2C: Optimize I2C library Ming Huang
2018-08-03 13:24   ` Leif Lindholm
2018-08-08 14:41     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 16/38] Silicon/Hisilicon/D06: Add I2C delay for HNS auto config Ming Huang
2018-08-03 13:28   ` Leif Lindholm
2018-08-09  3:59     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 17/38] Silicon/Hisilicon/D06: Optimize HNS config CDR post time Ming Huang
2018-08-03 13:30   ` Leif Lindholm
2018-08-08 14:54     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 18/38] Silicon/Hisilicon/Setup: Add Setup Item "EnableGOP" Ming Huang
2018-08-03 13:32   ` Leif Lindholm
2018-08-09  0:35     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 19/38] Hisilicon/Hi1620: Add ACPI PPTT table Ming Huang
2018-08-03 13:42   ` Leif Lindholm
2018-08-09  0:52     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 20/38] Platform/Hisilicon/D06: Enable ACPI PPTT Ming Huang
2018-08-03 13:43   ` Leif Lindholm
2018-07-24  7:09 ` [PATCH edk2-platforms v1 21/38] Silicon/Hisilicon/D0x: Move macro definition to PlatformArch.h Ming Huang
2018-08-03 13:44   ` Leif Lindholm
2018-07-24  7:09 ` [PATCH edk2-platforms v1 22/38] Platform/Hisilicon/D06: Add OemNicLib Ming Huang
2018-08-03 14:36   ` Leif Lindholm
2018-08-09  6:16     ` Ming
2018-08-09 10:19       ` Leif Lindholm
2018-08-09 14:41         ` Ming
2018-08-14  2:38         ` Ming
2018-08-14 15:48           ` Leif Lindholm
2018-08-15 11:08             ` Ming
2018-08-15 13:22               ` Leif Lindholm
2018-08-15 14:16                 ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 23/38] Hisilicon/D0X: Rename the global variable gDS3231RtcDevice Ming Huang
2018-08-03 15:20   ` Leif Lindholm
2018-08-09  6:22     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 24/38] Platform/Hisilicon/D06: Add OemNicConfig2P Driver Ming Huang
2018-08-03 15:23   ` Leif Lindholm
2018-08-09  6:24     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 25/38] Hisilicon/D0x: Update SMBIOS type9 info Ming Huang
2018-08-04  9:28   ` Leif Lindholm
2018-08-09  6:34     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 26/38] Silicon/Hisilicon/D0x: Move macro definition to PlatformArch.h Ming Huang
2018-08-04  9:34   ` Leif Lindholm
2018-08-09  6:37     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 27/38] Platform/Hisilicon/D06: Add EarlyConfigPeim peim Ming Huang
2018-08-04  9:59   ` Leif Lindholm
2018-08-09  7:07     ` Ming
2018-08-09 10:27       ` Leif Lindholm
2018-08-09 11:54         ` Ming
2018-08-14  2:31         ` Ming
2018-08-14 15:42           ` Leif Lindholm
2018-07-24  7:09 ` [PATCH edk2-platforms v1 28/38] Hisilicon/D0x: Unify FlashFvbDxe driver Ming Huang
2018-08-04 10:06   ` Leif Lindholm
2018-08-09  7:15     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 29/38] Platform/Hisilicon/D06: Add PciHostBridgeLib Ming Huang
2018-08-04 13:41   ` Leif Lindholm
2018-08-09  7:22     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 30/38] Hisilicon/D06: add apei driver Ming Huang
2018-08-04 14:47   ` Leif Lindholm
2018-08-10  2:46     ` Ming
2018-08-14 15:39       ` Leif Lindholm
2018-08-15  8:57         ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 31/38] Silicon/Hisilicon/D06: Add some Lpc macro to LpcLib.h Ming Huang
2018-08-04 14:58   ` Leif Lindholm
2018-08-09 12:02     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 32/38] Platform/Hisilicon/D06: Add capsule upgrade support Ming Huang
2018-08-04 15:08   ` Leif Lindholm
2018-07-24  7:09 ` [PATCH edk2-platforms v1 33/38] Silicon/Hisilicon/D06: Modify for close slave core clock Ming Huang
2018-08-04 15:14   ` Leif Lindholm
2018-08-09 12:15     ` Ming
2018-08-09 12:27       ` Leif Lindholm
2018-08-10  2:05         ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 34/38] Silicon/Hisilicon/D06: Add I2C Bus Exception deal function Ming Huang
2018-08-04 15:18   ` Leif Lindholm
2018-08-10  2:19     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 35/38] Silicon/Hisilicon/Setup: Support SPCR table switch Ming Huang
2018-08-04 15:20   ` Leif Lindholm
2018-08-09 14:17     ` Ming
2018-08-09 14:44       ` Leif Lindholm
2018-08-09 15:40         ` Ming
2018-08-09 15:48           ` Leif Lindholm
2018-07-24  7:09 ` [PATCH edk2-platforms v1 36/38] Silicon/Hisilicon/setup: Support SMMU switch Ming Huang
2018-08-06  9:59   ` Leif Lindholm
2018-08-09 14:19     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 37/38] Hisilicon/D06: Add PciPlatformLib Ming Huang
2018-08-06 10:01   ` Leif Lindholm
2018-08-09 14:27     ` Ming
2018-07-24  7:09 ` [PATCH edk2-platforms v1 38/38] Platform/Hisilicon/D0x: Update version string to 18.08 Ming Huang
2018-08-06 10:03   ` Leif Lindholm
2018-08-09 14:29     ` Ming
2018-08-01 21:56 ` [PATCH edk2-platforms v1 00/38] Upload for D06 platform Leif Lindholm
2018-08-02  1:46   ` Ming
2018-08-02  3:17     ` 答复: " Guoheyi
2018-08-02 10:12     ` Leif Lindholm
2018-08-02 15:36       ` Graeme Gregory
2018-08-04 14:26       ` Ming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180802144242.2t3lfo34dol53vic@bivouac.eciton.net \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox