From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x22d.google.com (mail-wm0-x22d.google.com [IPv6:2a00:1450:400c:c09::22d]) (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 7AB8A81D37 for ; Tue, 8 Nov 2016 12:43:17 -0800 (PST) Received: by mail-wm0-x22d.google.com with SMTP id t79so262968907wmt.0 for ; Tue, 08 Nov 2016 12:43:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=FQVV+HcPLF9V4aycVE6uodXPxMNiQujlA0MaMSzodpw=; b=a16ZYwwgBDs9Nggdc4C7yq7Y0xHwVL9rt4YRQ6R5x/5pP1nsd8uB1EX6UVCTFarGN1 9HmVqMt+GJ6YLJF4I+PSxpCLTddS4/2jQvqr3kXr7rSNlBs0Javql2SgSVxuKLXjtc0N XzBPsrTeip4fIreVm3Zr3J808qz4O81jMgRnw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=FQVV+HcPLF9V4aycVE6uodXPxMNiQujlA0MaMSzodpw=; b=VetH5liOzna7Z1CSz9qdk6evgsiY/Cs4kvwC/4epDXvOh/uDOTsD+zQbr5sfrPeGBX k1lYFUtR09ORS6iFXi1I40AvNxQl/yfPaE0v/qAwMtMuJ724PpM0Zr/XjJTs/uzU6Cty 9qodZ1T5076CNKd8XW3pqPfUFei6QCMA4IGgExC/lpNUriAaqIdEZhpHZhTENBDAJt2Q KvFVVKOvUCLv1IjCwKNKk7FTTWrOMsPEW1gKIXMvDvOUUtf00XIx5bST9wNwcuKM97AY YBwZpXcT1XM1XAfu794thAx564sHKF9EA6HHzxS8MOQ7mrtxzwlwP+Isg2REvPsURAkd Oapg== X-Gm-Message-State: ABUngvf099WJIl0GgdZJF06A1m95RsMsx8qah/602zEJurrI0tS6skzRWvhESaiSyKcp+TsJ X-Received: by 10.28.26.80 with SMTP id a77mr14076543wma.31.1478637799201; Tue, 08 Nov 2016 12:43:19 -0800 (PST) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id f3sm2980003wmf.10.2016.11.08.12.43.18 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 08 Nov 2016 12:43:18 -0800 (PST) Date: Tue, 8 Nov 2016 20:43:16 +0000 From: Leif Lindholm To: Haojian Zhuang Cc: ryan.harkin@linaro.org, edk2-devel@lists.01.org, ard.biesheuvel@linaro.org Message-ID: <20161108204316.GL27644@bivouac.eciton.net> References: <1478618476-12608-1-git-send-email-haojian.zhuang@linaro.org> <1478618476-12608-6-git-send-email-haojian.zhuang@linaro.org> MIME-Version: 1.0 In-Reply-To: <1478618476-12608-6-git-send-email-haojian.zhuang@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [PATCH v4 05/11] MmcDxe: declare ECSD structure X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Nov 2016 20:43:18 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > --- > 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 >