On 28/11/16 06:29, Ritesh Harjani wrote:
>
>
> On 11/25/2016 3:37 PM, Adrian Hunter wrote:
>> Add definitions relating to Command Queuing.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
>> ---
>> drivers/mmc/core/mmc.c | 17 +++++++++++++++++
>> include/linux/mmc/card.h | 2 ++
>> include/linux/mmc/mmc.h | 17 +++++++++++++++++
>> 3 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 3268fcd3378d..6e9830997eef 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -618,6 +618,23 @@ static int mmc_decode_ext_csd(struct mmc_card *card,
>> u8 *ext_csd)
>> (ext_csd[EXT_CSD_SUPPORTED_MODE] & 0x1) &&
>> !(ext_csd[EXT_CSD_FW_CONFIG] & 0x1);
>> }
>> +
>> + /* eMMC v5.1 or later */
>> + if (card->ext_csd.rev >= 8) {
>> + card->ext_csd.cmdq_support = ext_csd[EXT_CSD_CMDQ_SUPPORT] &
>> + EXT_CSD_CMDQ_SUPPORTED;
>> + card->ext_csd.cmdq_depth = (ext_csd[EXT_CSD_CMDQ_DEPTH] &
>> + EXT_CSD_CMDQ_DEPTH_MASK) + 1;
>> + if (card->ext_csd.cmdq_depth <= 2) {
>> + card->ext_csd.cmdq_support = false;
>> + card->ext_csd.cmdq_depth = 0;
>> + }
> Could you please explain, why for cmdq_depth <=2, we are disabling
> cmdq_support ? Maybe we can add a comment there.
It was in the original code. I presumed it was because such a small queue
did not give a performance advantage. Certainly a qdepth of 1 is not a
queue at all. However all the eMMC I have come in contact with have a queue
depth of at least 16, so it is unlikely to have any affect in practice. I
can add a comment.
>
>
>> + if (card->ext_csd.cmdq_support) {
>> + pr_debug("%s: Command Queue supported depth %u\n",
>> + mmc_hostname(card->host),
>> + card->ext_csd.cmdq_depth);
>> + }
>> + }
>> out:
>> return err;
>> }
>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>> index e49a3ff9d0e0..95d69d498296 100644
>> --- a/include/linux/mmc/card.h
>> +++ b/include/linux/mmc/card.h
>> @@ -89,6 +89,8 @@ struct mmc_ext_csd {
>> unsigned int boot_ro_lock; /* ro lock support */
>> bool boot_ro_lockable;
>> bool ffu_capable; /* Firmware upgrade support */
>> + bool cmdq_support; /* Command Queue supported */
>> + unsigned int cmdq_depth; /* Command Queue depth */
>> #define MMC_FIRMWARE_LEN 8
>> u8 fwrev[MMC_FIRMWARE_LEN]; /* FW version */
>> u8 raw_exception_status; /* 54 */
>> diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>> index c376209c70ef..672730acc705 100644
>> --- a/include/linux/mmc/mmc.h
>> +++ b/include/linux/mmc/mmc.h
>> @@ -84,6 +84,13 @@
>> #define MMC_APP_CMD 55 /* ac [31:16] RCA R1 */
>> #define MMC_GEN_CMD 56 /* adtc [0] RD/WR R1 */
>>
>> + /* class 11 */
>> +#define MMC_QUE_TASK_PARAMS 44 /* ac [20:16] task id R1 */
>> +#define MMC_QUE_TASK_ADDR 45 /* ac [31:0] data addr R1 */
>> +#define MMC_EXECUTE_READ_TASK 46 /* adtc [20:16] task id R1 */
>> +#define MMC_EXECUTE_WRITE_TASK 47 /* adtc [20:16] task id R1 */
>> +#define MMC_CMDQ_TASK_MGMT 48 /* ac [20:16] task id R1b */
>> +
>> static inline bool mmc_op_multi(u32 opcode)
>> {
>> return opcode == MMC_WRITE_MULTIPLE_BLOCK ||
>> @@ -272,6 +279,7 @@ struct _mmc_csd {
>> * EXT_CSD fields
>> */
>>
>> +#define EXT_CSD_CMDQ_MODE_EN 15 /* R/W */
>> #define EXT_CSD_FLUSH_CACHE 32 /* W */
>> #define EXT_CSD_CACHE_CTRL 33 /* R/W */
>> #define EXT_CSD_POWER_OFF_NOTIFICATION 34 /* R/W */
>> @@ -331,6 +339,8 @@ 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_SUPPORTED_MODE 493 /* RO */
>> #define EXT_CSD_TAG_UNIT_SIZE 498 /* RO */
>> #define EXT_CSD_DATA_TAG_SUPPORT 499 /* RO */
>> @@ -438,6 +448,13 @@ struct _mmc_csd {
>> #define EXT_CSD_MANUAL_BKOPS_MASK 0x01
>>
>> /*
>> + * Command Queue
>> + */
>> +#define EXT_CSD_CMDQ_MODE_ENABLED BIT(0)
>
> Is there a need for both EXT_CSD_CMDQ_MODE_ENABLED and
> EXT_CSD_CMDQ_MODE_SUPPORTED. Arent they doing same thing?
They are bits in different ext-csd bytes. EXT_CSD_CMDQ_MODE_ENABLED is in
the CMDQ_MODE_EN byte (15), and EXT_CSD_CMDQ_SUPPORTED is in CMDQ_SUPPORT
(308). AFAIK being the same bit number is coincidence.
>
>
>> +#define EXT_CSD_CMDQ_DEPTH_MASK GENMASK(4, 0)
>> +#define EXT_CSD_CMDQ_SUPPORTED BIT(0)
> Ditto
>
>> +
>> +/*
>> * MMC_SWITCH access modes
>> */
>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
