Re: [PATCH RFC 01/10] mmc: core: Add support to read command queue parameters

Ritesh Harjani
2016-06-27T07:59:24Z

Hi Shawn,

On 6/16/2016 1:42 PM, Shawn Lin wrote:
> On 2016/6/15 21:01, Ritesh Harjani wrote:
>> From: Asutosh Das <asutoshd@codeaurora.org>
>>
>> eMMC cards with EXT_CSD version >= 7, optionally support command
will change it here to "8"

>> queuing feature as defined by JEDEC eMMC5.1. Add support for probing
>> command queue feature for such type of cards.
>>
>> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>> [subhashj@codeaurora.org: fixed trivial merge conflicts]
>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>> ---
>> drivers/mmc/core/mmc.c | 18 ++++++++++++++++++
>> include/linux/mmc/card.h | 2 ++
>> include/linux/mmc/mmc.h | 3 +++
>> 3 files changed, 23 insertions(+)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 4dbe3df..ff560e9 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -578,6 +578,24 @@ static int mmc_decode_ext_csd(struct mmc_card
>> *card, u8 *ext_csd)
>> card->ext_csd.data_sector_size = 512;
>> }
>>
>> + if (card->ext_csd.rev >= 7) {
>
> Do you ever find one device claiming to support it with
> ext_csd.rev to be 7?
No, will change it to 8 (for 5.1 revision).

>
> I believe it should be 8 as 7 is for 5.0 and 5.01 which don't support
> hw-cmdq.
Yes, thanks for pointing out.

>
>> + card->ext_csd.cmdq_support = ext_csd[EXT_CSD_CMDQ_SUPPORT];
>> + if (card->ext_csd.cmdq_support) {
>> + /*
>> + * Queue Depth = N + 1,
>> + * see JEDEC JESD84-B51 section 7.4.19
>> + */
>> + card->ext_csd.cmdq_depth =
>> + ext_csd[EXT_CSD_CMDQ_DEPTH] + 1;
>> + pr_info("%s: CMDQ supported: depth: %d\n",
>> + mmc_hostname(card->host),
>> + card->ext_csd.cmdq_depth);
>
> pr_debug may be more appropriate?
Done.

>
>> + }
>> + } else {
>> + card->ext_csd.cmdq_support = 0;
>> + card->ext_csd.cmdq_depth = 0;
>> + }
>
> if the code fallback to here, card->ext_csd.cmdq_support must be
> zero already, and the ext_csd is already set to zero which also
> means you not need to zero it again here.
since card->ext_csd is zero for uninitialized variables,
we can remove these. Sure.

>
>> +
>> /* eMMC v5 or later */
>> if (card->ext_csd.rev >= 7) {
>> memcpy(card->ext_csd.fwrev, &ext_csd[EXT_CSD_FIRMWARE_VERSION],
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index eb0151b..f74db84 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -118,6 +118,8 @@ struct mmc_ext_csd {
>> u8 raw_pwr_cl_ddr_200_360; /* 253 */
>> u8 raw_bkops_status; /* 246 */
>> u8 raw_sectors[4]; /* 212 - 4 bytes */
>> + u8 cmdq_depth; /* 307 */
>> + u8 cmdq_support; /* 308 */
>>
>> unsigned int feature_support;
>> #define MMC_DISCARD_FEATURE BIT(0) /* CMD38
>> feature */
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index 15f2c4a..1c0ae75 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -330,6 +330,9 @@ struct _mmc_csd {
>> #define EXT_CSD_CACHE_SIZE 249 /* RO, 4 bytes */
>> #define EXT_CSD_PWR_CL_DDR_200_360 253 /* RO */
>> #define EXT_CSD_FIRMWARE_VERSION 254 /* RO, 8 bytes */
>> +#define EXT_CSD_CMDQ_DEPTH 307 /* RO */
>> +#define EXT_CSD_CMDQ_SUPPORT 308 /* RO */
>> + #define EXT_CSD_TAG_UNIT_SIZE 498 /* RO */
>
> an invalid space here
Done.

>
>> #define EXT_CSD_SUPPORTED_MODE 493 /* RO */
>> #define EXT_CSD_TAG_UNIT_SIZE 498 /* RO */
>> #define EXT_CSD_DATA_TAG_SUPPORT 499 /* RO */
>>
>
>

--
BR
Ritesh