From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::544; helo=mail-pg1-x544.google.com; envelope-from=ming.huang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pg1-x544.google.com (mail-pg1-x544.google.com [IPv6:2607:f8b0:4864:20::544]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E5124210D97A2 for ; Sat, 4 Aug 2018 20:35:58 -0700 (PDT) Received: by mail-pg1-x544.google.com with SMTP id y4-v6so4646980pgp.9 for ; Sat, 04 Aug 2018 20:35:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=H/um+gqtyAXI5DkugdIrf5GbK7SArZjv1TBSrE3q9W8=; b=PRGKjJX+TT85RkGSOYMqXut86djbMtBot+9npSZ/IFWs7alemB/UasRefqydVNykqU DqDM0sASsW6V5dnIv3CLDgX0l5BVYjnjSGqi3cl5ckjOjlMHWvCQMmzgnojTM32YMJlF 9bKnGC0g5r63cegOZqeLzCz/LZjceXWYXeyBU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=H/um+gqtyAXI5DkugdIrf5GbK7SArZjv1TBSrE3q9W8=; b=GgRzbxtZPXwgDvqa1f06WP8YBCfSyGMHlimzd7r9tofsJpkYjjRQ/fYFd+vInJAqC2 +0JMPxRFgT5qAK3L66OVVRb/8xInyjmKUpjisOqSWsJpyLRb3TQcImeGBbkm58l15AZ2 YNyYKmXi0UrBnmA1m7xWDn7slZWJeAUa0dHP3EdaX4I/M9poZs7oRViGMjTKh/nFz+S6 WjsiDVI5Ujn52sjvLDygObMgsqly3gZ3RQUe7vLOLl/6JFHy86AlvcQwo9A0TLGOfm1v uoEW9Y376dAuk+tUTiO7bmkJ10DyE4hHIT85A8djZbvCLZLeg0AV7Rmc4gJanuvRWrS8 g0eA== X-Gm-Message-State: AOUpUlGk7AXTDLvL6cb0PZcwqb62gOFqEKjWuI8ri5myYZN0GsEO0f8p KGsr9THt3C8I8uc1lYUxH37+NA== X-Google-Smtp-Source: AAOMgpfnpLAGsgDhdNJL01GLfO2YkgErjXs0877jMeVUCG0B56fZeaOEjhmMC4yFbb3K3/XR+G/HDg== X-Received: by 2002:a62:2744:: with SMTP id n65-v6mr11331527pfn.125.1533440158399; Sat, 04 Aug 2018 20:35:58 -0700 (PDT) Received: from [10.163.0.202] ([64.64.108.224]) by smtp.gmail.com with ESMTPSA id g5-v6sm5324086pfc.77.2018.08.04.20.35.45 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sat, 04 Aug 2018 20:35:57 -0700 (PDT) To: Leif Lindholm 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 References: <20180724070922.63362-1-ming.huang@linaro.org> <20180724070922.63362-2-ming.huang@linaro.org> <20180802144242.2t3lfo34dol53vic@bivouac.eciton.net> From: Ming Message-ID: <0e71892c-3313-c5e1-0b8b-36b2703c244c@linaro.org> Date: Sun, 5 Aug 2018 11:35:37 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180802144242.2t3lfo34dol53vic@bivouac.eciton.net> Subject: Re: [PATCH edk2-platforms v1 01/38] Silicon/Hisilicon: Modify the MRC interface for other module X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 05 Aug 2018 03:35:59 -0000 Content-Type: text/plain; charset=gbk Content-Transfer-Encoding: 8bit ÔÚ 8/2/2018 10:42 PM, Leif Lindholm дµÀ: > 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 >> Signed-off-by: Ming Huang >> --- >> 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 > >> 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 > Thanks for review detailed. All comments will be applied in v2. Ming >> >> typedef union { >> struct { >> -- >> 2.17.0 >>