RE: [PATCH V9 14/15] mmc: cqhci: support for command queue enabled host

From: Bough Chen
Date: Sun Sep 24 2017 - 22:39:43 EST


> -----Original Message-----
> From: Adrian Hunter [mailto:adrian.hunter@xxxxxxxxx]
> Sent: Friday, September 22, 2017 8:37 PM
> To: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
> Cc: linux-mmc <linux-mmc@xxxxxxxxxxxxxxx>; linux-block <linux-
> block@xxxxxxxxxxxxxxx>; linux-kernel <linux-kernel@xxxxxxxxxxxxxxx>; Bough
> Chen <haibo.chen@xxxxxxx>; Alex Lemberg <alex.lemberg@xxxxxxxxxxx>;
> Mateusz Nowak <mateusz.nowak@xxxxxxxxx>; Yuliy Izrailov
> <Yuliy.Izrailov@xxxxxxxxxxx>; Jaehoon Chung <jh80.chung@xxxxxxxxxxx>;
> Dong Aisheng <dongas86@xxxxxxxxx>; Das Asutosh
> <asutoshd@xxxxxxxxxxxxxx>; Zhangfei Gao <zhangfei.gao@xxxxxxxxx>;
> Sahitya Tummala <stummala@xxxxxxxxxxxxxx>; Harjani Ritesh
> <riteshh@xxxxxxxxxxxxxx>; Venu Byravarasu <vbyravarasu@xxxxxxxxxx>;
> Linus Walleij <linus.walleij@xxxxxxxxxx>; Shawn Lin <shawn.lin@rock-
> chips.com>; Christoph Hellwig <hch@xxxxxx>
> Subject: [PATCH V9 14/15] mmc: cqhci: support for command queue enabled
> host
>
> From: Venkat Gopalakrishnan <venkatg@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
> Signed-off-by: Sujit Reddy Thumma <sthumma@xxxxxxxxxxxxxx>
> Signed-off-by: Konstantin Dorfman <kdorfman@xxxxxxxxxxxxxx>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@xxxxxxxxxxxxxx>
> Signed-off-by: Subhash Jadavani <subhashj@xxxxxxxxxxxxxx>
> Signed-off-by: Ritesh Harjani <riteshh@xxxxxxxxxxxxxx>
> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> ---
> drivers/mmc/host/Kconfig | 13 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/cqhci.c | 1154
> +++++++++++++++++++++++++++++++++++++++++++++
> drivers/mmc/host/cqhci.h | 240 ++++++++++
> 4 files changed, 1408 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
> 17afe1ad3a03..f2751465bc54 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -843,6 +843,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
> 2b5a8133948d..f01d9915304d 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -90,6 +90,7 @@ 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_SDHCI_OMAP) += sdhci-omap.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..eb3c1695b0c7
> --- /dev/null
> +++ b/drivers/mmc/host/cqhci.c
> @@ -0,0 +1,1154 @@
> +/* Copyright (c) 2015, The Linux Foundation. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/highmem.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/slab.h>
> +#include <linux/scatterlist.h>
> +#include <linux/platform_device.h>
> +#include <linux/ktime.h>
> +
> +#include <linux/mmc/mmc.h>
> +#include <linux/mmc/host.h>
> +#include <linux/mmc/card.h>
> +
> +#include "cqhci.h"
> +
> +#define DCMD_SLOT 31
> +#define NUM_SLOTS 32
> +
> +struct cqhci_slot {
> + struct mmc_request *mrq;
> + unsigned int flags;
> +#define CQHCI_EXTERNAL_TIMEOUT BIT(0)
> +#define CQHCI_COMPLETED BIT(1)
> +#define CQHCI_HOST_CRC BIT(2)
> +#define CQHCI_HOST_TIMEOUT BIT(3)
> +#define CQHCI_HOST_OTHER BIT(4)
> +};
> +
> +static inline u8 *get_desc(struct cqhci_host *cq_host, u8 tag) {
> + return cq_host->desc_base + (tag * cq_host->slot_sz); }
> +
> +static inline u8 *get_link_desc(struct cqhci_host *cq_host, u8 tag) {
> + u8 *desc = get_desc(cq_host, tag);
> +
> + return desc + cq_host->task_desc_len;
> +}
> +
> +static inline dma_addr_t get_trans_desc_dma(struct cqhci_host *cq_host,
> +u8 tag) {
> + return cq_host->trans_desc_dma_base +
> + (cq_host->mmc->max_segs * tag *
> + cq_host->trans_desc_len);
> +}
> +
> +static inline u8 *get_trans_desc(struct cqhci_host *cq_host, u8 tag) {
> + return cq_host->trans_desc_base +
> + (cq_host->trans_desc_len * cq_host->mmc->max_segs * tag); }
> +
> +static void setup_trans_desc(struct cqhci_host *cq_host, u8 tag) {
> + u8 *link_temp;
> + dma_addr_t trans_temp;
> +
> + link_temp = get_link_desc(cq_host, tag);
> + trans_temp = get_trans_desc_dma(cq_host, tag);
> +
> + memset(link_temp, 0, cq_host->link_desc_len);
> + if (cq_host->link_desc_len > 8)
> + *(link_temp + 8) = 0;
> +
> + if (tag == DCMD_SLOT && (cq_host->mmc->caps2 &
> MMC_CAP2_CQE_DCMD)) {
> + *link_temp = CQHCI_VALID(0) | CQHCI_ACT(0) |
> CQHCI_END(1);
> + return;
> + }
> +
> + *link_temp = CQHCI_VALID(1) | CQHCI_ACT(0x6) | CQHCI_END(0);
> +
> + if (cq_host->dma64) {
> + __le64 *data_addr = (__le64 __force *)(link_temp + 4);
> +
> + data_addr[0] = cpu_to_le64(trans_temp);
> + } else {
> + __le32 *data_addr = (__le32 __force *)(link_temp + 4);
> +
> + data_addr[0] = cpu_to_le32(trans_temp);
> + }
> +}
> +
> +static void cqhci_set_irqs(struct cqhci_host *cq_host, u32 set) {
> + u32 ier;
> +
> + ier = cqhci_readl(cq_host, CQHCI_ISTE);
> + ier |= set;

Hi Adrian,

I think operation ' |= ' is not correct, since we will also call cqhci_set_irqs(cq_host, 0),
Which means to mask all cmdq irq, so I think better to directly write the parameter
'set' to ISTE and ISGE.

Best Regards,
Haibo Chen

> + cqhci_writel(cq_host, ier, CQHCI_ISTE);
> + cqhci_writel(cq_host, ier, CQHCI_ISGE); }
> +
> +#define DRV_NAME "cqhci"
> +