Re: [PATCH V4 10/11] mmc: cqhci: support for command queue enabled host

Adrian Hunter
2017-07-24T10:29:36Z

On 24/07/17 11:52, Bough Chen wrote:
> [...]
>
>> -----Original Message-----
>> From: Adrian Hunter [mailto:adrian.hunter@intel.com]
>> Sent: Friday, July 21, 2017 5:50 PM
>> To: Ulf Hansson <ulf.hansson@linaro.org>
>> Cc: linux-mmc <linux-mmc@vger.kernel.org>; Bough Chen
>> <haibo.chen@nxp.com>; Alex Lemberg <alex.lemberg@sandisk.com>;
>> Mateusz Nowak <mateusz.nowak@intel.com>; Yuliy Izrailov
>> <Yuliy.Izrailov@sandisk.com>; Jaehoon Chung <jh80.chung@samsung.com>;
>> Dong Aisheng <dongas86@gmail.com>; Das Asutosh
>> <asutoshd@codeaurora.org>; Zhangfei Gao <zhangfei.gao@gmail.com>;
>> Dorfman Konstantin <kdorfman@codeaurora.org>; David Griego
>> <david.griego@linaro.org>; Sahitya Tummala <stummala@codeaurora.org>;
>> Harjani Ritesh <riteshh@codeaurora.org>; Venu Byravarasu
>> <vbyravarasu@nvidia.com>; Linus Walleij <linus.walleij@linaro.org>; Shawn Lin
>> <shawn.lin@rock-chips.com>
>> Subject: [PATCH V4 10/11] mmc: cqhci: support for command queue enabled
>> host
>>
>> From: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>
>> This patch adds CMDQ support for command-queue compatible hosts.
>>
>> Command queue is added in eMMC-5.1 specification. This enables the
>> controller to process upto 32 requests at a time.
>>
>> Adrian Hunter contributed renaming to cqhci, recovery, suspend and resume,
>> cqhci_off, cqhci_wait_for_idle, and external timeout handling.
>>
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
>> Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>> drivers/mmc/host/Kconfig | 13 +
>> drivers/mmc/host/Makefile | 1 +
>> drivers/mmc/host/cqhci.c | 1146
>> +++++++++++++++++++++++++++++++++++++++++++++
>> drivers/mmc/host/cqhci.h | 240 ++++++++++
>> 4 files changed, 1400 insertions(+)
>> create mode 100644 drivers/mmc/host/cqhci.c create mode 100644
>> drivers/mmc/host/cqhci.h
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index
>> 2242633550df..3a164a03f2bc 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -834,6 +834,19 @@ config MMC_SUNXI
>> This selects support for the SD/MMC Host Controller on
>> Allwinner sunxi SoCs.
>>
>> +config MMC_CQHCI
>> + tristate "Command Queue Host Controller Interface support"
>> + depends on HAS_DMA
>> + help
>> + This selects the Command Queue Host Controller Interface (CQHCI)
>> + support present in host controllers of Qualcomm Technologies, Inc
>> + amongst others.
>> + This controller supports eMMC devices with command queue support.
>> +
>> + If you have a controller with this interface, say Y or M here.
>> +
>> + If unsure, say N.
>> +
>> config MMC_TOSHIBA_PCI
>> tristate "Toshiba Type A SD/MMC Card Interface Driver"
>> depends on PCI
>> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile index
>> 8c46766c000c..3ae71e006890 100644
>> --- a/drivers/mmc/host/Makefile
>> +++ b/drivers/mmc/host/Makefile
>> @@ -88,6 +88,7 @@ obj-$(CONFIG_MMC_SDHCI_MSM) +=
>> sdhci-msm.o
>> obj-$(CONFIG_MMC_SDHCI_ST) += sdhci-st.o
>> obj-$(CONFIG_MMC_SDHCI_MICROCHIP_PIC32) += sdhci-pic32.o
>> obj-$(CONFIG_MMC_SDHCI_BRCMSTB) += sdhci-brcmstb.o
>> +obj-$(CONFIG_MMC_CQHCI) += cqhci.o
>>
>> ifeq ($(CONFIG_CB710_DEBUG),y)
>> CFLAGS-cb710-mmc += -DDEBUG
>> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c new file
>> mode 100644 index 000000000000..302421a26230
>> --- /dev/null
>> +++ b/drivers/mmc/host/cqhci.c
>> @@ -0,0 +1,1146 @@
>
> <SNIP>
>
>> +
>> +static int cqhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>> +{
>> + int err = 0;
>> + u64 data = 0;
>> + u64 *task_desc = NULL;
>> + int tag = cqhci_tag(mrq);
>> + struct cqhci_host *cq_host = mmc->cqe_private;
>> + unsigned long flags;
>> +
>> + if (!cq_host->enabled) {
>> + pr_err("%s: cqhci: not enabled\n", mmc_hostname(mmc));
>> + return -EINVAL;
>> + }
>> +
>> + /* First request after resume has to re-enable */
>> + if (!cq_host->activated)
>> + __cqhci_enable(cq_host);
>> +
>> + if (!mmc->cqe_on) {
>> + cqhci_writel(cq_host, 0, CQHCI_CTL);
>> + mmc->cqe_on = true;
>> + pr_debug("%s: cqhci: CQE on\n", mmc_hostname(mmc));
>> + if (cqhci_readl(cq_host, CQHCI_CTL) && CQHCI_HALT) {
>> + pr_err("%s: cqhci: CQE failed to exit halt state\n",
>> + mmc_hostname(mmc));
>> + }
>> + if (cq_host->ops->enable)
>> + cq_host->ops->enable(mmc);
>> + }
>> +
>> + if (mrq->data) {
>> + task_desc = (__le64 __force *)get_desc(cq_host, tag);
>> + cqhci_prep_task_desc(mrq, &data, 1);
>> + *task_desc = cpu_to_le64(data);
>> + err = cqhci_prep_tran_desc(mrq, cq_host, tag);
>> + if (err) {
>> + pr_err("%s: cqhci: failed to setup tx desc: %d\n",
>> + mmc_hostname(mmc), err);
>> + return err;
>> + }
>> + } else {
>> + cqhci_prep_dcmd_desc(mmc, mrq);
>> + }
>> +
> Hi Adrian,
>
> For cqhci data request, I think we need to also config SDHCI_TRNS_BLK_CNT_EN and SDHCI_TRNS_DMA, just like function sdhci_set_transfer_mode(): set SDHCI_TRNS_BLK_CNT_EN and SDHCI_TRNS_DMA in every request.
>
> Currently, we do not do this for cqhci request, if support Runtime PM, when runtime resume,
> sdhci_runtime_resume_host() --> sdhci_init(host, 0) --> sdhci_do_reset(host, SDHCI_RESET_ALL)
> sdhci_reset() will do software reset for all, this will clear some SDHCI register, including SDHCI_TRNS_BLK_CNT_EN and SDHCI_TRNS_DMA, then when use cqhci request, error happens. I meet cqhci wait timeout error on our i.MX8. After set these two bits, this issue gone.

CQE anyway needs to write the transfer mode bit-4 so it is a bit strange
that it doesn't write the other bits too. Nevertheless anything you need to
set can be done in the ->enable() callback. For example see glk_cqe_enable().