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 ++++----------------
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{
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;
-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;
+};
+
+typedef struct _DDR_DIMM_DATA {
+ BOOLEAN Status;
+ UINT8 DramType; //Byte 2
+ UINT8 ModuleType; //Byte 3
+ UINT8 BankNum; //Byte 4,??DDR4,?????BankGroup??Bank??
+ 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;
+}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;
+ MEMORY_DATA mem;
+ NUMAINFO_DATA NumaInfo[MAX_SOCKET][MAX_NUM_PER_TYPE];
+}GBL_INTERFACE, *pGBL_INTERFACE;
typedef union {
struct {
--
2.17.0
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
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
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
在 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 <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
>
Thanks for review detailed. All comments will be applied in v2.
Ming
>>
>> typedef union {
>> struct {
>> --
>> 2.17.0
>>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2026 Red Hat, Inc.