On Tue, Nov 08, 2016 at 11:21:10PM +0800, Haojian Zhuang wrote: > Declare fields in ECSD structure. And drop the original 128 words > arrary.
Functionality-wise, I'm happy with this, if it is moved to directly after "add SPEC_VERS field in CSD structure". A few comments inline with regards to improving readability. > > Signed-off-by: Haojian Zhuang <haojian.zhu...@linaro.org> > --- > EmbeddedPkg/Universal/MmcDxe/Mmc.h | 157 > ++++++++++++++++++++++- > EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 4 +- > 2 files changed, 158 insertions(+), 3 deletions(-) > > diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h > b/EmbeddedPkg/Universal/MmcDxe/Mmc.h > index 8f30a9a..d3e9b71 100644 > --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h > +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h > @@ -133,13 +133,168 @@ typedef struct { > UINT8 CSD_STRUCTURE: 2; // CSD structure [127:126] > } CSD; > > +typedef struct { > + UINT8 RESERVED_1[16]; // Reserved [15:0] > + UINT8 SECURE_REMOVAL_TYPE; // Secure Removal Type [16:16] > + UINT8 PRODUCT_STATE_AWARENESS_ENABLEMENT; // Product state awareness > enablement [17:17] > + UINT8 MAX_PRE_LOADING_DATA_SIZE[4]; // MAX pre loading data size > [21:18] > + UINT8 PRE_LOADING_DATA_SIZE[4]; // Pre loading data size > [25:22] > + UINT8 FFU_STATUS; // FFU Status [26:26] > + UINT8 RESERVED_2[2]; // Reserved [28:27] > + UINT8 MODE_OPERATION_CODES; // Mode operation codes [29:29] > + UINT8 MODE_CONFIG; // Mode config [30:30] > + UINT8 RESERVED_3; // Reserved [31:31] > + UINT8 FLUSH_CACHE; // Flushing of the cache > [32:32] > + UINT8 CACHE_CTRL; // Control to turn the cache > ON/OFF [33:33] > + UINT8 POWER_OFF_NOTIFICATION; // Power Off Notification > [34:34] > + UINT8 PACKED_FAILURE_INDEX; // Packed command failure > index [35:35] > + UINT8 PACKED_COMMAND_STATUS; // Packed command status > [36:36] > + UINT8 CONTEXT_CONF[15]; // Context configuration > [51:37] > + UINT8 EXT_PARTITIONS_ATTRIBUTE[2]; // Extended partitions > attribute [53:52] > + UINT8 EXCEPTION_EVENTS_STATUS[2]; // Exception events status > [55:54] > + UINT8 EXCEPTION_EVENTS_CTRL[2]; // Exception events control > [57:56] > + UINT8 DYNCAP_NEEDED; // Number of addressed group > to be released [58:58] > + UINT8 CLASS_6_CTRL; // Class 6 commands control > [59:59] > + UINT8 INI_TIMEOUT_EMU; // 1st initialization after > disabling sector size emulation [60:60] > + UINT8 DATA_SECTOR_SIZE; // Sector size [61:61] > + UINT8 USE_NATIVE_SECTOR; // Sector size emulation > [62:62] > + UINT8 NATIVE_SECTOR_SIZE; // Native sector size [63:63] > + UINT8 VENDOR_SPECIFIC_FIELD[64]; // Vendor specific fields > [127:64] > + UINT8 RESERVED_4[2]; // Reserved [129:128] > + UINT8 PROGRAM_CID_CSD_DDR_SUPPORT; // Program CID/CSD in DDR mode > support [130:130] > + UINT8 PERIODIC_WAKEUP; // Periodic wake-up [131:131] > + UINT8 TCASE_SUPPORT; // Package case temperature is > controlled [132:132] > + UINT8 PRODUCTION_STATE_AWARENESS; // Production state awareness > [133:133] > + UINT8 SEC_BAD_BLK_MGMNT; // Bad block management mode > [134:134] > + UINT8 RESERVED_5; // Reserved [135:135] > + UINT8 ENH_START_ADDR[4]; // Enhanced user data start > address [139:136] > + UINT8 ENH_SIZE_MULT[3]; // Enhanced user data area > size [142:140] > + UINT8 GP_SIZE_MULT[12]; // General purpose partition > size [154:143] > + UINT8 PARTITION_SETTING_COMPLETED; // Partitioning setting > [155:155] > + UINT8 PARTITIONS_ATTRIBUTE; // Partitions attribute > [156:156] > + UINT8 MAX_ENH_SIZE_MULT[3]; // Max enhanced area size > [159:157] > + UINT8 PARTITIONING_SUPPORT; // Partitioning [160:160] > + UINT8 HPI_MGMT; // HPI management [161:161] > + UINT8 RST_N_FUNCTION; // H/W reset function [162:162] > + UINT8 BKOPS_EN; // Enable background > operations handshake [163:163] > + UINT8 BKOPS_START; // Manually start background > operations [164:164] > + UINT8 SANITIZE_START; // Start sanitize operation > [165:165] > + UINT8 WR_REL_PARAM; // Write reliability parameter > register [166:166] > + UINT8 WR_REL_SET; // Write reliability setting > register [167:167] > + UINT8 RPMB_SIZE_MULT; // RPMB size [168:168] > + UINT8 FW_CONFIG; // FW configuration [169:169] > + UINT8 RESERVED_6; // Reserved [170:170] > + UINT8 USER_WP; // User area write protection > register [171:171] > + UINT8 RESERVED_7; // Reserved [172:172] > + UINT8 BOOT_WP; // Boot area write protection > register [173:173] > + UINT8 BOOT_WP_STATUS; // Boot write protection > register [174:174] > + UINT8 ERASE_GROUP_DEF; // High-density erase group > definition [175:175] > + UINT8 RESERVED_8; // Reserved [176:176] > + UINT8 BOOT_BUS_CONDITIONS; // Boot bus conditions > [177:177] > + UINT8 BOOT_CONFIG_PROT; // Boot config protection > [178:178] > + UINT8 PARTITION_CONFIG; // Partition config [179:179] > + UINT8 RESERVED_9; // Reserved [180:180] > + UINT8 ERASED_MEM_CONT; // Erased memory content > [181:181] > + UINT8 RESERVED_10; // Reserved [182:182] > + UINT8 BUS_WIDTH; // Bus width mode [183:183] > + UINT8 RESERVED_11; // Reserved [184:184] > + UINT8 HS_TIMING; // High-speed interface timing > [185:185] > + UINT8 RESERVED_12; // Reserved [186:186] > + UINT8 POWER_CLASS; // Power class [187:187] > + UINT8 RESERVED_13; // Reserved [188:188] > + UINT8 CMD_SET_REV; // Command set revision > [189:189] > + UINT8 RESERVED_14; // Reserved [190:190] > + UINT8 CMD_SET; // Command set [191:191] > + UINT8 EXT_CSD_REV; // Extended CSD revision > [192:192] > + UINT8 RESERVED_15; // Reserved [193:193] > + UINT8 CSD_STRUCTURE; // CSD Structure [194:194] > + UINT8 RESERVED_16; // Reserved [195:195] > + UINT8 DEVICE_TYPE; // Device type [196:196] > + UINT8 DRIVER_STRENGTH; // I/O Driver strength > [197:197] > + UINT8 OUT_OF_INTERRUPT_TIME; // Out-of-interrupt busy > timing [198:198] > + UINT8 PARTITION_SWITCH_TIME; // Partition switching timing > [199:199] > + UINT8 PWR_CL_52_195; // Power class for 52MHz at > 1.95V 1 R [200:200] > + UINT8 PWR_CL_26_195; // Power class for 26MHz at > 1.95V 1 R [201:201] > + UINT8 PWR_CL_52_360; // Power class for 52MHz at > 3.6V 1 R [202:202] > + UINT8 PWR_CL_26_360; // Power class for 26MHz at > 3.6V 1 R [203:203] > + UINT8 RESERVED_17; // Reserved [204:204] > + UINT8 MIN_PERF_R_4_26; // Minimum read performance > for 4bit at 26MHz [205:205] > + UINT8 MIN_PERF_W_4_26; // Minimum write performance > for 4bit at 26MHz [206:206] > + UINT8 MIN_PERF_R_8_26_4_52; // Minimum read performance > for 8bit at 26MHz, for 4bit at 52MHz [207:207] > + UINT8 MIN_PERF_W_8_26_4_52; // Minimum write performance > for 8bit at 26MHz, for 4bit at 52MHz [208:208] > + UINT8 MIN_PERF_R_8_52; // Minimum read performance > for 8bit at 52MHz [209:209] > + UINT8 MIN_PERF_W_8_52; // Minimum write performance > for 8bit at 52MHz [210:210] > + UINT8 RESERVED_18; // Reserved [211:211] > + UINT32 SEC_COUNT; // Sector count [215:212] So, this field is the only one we currently look at. In the code below, this replaces the field previously identified vi EMMC_ECSD_SIZE_OFFSET. Where does this change in name come from? Could all SEC_ fields referring to SECTOR rbe renamed SECTOR_, and all referring to SECURE be renamed SECURE_? > + UINT8 SLEEP_NOTIFICATION_TIME; // Sleep notification timout > [216:216] > + UINT8 S_A_TIMEOUT; // Sleep/awake timeout > [217:217] > + UINT8 PRODUCTION_STATE_AWARENESS_TIMEOUT; // Production state awareness > timeout [218:218] > + UINT8 S_C_VCCQ; // Sleep current (VCCQ) > [219:219] > + UINT8 S_C_VCC; // Sleep current (VCC) > [220:220] > + UINT8 HC_WP_GRP_SIZE; // High-capacity write protect > group size [221:221] > + UINT8 REL_WR_SEC_C; // Reliable write sector count > [222:222] > + UINT8 ERASE_TIMEOUT_MULT; // High-capacity erase timeout > [223:223] > + UINT8 HC_ERASE_GRP_SIZE; // High-capacity erase unit > size [224:224] > + UINT8 ACC_SIZE; // Access size [225:225] > + UINT8 BOOT_SIZE_MULTI; // Boot partition size > [226:226] > + UINT8 RESERVED_19; // Reserved [227:227] > + UINT8 BOOT_INFO; // Boot information [228:228] > + UINT8 SEC_TRIM_MULT; // Secure TRIM Multiplier > [229:229] > + UINT8 SEC_ERASE_MULT; // Secure Erase Multiplier > [230:230] > + UINT8 SEC_FEATURE_SUPPORT; // Secure Feature Support > [231:231] > + UINT8 TRIM_MULT; // TRIM Multiplier [232:232] > + UINT8 RESERVED_20; // Reserved [233:233] > + UINT8 MIN_PREF_DDR_R_8_52; // Minimum read performance > for 8bit at 52MHz in DDR mode [234:234] > + UINT8 MIN_PREF_DDR_W_8_52; // Minimum write performance > for 8bit at 52MHz in DDR mode [235:235] > + UINT8 PWR_CL_200_130; // Power class for 200MHz at > VCCQ=1.3V, VCC=3.6V [236:236] > + UINT8 PWR_CL_200_195; // Power class for 200MHz at > VCCQ=1.95V, VCC=3.6V [237:237] > + UINT8 PWR_CL_DDR_52_195; // Power class for 52MHz, DDR > at 1.95V [238:238] > + UINT8 PWR_CL_DDR_52_360; // Power class for 52Mhz, DDR > at 3.6V [239:239] > + UINT8 RESERVED_21; // Reserved [240:240] > + UINT8 INI_TIMEOUT_AP; // 1st initialization time > after partitioning [241:241] > + UINT8 CORRECTLY_PRG_SECTORS_NUM[4]; // Number of correctly > programmed sectors [245:242] > + UINT8 BKOPS_STATUS; // Background operations > status [246:246] > + UINT8 POWER_OFF_LONG_TIME; // Power off notification > (long) timeout [247:247] > + UINT8 GENERIC_CMD6_TIME; // Generic CMD6 timeout > [248:248] > + UINT8 CACHE_SIZE[4]; // Cache size [252:249] > + UINT8 PWR_CL_DDR_200_360; // Power class for 200MHz, DDR > at VCC=3.6V [253:253] > + UINT8 FIRMWARE_VERSION[8]; // Firmware version [261:254] > + UINT8 DEVICE_VERSION[2]; // Device version [263:262] > + UINT8 OPTIMAL_TRIM_UNIT_SIZE; // Optimal trim unit size > [264:264] > + UINT8 OPTIMAL_WRITE_SIZE; // Optimal write size [265:265] > + UINT8 OPTIMAL_READ_SIZE; // Optimal read size [266:266] > + UINT8 PRE_EOL_INFO; // Pre EOL information > [267:267] > + UINT8 DEVICE_LIFE_TIME_EST_TYP_A; // Device life time estimation > type A [268:268] > + UINT8 DEVICE_LIFE_TIME_EST_TYP_B; // Device life time estimation > type B [269:269] > + UINT8 VENDOR_PROPRIETARY_HEALTH_REPORT[32]; // Vendor > proprietary health report [301:270] > + UINT8 NUMBER_OF_FW_SECTORS_CORRECTLY_PROGRAMMED[4]; // Number of FW > sectors correctly programmed [305:302] > + UINT8 RESERVED_22[181]; // Reserved [486:306] > + UINT8 FFU_ARG[4]; // FFU argument [490:487] > + UINT8 OPERATION_CODE_TIMEOUT; // Operation codes timeout > [491:491] > + UINT8 FFU_FEATURES; // FFU features [492:492] > + UINT8 SUPPORTED_MODES; // Supported modes [493:493] > + UINT8 EXT_SUPPORT; // Extended partitions > attribute support [494:494] > + UINT8 LARGE_UNIT_SIZE_M1; // Large unit size [495:495] > + UINT8 CONTEXT_CAPABILITIES; // Context management > capabilities [496:496] > + UINT8 TAG_RES_SIZE; // Tag resource size [497:497] > + UINT8 TAG_UNIT_SIZE; // Tag unit size [498:498] > + UINT8 DATA_TAG_SUPPORT; // Data tag support [499:499] > + UINT8 MAX_PACKED_WRITES; // Max packed write commands > [500:500] > + UINT8 MAX_PACKED_READS; // Max packed read commands > [501:501] > + UINT8 BKOPS_SUPPORT; // Background operations > support [502:502] > + UINT8 HPI_FEATURES; // HPI features [503:503] > + UINT8 S_CMD_SET; // Supported command sets > [504:504] > + UINT8 EXT_SECURITY_ERR; // Extended security commands > error [505:505] > + UINT8 RESERVED_23[6]; // Reserved [511:506] > +} ECSD; > + > typedef struct { > UINT16 RCA; > CARD_TYPE CardType; > OCR OCRData; > CID CIDData; > CSD CSDData; > - UINT32 ECSD[128]; // MMC V4 extended card specific > + ECSD ECSDData; // MMC V4 extended card > specific > } CARD_INFO; > > typedef struct _MMC_HOST_INSTANCE { > diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > index 578fcb6..c5897de 100644 > --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c > @@ -90,7 +90,7 @@ EmmcIdentificationMode ( > DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD fetch error, > Status=%r.\n", Status)); > } > > - Status = Host->ReadBlockData (Host, 0, 512, (UINT32 > *)&(MmcHostInstance->CardInfo.ECSD)); > + Status = Host->ReadBlockData (Host, 0, 512, (UINT32 > *)&(MmcHostInstance->CardInfo.ECSDData)); > if (EFI_ERROR (Status)) { > DEBUG ((EFI_D_ERROR, "EmmcIdentificationMode(): ECSD read error, > Status=%r.\n", Status)); > return Status; > @@ -103,7 +103,7 @@ EmmcIdentificationMode ( > Media->LogicalBlocksPerPhysicalBlock = 1; > Media->IoAlign = 4; > // Compute last block using bits [215:212] of the ECSD > - Media->LastBlock = MmcHostInstance->CardInfo.ECSD[EMMC_ECSD_SIZE_OFFSET] - > 1; // eMMC isn't supposed to report this for > + Media->LastBlock = MmcHostInstance->CardInfo.ECSDData.SEC_COUNT - 1; // > eMMC isn't supposed to report this for This deletes the only use of the EMMC_ECSD_SIZE_OFFSET define - could we delete the definition of that too? > // Cards <2GB in size, but the model does. > > // Setup card type > -- > 2.7.4 > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel