From: Leif Lindholm <leif.lindholm@linaro.org>
To: Haojian Zhuang <haojian.zhuang@linaro.org>
Cc: ryan.harkin@linaro.org, edk2-devel@lists.01.org,
ard.biesheuvel@linaro.org
Subject: Re: [PATCH v4 05/11] MmcDxe: declare ECSD structure
Date: Tue, 8 Nov 2016 20:43:16 +0000 [thread overview]
Message-ID: <20161108204316.GL27644@bivouac.eciton.net> (raw)
In-Reply-To: <1478618476-12608-6-git-send-email-haojian.zhuang@linaro.org>
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.zhuang@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
>
next prev parent reply other threads:[~2016-11-08 20:43 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-08 15:21 [PATCH v4 00/11] enhance MMC Haojian Zhuang
2016-11-08 15:21 ` [PATCH v4 01/11] MmcDxe: wait OCR busy bit free Haojian Zhuang
2016-11-08 19:27 ` Leif Lindholm
2016-11-08 15:21 ` [PATCH v4 02/11] MmcDxe: move ECSD into CardInfo structure Haojian Zhuang
2016-11-08 20:27 ` Leif Lindholm
2016-11-08 15:21 ` [PATCH v4 03/11] MmcDxe: add SPEC_VERS field in CSD structure Haojian Zhuang
2016-11-08 20:27 ` Leif Lindholm
2016-11-08 15:21 ` [PATCH v4 04/11] MmcDxe: add interface to change io width and speed Haojian Zhuang
2016-11-08 15:21 ` [PATCH v4 05/11] MmcDxe: declare ECSD structure Haojian Zhuang
2016-11-08 20:43 ` Leif Lindholm [this message]
2016-11-09 0:34 ` Haojian Zhuang
2016-11-08 15:21 ` [PATCH v4 06/11] MmcDxe: set io bus width before reading EXTCSD Haojian Zhuang
2016-11-08 15:21 ` [PATCH v4 07/11] MmcDxe: Fix uninitialized mediaid for SD Haojian Zhuang
2016-11-08 23:59 ` Leif Lindholm
2016-11-13 6:46 ` Haojian Zhuang
2016-11-08 15:21 ` [PATCH v4 08/11] MmcDxe: set iospeed and bus width in SD stack Haojian Zhuang
2016-11-08 15:21 ` [PATCH v4 09/11] MmcDxe: expand to support multiple blocks Haojian Zhuang
2016-11-09 0:14 ` Leif Lindholm
2016-11-08 15:21 ` [PATCH v4 10/11] PL180: update for indentifying SD Haojian Zhuang
2016-11-08 15:21 ` [PATCH v4 11/11] PL180: set variable length for CMD6 and ACMD51 Haojian Zhuang
2016-11-08 17:32 ` [PATCH v4 00/11] enhance MMC Ryan Harkin
2016-11-09 0:30 ` Haojian Zhuang
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=20161108204316.GL27644@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