From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ritesh Harjani Subject: Re: [PATCH V7 10/25] mmc: mmc: Add functions to enable / disable the Command Queue Date: Mon, 28 Nov 2016 19:30:11 +0530 Message-ID: References: <1480068442-5169-1-git-send-email-adrian.hunter@intel.com> <1480068442-5169-11-git-send-email-adrian.hunter@intel.com> <9984a3e0-a834-41f7-1c59-6f3d78e63ab0@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:37734 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932294AbcK1OAU (ORCPT ); Mon, 28 Nov 2016 09:00:20 -0500 In-Reply-To: <9984a3e0-a834-41f7-1c59-6f3d78e63ab0@intel.com> Sender: linux-mmc-owner@vger.kernel.org List-Id: linux-mmc@vger.kernel.org To: Adrian Hunter Cc: Ulf Hansson , linux-mmc , Alex Lemberg , Mateusz Nowak , Yuliy Izrailov , Jaehoon Chung , Dong Aisheng , Das Asutosh , Zhangfei Gao , Dorfman Konstantin , David Griego , Sahitya Tummala , Venu Byravarasu , Linus Walleij On 11/28/2016 6:53 PM, Adrian Hunter wrote: > On 28/11/16 06:36, Ritesh Harjani wrote: >> >> >> On 11/25/2016 3:37 PM, Adrian Hunter wrote: >>> Add helper functions to enable or disable the Command Queue. >>> >>> Signed-off-by: Adrian Hunter >>> --- >>> Documentation/mmc/mmc-dev-attrs.txt | 1 + >>> drivers/mmc/core/mmc.c | 2 ++ >>> drivers/mmc/core/mmc_ops.c | 27 +++++++++++++++++++++++++++ >>> include/linux/mmc/card.h | 1 + >>> include/linux/mmc/core.h | 2 ++ >>> 5 files changed, 33 insertions(+) >>> >>> diff --git a/Documentation/mmc/mmc-dev-attrs.txt >>> b/Documentation/mmc/mmc-dev-attrs.txt >>> index 404a0e9e92b0..dcd1252877fb 100644 >>> --- a/Documentation/mmc/mmc-dev-attrs.txt >>> +++ b/Documentation/mmc/mmc-dev-attrs.txt >>> @@ -30,6 +30,7 @@ All attributes are read-only. >>> rel_sectors Reliable write sector count >>> ocr Operation Conditions Register >>> dsr Driver Stage Register >>> + cmdq_en Command Queue enabled: 1 => enabled, 0 => not enabled >>> >>> Note on Erase Size and Preferred Erase Size: >>> >>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >>> index 6e9830997eef..d6a30bbd399d 100644 >>> --- a/drivers/mmc/core/mmc.c >>> +++ b/drivers/mmc/core/mmc.c >>> @@ -770,6 +770,7 @@ static int mmc_compare_ext_csds(struct mmc_card *card, >>> unsigned bus_width) >>> MMC_DEV_ATTR(raw_rpmb_size_mult, "%#x\n", card->ext_csd.raw_rpmb_size_mult); >>> MMC_DEV_ATTR(rel_sectors, "%#x\n", card->ext_csd.rel_sectors); >>> MMC_DEV_ATTR(ocr, "%08x\n", card->ocr); >>> +MMC_DEV_ATTR(cmdq_en, "%d\n", card->ext_csd.cmdq_en); >>> >>> static ssize_t mmc_fwrev_show(struct device *dev, >>> struct device_attribute *attr, >>> @@ -823,6 +824,7 @@ static ssize_t mmc_dsr_show(struct device *dev, >>> &dev_attr_rel_sectors.attr, >>> &dev_attr_ocr.attr, >>> &dev_attr_dsr.attr, >>> + &dev_attr_cmdq_en.attr, >>> NULL, >>> }; >>> ATTRIBUTE_GROUPS(mmc_std); >>> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c >>> index 9b2617cfff67..92a1de9b4981 100644 >>> --- a/drivers/mmc/core/mmc_ops.c >>> +++ b/drivers/mmc/core/mmc_ops.c >>> @@ -824,3 +824,30 @@ int mmc_can_ext_csd(struct mmc_card *card) >>> { >>> return (card && card->csd.mmca_vsn > CSD_SPEC_VER_3); >>> } >>> + >>> +int mmc_cmdq_switch(struct mmc_card *card, int enable) >>> +{ >>> + int err; >>> + >>> + if (!card->ext_csd.cmdq_support) >>> + return -EOPNOTSUPP; >>> + >>> + err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_CMDQ_MODE_EN, >>> + enable, card->ext_csd.generic_cmd6_time); >>> + if (!err) >>> + card->ext_csd.cmdq_en = enable; >>> + >>> + return err; >>> +} >>> + >>> +int mmc_cmdq_enable(struct mmc_card *card) >>> +{ >>> + return mmc_cmdq_switch(card, EXT_CSD_CMDQ_MODE_ENABLED); >> >> EXT_CSD_CMDQ_MODE_ENABLED is defined as a BIT(0), but getting used here as a >> value of 1. EXT_CSD_CMDQ_MODE_ENABLED seems redundant anyways. >> Do you think we can remove it and pass 1 directly? > > How about: > > static int mmc_cmdq_switch(struct mmc_card *card, bool enable) > { > u8 val = enable ? EXT_CSD_CMDQ_MODE_ENABLED : 0; > int err; > > if (!card->ext_csd.cmdq_support) > return -EOPNOTSUPP; > > err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_CMDQ_MODE_EN, > val, card->ext_csd.generic_cmd6_time); > if (!err) > card->ext_csd.cmdq_en = enable; > > return err; > } > > int mmc_cmdq_enable(struct mmc_card *card) > { > return mmc_cmdq_switch(card, true); > } > EXPORT_SYMBOL_GPL(mmc_cmdq_enable); > > int mmc_cmdq_disable(struct mmc_card *card) > { > return mmc_cmdq_switch(card, false); > } > EXPORT_SYMBOL_GPL(mmc_cmdq_disable); > yes. This looks good. >> >>> +} >>> +EXPORT_SYMBOL_GPL(mmc_cmdq_enable); >>> + >>> +int mmc_cmdq_disable(struct mmc_card *card) >>> +{ >>> + return mmc_cmdq_switch(card, 0); >>> +} >>> +EXPORT_SYMBOL_GPL(mmc_cmdq_disable); >>> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h >>> index 95d69d498296..2d9c24f4e88e 100644 >>> --- a/include/linux/mmc/card.h >>> +++ b/include/linux/mmc/card.h >>> @@ -89,6 +89,7 @@ struct mmc_ext_csd { >>> unsigned int boot_ro_lock; /* ro lock support */ >>> bool boot_ro_lockable; >>> bool ffu_capable; /* Firmware upgrade support */ >>> + bool cmdq_en; /* Command Queue enabled */ >>> bool cmdq_support; /* Command Queue supported */ >>> unsigned int cmdq_depth; /* Command Queue depth */ >>> #define MMC_FIRMWARE_LEN 8 >>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h >>> index 0ce928b3ce90..d045b06fc7ea 100644 >>> --- a/include/linux/mmc/core.h >>> +++ b/include/linux/mmc/core.h >>> @@ -177,6 +177,8 @@ extern int mmc_wait_for_app_cmd(struct mmc_host *, >>> struct mmc_card *, >>> extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int); >>> extern int mmc_send_tuning(struct mmc_host *host, u32 opcode, int >>> *cmd_error); >>> extern int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd); >>> +extern int mmc_cmdq_enable(struct mmc_card *card); >>> +extern int mmc_cmdq_disable(struct mmc_card *card); >>> >>> #define MMC_ERASE_ARG 0x00000000 >>> #define MMC_SECURE_ERASE_ARG 0x80000000 >>> >> > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project