[PATCH RFCv2 00/10] mmc: Add HW Command Queuing Support

[PATCH RFCv2 00/10] mmc: Add HW Command Queuing Support

[Ritesh Harjani]

Hi All, 

This patch series refreshes the older patch series sent a
while ago by Asutosh Das[1].

Current set of patch series is only introducing the basic CMDQ
driver to get more review comments. Based on the reviews, will
add other features as well to the patch list.

Below paras describes about the CMDQ feature and a brief
implementaion details of HW CMDQ :-

In this patch series, we propose a method to add support for
Command Queueing(CQ) feature added to eMMC-5.1 specification.
This feature includes new commands for issuing tasks to the
device and orders the execution of tasks to the device. It
also has task management functions.

Working of CQ with HW CQ support :-
Both card and controller(HC) maintains a queue where tasks can be
queued for execution. SW pulls the requests from block layer,
prepares request's task/transfer descriptors and issues
(by ringing the doorbell) it to CMDQ HC. CMDQ HC then queues this
task to the card queue (can queue while data transfer is in progress)
by preparing and sending CMD44 followed by CMD45.

While SW driver can issues the tasks asynchronously to CMDQ HC,
until the queue is full, HC can in parallel, send SQS
(send queue status) to card to read QSR (queue status register),
to know which tasks are ready for execution in card.
Based on the QSR, HC can send CMD46/47 (read/write) for the
task which was ready for execution in card.

Main advantage of CQ comes for Random requests and thus
we see that in below performance table that it's mainly
the random requests that gets most of the benefit.

Brief on implementation details :-
The initialization of CQ is decided based on the underlying
driver capability and the capability advertised by the card
through ext_csd.

In order to support queueing of multiple requests, we have
added a new issue function to mmc-queue. This selectively
pulls the requests and prepares and issues it to the underlying
driver. We have used the inherent tagging mechanism of kernel
block layer to keep track and map tags to the slots of underlying
driver. The current design doesn't block for the request to
complete. We have separated the issuing and completion path
of the request and tracking is done using the tag assigned to
the request.

We have introduced a number of APIs to mmc block layer to
facilitate servicing of requests.

The completion of requests is handled in a softirq registered
with the kernel block layer during initialization.

A new layer has been introduced to serve the CQ compliant drivers.
This layer (cq_hci) has all the standard functionality implemented.
It also has necessary hooks for convenience of platform drivers.


Performance Data :-
|------------------------------------
|Androbench	| CQ	|  Legacy  |
|---------------|-------|----------|
|Seq Read(MBps)	| 227.41|  216.97  |	
|---------------|-------|----------|
|Seq Write(MBps)| 56.6	|  56.44   |
|---------------|-------|----------|
|Rand.Read(IOPs)| 7144	|  4554    |
|---------------|-------|----------|
|Rand.Write(IOPS| 2374  |  1629	   |
|----------------------------------|


Patch Description in brief:-
Patch 1/10 - Reads ext_csd to know whether the card supports
CMDQ or not.

Patch 2/10 - Adds support for a seperate queue and
mmc_cmdq_thread for pulling and issuing the requests.
Since this patch series adds a seperate queue for HW CMDQ
support, this should not conflict with SW CMDQ series,
except in places where we are (initializing/enabling? and)
defining CMDQ definitions for card.

Patch 3/10 - Adds initialization and enabling of CMDQ in
card and controller if both supports it.

Patch 4-5,8,9/10 - Add read/write/flush/halt support to CMDQ

Patch 6/10 - This change does not relate to CMDQ in general,
but only w.r.t. SDMA & ADMA.

Patch 7/10 - Add CMDQ_HCI file for host controllers that support
CMDQ.

Patch 10/10 - Add CQ support to sdhci.

[1]:- http://www.spinics.net/lists/linux-arm-msm/msg12692.html


Changes in RFCv2:
- Addressed comments from Shawn.

Asutosh Das (7):
  mmc: core: Add support to read command queue parameters
  mmc: queue: initialization of command queue
  mmc: core: Add command queue initialzation support
  mmc: core: add flush request support to command queue
  mmc: core: Add halt support
  mmc: cmdq-host: add halt support to command queue host
  mmc: sdhci: add command queue support to sdhci

Subhash Jadavani (1):
  mmc: host: sdhci: don't set SDMA buffer boundary in ADMA mode

Venkat Gopalakrishnan (2):
  mmc: card: add read/write support in command queue mode
  mmc: cmdq: support for command queue enabled host

 drivers/mmc/card/block.c    | 315 ++++++++++++++++++++-
 drivers/mmc/card/queue.c    | 188 ++++++++++++-
 drivers/mmc/card/queue.h    |  10 +-
 drivers/mmc/core/core.c     |  98 +++++++
 drivers/mmc/core/mmc.c      |  65 +++++
 drivers/mmc/core/mmc_ops.c  |  47 +++-
 drivers/mmc/host/Kconfig    |  13 +
 drivers/mmc/host/Makefile   |   1 +
 drivers/mmc/host/cmdq_hci.c | 671 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/cmdq_hci.h | 207 ++++++++++++++
 drivers/mmc/host/sdhci.c    | 169 ++++++++++-
 drivers/mmc/host/sdhci.h    |   6 +
 include/linux/mmc/card.h    |  11 +-
 include/linux/mmc/core.h    |  13 +
 include/linux/mmc/host.h    |  75 +++++
 include/linux/mmc/mmc.h     |   3 +
 16 files changed, 1868 insertions(+), 24 deletions(-)
 create mode 100644 drivers/mmc/host/cmdq_hci.c
 create mode 100644 drivers/mmc/host/cmdq_hci.h

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

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

[Ritesh Harjani]

From: Asutosh Das <asutoshd@codeaurora.org>

eMMC cards with EXT_CSD version >= 8, optionally support command
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>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/core/mmc.c   | 15 +++++++++++++++
 include/linux/mmc/card.h |  2 ++
 include/linux/mmc/mmc.h  |  2 ++
 3 files changed, 19 insertions(+)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 5d438ad..77734c8 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -585,6 +585,21 @@ 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 >= 8) {
+		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_debug("%s: CMDQ supported: depth: %d\n",
+				mmc_hostname(card->host),
+				card->ext_csd.cmdq_depth);
+		}
+	}
+
 	/* 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..694c189 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -330,6 +330,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 */
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH RFCv2 02/10] mmc: queue: initialization of command queue

[Ritesh Harjani]

From: Asutosh Das <asutoshd@codeaurora.org>

Command Queueing (CQ) feature is introduced to eMMC
standard in revision 5.1. CQ includes new commands
for issuing tasks to the device, for ordering the
execution of previously issued tasks and for
additional task management functions.

The idea is to keep the legacy and CQ code as discrete
as possible. Hence, a separate queue is created for CQ.
The issuing path is non-blocking since several requests
(max. 32) can be queued at a time.

Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
[subhashj@codeaurora.org: fixed trivial merge conflicts & compilation
error]
Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/card/block.c |   2 +-
 drivers/mmc/card/queue.c | 186 ++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/mmc/card/queue.h |   9 ++-
 include/linux/mmc/host.h |  17 +++++
 4 files changed, 210 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index e62fde3..ec99f57 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -2257,7 +2257,7 @@ again:
 	INIT_LIST_HEAD(&md->part);
 	md->usage = 1;
 
-	ret = mmc_init_queue(&md->queue, card, &md->lock, subname);
+	ret = mmc_init_queue(&md->queue, card, &md->lock, subname, area_type);
 	if (ret)
 		goto err_putdisk;
 
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 6f4323c..bb885f4 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -46,6 +46,69 @@ static int mmc_prep_request(struct request_queue *q, struct request *req)
 	return BLKPREP_OK;
 }
 
+static inline bool mmc_cmdq_should_pull_reqs(struct mmc_host *host,
+					struct mmc_cmdq_context_info *ctx)
+{
+	if (test_bit(CMDQ_STATE_ERR, &ctx->curr_state)) {
+		pr_debug("%s: %s: skip pulling reqs: state: %lu\n",
+			 mmc_hostname(host), __func__, ctx->curr_state);
+		return false;
+	}
+	return true;
+}
+
+static int mmc_cmdq_thread(void *d)
+{
+	struct mmc_queue *mq = d;
+	struct request_queue *q = mq->queue;
+	struct mmc_card *card = mq->card;
+
+	struct request *req;
+	struct mmc_host *host = card->host;
+	struct mmc_cmdq_context_info *ctx = &host->cmdq_ctx;
+	unsigned long flags;
+
+	current->flags |= PF_MEMALLOC;
+
+	while (1) {
+		int ret = 0;
+
+		if (!mmc_cmdq_should_pull_reqs(host, ctx)) {
+			test_and_set_bit(0, &ctx->req_starved);
+			schedule();
+		}
+
+		spin_lock_irqsave(q->queue_lock, flags);
+		req = blk_peek_request(q);
+		if (req) {
+			ret = blk_queue_start_tag(q, req);
+			spin_unlock_irqrestore(q->queue_lock, flags);
+			if (ret) {
+				test_and_set_bit(0, &ctx->req_starved);
+				schedule();
+			} else {
+				ret = mq->cmdq_issue_fn(mq, req);
+				if (ret) {
+					pr_err("%s: failed (%d) to issue req, requeue\n",
+					       mmc_hostname(host), ret);
+					spin_lock_irqsave(q->queue_lock, flags);
+					blk_requeue_request(q, req);
+					spin_unlock_irqrestore(q->queue_lock,
+							       flags);
+				}
+			}
+		} else {
+			spin_unlock_irqrestore(q->queue_lock, flags);
+			if (kthread_should_stop()) {
+				set_current_state(TASK_RUNNING);
+				break;
+			}
+			schedule();
+		}
+	} /* loop */
+	return 0;
+}
+
 static int mmc_queue_thread(void *d)
 {
 	struct mmc_queue *mq = d;
@@ -102,6 +165,13 @@ static int mmc_queue_thread(void *d)
 	return 0;
 }
 
+static void mmc_cmdq_dispatch_req(struct request_queue *q)
+{
+	struct mmc_queue *mq = q->queuedata;
+
+	wake_up_process(mq->thread);
+}
+
 /*
  * Generic MMC request handler.  This is called for any queue on a
  * particular host.  When the host is not busy, we look for a request
@@ -177,6 +247,84 @@ static void mmc_queue_setup_discard(struct request_queue *q,
 }
 
 /**
+ * mmc_blk_cmdq_setup_queue
+ * @mq: mmc queue
+ * @card: card to attach to this queue
+ *
+ * Setup queue for CMDQ supporting MMC card
+ */
+static void mmc_cmdq_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
+{
+	u64 limit = BLK_BOUNCE_HIGH;
+	struct mmc_host *host = card->host;
+
+	queue_flag_set_unlocked(QUEUE_FLAG_NONROT, mq->queue);
+	if (mmc_can_erase(card))
+		mmc_queue_setup_discard(mq->queue, card);
+
+	blk_queue_bounce_limit(mq->queue, limit);
+	blk_queue_max_hw_sectors(mq->queue, min(host->max_blk_count,
+						host->max_req_size / 512));
+	blk_queue_max_segment_size(mq->queue, host->max_seg_size);
+	blk_queue_max_segments(mq->queue, host->max_segs);
+}
+
+static void mmc_cmdq_softirq_done(struct request *rq)
+{
+	struct mmc_queue *mq = rq->q->queuedata;
+
+	mq->cmdq_complete_fn(rq);
+}
+
+static int mmc_cmdq_init(struct mmc_queue *mq, struct mmc_card *card)
+{
+	int i, ret = 0;
+	/* one slot is reserved for dcmd requests */
+	int q_depth = card->ext_csd.cmdq_depth - 1;
+
+	if (!(card->host->caps2 & MMC_CAP2_CMD_QUEUE)) {
+		ret = -ENOTSUPP;
+		goto out;
+	}
+
+	mq->mqrq_cmdq = kzalloc(
+			sizeof(struct mmc_queue_req) * q_depth, GFP_KERNEL);
+	if (!mq->mqrq_cmdq) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* sg is allocated for data request slots only */
+	for (i = 0; i < q_depth; i++) {
+		mq->mqrq_cmdq[i].sg = mmc_alloc_sg(card->host->max_segs, &ret);
+		if (ret) {
+			pr_warn("%s: unable to allocate cmdq sg of size %d\n",
+				mmc_card_name(card),
+				card->host->max_segs);
+			goto free_mqrq_sg;
+		}
+	}
+
+	ret = blk_queue_init_tags(mq->queue, q_depth, NULL, BLK_TAG_ALLOC_FIFO);
+	if (ret) {
+		pr_warn("%s: unable to allocate cmdq tags %d\n",
+				mmc_card_name(card), q_depth);
+		goto free_mqrq_sg;
+	}
+
+	blk_queue_softirq_done(mq->queue, mmc_cmdq_softirq_done);
+	goto out;
+
+free_mqrq_sg:
+	for (i = 0; i < q_depth; i++)
+		kfree(mq->mqrq_cmdq[i].sg);
+	kfree(mq->mqrq_cmdq);
+	mq->mqrq_cmdq = NULL;
+out:
+	return ret;
+}
+
+/**
  * mmc_init_queue - initialise a queue structure.
  * @mq: mmc queue
  * @card: mmc card to attach this queue
@@ -186,7 +334,7 @@ static void mmc_queue_setup_discard(struct request_queue *q,
  * Initialise a MMC card request queue.
  */
 int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
-		   spinlock_t *lock, const char *subname)
+		   spinlock_t *lock, const char *subname, int area_type)
 {
 	struct mmc_host *host = card->host;
 	u64 limit = BLK_BOUNCE_HIGH;
@@ -198,6 +346,27 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
 		limit = (u64)dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
 
 	mq->card = card;
+	if (card->ext_csd.cmdq_support &&
+	    (area_type == MMC_BLK_DATA_AREA_MAIN)) {
+		mq->queue = blk_init_queue(mmc_cmdq_dispatch_req, lock);
+		if (!mq->queue)
+			return -ENOMEM;
+		mmc_cmdq_setup_queue(mq, card);
+		ret = mmc_cmdq_init(mq, card);
+		if (ret) {
+			pr_err("%s: %d: cmdq: unable to set-up\n",
+			       mmc_hostname(card->host), ret);
+			blk_cleanup_queue(mq->queue);
+		} else {
+			mq->queue->queuedata = mq;
+			mq->thread = kthread_run(mmc_cmdq_thread, mq,
+						 "mmc-cmdqd/%d%s",
+						 host->index,
+						 subname ? subname : "");
+			return ret;
+		}
+	}
+
 	mq->queue = blk_init_queue(mmc_request_fn, lock);
 	if (!mq->queue)
 		return -ENOMEM;
@@ -402,6 +571,21 @@ void mmc_packed_clean(struct mmc_queue *mq)
 	mqrq_prev->packed = NULL;
 }
 
+void mmc_cmdq_clean(struct mmc_queue *mq, struct mmc_card *card)
+{
+	int i;
+	int q_depth = card->ext_csd.cmdq_depth - 1;
+
+	blk_free_tags(mq->queue->queue_tags);
+	mq->queue->queue_tags = NULL;
+	blk_queue_free_tags(mq->queue);
+
+	for (i = 0; i < q_depth; i++)
+		kfree(mq->mqrq_cmdq[i].sg);
+	kfree(mq->mqrq_cmdq);
+	mq->mqrq_cmdq = NULL;
+}
+
 /**
  * mmc_queue_suspend - suspend a MMC request queue
  * @mq: MMC queue to suspend
diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
index 36cddab..6420896 100644
--- a/drivers/mmc/card/queue.h
+++ b/drivers/mmc/card/queue.h
@@ -52,16 +52,20 @@ struct mmc_queue {
 #define MMC_QUEUE_SUSPENDED	(1 << 0)
 #define MMC_QUEUE_NEW_REQUEST	(1 << 1)
 
-	int			(*issue_fn)(struct mmc_queue *, struct request *);
+	int (*issue_fn)(struct mmc_queue *, struct request *);
+	int (*cmdq_issue_fn)(struct mmc_queue *,
+			     struct request *);
+	void (*cmdq_complete_fn)(struct request *);
 	void			*data;
 	struct request_queue	*queue;
 	struct mmc_queue_req	mqrq[2];
 	struct mmc_queue_req	*mqrq_cur;
 	struct mmc_queue_req	*mqrq_prev;
+	struct mmc_queue_req	*mqrq_cmdq;
 };
 
 extern int mmc_init_queue(struct mmc_queue *, struct mmc_card *, spinlock_t *,
-			  const char *);
+			  const char *, int);
 extern void mmc_cleanup_queue(struct mmc_queue *);
 extern void mmc_queue_suspend(struct mmc_queue *);
 extern void mmc_queue_resume(struct mmc_queue *);
@@ -76,4 +80,5 @@ extern void mmc_packed_clean(struct mmc_queue *);
 
 extern int mmc_access_rpmb(struct mmc_queue *);
 
+extern void mmc_cmdq_clean(struct mmc_queue *mq, struct mmc_card *card);
 #endif
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 45cde8c..3842208 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -187,6 +187,21 @@ struct mmc_slot {
 };
 
 /**
+ * mmc_cmdq_context_info - describes the contexts of cmdq
+ * @active_reqs		requests being processed
+ * @curr_state		state of cmdq engine
+ * @req_starved		completion should invoke the request_fn since
+ *			no tags were available
+ */
+struct mmc_cmdq_context_info {
+	unsigned long	active_reqs; /* in-flight requests */
+	unsigned long	curr_state;
+#define	CMDQ_STATE_ERR 0
+	/* no free tag available */
+	unsigned long	req_starved;
+};
+
+/**
  * mmc_context_info - synchronization details for mmc context
  * @is_done_rcv		wake up reason was done request
  * @is_new_req		wake up reason was new request
@@ -302,6 +317,7 @@ struct mmc_host {
 #define MMC_CAP2_SDIO_IRQ_NOTHREAD (1 << 17)
 #define MMC_CAP2_NO_WRITE_PROTECT (1 << 18)	/* No physical write protect pin, assume that card is always read-write */
 #define MMC_CAP2_NO_SDIO	(1 << 19)	/* Do not send SDIO commands during initialization */
+#define MMC_CAP2_CMD_QUEUE	(1 << 20)	/* support eMMC command queue */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 
@@ -384,6 +400,7 @@ struct mmc_host {
 	int			dsr_req;	/* DSR value is valid */
 	u32			dsr;	/* optional driver stage (DSR) value */
 
+	struct mmc_cmdq_context_info	cmdq_ctx;
 	unsigned long		private[0] ____cacheline_aligned;
 };
 
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH RFCv2 03/10] mmc: core: Add command queue initialzation support

[Ritesh Harjani]

From: Asutosh Das <asutoshd@codeaurora.org>

Command Queueing (CQ) feature is introduced to eMMC
standard in revision 5.1. CQ includes new commands
for issuing tasks to the device, for ordering the
execution of previously issued tasks and for
additional task management functions.

This patch adds initialization and enabling of command
queue in the card and controller. If the card and the
controller support CQ, then it is enabled.

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>
[riteshh@codeaurora.org: fixed trivial merge conflicts]
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/core/mmc.c   | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mmc/card.h |  6 ++++++
 include/linux/mmc/host.h |  6 ++++++
 include/linux/mmc/mmc.h  |  1 +
 4 files changed, 63 insertions(+)

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 77734c8..e0432b0 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1371,6 +1371,43 @@ static int mmc_hs200_tuning(struct mmc_card *card)
 	return mmc_execute_tuning(card);
 }
 
+static int mmc_select_cmdq(struct mmc_card *card)
+{
+	struct mmc_host *host = card->host;
+	int ret = 0;
+
+	if (!host->cmdq_ops) {
+		pr_err("%s: host controller doesn't support CMDQ\n",
+		       mmc_hostname(host));
+		return 0;
+	}
+
+	ret = mmc_set_blocklen(card, MMC_CARD_CMDQ_BLK_SIZE);
+	if (ret)
+		goto out;
+
+	ret = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_CMDQ, 1,
+			 card->ext_csd.generic_cmd6_time);
+	if (ret)
+		goto out;
+
+	mmc_card_set_cmdq(card);
+	ret = host->cmdq_ops->enable(card->host);
+	if (ret) {
+		pr_err("%s: failed (%d) enabling CMDQ on host\n",
+			mmc_hostname(host), ret);
+		mmc_card_clr_cmdq(card);
+		ret = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_CMDQ, 0,
+				 card->ext_csd.generic_cmd6_time);
+		if (ret)
+			goto out;
+	}
+
+	pr_debug("%s: CMDQ enabled on card\n", mmc_hostname(host));
+out:
+	return ret;
+}
+
 /*
  * Handle the detection and initialisation of a card.
  *
@@ -1399,6 +1436,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	 * respond.
 	 * mmc_go_idle is needed for eMMC that are asleep
 	 */
+reinit:
 	mmc_go_idle(host);
 
 	/* The extra bit indicates that we support high capacity */
@@ -1677,6 +1715,18 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	if (!oldcard)
 		host->card = card;
 
+	if (card->ext_csd.cmdq_support && (card->host->caps2 &
+					   MMC_CAP2_CMD_QUEUE)) {
+		err = mmc_select_cmdq(card);
+		if (err) {
+			pr_err("%s: selecting CMDQ mode: failed: %d\n",
+					   mmc_hostname(card->host), err);
+			card->ext_csd.cmdq_support = 0;
+			oldcard = card;
+			goto reinit;
+		}
+	}
+
 	return 0;
 
 free_card:
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index f74db84..85a0d8d 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -14,6 +14,8 @@
 #include <linux/mmc/core.h>
 #include <linux/mod_devicetable.h>
 
+#define MMC_CARD_CMDQ_BLK_SIZE 512
+
 struct mmc_cid {
 	unsigned int		manfid;
 	char			prod_name[8];
@@ -265,6 +267,7 @@ struct mmc_card {
 #define MMC_CARD_REMOVED	(1<<4)		/* card has been removed */
 #define MMC_STATE_DOING_BKOPS	(1<<5)		/* card is doing BKOPS */
 #define MMC_STATE_SUSPENDED	(1<<6)		/* card is suspended */
+#define MMC_STATE_CMDQ		(1<<12)         /* card is in cmd queue mode */
 	unsigned int		quirks; 	/* card quirks */
 #define MMC_QUIRK_LENIENT_FN0	(1<<0)		/* allow SDIO FN0 writes outside of the VS CCCR range */
 #define MMC_QUIRK_BLKSZ_FOR_BYTE_MODE (1<<1)	/* use func->cur_blksize */
@@ -433,6 +436,7 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 #define mmc_card_removed(c)	((c) && ((c)->state & MMC_CARD_REMOVED))
 #define mmc_card_doing_bkops(c)	((c)->state & MMC_STATE_DOING_BKOPS)
 #define mmc_card_suspended(c)	((c)->state & MMC_STATE_SUSPENDED)
+#define mmc_card_cmdq(c)       ((c)->state & MMC_STATE_CMDQ)
 
 #define mmc_card_set_present(c)	((c)->state |= MMC_STATE_PRESENT)
 #define mmc_card_set_readonly(c) ((c)->state |= MMC_STATE_READONLY)
@@ -443,6 +447,8 @@ static inline void __maybe_unused remove_quirk(struct mmc_card *card, int data)
 #define mmc_card_clr_doing_bkops(c)	((c)->state &= ~MMC_STATE_DOING_BKOPS)
 #define mmc_card_set_suspended(c) ((c)->state |= MMC_STATE_SUSPENDED)
 #define mmc_card_clr_suspended(c) ((c)->state &= ~MMC_STATE_SUSPENDED)
+#define mmc_card_set_cmdq(c)           ((c)->state |= MMC_STATE_CMDQ)
+#define mmc_card_clr_cmdq(c)           ((c)->state &= ~MMC_STATE_CMDQ)
 
 /*
  * Quirk add/remove for MMC products.
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 3842208..3c4a569 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -79,6 +79,11 @@ struct mmc_ios {
 #define MMC_SET_DRIVER_TYPE_D	3
 };
 
+struct mmc_cmdq_host_ops {
+	int (*enable)(struct mmc_host *host);
+	void (*disable)(struct mmc_host *host, bool soft);
+};
+
 struct mmc_host_ops {
 	/*
 	 * It is optional for the host to implement pre_req and post_req in
@@ -230,6 +235,7 @@ struct mmc_host {
 	struct device		class_dev;
 	int			index;
 	const struct mmc_host_ops *ops;
+	const struct mmc_cmdq_host_ops *cmdq_ops;
 	struct mmc_pwrseq	*pwrseq;
 	unsigned int		f_min;
 	unsigned int		f_max;
diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
index 694c189..55913d9 100644
--- a/include/linux/mmc/mmc.h
+++ b/include/linux/mmc/mmc.h
@@ -272,6 +272,7 @@ struct _mmc_csd {
  * EXT_CSD fields
  */
 
+#define EXT_CSD_CMDQ			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 */
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH RFCv2 04/10] mmc: card: add read/write support in command queue mode

[Ritesh Harjani]

From: Venkat Gopalakrishnan <venkatg@codeaurora.org>

Command queueing is defined in eMMC-5.1. It is designed for
higher performance by ensuring upto 32 requests to be serviced
at a time.

Adds read/write support for CMDQ enabled devices.

Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Konstantin Dorfman <kdorfman@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>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/card/block.c | 242 ++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/mmc/card/queue.h |   1 +
 drivers/mmc/core/core.c  |  57 +++++++++++
 include/linux/mmc/card.h |   3 +-
 include/linux/mmc/core.h |   8 ++
 include/linux/mmc/host.h |  22 +++++
 6 files changed, 330 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index ec99f57..57563a5 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -36,6 +36,7 @@
 #include <linux/compat.h>
 #include <linux/pm_runtime.h>
 #include <linux/idr.h>
+#include <linux/ioprio.h>
 
 #include <linux/mmc/ioctl.h>
 #include <linux/mmc/card.h>
@@ -101,6 +102,7 @@ struct mmc_blk_data {
 #define MMC_BLK_CMD23	(1 << 0)	/* Can do SET_BLOCK_COUNT for multiblock */
 #define MMC_BLK_REL_WR	(1 << 1)	/* MMC Reliable write support */
 #define MMC_BLK_PACKED_CMD	(1 << 2)	/* MMC packed command support */
+#define MMC_BLK_CMD_QUEUE	(1 << 3) /* MMC command queue support */
 
 	unsigned int	usage;
 	unsigned int	read_only;
@@ -136,6 +138,8 @@ MODULE_PARM_DESC(perdev_minors, "Minors numbers to allocate per device");
 static inline int mmc_blk_part_switch(struct mmc_card *card,
 				      struct mmc_blk_data *md);
 static int get_card_status(struct mmc_card *card, u32 *status, int retries);
+static int mmc_blk_cmdq_switch(struct mmc_card *card,
+			       struct mmc_blk_data *md, bool enable);
 
 static inline void mmc_blk_clear_packed(struct mmc_queue_req *mqrq)
 {
@@ -744,6 +748,44 @@ static const struct block_device_operations mmc_bdops = {
 #endif
 };
 
+static int mmc_blk_cmdq_switch(struct mmc_card *card,
+			       struct mmc_blk_data *md, bool enable)
+{
+	int ret = 0;
+	bool cmdq_mode = !!mmc_card_cmdq(card);
+
+	if (!(card->host->caps2 & MMC_CAP2_CMD_QUEUE) ||
+	    !card->ext_csd.cmdq_support ||
+	    (enable && !(md->flags & MMC_BLK_CMD_QUEUE)) ||
+	    (cmdq_mode == enable))
+		return 0;
+
+	if (enable) {
+		ret = mmc_set_blocklen(card, MMC_CARD_CMDQ_BLK_SIZE);
+		if (ret) {
+			pr_err("%s: failed (%d) to set block-size to %d\n",
+			       __func__, ret, MMC_CARD_CMDQ_BLK_SIZE);
+			goto out;
+		}
+	}
+
+	ret = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
+			 EXT_CSD_CMDQ, enable,
+			 card->ext_csd.generic_cmd6_time);
+	if (ret) {
+		pr_err("%s: cmdq mode %sable failed %d\n",
+		       md->disk->disk_name, enable ? "en" : "dis", ret);
+		goto out;
+	}
+
+	if (enable)
+		mmc_card_set_cmdq(card);
+	else
+		mmc_card_clr_cmdq(card);
+out:
+	return ret;
+}
+
 static inline int mmc_blk_part_switch(struct mmc_card *card,
 				      struct mmc_blk_data *md)
 {
@@ -759,6 +801,13 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
 		if (md->part_type == EXT_CSD_PART_CONFIG_ACC_RPMB)
 			mmc_retune_pause(card->host);
 
+		if (md->part_type) {
+			/* disable CQ mode for non-user data partitions */
+			ret = mmc_blk_cmdq_switch(card, md, false);
+			if (ret)
+				return ret;
+		}
+
 		part_config &= ~EXT_CSD_PART_CONFIG_ACC_MASK;
 		part_config |= md->part_type;
 
@@ -1949,6 +1998,165 @@ static void mmc_blk_revert_packed_req(struct mmc_queue *mq,
 	mmc_blk_clear_packed(mq_rq);
 }
 
+static int mmc_blk_cmdq_start_req(struct mmc_host *host,
+				  struct mmc_cmdq_req *cmdq_req)
+{
+	struct mmc_request *mrq = &cmdq_req->mrq;
+
+	mrq->done = mmc_blk_cmdq_req_done;
+	return mmc_cmdq_start_req(host, cmdq_req);
+}
+
+#define IS_RT_CLASS_REQ(x)     \
+	(IOPRIO_PRIO_CLASS(req_get_ioprio(x)) == IOPRIO_CLASS_RT)
+
+static struct mmc_cmdq_req *mmc_blk_cmdq_rw_prep(
+		struct mmc_queue_req *mqrq, struct mmc_queue *mq)
+{
+	struct mmc_card *card = mq->card;
+	struct request *req = mqrq->req;
+	struct mmc_blk_data *md = mq->data;
+	bool do_rel_wr = mmc_req_rel_wr(req) && (md->flags & MMC_BLK_REL_WR);
+	bool do_data_tag;
+	bool read_dir = (rq_data_dir(req) == READ);
+	bool prio = IS_RT_CLASS_REQ(req);
+	struct mmc_cmdq_req *cmdq_rq = &mqrq->cmdq_req;
+
+	memset(&mqrq->cmdq_req, 0, sizeof(struct mmc_cmdq_req));
+
+	cmdq_rq->tag = req->tag;
+	if (read_dir) {
+		cmdq_rq->cmdq_req_flags |= DIR;
+		cmdq_rq->data.flags = MMC_DATA_READ;
+	} else {
+		cmdq_rq->data.flags = MMC_DATA_WRITE;
+	}
+	if (prio)
+		cmdq_rq->cmdq_req_flags |= PRIO;
+
+	if (do_rel_wr)
+		cmdq_rq->cmdq_req_flags |= REL_WR;
+
+	cmdq_rq->data.blocks = blk_rq_sectors(req);
+	cmdq_rq->blk_addr = blk_rq_pos(req);
+	cmdq_rq->data.blksz = MMC_CARD_CMDQ_BLK_SIZE;
+
+	mmc_set_data_timeout(&cmdq_rq->data, card);
+
+	do_data_tag = (card->ext_csd.data_tag_unit_size) &&
+		(req->cmd_flags & REQ_META) &&
+		(rq_data_dir(req) == WRITE) &&
+		((cmdq_rq->data.blocks * cmdq_rq->data.blksz) >=
+		 card->ext_csd.data_tag_unit_size);
+	if (do_data_tag)
+		cmdq_rq->cmdq_req_flags |= DAT_TAG;
+	cmdq_rq->data.sg = mqrq->sg;
+	cmdq_rq->data.sg_len = mmc_queue_map_sg(mq, mqrq);
+
+	/*
+	 * Adjust the sg list so it is the same size as the
+	 * request.
+	 */
+	if (cmdq_rq->data.blocks > card->host->max_blk_count)
+		cmdq_rq->data.blocks = card->host->max_blk_count;
+
+	if (cmdq_rq->data.blocks != blk_rq_sectors(req)) {
+		int i, data_size = cmdq_rq->data.blocks << 9;
+		struct scatterlist *sg;
+
+		for_each_sg(cmdq_rq->data.sg, sg, cmdq_rq->data.sg_len, i) {
+			data_size -= sg->length;
+			if (data_size <= 0) {
+				sg->length += data_size;
+				i++;
+				break;
+			}
+		}
+		cmdq_rq->data.sg_len = i;
+	}
+
+	mqrq->cmdq_req.cmd_flags = req->cmd_flags;
+	mqrq->cmdq_req.mrq.req = mqrq->req;
+	mqrq->cmdq_req.mrq.cmdq_req = &mqrq->cmdq_req;
+	mqrq->cmdq_req.mrq.data = &mqrq->cmdq_req.data;
+	mqrq->req->special = mqrq;
+
+	pr_debug("%s: %s: mrq: 0x%p req: 0x%p mqrq: 0x%p bytes to xf: %d mmc_cmdq_req: 0x%p card-addr: 0x%08x dir(r-1/w-0): %d\n",
+		 mmc_hostname(card->host), __func__, &mqrq->cmdq_req.mrq,
+		 mqrq->req, mqrq, (cmdq_rq->data.blocks * cmdq_rq->data.blksz),
+		 cmdq_rq, cmdq_rq->blk_addr,
+		 (cmdq_rq->cmdq_req_flags & DIR) ? 1 : 0);
+
+	return &mqrq->cmdq_req;
+}
+
+static int mmc_blk_cmdq_issue_rw_rq(struct mmc_queue *mq, struct request *req)
+{
+	struct mmc_queue_req *active_mqrq;
+	struct mmc_card *card = mq->card;
+	struct mmc_host *host = card->host;
+	struct mmc_cmdq_req *mc_rq;
+	int ret = 0;
+
+	BUG_ON((req->tag < 0) || (req->tag > card->ext_csd.cmdq_depth));
+	BUG_ON(test_and_set_bit(req->tag, &host->cmdq_ctx.active_reqs));
+
+	active_mqrq = &mq->mqrq_cmdq[req->tag];
+	active_mqrq->req = req;
+
+	mc_rq = mmc_blk_cmdq_rw_prep(active_mqrq, mq);
+
+	ret = mmc_blk_cmdq_start_req(card->host, mc_rq);
+	return ret;
+}
+
+/* invoked by block layer in softirq context */
+void mmc_blk_cmdq_complete_rq(struct request *rq)
+{
+	struct mmc_queue_req *mq_rq = rq->special;
+	struct mmc_request *mrq = &mq_rq->cmdq_req.mrq;
+	struct mmc_host *host = mrq->host;
+	struct mmc_cmdq_context_info *ctx_info = &host->cmdq_ctx;
+	struct mmc_cmdq_req *cmdq_req = &mq_rq->cmdq_req;
+	struct mmc_queue *mq = (struct mmc_queue *)rq->q->queuedata;
+	int err = 0;
+
+	if (mrq->cmd && mrq->cmd->error)
+		err = mrq->cmd->error;
+	else if (mrq->data && mrq->data->error)
+		err = mrq->data->error;
+
+	mmc_cmdq_post_req(host, mrq, err);
+	if (err) {
+		pr_err("%s: %s: txfr error: %d\n", mmc_hostname(mrq->host),
+		       __func__, err);
+		set_bit(CMDQ_STATE_ERR, &ctx_info->curr_state);
+		WARN_ON(1);
+	}
+
+	BUG_ON(!test_and_clear_bit(cmdq_req->tag,
+				   &ctx_info->active_reqs));
+
+	blk_end_request(rq, err, cmdq_req->data.bytes_xfered);
+
+	if (test_and_clear_bit(0, &ctx_info->req_starved))
+		blk_run_queue(mq->queue);
+
+	mmc_release_host(host);
+}
+
+/*
+ * Complete reqs from block layer softirq context
+ * Invoked in irq context
+ */
+void mmc_blk_cmdq_req_done(struct mmc_request *mrq)
+{
+	struct request *req = mrq->req;
+
+	blk_complete_request(req);
+}
+EXPORT_SYMBOL(mmc_blk_cmdq_req_done);
+
 static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 {
 	struct mmc_blk_data *md = mq->data;
@@ -2141,6 +2349,28 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
 	return 0;
 }
 
+static int mmc_blk_cmdq_issue_rq(struct mmc_queue *mq, struct request *req)
+{
+	int ret;
+	struct mmc_blk_data *md = mq->data;
+	struct mmc_card *card = md->queue.card;
+
+	mmc_claim_host(card->host);
+	ret = mmc_blk_part_switch(card, md);
+	if (ret) {
+		pr_err("%s: %s: partition switch failed %d\n",
+				md->disk->disk_name, __func__, ret);
+		blk_end_request_all(req, ret);
+		mmc_release_host(card->host);
+		goto switch_failure;
+	}
+
+	ret = mmc_blk_cmdq_issue_rw_rq(mq, req);
+
+switch_failure:
+	return ret;
+}
+
 static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
 {
 	int ret;
@@ -2308,12 +2538,18 @@ again:
 	if (mmc_card_mmc(card) &&
 	    md->flags & MMC_BLK_CMD23 &&
 	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
-	     card->ext_csd.rel_sectors)) {
+	     card->ext_csd.rel_sectors) && !card->cmdq_init) {
 		md->flags |= MMC_BLK_REL_WR;
 		blk_queue_write_cache(md->queue.queue, true, true);
 	}
 
-	if (mmc_card_mmc(card) &&
+	if (card->cmdq_init) {
+		md->flags |= MMC_BLK_CMD_QUEUE;
+		md->queue.cmdq_complete_fn = mmc_blk_cmdq_complete_rq;
+		md->queue.cmdq_issue_fn = mmc_blk_cmdq_issue_rq;
+	}
+
+	if (mmc_card_mmc(card) && !card->cmdq_init &&
 	    (area_type == MMC_BLK_DATA_AREA_MAIN) &&
 	    (md->flags & MMC_BLK_CMD23) &&
 	    card->ext_csd.packed_event_en) {
@@ -2426,6 +2662,8 @@ static void mmc_blk_remove_req(struct mmc_blk_data *md)
 		mmc_cleanup_queue(&md->queue);
 		if (md->flags & MMC_BLK_PACKED_CMD)
 			mmc_packed_clean(&md->queue);
+		if (md->flags & MMC_BLK_CMD_QUEUE)
+			mmc_cmdq_clean(&md->queue, card);
 		if (md->disk->flags & GENHD_FL_UP) {
 			device_remove_file(disk_to_dev(md->disk), &md->force_ro);
 			if ((md->area_type & MMC_BLK_DATA_AREA_BOOT) &&
diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
index 6420896..34d7800 100644
--- a/drivers/mmc/card/queue.h
+++ b/drivers/mmc/card/queue.h
@@ -42,6 +42,7 @@ struct mmc_queue_req {
 	struct mmc_async_req	mmc_active;
 	enum mmc_packed_type	cmd_type;
 	struct mmc_packed	*packed;
+	struct mmc_cmdq_req	cmdq_req;
 };
 
 struct mmc_queue {
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 8b4dfd4..32b6790 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -297,6 +297,36 @@ static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 	return 0;
 }
 
+static void mmc_start_cmdq_request(struct mmc_host *host,
+				   struct mmc_request *mrq)
+{
+	if (mrq->data) {
+		pr_debug("%s:     blksz %d blocks %d flags %08x tsac %lu ms nsac %d\n",
+			mmc_hostname(host), mrq->data->blksz,
+			mrq->data->blocks, mrq->data->flags,
+			mrq->data->timeout_ns / NSEC_PER_MSEC,
+			mrq->data->timeout_clks);
+
+		BUG_ON(mrq->data->blksz > host->max_blk_size);
+		BUG_ON(mrq->data->blocks > host->max_blk_count);
+		BUG_ON(mrq->data->blocks * mrq->data->blksz >
+			host->max_req_size);
+		mrq->data->error = 0;
+		mrq->data->mrq = mrq;
+	}
+
+	if (mrq->cmd) {
+		mrq->cmd->error = 0;
+		mrq->cmd->mrq = mrq;
+	}
+
+	if (likely(host->cmdq_ops->request))
+		host->cmdq_ops->request(host, mrq);
+	else
+		pr_err("%s: %s: issue request failed\n", mmc_hostname(host),
+				__func__);
+}
+
 /**
  *	mmc_start_bkops - start BKOPS for supported cards
  *	@card: MMC card to start BKOPS
@@ -561,6 +591,33 @@ static void mmc_post_req(struct mmc_host *host, struct mmc_request *mrq,
 }
 
 /**
+ *	mmc_cmdq_post_req - post process of a completed request
+ *	@host: host instance
+ *	@mrq: the request to be processed
+ *	@err: non-zero is error, success otherwise
+ */
+void mmc_cmdq_post_req(struct mmc_host *host, struct mmc_request *mrq, int err)
+{
+	if (likely(host->cmdq_ops->post_req))
+		host->cmdq_ops->post_req(host, mrq, err);
+}
+EXPORT_SYMBOL(mmc_cmdq_post_req);
+
+int mmc_cmdq_start_req(struct mmc_host *host, struct mmc_cmdq_req *cmdq_req)
+{
+	struct mmc_request *mrq = &cmdq_req->mrq;
+
+	mrq->host = host;
+	if (mmc_card_removed(host->card)) {
+		mrq->cmd->error = -ENOMEDIUM;
+		return -ENOMEDIUM;
+	}
+	mmc_start_cmdq_request(host, mrq);
+	return 0;
+}
+EXPORT_SYMBOL(mmc_cmdq_start_req);
+
+/**
  *	mmc_start_req - start a non-blocking request
  *	@host: MMC host to start command
  *	@areq: async request to start
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 85a0d8d..8bf1742 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -318,6 +318,7 @@ struct mmc_card {
 	struct dentry		*debugfs_root;
 	struct mmc_part	part[MMC_NUM_PHY_PARTITION]; /* physical partitions */
 	unsigned int    nr_parts;
+	bool cmdq_init;
 };
 
 /*
@@ -539,5 +540,5 @@ extern void mmc_unregister_driver(struct mmc_driver *);
 
 extern void mmc_fixup_device(struct mmc_card *card,
 			     const struct mmc_fixup *table);
-
+extern void mmc_blk_cmdq_req_done(struct mmc_request *mrq);
 #endif /* LINUX_MMC_CARD_H */
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index b01e77d..a93e6f8 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -135,10 +135,18 @@ struct mmc_request {
 	struct completion	completion;
 	void			(*done)(struct mmc_request *);/* completion function */
 	struct mmc_host		*host;
+	struct mmc_cmdq_req	*cmdq_req;
+	struct request		*req;		/* associated block request */
 };
 
 struct mmc_card;
 struct mmc_async_req;
+struct mmc_cmdq_req;
+
+extern void mmc_cmdq_post_req(struct mmc_host *host, struct mmc_request *mrq,
+			      int err);
+extern int mmc_cmdq_start_req(struct mmc_host *host,
+			      struct mmc_cmdq_req *cmdq_req);
 
 extern int mmc_stop_bkops(struct mmc_card *);
 extern int mmc_read_bkops_status(struct mmc_card *);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 3c4a569..319501e 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -82,6 +82,9 @@ struct mmc_ios {
 struct mmc_cmdq_host_ops {
 	int (*enable)(struct mmc_host *host);
 	void (*disable)(struct mmc_host *host, bool soft);
+	int (*request)(struct mmc_host *host, struct mmc_request *mrq);
+	void (*post_req)(struct mmc_host *host, struct mmc_request *mrq,
+			 int err);
 };
 
 struct mmc_host_ops {
@@ -165,6 +168,25 @@ struct mmc_host_ops {
 struct mmc_card;
 struct device;
 
+struct mmc_cmdq_req {
+	unsigned int cmd_flags;
+	u32 blk_addr;
+	/* active mmc request */
+	struct mmc_request	mrq;
+	struct mmc_data		data;
+	struct mmc_command	cmd;
+#define DCMD		(1 << 0)
+#define QBR		(1 << 1)
+#define DIR		(1 << 2)
+#define PRIO		(1 << 3)
+#define REL_WR		(1 << 4)
+#define DAT_TAG	(1 << 5)
+#define FORCED_PRG	(1 << 6)
+	unsigned int		cmdq_req_flags;
+	int			tag; /* used for command queuing */
+	u8			ctx_id;
+};
+
 struct mmc_async_req {
 	/* active mmc request */
 	struct mmc_request	*mrq;
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH RFCv2 05/10] mmc: core: add flush request support to command queue

[Ritesh Harjani]

From: Asutosh Das <asutoshd@codeaurora.org>

Adds flush request support to command-queue. This uses DCMD
feature of the controller for sending commands in
command-queue mode. DCMD is a direct command feature that uses
a pre-configured slot for sending commands other than Class 11.

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/card/block.c   | 81 +++++++++++++++++++++++++++++++++++++++++++---
 drivers/mmc/card/queue.c   |  3 +-
 drivers/mmc/core/core.c    |  8 +++++
 drivers/mmc/core/mmc_ops.c | 47 +++++++++++++++++++++++----
 include/linux/mmc/core.h   |  4 +++
 include/linux/mmc/host.h   |  1 +
 6 files changed, 133 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 57563a5..51c640b 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -2007,6 +2007,26 @@ static int mmc_blk_cmdq_start_req(struct mmc_host *host,
 	return mmc_cmdq_start_req(host, cmdq_req);
 }
 
+/* prepare for non-data commands */
+static struct mmc_cmdq_req *mmc_cmdq_prep_dcmd(
+		struct mmc_queue_req *mqrq, struct mmc_queue *mq)
+{
+	struct request *req = mqrq->req;
+	struct mmc_cmdq_req *cmdq_req = &mqrq->cmdq_req;
+
+	memset(&mqrq->cmdq_req, 0, sizeof(struct mmc_cmdq_req));
+
+	cmdq_req->mrq.data = NULL;
+	cmdq_req->cmd_flags = req->cmd_flags;
+	cmdq_req->mrq.req = mqrq->req;
+	req->special = mqrq;
+	cmdq_req->cmdq_req_flags |= DCMD;
+	cmdq_req->mrq.cmdq_req = cmdq_req;
+
+	return &mqrq->cmdq_req;
+}
+
+
 #define IS_RT_CLASS_REQ(x)     \
 	(IOPRIO_PRIO_CLASS(req_get_ioprio(x)) == IOPRIO_CLASS_RT)
 
@@ -2110,6 +2130,47 @@ static int mmc_blk_cmdq_issue_rw_rq(struct mmc_queue *mq, struct request *req)
 	return ret;
 }
 
+/*
+ * Issues a flush (dcmd) request
+ */
+int mmc_blk_cmdq_issue_flush_rq(struct mmc_queue *mq, struct request *req)
+{
+	int err;
+	struct mmc_queue_req *active_mqrq;
+	struct mmc_card *card = mq->card;
+	struct mmc_host *host;
+	struct mmc_cmdq_req *cmdq_req;
+	struct mmc_cmdq_context_info *ctx_info;
+
+	BUG_ON(!card);
+	host = card->host;
+	BUG_ON(!host);
+	BUG_ON(req->tag > card->ext_csd.cmdq_depth);
+	BUG_ON(test_and_set_bit(req->tag, &host->cmdq_ctx.active_reqs));
+
+	ctx_info = &host->cmdq_ctx;
+
+	set_bit(CMDQ_STATE_DCMD_ACTIVE, &ctx_info->curr_state);
+
+	active_mqrq = &mq->mqrq_cmdq[req->tag];
+	active_mqrq->req = req;
+
+	cmdq_req = mmc_cmdq_prep_dcmd(active_mqrq, mq);
+	cmdq_req->cmdq_req_flags |= QBR;
+	cmdq_req->mrq.cmd = &cmdq_req->cmd;
+	cmdq_req->tag = req->tag;
+
+	err = mmc_cmdq_prepare_flush(cmdq_req->mrq.cmd);
+	if (err) {
+		pr_err("%s: failed (%d) preparing flush req\n",
+		       mmc_hostname(host), err);
+		return err;
+	}
+	err = mmc_blk_cmdq_start_req(card->host, cmdq_req);
+	return err;
+}
+EXPORT_SYMBOL(mmc_blk_cmdq_issue_flush_rq);
+
 /* invoked by block layer in softirq context */
 void mmc_blk_cmdq_complete_rq(struct request *rq)
 {
@@ -2136,12 +2197,17 @@ void mmc_blk_cmdq_complete_rq(struct request *rq)
 
 	BUG_ON(!test_and_clear_bit(cmdq_req->tag,
 				   &ctx_info->active_reqs));
+	if (cmdq_req->cmdq_req_flags & DCMD) {
+		clear_bit(CMDQ_STATE_DCMD_ACTIVE, &ctx_info->curr_state);
+		blk_end_request_all(rq, 0);
+		goto out;
+	}
 
 	blk_end_request(rq, err, cmdq_req->data.bytes_xfered);
 
+out:
 	if (test_and_clear_bit(0, &ctx_info->req_starved))
 		blk_run_queue(mq->queue);
-
 	mmc_release_host(host);
 }
 
@@ -2354,18 +2420,25 @@ static int mmc_blk_cmdq_issue_rq(struct mmc_queue *mq, struct request *req)
 	int ret;
 	struct mmc_blk_data *md = mq->data;
 	struct mmc_card *card = md->queue.card;
+	unsigned int cmd_flags = req ? req->cmd_flags : 0;
 
 	mmc_claim_host(card->host);
 	ret = mmc_blk_part_switch(card, md);
 	if (ret) {
 		pr_err("%s: %s: partition switch failed %d\n",
 				md->disk->disk_name, __func__, ret);
-		blk_end_request_all(req, ret);
+		if (req)
+			blk_end_request_all(req, ret);
 		mmc_release_host(card->host);
 		goto switch_failure;
 	}
 
-	ret = mmc_blk_cmdq_issue_rw_rq(mq, req);
+	if (req) {
+		if (cmd_flags & REQ_FLUSH)
+			ret = mmc_blk_cmdq_issue_flush_rq(mq, req);
+		else
+			ret = mmc_blk_cmdq_issue_rw_rq(mq, req);
+	}
 
 switch_failure:
 	return ret;
@@ -2538,7 +2611,7 @@ again:
 	if (mmc_card_mmc(card) &&
 	    md->flags & MMC_BLK_CMD23 &&
 	    ((card->ext_csd.rel_param & EXT_CSD_WR_REL_PARAM_EN) ||
-	     card->ext_csd.rel_sectors) && !card->cmdq_init) {
+	     card->ext_csd.rel_sectors)) {
 		md->flags |= MMC_BLK_REL_WR;
 		blk_queue_write_cache(md->queue.queue, true, true);
 	}
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index bb885f4..2697529 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -49,7 +49,8 @@ static int mmc_prep_request(struct request_queue *q, struct request *req)
 static inline bool mmc_cmdq_should_pull_reqs(struct mmc_host *host,
 					struct mmc_cmdq_context_info *ctx)
 {
-	if (test_bit(CMDQ_STATE_ERR, &ctx->curr_state)) {
+	if (test_bit(CMDQ_STATE_DCMD_ACTIVE, &ctx->curr_state) ||
+		test_bit(CMDQ_STATE_ERR, &ctx->curr_state)) {
 		pr_debug("%s: %s: skip pulling reqs: state: %lu\n",
 			 mmc_hostname(host), __func__, ctx->curr_state);
 		return false;
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 32b6790..fdb24cc 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -617,6 +617,14 @@ int mmc_cmdq_start_req(struct mmc_host *host, struct mmc_cmdq_req *cmdq_req)
 }
 EXPORT_SYMBOL(mmc_cmdq_start_req);
 
+int mmc_cmdq_prepare_flush(struct mmc_command *cmd)
+{
+	return   __mmc_switch_cmdq_mode(cmd, EXT_CSD_CMD_SET_NORMAL,
+				     EXT_CSD_FLUSH_CACHE, 1,
+				     0, true, true);
+}
+EXPORT_SYMBOL(mmc_cmdq_prepare_flush);
+
 /**
  *	mmc_start_req - start a non-blocking request
  *	@host: MMC host to start command
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 62355bd..c15df0b 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -456,6 +456,45 @@ int mmc_switch_status_error(struct mmc_host *host, u32 status)
 }
 
 /**
+ *	mmc_prepare_switch - helper; prepare to modify EXT_CSD register
+ *	@card: the MMC card associated with the data transfer
+ *	@set: cmd set values
+ *	@index: EXT_CSD register index
+ *	@value: value to program into EXT_CSD register
+ *	@tout_ms: timeout (ms) for operation performed by register write,
+ *                   timeout of zero implies maximum possible timeout
+ *	@use_busy_signal: use the busy signal as response type
+ *
+ *	Helper to prepare to modify EXT_CSD register for selected card.
+ */
+
+static inline void mmc_prepare_switch(struct mmc_command *cmd, u8 index,
+				      u8 value, u8 set, unsigned int tout_ms,
+				      bool use_busy_signal)
+{
+	cmd->opcode = MMC_SWITCH;
+	cmd->arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) |
+		  (index << 16) |
+		  (value << 8) |
+		  set;
+	cmd->flags = MMC_CMD_AC;
+	cmd->busy_timeout = tout_ms;
+	if (use_busy_signal)
+		cmd->flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
+	else
+		cmd->flags |= MMC_RSP_SPI_R1 | MMC_RSP_R1;
+}
+
+int __mmc_switch_cmdq_mode(struct mmc_command *cmd, u8 set, u8 index, u8 value,
+			   unsigned int timeout_ms, bool use_busy_signal,
+			   bool ignore_timeout)
+{
+	mmc_prepare_switch(cmd, index, value, set, timeout_ms, use_busy_signal);
+	return 0;
+}
+EXPORT_SYMBOL(__mmc_switch_cmdq_mode);
+
+/**
  *	__mmc_switch - modify EXT_CSD register
  *	@card: the MMC card associated with the data transfer
  *	@set: cmd set values
@@ -493,12 +532,8 @@ int __mmc_switch(struct mmc_card *card, u8 set, u8 index, u8 value,
 		(timeout_ms > host->max_busy_timeout))
 		use_r1b_resp = false;
 
-	cmd.opcode = MMC_SWITCH;
-	cmd.arg = (MMC_SWITCH_MODE_WRITE_BYTE << 24) |
-		  (index << 16) |
-		  (value << 8) |
-		  set;
-	cmd.flags = MMC_CMD_AC;
+	mmc_prepare_switch(&cmd, index, value, set, timeout_ms,
+			   use_r1b_resp);
 	if (use_r1b_resp) {
 		cmd.flags |= MMC_RSP_SPI_R1B | MMC_RSP_R1B;
 		/*
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index a93e6f8..7b3a60c 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -147,6 +147,7 @@ extern void mmc_cmdq_post_req(struct mmc_host *host, struct mmc_request *mrq,
 			      int err);
 extern int mmc_cmdq_start_req(struct mmc_host *host,
 			      struct mmc_cmdq_req *cmdq_req);
+extern int mmc_cmdq_prepare_flush(struct mmc_command *cmd);
 
 extern int mmc_stop_bkops(struct mmc_card *);
 extern int mmc_read_bkops_status(struct mmc_card *);
@@ -160,6 +161,9 @@ extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
 	struct mmc_command *, int);
 extern void mmc_start_bkops(struct mmc_card *card, bool from_exception);
 extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
+extern int __mmc_switch_cmdq_mode(struct mmc_command *cmd, u8 set, u8 index,
+				  u8 value, unsigned int timeout_ms,
+				  bool use_busy_signal, bool ignore_timeout);
 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);
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 319501e..c73cf27 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -224,6 +224,7 @@ struct mmc_cmdq_context_info {
 	unsigned long	active_reqs; /* in-flight requests */
 	unsigned long	curr_state;
 #define	CMDQ_STATE_ERR 0
+#define	CMDQ_STATE_DCMD_ACTIVE 1
 	/* no free tag available */
 	unsigned long	req_starved;
 };
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH RFCv2 06/10] mmc: host: sdhci: don't set SDMA buffer boundary in ADMA mode

[Ritesh Harjani]

From: Subhash Jadavani <subhashj@codeaurora.org>

SDMA buffer boundary size parameter in block size register should only be
programmed if host controller DMA is operating in SDMA mode otherwise its
better not to set this parameter to avoid any side effect when DMA is
operating in ADMA mode operation.

Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
[venkatg@codeaurora.org: fix trivial merge conflict]
Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
---
 drivers/mmc/host/sdhci.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0e3d7c0..9f5cdaa 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -742,6 +742,17 @@ static void sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 	}
 }
 
+static void sdhci_set_blk_size_reg(struct sdhci_host *host, unsigned int blksz,
+				   unsigned int sdma_boundary)
+{
+	if (host->flags & SDHCI_USE_ADMA)
+		sdhci_writew(host, SDHCI_MAKE_BLKSZ(0, blksz),
+			     SDHCI_BLOCK_SIZE);
+	else
+		sdhci_writew(host, SDHCI_MAKE_BLKSZ(sdma_boundary, blksz),
+			     SDHCI_BLOCK_SIZE);
+}
+
 static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	u8 ctrl;
@@ -874,8 +885,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
 	sdhci_set_transfer_irqs(host);
 
 	/* Set the DMA boundary value and block size */
-	sdhci_writew(host, SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG,
-		data->blksz), SDHCI_BLOCK_SIZE);
+	sdhci_set_blk_size_reg(host, data->blksz, SDHCI_DEFAULT_BOUNDARY_ARG);
 	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
 }
 
@@ -1935,14 +1945,11 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 		 */
 		if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200) {
 			if (mmc->ios.bus_width == MMC_BUS_WIDTH_8)
-				sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128),
-					     SDHCI_BLOCK_SIZE);
+				sdhci_set_blk_size_reg(host, 128, 7);
 			else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4)
-				sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
-					     SDHCI_BLOCK_SIZE);
+				sdhci_set_blk_size_reg(host, 64, 7);
 		} else {
-			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
-				     SDHCI_BLOCK_SIZE);
+			sdhci_set_blk_size_reg(host, 64, 7);
 		}
 
 		/*
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH RFCv2 07/10] mmc: cmdq: support for command queue enabled host

[Ritesh Harjani]

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.

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>
[subhashj@codeaurora.org: fixed trivial merge conflicts]
Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/Kconfig    |  13 +
 drivers/mmc/host/Makefile   |   1 +
 drivers/mmc/host/cmdq_hci.c | 636 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/mmc/host/cmdq_hci.h | 207 ++++++++++++++
 include/linux/mmc/host.h    |  12 +
 5 files changed, 869 insertions(+)
 create mode 100644 drivers/mmc/host/cmdq_hci.c
 create mode 100644 drivers/mmc/host/cmdq_hci.h

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 0aa484c..0381389 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -774,6 +774,19 @@ config MMC_SUNXI
 	  This selects support for the SD/MMC Host Controller on
 	  Allwinner sunxi SoCs.
 
+config MMC_CQ_HCI
+	tristate "Command Queue 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 af918d2..3715f73 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -76,6 +76,7 @@ obj-$(CONFIG_MMC_SDHCI_IPROC)		+= sdhci-iproc.o
 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_CQ_HCI)		+= cmdq_hci.o
 
 ifeq ($(CONFIG_CB710_DEBUG),y)
 	CFLAGS-cb710-mmc	+= -DDEBUG
diff --git a/drivers/mmc/host/cmdq_hci.c b/drivers/mmc/host/cmdq_hci.c
new file mode 100644
index 0000000..322b77c
--- /dev/null
+++ b/drivers/mmc/host/cmdq_hci.c
@@ -0,0 +1,636 @@
+/* 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/blkdev.h>
+
+#include <linux/mmc/mmc.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/card.h>
+
+#include "cmdq_hci.h"
+
+#define DCMD_SLOT 31
+#define NUM_SLOTS 32
+
+static inline u8 *get_desc(struct cmdq_host *cq_host, u8 tag)
+{
+	return cq_host->desc_base + (tag * cq_host->slot_sz);
+}
+
+static inline u8 *get_link_desc(struct cmdq_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 cmdq_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 cmdq_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 cmdq_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) {
+		*link_temp = VALID(0) | ACT(0) | END(1);
+		return;
+	}
+
+	*link_temp = VALID(1) | ACT(0x6) | 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 cmdq_clear_set_irqs(struct cmdq_host *cq_host, u32 clear, u32 set)
+{
+	u32 ier;
+
+	ier = cmdq_readl(cq_host, CQISTE);
+	ier &= ~clear;
+	ier |= set;
+	cmdq_writel(cq_host, ier, CQISTE);
+	cmdq_writel(cq_host, ier, CQISGE);
+	/* ensure the writes are done */
+	mb();
+}
+
+
+#define DRV_NAME "cmdq-host"
+
+static void cmdq_dumpregs(struct cmdq_host *cq_host)
+{
+	struct mmc_host *mmc = cq_host->mmc;
+
+	pr_info(DRV_NAME ": ========== REGISTER DUMP (%s)==========\n",
+		mmc_hostname(mmc));
+
+	pr_info(DRV_NAME ": Caps: 0x%08x	  | Version:  0x%08x\n",
+		cmdq_readl(cq_host, CQCAP),
+		cmdq_readl(cq_host, CQVER));
+	pr_info(DRV_NAME ": Queing config: 0x%08x | Queue Ctrl:  0x%08x\n",
+		cmdq_readl(cq_host, CQCFG),
+		cmdq_readl(cq_host, CQCTL));
+	pr_info(DRV_NAME ": Int stat: 0x%08x	  | Int enab:  0x%08x\n",
+		cmdq_readl(cq_host, CQIS),
+		cmdq_readl(cq_host, CQISTE));
+	pr_info(DRV_NAME ": Int sig: 0x%08x	  | Int Coal:  0x%08x\n",
+		cmdq_readl(cq_host, CQISGE),
+		cmdq_readl(cq_host, CQIC));
+	pr_info(DRV_NAME ": TDL base: 0x%08x	  | TDL up32:  0x%08x\n",
+		cmdq_readl(cq_host, CQTDLBA),
+		cmdq_readl(cq_host, CQTDLBAU));
+	pr_info(DRV_NAME ": Doorbell: 0x%08x	  | Comp Notif:  0x%08x\n",
+		cmdq_readl(cq_host, CQTDBR),
+		cmdq_readl(cq_host, CQTCN));
+	pr_info(DRV_NAME ": Dev queue: 0x%08x	  | Dev Pend:  0x%08x\n",
+		cmdq_readl(cq_host, CQDQS),
+		cmdq_readl(cq_host, CQDPT));
+	pr_info(DRV_NAME ": Task clr: 0x%08x	  | Send stat 1:  0x%08x\n",
+		cmdq_readl(cq_host, CQTCLR),
+		cmdq_readl(cq_host, CQSSC1));
+	pr_info(DRV_NAME ": Send stat 2: 0x%08x	  | DCMD resp:  0x%08x\n",
+		cmdq_readl(cq_host, CQSSC2),
+		cmdq_readl(cq_host, CQCRDCT));
+	pr_info(DRV_NAME ": Resp err mask: 0x%08x | Task err:  0x%08x\n",
+		cmdq_readl(cq_host, CQRMEM),
+		cmdq_readl(cq_host, CQTERRI));
+	pr_info(DRV_NAME ": Resp idx 0x%08x	  | Resp arg:  0x%08x\n",
+		cmdq_readl(cq_host, CQCRI),
+		cmdq_readl(cq_host, CQCRA));
+	pr_info(DRV_NAME ": ===========================================\n");
+
+	if (cq_host->ops->dump_vendor_regs)
+		cq_host->ops->dump_vendor_regs(mmc);
+}
+
+/**
+ * The allocated descriptor table for task, link & transfer descritors
+ * looks like:
+ * |----------|
+ * |task desc |  |->|----------|
+ * |----------|  |  |trans desc|
+ * |link desc-|->|  |----------|
+ * |----------|          .
+ *      .                .
+ *  no. of slots      max-segs
+ *      .           |----------|
+ * |----------|
+ * The idea here is to create the [task+trans] table and mark & point the
+ * link desc to the transfer desc table on a per slot basis.
+ */
+static int cmdq_host_alloc_tdl(struct cmdq_host *cq_host)
+{
+
+	size_t desc_size;
+	size_t data_size;
+	int i = 0;
+
+	/* task descriptor can be 64/128 bit irrespective of arch */
+	if (cq_host->caps & CMDQ_TASK_DESC_SZ_128) {
+		cmdq_writel(cq_host, cmdq_readl(cq_host, CQCFG) |
+			       CQ_TASK_DESC_SZ, CQCFG);
+		cq_host->task_desc_len = 16;
+	} else {
+		cq_host->task_desc_len = 8;
+	}
+
+	/*
+	 * 96 bits length of transfer desc instead of 128 bits which means
+	 * ADMA would expect next valid descriptor at the 96th bit
+	 * or 128th bit
+	 */
+	if (cq_host->dma64) {
+		if (cq_host->quirks & CMDQ_QUIRK_SHORT_TXFR_DESC_SZ)
+			cq_host->trans_desc_len = 12;
+		else
+			cq_host->trans_desc_len = 16;
+		cq_host->link_desc_len = 16;
+	} else {
+		cq_host->trans_desc_len = 8;
+		cq_host->link_desc_len = 8;
+	}
+
+	/* total size of a slot: 1 task & 1 transfer (link) */
+	cq_host->slot_sz = cq_host->task_desc_len + cq_host->link_desc_len;
+
+	desc_size = cq_host->slot_sz * cq_host->num_slots;
+
+	data_size = cq_host->trans_desc_len * cq_host->mmc->max_segs *
+		(cq_host->num_slots - 1);
+
+	pr_info("%s: desc_size: %d data_sz: %d slot-sz: %d\n", __func__,
+		(int)desc_size, (int)data_size, cq_host->slot_sz);
+
+	/*
+	 * allocate a dma-mapped chunk of memory for the descriptors
+	 * allocate a dma-mapped chunk of memory for link descriptors
+	 * setup each link-desc memory offset per slot-number to
+	 * the descriptor table.
+	 */
+	cq_host->desc_base = dmam_alloc_coherent(mmc_dev(cq_host->mmc),
+						 desc_size,
+						 &cq_host->desc_dma_base,
+						 GFP_KERNEL);
+	cq_host->trans_desc_base = dmam_alloc_coherent(mmc_dev(cq_host->mmc),
+					      data_size,
+					      &cq_host->trans_desc_dma_base,
+					      GFP_KERNEL);
+	if (!cq_host->desc_base || !cq_host->trans_desc_base)
+		return -ENOMEM;
+
+	pr_info("desc-base: 0x%p trans-base: 0x%p\n desc_dma 0x%llx trans_dma: 0x%llx\n",
+		 cq_host->desc_base, cq_host->trans_desc_base,
+		(unsigned long long)cq_host->desc_dma_base,
+		(unsigned long long) cq_host->trans_desc_dma_base);
+
+	for (; i < (cq_host->num_slots); i++)
+		setup_trans_desc(cq_host, i);
+
+	return 0;
+}
+
+static int cmdq_enable(struct mmc_host *mmc)
+{
+	int err = 0;
+	u32 cqcfg;
+	bool dcmd_enable;
+	struct cmdq_host *cq_host = mmc_cmdq_private(mmc);
+
+	if (!cq_host || !mmc->card || !mmc_card_cmdq(mmc->card)) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	if (cq_host->enabled)
+		goto out;
+
+	cqcfg = cmdq_readl(cq_host, CQCFG);
+	if (cqcfg & 0x1) {
+		pr_info("%s: %s: cq_host is already enabled\n",
+				mmc_hostname(mmc), __func__);
+		WARN_ON(1);
+		goto out;
+	}
+
+	if (cq_host->quirks & CMDQ_QUIRK_NO_DCMD)
+		dcmd_enable = false;
+	else
+		dcmd_enable = true;
+
+	cqcfg = ((cq_host->caps & CMDQ_TASK_DESC_SZ_128 ? CQ_TASK_DESC_SZ : 0) |
+			(dcmd_enable ? CQ_DCMD : 0));
+
+	cmdq_writel(cq_host, cqcfg, CQCFG);
+	/* enable CQ_HOST */
+	cmdq_writel(cq_host, cmdq_readl(cq_host, CQCFG) | CQ_ENABLE,
+		    CQCFG);
+
+	if (!cq_host->desc_base ||
+			!cq_host->trans_desc_base) {
+		err = cmdq_host_alloc_tdl(cq_host);
+		if (err)
+			goto out;
+		cmdq_writel(cq_host, lower_32_bits(cq_host->desc_dma_base),
+				CQTDLBA);
+		cmdq_writel(cq_host, upper_32_bits(cq_host->desc_dma_base),
+				CQTDLBAU);
+		cmdq_dumpregs(cq_host);
+	}
+
+	/*
+	 * disable all vendor interrupts
+	 * enable CMDQ interrupts
+	 * enable the vendor error interrupts
+	 */
+	if (cq_host->ops->clear_set_irqs)
+		cq_host->ops->clear_set_irqs(mmc, true);
+
+	cmdq_clear_set_irqs(cq_host, 0x0, CQ_INT_ALL);
+
+	/* cq_host would use this rca to address the card */
+	cmdq_writel(cq_host, mmc->card->rca, CQSSC2);
+
+	/* send QSR at lesser intervals than the default */
+	cmdq_writel(cq_host, cmdq_readl(cq_host, CQSSC1) | SEND_QSR_INTERVAL,
+				CQSSC1);
+
+	/* ensure the writes are done before enabling CQE */
+	mb();
+
+	cq_host->enabled = true;
+
+	if (cq_host->ops->set_block_size)
+		cq_host->ops->set_block_size(cq_host->mmc);
+
+	if (cq_host->ops->set_data_timeout)
+		cq_host->ops->set_data_timeout(mmc, 0xf);
+
+	if (cq_host->ops->clear_set_dumpregs)
+		cq_host->ops->clear_set_dumpregs(mmc, 1);
+
+out:
+	return err;
+}
+
+static void cmdq_disable(struct mmc_host *mmc, bool soft)
+{
+	struct cmdq_host *cq_host = (struct cmdq_host *)mmc_cmdq_private(mmc);
+
+	if (soft)
+		cmdq_writel(cq_host, cmdq_readl(
+				    cq_host, CQCFG) & ~(CQ_ENABLE),
+			    CQCFG);
+	cq_host->enabled = false;
+}
+
+static void cmdq_prep_task_desc(struct mmc_request *mrq,
+					u64 *data, bool intr, bool qbr)
+{
+	struct mmc_cmdq_req *cmdq_req = mrq->cmdq_req;
+	u32 req_flags = cmdq_req->cmdq_req_flags;
+
+	pr_debug("%s: %s: data-tag: 0x%08x - dir: %d - prio: %d - cnt: 0x%08x -	addr: 0x%llx\n",
+		 mmc_hostname(mrq->host), __func__,
+		 !!(req_flags & DAT_TAG), !!(req_flags & DIR),
+		 !!(req_flags & PRIO), cmdq_req->data.blocks,
+		 (u64)mrq->cmdq_req->blk_addr);
+
+	*data = VALID(1) |
+		END(1) |
+		INT(intr) |
+		ACT(0x5) |
+		FORCED_PROG(!!(req_flags & FORCED_PRG)) |
+		CONTEXT(mrq->cmdq_req->ctx_id) |
+		DATA_TAG(!!(req_flags & DAT_TAG)) |
+		DATA_DIR(!!(req_flags & DIR)) |
+		PRIORITY(!!(req_flags & PRIO)) |
+		QBAR(qbr) |
+		REL_WRITE(!!(req_flags & REL_WR)) |
+		BLK_COUNT(mrq->cmdq_req->data.blocks) |
+		BLK_ADDR((u64)mrq->cmdq_req->blk_addr);
+}
+
+static int cmdq_dma_map(struct mmc_host *host, struct mmc_request *mrq)
+{
+	int sg_count;
+	struct mmc_data *data = mrq->data;
+
+	if (!data)
+		return -EINVAL;
+
+	sg_count = dma_map_sg(mmc_dev(host), data->sg,
+			      data->sg_len,
+			      (data->flags & MMC_DATA_WRITE) ?
+			      DMA_TO_DEVICE : DMA_FROM_DEVICE);
+	if (!sg_count) {
+		pr_err("%s: sg-len: %d\n", __func__, data->sg_len);
+		return -ENOMEM;
+	}
+
+	return sg_count;
+}
+
+static void cmdq_set_tran_desc(u8 *desc,
+				 dma_addr_t addr, int len, bool end)
+{
+	__le64 *dataddr = (__le64 __force *)(desc + 4);
+	__le32 *attr = (__le32 __force *)desc;
+
+	*attr = (VALID(1) |
+		 END(end ? 1 : 0) |
+		 INT(0) |
+		 ACT(0x4) |
+		 DAT_LENGTH(len));
+
+	dataddr[0] = cpu_to_le64(addr);
+}
+
+static int cmdq_prep_tran_desc(struct mmc_request *mrq,
+			       struct cmdq_host *cq_host, int tag)
+{
+	struct mmc_data *data = mrq->data;
+	int i, sg_count, len;
+	bool end = false;
+	dma_addr_t addr;
+	u8 *desc;
+	struct scatterlist *sg;
+
+	sg_count = cmdq_dma_map(mrq->host, mrq);
+	if (sg_count < 0) {
+		pr_err("%s: %s: unable to map sg lists, %d\n",
+				mmc_hostname(mrq->host), __func__, sg_count);
+		return sg_count;
+	}
+
+	desc = get_trans_desc(cq_host, tag);
+	memset(desc, 0, cq_host->trans_desc_len * cq_host->mmc->max_segs);
+
+	for_each_sg(data->sg, sg, sg_count, i) {
+		addr = sg_dma_address(sg);
+		len = sg_dma_len(sg);
+
+		if ((i+1) == sg_count)
+			end = true;
+		cmdq_set_tran_desc(desc, addr, len, end);
+		desc += cq_host->trans_desc_len;
+	}
+
+	pr_debug("%s: req: 0x%p tag: %d calc_trans_des: 0x%p sg-cnt: %d\n",
+		__func__, mrq->req, tag, desc, sg_count);
+
+	return 0;
+}
+
+static void cmdq_prep_dcmd_desc(struct mmc_host *mmc,
+				   struct mmc_request *mrq)
+{
+	u64 *task_desc = NULL;
+	u64 data = 0;
+	u8 resp_type;
+	u8 *desc;
+	__le64 *dataddr;
+	struct cmdq_host *cq_host = mmc_cmdq_private(mmc);
+	u8 timing;
+
+	if (!(mrq->cmd->flags & MMC_RSP_PRESENT)) {
+		resp_type = 0x0;
+		timing = 0x1;
+	} else {
+		if (mrq->cmd->flags & MMC_RSP_R1B) {
+			resp_type = 0x3;
+			timing = 0x0;
+		} else {
+			resp_type = 0x2;
+			timing = 0x1;
+		}
+	}
+
+	task_desc = (__le64 __force *)get_desc(cq_host, cq_host->dcmd_slot);
+	memset(task_desc, 0, cq_host->task_desc_len);
+	data |= (VALID(1) |
+		 END(1) |
+		 INT(1) |
+		 QBAR(1) |
+		 ACT(0x5) |
+		 CMD_INDEX(mrq->cmd->opcode) |
+		 CMD_TIMING(timing) | RESP_TYPE(resp_type));
+	*task_desc |= data;
+	desc = (u8 *)task_desc;
+	pr_debug("cmdq: dcmd: cmd: %d timing: %d resp: %d\n",
+		mrq->cmd->opcode, timing, resp_type);
+	dataddr = (__le64 __force *)(desc + 4);
+	dataddr[0] = cpu_to_le64((u64)mrq->cmd->arg);
+
+}
+
+static int cmdq_request(struct mmc_host *mmc, struct mmc_request *mrq)
+{
+	int err;
+	u64 data = 0;
+	u64 *task_desc = NULL;
+	u32 tag = mrq->cmdq_req->tag;
+	struct cmdq_host *cq_host = (struct cmdq_host *)mmc_cmdq_private(mmc);
+
+	if (!cq_host->enabled) {
+		pr_err("%s: CMDQ host not enabled yet !!!\n",
+		       mmc_hostname(mmc));
+		err = -EINVAL;
+		goto out;
+	}
+
+	if (mrq->cmdq_req->cmdq_req_flags & DCMD) {
+		cmdq_prep_dcmd_desc(mmc, mrq);
+		cq_host->mrq_slot[DCMD_SLOT] = mrq;
+		cmdq_writel(cq_host, 1 << DCMD_SLOT, CQTDBR);
+		return 0;
+	}
+
+	task_desc = (__le64 __force *)get_desc(cq_host, tag);
+
+	cmdq_prep_task_desc(mrq, &data, 1,
+			    (mrq->cmdq_req->cmdq_req_flags & QBR));
+	*task_desc = cpu_to_le64(data);
+
+	err = cmdq_prep_tran_desc(mrq, cq_host, tag);
+	if (err) {
+		pr_err("%s: %s: failed to setup tx desc: %d\n",
+		       mmc_hostname(mmc), __func__, err);
+		return err;
+	}
+
+	BUG_ON(cmdq_readl(cq_host, CQTDBR) & (1 << tag));
+
+	cq_host->mrq_slot[tag] = mrq;
+	if (cq_host->ops->set_transfer_params)
+		cq_host->ops->set_transfer_params(mmc);
+
+	cmdq_writel(cq_host, 1 << tag, CQTDBR);
+
+out:
+	return err;
+}
+
+static void cmdq_finish_data(struct mmc_host *mmc, unsigned int tag)
+{
+	struct mmc_request *mrq;
+	struct cmdq_host *cq_host = (struct cmdq_host *)mmc_cmdq_private(mmc);
+
+	mrq = cq_host->mrq_slot[tag];
+	mrq->done(mrq);
+}
+
+irqreturn_t cmdq_irq(struct mmc_host *mmc, u32 intmask)
+{
+	u32 status;
+	unsigned long tag = 0, comp_status;
+	struct cmdq_host *cq_host = (struct cmdq_host *)mmc_cmdq_private(mmc);
+
+	status = cmdq_readl(cq_host, CQIS);
+	cmdq_writel(cq_host, status, CQIS);
+
+	if (status & CQIS_TCC) {
+		/* read QCTCN and complete the request */
+		comp_status = cmdq_readl(cq_host, CQTCN);
+		if (!comp_status)
+			goto out;
+
+		for_each_set_bit(tag, &comp_status, cq_host->num_slots) {
+			/* complete the corresponding mrq */
+			pr_debug("%s: completing tag -> %lu\n",
+				 mmc_hostname(mmc), tag);
+			cmdq_finish_data(mmc, tag);
+		}
+		cmdq_writel(cq_host, comp_status, CQTCN);
+	}
+
+	if (status & CQIS_RED) {
+		/* task response has an error */
+		pr_err("%s: RED error %d !!!\n", mmc_hostname(mmc), status);
+		cmdq_dumpregs(cq_host);
+	}
+
+out:
+	return IRQ_HANDLED;
+}
+EXPORT_SYMBOL(cmdq_irq);
+
+static void cmdq_post_req(struct mmc_host *host, struct mmc_request *mrq,
+			  int err)
+{
+	struct mmc_data *data = mrq->data;
+
+	if (data) {
+		data->error = err;
+		dma_unmap_sg(mmc_dev(host), data->sg, data->sg_len,
+			     (data->flags & MMC_DATA_READ) ?
+			     DMA_FROM_DEVICE : DMA_TO_DEVICE);
+		if (err)
+			data->bytes_xfered = 0;
+		else
+			data->bytes_xfered = blk_rq_bytes(mrq->req);
+	}
+}
+
+static const struct mmc_cmdq_host_ops cmdq_host_ops = {
+	.enable = cmdq_enable,
+	.disable = cmdq_disable,
+	.request = cmdq_request,
+	.post_req = cmdq_post_req,
+};
+
+struct cmdq_host *cmdq_pltfm_init(struct platform_device *pdev)
+{
+	struct cmdq_host *cq_host;
+	struct resource *cmdq_memres = NULL;
+
+	/* check and setup CMDQ interface */
+	cmdq_memres = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   "cmdq_mem");
+	if (!cmdq_memres) {
+		dev_dbg(&pdev->dev, "CMDQ not supported\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	cq_host = kzalloc(sizeof(*cq_host), GFP_KERNEL);
+	if (!cq_host)
+		return ERR_PTR(-ENOMEM);
+	cq_host->mmio = devm_ioremap(&pdev->dev,
+				     cmdq_memres->start,
+				     resource_size(cmdq_memres));
+	if (!cq_host->mmio) {
+		dev_err(&pdev->dev, "failed to remap cmdq regs\n");
+		kfree(cq_host);
+		return ERR_PTR(-EBUSY);
+	}
+	dev_dbg(&pdev->dev, "CMDQ ioremap: done\n");
+
+	return cq_host;
+}
+EXPORT_SYMBOL(cmdq_pltfm_init);
+
+int cmdq_init(struct cmdq_host *cq_host, struct mmc_host *mmc,
+	      bool dma64)
+{
+	int err = 0;
+
+	cq_host->dma64 = dma64;
+	cq_host->mmc = mmc;
+	cq_host->mmc->cmdq_private = cq_host;
+
+	cq_host->num_slots = NUM_SLOTS;
+	cq_host->dcmd_slot = DCMD_SLOT;
+
+	mmc->cmdq_ops = &cmdq_host_ops;
+
+	cq_host->mrq_slot = kzalloc(sizeof(cq_host->mrq_slot) *
+				    cq_host->num_slots, GFP_KERNEL);
+	if (!cq_host->mrq_slot)
+		return -ENOMEM;
+
+	init_completion(&cq_host->halt_comp);
+	return err;
+}
+EXPORT_SYMBOL(cmdq_init);
diff --git a/drivers/mmc/host/cmdq_hci.h b/drivers/mmc/host/cmdq_hci.h
new file mode 100644
index 0000000..3231864
--- /dev/null
+++ b/drivers/mmc/host/cmdq_hci.h
@@ -0,0 +1,207 @@
+/* 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.
+ */
+#ifndef LINUX_MMC_CQ_HCI_H
+#define LINUX_MMC_CQ_HCI_H
+#include <linux/mmc/core.h>
+
+/* registers */
+/* version */
+#define CQVER		0x00
+/* capabilities */
+#define CQCAP		0x04
+/* configuration */
+#define CQCFG		0x08
+#define CQ_DCMD		0x00001000
+#define CQ_TASK_DESC_SZ 0x00000100
+#define CQ_ENABLE	0x00000001
+
+/* control */
+#define CQCTL		0x0C
+#define CLEAR_ALL_TASKS 0x00000100
+#define HALT		0x00000001
+
+/* interrupt status */
+#define CQIS		0x10
+#define CQIS_HAC	BIT(0)
+#define CQIS_TCC	BIT(1)
+#define CQIS_RED	BIT(2)
+#define CQIS_TCL	BIT(3)
+
+/* interrupt status enable */
+#define CQISTE		0x14
+
+/* interrupt signal enable */
+#define CQISGE		0x18
+
+/* interrupt coalescing */
+#define CQIC		0x1C
+#define CQIC_ENABLE	BIT(31)
+#define CQIC_RESET	BIT(16)
+#define CQIC_ICCTHWEN	BIT(15)
+#define CQIC_ICCTH(x)	((x & 0x1F) << 8)
+#define CQIC_ICTOVALWEN BIT(7)
+#define CQIC_ICTOVAL(x) (x & 0x7F)
+
+/* task list base address */
+#define CQTDLBA		0x20
+
+/* task list base address upper */
+#define CQTDLBAU	0x24
+
+/* door-bell */
+#define CQTDBR		0x28
+
+/* task completion notification */
+#define CQTCN		0x2C
+
+/* device queue status */
+#define CQDQS		0x30
+
+/* device pending tasks */
+#define CQDPT		0x34
+
+/* task clear */
+#define CQTCLR		0x38
+
+/* send status config 1 */
+#define CQSSC1		0x40
+/*
+ * Value n means CQE would send CMD13 during the transfer of data block
+ * BLOCK_CNT-n
+ */
+#define SEND_QSR_INTERVAL 0x70000
+
+/* send status config 2 */
+#define CQSSC2		0x44
+
+/* response for dcmd */
+#define CQCRDCT		0x48
+
+/* response mode error mask */
+#define CQRMEM		0x50
+
+/* task error info */
+#define CQTERRI		0x54
+
+/* command response index */
+#define CQCRI		0x58
+
+/* command response argument */
+#define CQCRA		0x5C
+
+#define CQ_INT_ALL	0xF
+#define CQIC_DEFAULT_ICCTH 31
+#define CQIC_DEFAULT_ICTOVAL 1
+
+/* attribute fields */
+#define VALID(x)	((x & 1) << 0)
+#define END(x)		((x & 1) << 1)
+#define INT(x)		((x & 1) << 2)
+#define ACT(x)		((x & 0x7) << 3)
+
+/* data command task descriptor fields */
+#define FORCED_PROG(x)	((x & 1) << 6)
+#define CONTEXT(x)	((x & 0xF) << 7)
+#define DATA_TAG(x)	((x & 1) << 11)
+#define DATA_DIR(x)	((x & 1) << 12)
+#define PRIORITY(x)	((x & 1) << 13)
+#define QBAR(x)		((x & 1) << 14)
+#define REL_WRITE(x)	((x & 1) << 15)
+#define BLK_COUNT(x)	((x & 0xFFFF) << 16)
+#define BLK_ADDR(x)	((x & 0xFFFFFFFF) << 32)
+
+/* direct command task descriptor fields */
+#define CMD_INDEX(x)	((x & 0x3F) << 16)
+#define CMD_TIMING(x)	((x & 1) << 22)
+#define RESP_TYPE(x)	((x & 0x3) << 23)
+
+/* transfer descriptor fields */
+#define DAT_LENGTH(x)	((x & 0xFFFF) << 16)
+#define DAT_ADDR_LO(x)	((x & 0xFFFFFFFF) << 32)
+#define DAT_ADDR_HI(x)	((x & 0xFFFFFFFF) << 0)
+
+struct cmdq_host {
+	const struct cmdq_host_ops *ops;
+	void __iomem *mmio;
+	struct mmc_host *mmc;
+
+	/* 64 bit DMA */
+	bool dma64;
+	int num_slots;
+
+	u32 dcmd_slot;
+	u32 caps;
+#define CMDQ_TASK_DESC_SZ_128 0x1
+
+	u32 quirks;
+#define CMDQ_QUIRK_SHORT_TXFR_DESC_SZ 0x1
+#define CMDQ_QUIRK_NO_DCMD	0x2
+
+	bool enabled;
+	bool halted;
+	bool init_done;
+
+	u8 *desc_base;
+
+	/* total descriptor size */
+	u8 slot_sz;
+
+	/* 64/128 bit depends on CQCFG */
+	u8 task_desc_len;
+
+	/* 64 bit on 32-bit arch, 128 bit on 64-bit */
+	u8 link_desc_len;
+
+	u8 *trans_desc_base;
+	/* same length as transfer descriptor */
+	u8 trans_desc_len;
+
+	dma_addr_t desc_dma_base;
+	dma_addr_t trans_desc_dma_base;
+
+	struct completion halt_comp;
+	struct mmc_request **mrq_slot;
+	void *private;
+};
+
+struct cmdq_host_ops {
+	void (*set_transfer_params)(struct mmc_host *mmc);
+	void (*set_data_timeout)(struct mmc_host *mmc, u32 val);
+	void (*clear_set_irqs)(struct mmc_host *mmc, bool clear);
+	void (*set_block_size)(struct mmc_host *mmc);
+	void (*dump_vendor_regs)(struct mmc_host *mmc);
+	void (*write_l)(struct cmdq_host *host, u32 val, int reg);
+	u32 (*read_l)(struct cmdq_host *host, int reg);
+	void (*clear_set_dumpregs)(struct mmc_host *mmc, bool set);
+};
+
+static inline void cmdq_writel(struct cmdq_host *host, u32 val, int reg)
+{
+	if (unlikely(host->ops->write_l))
+		host->ops->write_l(host, val, reg);
+	else
+		writel_relaxed(val, host->mmio + reg);
+}
+
+static inline u32 cmdq_readl(struct cmdq_host *host, int reg)
+{
+	if (unlikely(host->ops->read_l))
+		return host->ops->read_l(host, reg);
+	else
+		return readl_relaxed(host->mmio + reg);
+}
+
+extern irqreturn_t cmdq_irq(struct mmc_host *mmc, u32 intmask);
+extern int cmdq_init(struct cmdq_host *cq_host, struct mmc_host *mmc,
+		     bool dma64);
+extern struct cmdq_host *cmdq_pltfm_init(struct platform_device *pdev);
+#endif
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index c73cf27..41b4e25 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -430,6 +430,13 @@ struct mmc_host {
 	u32			dsr;	/* optional driver stage (DSR) value */
 
 	struct mmc_cmdq_context_info	cmdq_ctx;
+	/*
+	 * several cmdq supporting host controllers are extensions
+	 * of legacy controllers. This variable can be used to store
+	 * a reference to the cmdq extension of the existing host
+	 * controller.
+	 */
+	void *cmdq_private;
 	unsigned long		private[0] ____cacheline_aligned;
 };
 
@@ -444,6 +451,11 @@ static inline void *mmc_priv(struct mmc_host *host)
 	return (void *)host->private;
 }
 
+static inline void *mmc_cmdq_private(struct mmc_host *host)
+{
+	return host->cmdq_private;
+}
+
 #define mmc_host_is_spi(host)	((host)->caps & MMC_CAP_SPI)
 
 #define mmc_dev(x)	((x)->parent)
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH RFCv2 08/10] mmc: core: Add halt support

[Ritesh Harjani]

From: Asutosh Das <asutoshd@codeaurora.org>

Halt is a controller feature that can change the controller mode
from command-queue to legacy. This feature is very helpful in
error cases.

Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
[riteshh@codeaurora.org: fixed merge conflicts]
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/card/queue.c |  1 +
 drivers/mmc/core/core.c  | 33 +++++++++++++++++++++++++++++++++
 include/linux/mmc/core.h |  1 +
 include/linux/mmc/host.h | 17 +++++++++++++++++
 4 files changed, 52 insertions(+)

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 2697529..054a0b3 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -50,6 +50,7 @@ static inline bool mmc_cmdq_should_pull_reqs(struct mmc_host *host,
 					struct mmc_cmdq_context_info *ctx)
 {
 	if (test_bit(CMDQ_STATE_DCMD_ACTIVE, &ctx->curr_state) ||
+		mmc_host_halt(host) ||
 		test_bit(CMDQ_STATE_ERR, &ctx->curr_state)) {
 		pr_debug("%s: %s: skip pulling reqs: state: %lu\n",
 			 mmc_hostname(host), __func__, ctx->curr_state);
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index fdb24cc..ab7f7fa 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -603,6 +603,39 @@ void mmc_cmdq_post_req(struct mmc_host *host, struct mmc_request *mrq, int err)
 }
 EXPORT_SYMBOL(mmc_cmdq_post_req);
 
+/**
+ *	mmc_cmdq_halt - halt/un-halt the command queue engine
+ *	@host: host instance
+ *	@halt: true - halt, un-halt otherwise
+ *
+ *	Host halts the command queue engine. It should complete
+ *	the ongoing transfer and release the bus.
+ *	All legacy commands can be sent upon successful
+ *	completion of this function.
+ *	Returns 0 on success, negative otherwise
+ */
+int mmc_cmdq_halt(struct mmc_host *host, bool halt)
+{
+	int err = 0;
+
+	if ((halt && mmc_host_halt(host)) ||
+	    (!halt && !mmc_host_halt(host)))
+		return -EINVAL;
+
+	if (host->cmdq_ops->halt) {
+		err = host->cmdq_ops->halt(host, halt);
+		if (!err && halt)
+			mmc_host_set_halt(host);
+		else if (!err && !halt)
+			mmc_host_clr_halt(host);
+	} else {
+		err = -ENOSYS;
+	}
+
+	return err;
+}
+EXPORT_SYMBOL(mmc_cmdq_halt);
+
 int mmc_cmdq_start_req(struct mmc_host *host, struct mmc_cmdq_req *cmdq_req)
 {
 	struct mmc_request *mrq = &cmdq_req->mrq;
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 7b3a60c..92aa8cc 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -143,6 +143,7 @@ struct mmc_card;
 struct mmc_async_req;
 struct mmc_cmdq_req;
 
+extern int mmc_cmdq_halt(struct mmc_host *host, bool enable);
 extern void mmc_cmdq_post_req(struct mmc_host *host, struct mmc_request *mrq,
 			      int err);
 extern int mmc_cmdq_start_req(struct mmc_host *host,
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 41b4e25..7f2ae52 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -85,6 +85,7 @@ struct mmc_cmdq_host_ops {
 	int (*request)(struct mmc_host *host, struct mmc_request *mrq);
 	void (*post_req)(struct mmc_host *host, struct mmc_request *mrq,
 			 int err);
+	int (*halt)(struct mmc_host *host, bool halt);
 };
 
 struct mmc_host_ops {
@@ -225,6 +226,7 @@ struct mmc_cmdq_context_info {
 	unsigned long	curr_state;
 #define	CMDQ_STATE_ERR 0
 #define	CMDQ_STATE_DCMD_ACTIVE 1
+#define	CMDQ_STATE_HALT 2
 	/* no free tag available */
 	unsigned long	req_starved;
 };
@@ -544,6 +546,21 @@ static inline int mmc_host_packed_wr(struct mmc_host *host)
 	return host->caps2 & MMC_CAP2_PACKED_WR;
 }
 
+static inline void mmc_host_set_halt(struct mmc_host *host)
+{
+	set_bit(CMDQ_STATE_HALT, &host->cmdq_ctx.curr_state);
+}
+
+static inline void mmc_host_clr_halt(struct mmc_host *host)
+{
+	clear_bit(CMDQ_STATE_HALT, &host->cmdq_ctx.curr_state);
+}
+
+static inline int mmc_host_halt(struct mmc_host *host)
+{
+	return test_bit(CMDQ_STATE_HALT, &host->cmdq_ctx.curr_state);
+}
+
 static inline int mmc_card_hs(struct mmc_card *card)
 {
 	return card->host->ios.timing == MMC_TIMING_SD_HS ||
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH RFCv2 09/10] mmc: cmdq-host: add halt support to command queue host

[Ritesh Harjani]

From: Asutosh Das <asutoshd@codeaurora.org>

Halt can be used in error cases to get control of the
bus. This is used to remove a task from device queue
and/or other recovery mechanisms.

Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
[riteshh@codeaurora.org: fixed merge conflicts]
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/cmdq_hci.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/mmc/host/cmdq_hci.c b/drivers/mmc/host/cmdq_hci.c
index 322b77c..33c860b 100644
--- a/drivers/mmc/host/cmdq_hci.c
+++ b/drivers/mmc/host/cmdq_hci.c
@@ -29,6 +29,9 @@
 #define DCMD_SLOT 31
 #define NUM_SLOTS 32
 
+/* 1 sec */
+#define HALT_TIMEOUT_MS 1000
+
 static inline u8 *get_desc(struct cmdq_host *cq_host, u8 tag)
 {
 	return cq_host->desc_base + (tag * cq_host->slot_sz);
@@ -552,11 +555,42 @@ irqreturn_t cmdq_irq(struct mmc_host *mmc, u32 intmask)
 		cmdq_dumpregs(cq_host);
 	}
 
+	if (status & CQIS_HAC) {
+		/* halt is completed, wakeup waiting thread */
+		complete(&cq_host->halt_comp);
+	}
+
 out:
 	return IRQ_HANDLED;
 }
 EXPORT_SYMBOL(cmdq_irq);
 
+/* May sleep */
+static int cmdq_halt(struct mmc_host *mmc, bool halt)
+{
+	struct cmdq_host *cq_host = (struct cmdq_host *)mmc_cmdq_private(mmc);
+	u32 val = 0;
+
+	if (halt) {
+		cmdq_writel(cq_host, cmdq_readl(cq_host, CQCTL) | HALT,
+			    CQCTL);
+		val = wait_for_completion_timeout(&cq_host->halt_comp,
+					  msecs_to_jiffies(HALT_TIMEOUT_MS));
+		/* halt done: re-enable legacy interrupts */
+		if (cq_host->ops->clear_set_irqs)
+			cq_host->ops->clear_set_irqs(mmc, false);
+
+		val = val ? 0 : -ETIMEDOUT;
+	} else {
+		if (cq_host->ops->clear_set_irqs)
+			cq_host->ops->clear_set_irqs(mmc, true);
+		cmdq_writel(cq_host, cmdq_readl(cq_host, CQCTL) & ~HALT,
+			    CQCTL);
+	}
+
+	return val;
+}
+
 static void cmdq_post_req(struct mmc_host *host, struct mmc_request *mrq,
 			  int err)
 {
@@ -579,6 +613,7 @@ static const struct mmc_cmdq_host_ops cmdq_host_ops = {
 	.disable = cmdq_disable,
 	.request = cmdq_request,
 	.post_req = cmdq_post_req,
+	.halt = cmdq_halt,
 };
 
 struct cmdq_host *cmdq_pltfm_init(struct platform_device *pdev)
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

[PATCH RFCv2 10/10] mmc: sdhci: add command queue support to sdhci

[Ritesh Harjani]

From: Asutosh Das <asutoshd@codeaurora.org>

Adds command-queue support to SDHCi compliant drivers.

Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
[subhashj@codeaurora.org: fixed trivial merge conflicts and
compilation error]
Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
[riteshh@codeaurora.org: fixed merge conflicts]
Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
---
 drivers/mmc/host/sdhci.c | 146 +++++++++++++++++++++++++++++++++++++++++++++--
 drivers/mmc/host/sdhci.h |   6 ++
 2 files changed, 148 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9f5cdaa..0ed9c45 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -32,6 +32,7 @@
 #include <linux/mmc/slot-gpio.h>
 
 #include "sdhci.h"
+#include "cmdq_hci.h"
 
 #define DRIVER_NAME "sdhci"
 
@@ -2474,6 +2475,20 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
 	}
 }
 
+#ifdef CONFIG_MMC_CQ_HCI
+static irqreturn_t sdhci_cmdq_irq(struct mmc_host *mmc, u32 intmask)
+{
+	return cmdq_irq(mmc, intmask);
+}
+
+#else
+static irqreturn_t sdhci_cmdq_irq(struct mmc_host *mmc, u32 intmask)
+{
+	pr_err("%s: rxd cmdq-irq when disabled !!!!\n", mmc_hostname(mmc));
+	return IRQ_NONE;
+}
+#endif
+
 static irqreturn_t sdhci_irq(int irq, void *dev_id)
 {
 	irqreturn_t result = IRQ_NONE;
@@ -2495,6 +2510,15 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
 	}
 
 	do {
+		if (host->mmc->card && mmc_card_cmdq(host->mmc->card) &&
+		    !mmc_host_halt(host->mmc)) {
+			pr_debug("*** %s: cmdq intr: 0x%08x\n",
+					mmc_hostname(host->mmc),
+					intmask);
+			result = sdhci_cmdq_irq(host->mmc, intmask);
+			goto out;
+		}
+
 		/* Clear selected interrupts. */
 		mask = intmask & (SDHCI_INT_CMD_MASK | SDHCI_INT_DATA_MASK |
 				  SDHCI_INT_BUS_POWER);
@@ -2823,6 +2847,106 @@ static int sdhci_set_dma_mask(struct sdhci_host *host)
 	return ret;
 }
 
+#ifdef CONFIG_MMC_CQ_HCI
+static void sdhci_cmdq_clear_set_irqs(struct mmc_host *mmc, bool clear)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	u32 ier = 0;
+
+	ier &= ~SDHCI_INT_ALL_MASK;
+
+	if (clear) {
+		ier = SDHCI_INT_CMDQ_EN | SDHCI_INT_ERROR_MASK;
+		sdhci_writel(host, ier, SDHCI_INT_ENABLE);
+		sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
+	} else {
+		ier = SDHCI_INT_BUS_POWER | SDHCI_INT_DATA_END_BIT |
+			     SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT |
+			     SDHCI_INT_INDEX | SDHCI_INT_END_BIT |
+			     SDHCI_INT_CRC | SDHCI_INT_TIMEOUT |
+			     SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
+			     SDHCI_INT_ACMD12ERR;
+		sdhci_writel(host, ier, SDHCI_INT_ENABLE);
+		sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
+	}
+}
+
+static void sdhci_cmdq_set_data_timeout(struct mmc_host *mmc, u32 val)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	sdhci_writeb(host, val, SDHCI_TIMEOUT_CONTROL);
+}
+
+static void sdhci_cmdq_dump_vendor_regs(struct mmc_host *mmc)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	sdhci_dumpregs(host);
+}
+
+static int sdhci_cmdq_init(struct sdhci_host *host, struct mmc_host *mmc,
+			   bool dma64)
+{
+	return cmdq_init(host->cq_host, mmc, dma64);
+}
+
+static void sdhci_cmdq_set_block_size(struct mmc_host *mmc)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	sdhci_set_blk_size_reg(host, 512, 0);
+}
+
+static void sdhci_cmdq_clear_set_dumpregs(struct mmc_host *mmc, bool set)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+
+	if (host->ops->clear_set_dumpregs)
+		host->ops->clear_set_dumpregs(host, set);
+}
+#else
+static void sdhci_cmdq_clear_set_irqs(struct mmc_host *mmc, bool clear)
+{
+
+}
+
+static void sdhci_cmdq_set_data_timeout(struct mmc_host *mmc, u32 val)
+{
+
+}
+
+static void sdhci_cmdq_dump_vendor_regs(struct mmc_host *mmc)
+{
+
+}
+
+static int sdhci_cmdq_init(struct sdhci_host *host, struct mmc_host *mmc,
+			   bool dma64)
+{
+	return -ENOSYS;
+}
+
+static void sdhci_cmdq_set_block_size(struct mmc_host *mmc)
+{
+
+}
+
+static void sdhci_cmdq_clear_set_dumpregs(struct mmc_host *mmc, bool set)
+{
+
+}
+
+#endif
+
+static const struct cmdq_host_ops sdhci_cmdq_ops = {
+	.clear_set_irqs = sdhci_cmdq_clear_set_irqs,
+	.set_data_timeout = sdhci_cmdq_set_data_timeout,
+	.dump_vendor_regs = sdhci_cmdq_dump_vendor_regs,
+	.set_block_size = sdhci_cmdq_set_block_size,
+	.clear_set_dumpregs = sdhci_cmdq_clear_set_dumpregs,
+};
+
 int sdhci_add_host(struct sdhci_host *host)
 {
 	struct mmc_host *mmc;
@@ -3341,11 +3465,25 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (ret)
 		goto unled;
 
-	pr_info("%s: SDHCI controller on %s [%s] using %s\n",
-		mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
+	if (mmc->caps2 &  MMC_CAP2_CMD_QUEUE) {
+		bool dma64 = (host->flags & SDHCI_USE_ADMA_64BIT) ?
+			true : false;
+		ret = sdhci_cmdq_init(host, mmc, dma64);
+		if (ret)
+			pr_err("%s: CMDQ init: failed (%d)\n",
+			       mmc_hostname(host->mmc), ret);
+		else
+			host->cq_host->ops = &sdhci_cmdq_ops;
+	}
+
+	pr_info("%s: SDHCI controller on %s [%s] using %s in %s mode\n",
+	mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
 		(host->flags & SDHCI_USE_ADMA) ?
-		(host->flags & SDHCI_USE_64_BIT_DMA) ? "ADMA 64-bit" : "ADMA" :
-		(host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO");
+		((host->flags & SDHCI_USE_ADMA_64BIT) ?
+		"64-bit ADMA" : "32-bit ADMA") :
+		((host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO"),
+		((mmc->caps2 &  MMC_CAP2_CMD_QUEUE) && !ret) ?
+		"CMDQ" : "legacy");
 
 	sdhci_enable_card_detection(host);
 
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 609f87c..fccc750 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -150,6 +150,8 @@
 		SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_DATA_CRC | \
 		SDHCI_INT_DATA_END_BIT | SDHCI_INT_ADMA_ERROR | \
 		SDHCI_INT_BLK_GAP)
+
+#define SDHCI_INT_CMDQ_EN	(0x1 << 14)
 #define SDHCI_INT_ALL_MASK	((unsigned int)-1)
 
 #define SDHCI_ACMD12_ERR	0x3C
@@ -446,6 +448,7 @@ struct sdhci_host {
 #define SDHCI_PV_ENABLED	(1<<8)	/* Preset value enabled */
 #define SDHCI_SDIO_IRQ_ENABLED	(1<<9)	/* SDIO irq enabled */
 #define SDHCI_USE_64_BIT_DMA	(1<<12)	/* Use 64-bit DMA */
+#define SDHCI_USE_ADMA_64BIT	(1<<12)	/* Host is 64-bit ADMA capable */
 #define SDHCI_HS400_TUNING	(1<<13)	/* Tuning for HS400 */
 
 	unsigned int version;	/* SDHCI spec. version */
@@ -509,6 +512,8 @@ struct sdhci_host {
 	unsigned int		tuning_mode;	/* Re-tuning mode supported by host */
 #define SDHCI_TUNING_MODE_1	0
 
+	struct cmdq_host *cq_host;
+
 	unsigned long private[0] ____cacheline_aligned;
 };
 
@@ -544,6 +549,7 @@ struct sdhci_ops {
 	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
 	void	(*platform_init)(struct sdhci_host *host);
 	void    (*card_event)(struct sdhci_host *host);
+	void	(*clear_set_dumpregs)(struct sdhci_host *host, bool set);
 	void	(*voltage_switch)(struct sdhci_host *host);
 	int	(*select_drive_strength)(struct sdhci_host *host,
 					 struct mmc_card *card,
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
a Linux Foundation Collaborative Project.

Re: [PATCH RFCv2 06/10] mmc: host: sdhci: don't set SDMA buffer boundary in ADMA mode

[Adrian Hunter]

On 27/06/16 16:22, Ritesh Harjani wrote:
> From: Subhash Jadavani <subhashj@codeaurora.org>
> 
> SDMA buffer boundary size parameter in block size register should only be
> programmed if host controller DMA is operating in SDMA mode otherwise its
> better not to set this parameter to avoid any side effect when DMA is
> operating in ADMA mode operation.

Pedantically, the SDHCI specification does not say the SDMA Buffer Boundary
should not be set in ADMA mode.  All it says is that ADMA does not use it.
In fact it is impossible to avoid writing to it because it is part of the
Block Size register.  Unfortunately the value 0 does not mean "not used" but
instead means the boundary is 4K whereas the value presently being written
(7 which is the highest) means 512K.

Given that we have always been writing 7 to it, I don't see any reason to
change.  Murphy's law says if we do change it, someone else's driver will break.

However I presume you have hardware where value 7 doesn't work.  Can you
clarify that?

> 
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> [venkatg@codeaurora.org: fix trivial merge conflict]
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 0e3d7c0..9f5cdaa 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -742,6 +742,17 @@ static void sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>  	}
>  }
>  
> +static void sdhci_set_blk_size_reg(struct sdhci_host *host, unsigned int blksz,
> +				   unsigned int sdma_boundary)
> +{
> +	if (host->flags & SDHCI_USE_ADMA)
> +		sdhci_writew(host, SDHCI_MAKE_BLKSZ(0, blksz),
> +			     SDHCI_BLOCK_SIZE);
> +	else
> +		sdhci_writew(host, SDHCI_MAKE_BLKSZ(sdma_boundary, blksz),
> +			     SDHCI_BLOCK_SIZE);
> +}
> +
>  static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
>  {
>  	u8 ctrl;
> @@ -874,8 +885,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
>  	sdhci_set_transfer_irqs(host);
>  
>  	/* Set the DMA boundary value and block size */
> -	sdhci_writew(host, SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG,
> -		data->blksz), SDHCI_BLOCK_SIZE);
> +	sdhci_set_blk_size_reg(host, data->blksz, SDHCI_DEFAULT_BOUNDARY_ARG);
>  	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
>  }
>  
> @@ -1935,14 +1945,11 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  		 */
>  		if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200) {
>  			if (mmc->ios.bus_width == MMC_BUS_WIDTH_8)
> -				sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128),
> -					     SDHCI_BLOCK_SIZE);
> +				sdhci_set_blk_size_reg(host, 128, 7);
>  			else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4)
> -				sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
> -					     SDHCI_BLOCK_SIZE);
> +				sdhci_set_blk_size_reg(host, 64, 7);
>  		} else {
> -			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
> -				     SDHCI_BLOCK_SIZE);
> +			sdhci_set_blk_size_reg(host, 64, 7);
>  		}
>  
>  		/*
> 

Re: [PATCH RFCv2 06/10] mmc: host: sdhci: don't set SDMA buffer boundary in ADMA mode

[Ritesh Harjani]

Hi Adrian,

On 6/29/2016 4:25 PM, Adrian Hunter wrote:
> On 27/06/16 16:22, Ritesh Harjani wrote:
>> From: Subhash Jadavani <subhashj@codeaurora.org>
>>
>> SDMA buffer boundary size parameter in block size register should only be
>> programmed if host controller DMA is operating in SDMA mode otherwise its
>> better not to set this parameter to avoid any side effect when DMA is
>> operating in ADMA mode operation.
>
> Pedantically, the SDHCI specification does not say the SDMA Buffer Boundary
> should not be set in ADMA mode.  All it says is that ADMA does not use it.
> In fact it is impossible to avoid writing to it because it is part of the
> Block Size register.  Unfortunately the value 0 does not mean "not used" but
> instead means the boundary is 4K whereas the value presently being written
> (7 which is the highest) means 512K.
Ok.

>
> Given that we have always been writing 7 to it, I don't see any reason to
> change.  Murphy's law says if we do change it, someone else's driver will break.
>
> However I presume you have hardware where value 7 doesn't work.  Can you
> clarify that?
I think this was good to have patch, since SDHCI spec does not 
explicitly mention about this for ADMA. But agree with what you have 
mentioned. I will drop this patch then.


>
>>
>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>> [venkatg@codeaurora.org: fix trivial merge conflict]
>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci.c | 23 +++++++++++++++--------
>>  1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 0e3d7c0..9f5cdaa 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -742,6 +742,17 @@ static void sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
>>  	}
>>  }
>>
>> +static void sdhci_set_blk_size_reg(struct sdhci_host *host, unsigned int blksz,
>> +				   unsigned int sdma_boundary)
>> +{
>> +	if (host->flags & SDHCI_USE_ADMA)
>> +		sdhci_writew(host, SDHCI_MAKE_BLKSZ(0, blksz),
>> +			     SDHCI_BLOCK_SIZE);
>> +	else
>> +		sdhci_writew(host, SDHCI_MAKE_BLKSZ(sdma_boundary, blksz),
>> +			     SDHCI_BLOCK_SIZE);
>> +}
>> +
>>  static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
>>  {
>>  	u8 ctrl;
>> @@ -874,8 +885,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
>>  	sdhci_set_transfer_irqs(host);
>>
>>  	/* Set the DMA boundary value and block size */
>> -	sdhci_writew(host, SDHCI_MAKE_BLKSZ(SDHCI_DEFAULT_BOUNDARY_ARG,
>> -		data->blksz), SDHCI_BLOCK_SIZE);
>> +	sdhci_set_blk_size_reg(host, data->blksz, SDHCI_DEFAULT_BOUNDARY_ARG);
>>  	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
>>  }
>>
>> @@ -1935,14 +1945,11 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>>  		 */
>>  		if (cmd.opcode == MMC_SEND_TUNING_BLOCK_HS200) {
>>  			if (mmc->ios.bus_width == MMC_BUS_WIDTH_8)
>> -				sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 128),
>> -					     SDHCI_BLOCK_SIZE);
>> +				sdhci_set_blk_size_reg(host, 128, 7);
>>  			else if (mmc->ios.bus_width == MMC_BUS_WIDTH_4)
>> -				sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
>> -					     SDHCI_BLOCK_SIZE);
>> +				sdhci_set_blk_size_reg(host, 64, 7);
>>  		} else {
>> -			sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, 64),
>> -				     SDHCI_BLOCK_SIZE);
>> +			sdhci_set_blk_size_reg(host, 64, 7);
>>  		}
>>
>>  		/*
>>
>

Re: [PATCH RFCv2 10/10] mmc: sdhci: add command queue support to sdhci

[Adrian Hunter]

On 27/06/16 16:22, Ritesh Harjani wrote:
> From: Asutosh Das <asutoshd@codeaurora.org>
> 
> Adds command-queue support to SDHCi compliant drivers.

CQHCI is not recognized in the SDHCI specification, and SDHCI should
facilitate any CQE driver implementation not just CQHCI.  That means there
are 2 options:

1. Provide minimal support to allow individual host drivers to deal with
CQHCI directly.

2. Create explicit support for a CQE companion driver and use that to
provide common support for CQHCI.

I started looking at option 2 and concluded that it was taking SDHCI in the
wrong direction because it made drivers more dependent on SDHCI and gave
them less flexibility, and it seemed inconsistent with the aim of making
SDHCI more like a library.

Consequently, I suggest the following:

1. Individual drivers are responsible for initializing and setting up CQHCI
and its callbacks, and shutting it down.

2. SDHCI provides a new callback so that individual drivers can process
interrupts and pass them to CQHCI.

3. SDHCI provides and exports any common helper functions that do not depend
directly on CQHCI.  For example the functions to set interrupts, timeout,
blocks size and dump registers.

> 
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> [subhashj@codeaurora.org: fixed trivial merge conflicts and
> compilation error]
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> [riteshh@codeaurora.org: fixed merge conflicts]
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci.c | 146 +++++++++++++++++++++++++++++++++++++++++++++--
>  drivers/mmc/host/sdhci.h |   6 ++
>  2 files changed, 148 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index 9f5cdaa..0ed9c45 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -32,6 +32,7 @@
>  #include <linux/mmc/slot-gpio.h>
>  
>  #include "sdhci.h"
> +#include "cmdq_hci.h"

As discussed above, SDHCI should not have any references to CQHCI

>  
>  #define DRIVER_NAME "sdhci"
>  
> @@ -2474,6 +2475,20 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
>  	}
>  }
>  
> +#ifdef CONFIG_MMC_CQ_HCI

This won't work if CQHCI is a module.  If you fix the dependencies then you
can use:

#if IS_ENABLED(CONFIG_MMC_CQ_HCI)

Or if you select CONFIG_MMC_CQ_HCI in Kconfig then you don't need ifdef at all.

> +static irqreturn_t sdhci_cmdq_irq(struct mmc_host *mmc, u32 intmask)
> +{
> +	return cmdq_irq(mmc, intmask);

This will give a linker error if the driver is built-in but CQHCI is a
module.  Probably drivers that use CQHCI should just select it in Kconfig.

Another issue is that CQHCI won't be able to interpret SDHCI interrupt bits.
 I suggest you define a generic set of bit flags not dependent on SDHCI or
CQHCI and then we can create a function to convert the SDHCI interrupt status.

> +}
> +
> +#else
> +static irqreturn_t sdhci_cmdq_irq(struct mmc_host *mmc, u32 intmask)
> +{
> +	pr_err("%s: rxd cmdq-irq when disabled !!!!\n", mmc_hostname(mmc));
> +	return IRQ_NONE;
> +}
> +#endif
> +
>  static irqreturn_t sdhci_irq(int irq, void *dev_id)
>  {
>  	irqreturn_t result = IRQ_NONE;
> @@ -2495,6 +2510,15 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
>  	}
>  
>  	do {
> +		if (host->mmc->card && mmc_card_cmdq(host->mmc->card) &&
> +		    !mmc_host_halt(host->mmc)) {
> +			pr_debug("*** %s: cmdq intr: 0x%08x\n",
> +					mmc_hostname(host->mmc),
> +					intmask);
> +			result = sdhci_cmdq_irq(host->mmc, intmask);
> +			goto out;
> +		}
> +

We need to create a new SDHCI host op to enable individual drivers to
handle the IRQ if they choose.  Then it is up to individual drivers to call
into CQHCI.

>  		/* Clear selected interrupts. */
>  		mask = intmask & (SDHCI_INT_CMD_MASK | SDHCI_INT_DATA_MASK |
>  				  SDHCI_INT_BUS_POWER);
> @@ -2823,6 +2847,106 @@ static int sdhci_set_dma_mask(struct sdhci_host *host)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_MMC_CQ_HCI
> +static void sdhci_cmdq_clear_set_irqs(struct mmc_host *mmc, bool clear)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +	u32 ier = 0;
> +
> +	ier &= ~SDHCI_INT_ALL_MASK;
> +
> +	if (clear) {
> +		ier = SDHCI_INT_CMDQ_EN | SDHCI_INT_ERROR_MASK;

SDHCI_INT_ERROR_MASK is not ideal here.  I would expect to set only the bits
we know and want.

> +		sdhci_writel(host, ier, SDHCI_INT_ENABLE);
> +		sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
> +	} else {
> +		ier = SDHCI_INT_BUS_POWER | SDHCI_INT_DATA_END_BIT |
> +			     SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT |
> +			     SDHCI_INT_INDEX | SDHCI_INT_END_BIT |
> +			     SDHCI_INT_CRC | SDHCI_INT_TIMEOUT |
> +			     SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
> +			     SDHCI_INT_ACMD12ERR;

We ought to have the default interrupts defined, so that the same ones are
set here.

> +		sdhci_writel(host, ier, SDHCI_INT_ENABLE);
> +		sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
> +	}
> +}
> +
> +static void sdhci_cmdq_set_data_timeout(struct mmc_host *mmc, u32 val)

Do we really need different callbacks for ->clear_set_irqs(),
->set_data_timeout(), ->set_block_size() and ->clear_set_dumpregs()?  It
looks like we could consolidate them all into just 2 i.e. to start and stop
CQ mode.

> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +
> +	sdhci_writeb(host, val, SDHCI_TIMEOUT_CONTROL);
> +}

We can't expect CQHCI to provide the SDHCI register value.  Ideally we don't
want to set the highest value but instead calculate the value based on the
maximum sized transfer.

> +
> +static void sdhci_cmdq_dump_vendor_regs(struct mmc_host *mmc)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +
> +	sdhci_dumpregs(host);
> +}

Instead you should export sdhci_dumpregs() and let the individual drivers
connect it to CQHCI.

> +
> +static int sdhci_cmdq_init(struct sdhci_host *host, struct mmc_host *mmc,
> +			   bool dma64)
> +{
> +	return cmdq_init(host->cq_host, mmc, dma64);
> +}

Individual drivers should initialize their CQE driver implementation.  It is
not necessarily CQHCI.

> +
> +static void sdhci_cmdq_set_block_size(struct mmc_host *mmc)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +
> +	sdhci_set_blk_size_reg(host, 512, 0);
> +}
> +
> +static void sdhci_cmdq_clear_set_dumpregs(struct mmc_host *mmc, bool set)
> +{
> +	struct sdhci_host *host = mmc_priv(mmc);
> +
> +	if (host->ops->clear_set_dumpregs)
> +		host->ops->clear_set_dumpregs(host, set);
> +}

As questioned further below, what is this for?

> +#else
> +static void sdhci_cmdq_clear_set_irqs(struct mmc_host *mmc, bool clear)
> +{
> +
> +}
> +
> +static void sdhci_cmdq_set_data_timeout(struct mmc_host *mmc, u32 val)
> +{
> +
> +}
> +
> +static void sdhci_cmdq_dump_vendor_regs(struct mmc_host *mmc)
> +{
> +
> +}
> +
> +static int sdhci_cmdq_init(struct sdhci_host *host, struct mmc_host *mmc,
> +			   bool dma64)
> +{
> +	return -ENOSYS;
> +}
> +
> +static void sdhci_cmdq_set_block_size(struct mmc_host *mmc)
> +{
> +
> +}
> +
> +static void sdhci_cmdq_clear_set_dumpregs(struct mmc_host *mmc, bool set)
> +{
> +
> +}
> +
> +#endif
> +
> +static const struct cmdq_host_ops sdhci_cmdq_ops = {
> +	.clear_set_irqs = sdhci_cmdq_clear_set_irqs,
> +	.set_data_timeout = sdhci_cmdq_set_data_timeout,
> +	.dump_vendor_regs = sdhci_cmdq_dump_vendor_regs,
> +	.set_block_size = sdhci_cmdq_set_block_size,
> +	.clear_set_dumpregs = sdhci_cmdq_clear_set_dumpregs,
> +};

It would be up to individual drivers to provide CQHCI ops.

> +
>  int sdhci_add_host(struct sdhci_host *host)
>  {
>  	struct mmc_host *mmc;
> @@ -3341,11 +3465,25 @@ int sdhci_add_host(struct sdhci_host *host)
>  	if (ret)
>  		goto unled;
>  
> -	pr_info("%s: SDHCI controller on %s [%s] using %s\n",
> -		mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
> +	if (mmc->caps2 &  MMC_CAP2_CMD_QUEUE) {
> +		bool dma64 = (host->flags & SDHCI_USE_ADMA_64BIT) ?
> +			true : false;
> +		ret = sdhci_cmdq_init(host, mmc, dma64);

You must initialize before mmc_add_host() because mmc_add_host() also starts
the host.  If you re-base on top of the SDHCI patches, then individual
drivers can take advantage of the new sdhci_setup_host() /
__sdhci_add_host() split.

> +		if (ret)
> +			pr_err("%s: CMDQ init: failed (%d)\n",
> +			       mmc_hostname(host->mmc), ret);
> +		else
> +			host->cq_host->ops = &sdhci_cmdq_ops;
> +	}
> +
> +	pr_info("%s: SDHCI controller on %s [%s] using %s in %s mode\n",
> +	mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
>  		(host->flags & SDHCI_USE_ADMA) ?
> -		(host->flags & SDHCI_USE_64_BIT_DMA) ? "ADMA 64-bit" : "ADMA" :
> -		(host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO");
> +		((host->flags & SDHCI_USE_ADMA_64BIT) ?
> +		"64-bit ADMA" : "32-bit ADMA") :
> +		((host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO"),
> +		((mmc->caps2 &  MMC_CAP2_CMD_QUEUE) && !ret) ?
> +		"CMDQ" : "legacy");

It is probably better if CQHCI prints its own info.  That way it you get a
consistent message irrespective of whether the driver uses SDHCI.  Also you
can print other CQHCI details such as the version.

>  
>  	sdhci_enable_card_detection(host);
>  
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 609f87c..fccc750 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -150,6 +150,8 @@
>  		SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_DATA_CRC | \
>  		SDHCI_INT_DATA_END_BIT | SDHCI_INT_ADMA_ERROR | \
>  		SDHCI_INT_BLK_GAP)
> +
> +#define SDHCI_INT_CMDQ_EN	(0x1 << 14)

We can define this bit for convenience but we need a comment to say that it
is non-standard.

>  #define SDHCI_INT_ALL_MASK	((unsigned int)-1)
>  
>  #define SDHCI_ACMD12_ERR	0x3C
> @@ -446,6 +448,7 @@ struct sdhci_host {
>  #define SDHCI_PV_ENABLED	(1<<8)	/* Preset value enabled */
>  #define SDHCI_SDIO_IRQ_ENABLED	(1<<9)	/* SDIO irq enabled */
>  #define SDHCI_USE_64_BIT_DMA	(1<<12)	/* Use 64-bit DMA */
> +#define SDHCI_USE_ADMA_64BIT	(1<<12)	/* Host is 64-bit ADMA capable */

SDHCI_USE_ADMA_64BIT looks redundant.

>  #define SDHCI_HS400_TUNING	(1<<13)	/* Tuning for HS400 */
>  
>  	unsigned int version;	/* SDHCI spec. version */
> @@ -509,6 +512,8 @@ struct sdhci_host {
>  	unsigned int		tuning_mode;	/* Re-tuning mode supported by host */
>  #define SDHCI_TUNING_MODE_1	0
>  
> +	struct cmdq_host *cq_host;
> +

The individual drivers will have to have their own reference to CQHCI.

>  	unsigned long private[0] ____cacheline_aligned;
>  };
>  
> @@ -544,6 +549,7 @@ struct sdhci_ops {
>  	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
>  	void	(*platform_init)(struct sdhci_host *host);
>  	void    (*card_event)(struct sdhci_host *host);
> +	void	(*clear_set_dumpregs)(struct sdhci_host *host, bool set);

What is this for?

>  	void	(*voltage_switch)(struct sdhci_host *host);
>  	int	(*select_drive_strength)(struct sdhci_host *host,
>  					 struct mmc_card *card,
> 

Re: [PATCH RFCv2 10/10] mmc: sdhci: add command queue support to sdhci

[Adrian Hunter]

On 05/07/16 14:15, Adrian Hunter wrote:
> On 27/06/16 16:22, Ritesh Harjani wrote:

<SNIP>

>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +	sdhci_writeb(host, val, SDHCI_TIMEOUT_CONTROL);
>> +}
> 
> We can't expect CQHCI to provide the SDHCI register value.  Ideally we don't
> want to set the highest value but instead calculate the value based on the
> maximum sized transfer.

Except cache flush doesn't have a timeout, so we will have to set the
highest value if we issue cache flush via DCMD.

Re: [PATCH RFCv2 10/10] mmc: sdhci: add command queue support to sdhci

[Ritesh Harjani]

Hi Adrian,

Sorry about this long delay, was pulled into some release.
Will be more responsive from now.


We have gone through your comments. We had some queries and wanted to 
discuss more about your initial comments before going onto individual 
comments in the code. It would be great if you could help us understand 
more on your initial comments by answering to our queries.


On 7/5/2016 4:45 PM, Adrian Hunter wrote:
> On 27/06/16 16:22, Ritesh Harjani wrote:
>> From: Asutosh Das <asutoshd@codeaurora.org>
>>
>> Adds command-queue support to SDHCi compliant drivers.
>
> CQHCI is not recognized in the SDHCI specification,
Yes. Not sure about future plan though.

> and SDHCI should
> facilitate any CQE driver implementation not just CQHCI.  That means there
> are 2 options:

Are we aware of any other vendor specific implementation of CQHCI? Or do 
you foresee this coming through?
Actually CQ host controller interface(CQHCI) is what is proposed in emmc 
JEDEC5.1 which is mentioned as extension to HCI in block diagram. (Fig 
B.110 in emmc-5.1 JEDEC).


>
> 1. Provide minimal support to allow individual host drivers to deal with
> CQHCI directly.
>
> 2. Create explicit support for a CQE companion driver and use that to
> provide common support for CQHCI.
>
> I started looking at option 2 and concluded that it was taking SDHCI in the
> wrong direction because it made drivers more dependent on SDHCI and gave
> them less flexibility, and it seemed inconsistent with the aim of making
> SDHCI more like a library.
>
> Consequently, I suggest the following:
>
> 1. Individual drivers are responsible for initializing and setting up CQHCI
> and its callbacks, and shutting it down.

CQHCI callbacks is what the concern is. Currently the callbacks are 
placed in SDHCI, because these callbacks are doing nothing other than 
setting up SDHCI registers itself for CQHCI. There is nothing platform 
specific which is happening in these.
So why do you think that these should be implemented in individual 
platform specific drivers?
Will that not result into same code duplication in all SDHCI compliant 
platform drivers?


>
> 2. SDHCI provides a new callback so that individual drivers can process
> interrupts and pass them to CQHCI.
This again will depend upon the initial ask on - do you foresee SDHCI 
facilitating other CQE implementations? Or if you are already aware of 
something else other than CQHCI?


>
> 3. SDHCI provides and exports any common helper functions that do not depend
> directly on CQHCI.  For example the functions to set interrupts, timeout,
> blocks size and dump registers.
>
>>
>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>> Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>> [subhashj@codeaurora.org: fixed trivial merge conflicts and
>> compilation error]
>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>> [riteshh@codeaurora.org: fixed merge conflicts]
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>> ---
>>  drivers/mmc/host/sdhci.c | 146 +++++++++++++++++++++++++++++++++++++++++++++--
>>  drivers/mmc/host/sdhci.h |   6 ++
>>  2 files changed, 148 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>> index 9f5cdaa..0ed9c45 100644
>> --- a/drivers/mmc/host/sdhci.c
>> +++ b/drivers/mmc/host/sdhci.c
>> @@ -32,6 +32,7 @@
>>  #include <linux/mmc/slot-gpio.h>
>>
>>  #include "sdhci.h"
>> +#include "cmdq_hci.h"
>
> As discussed above, SDHCI should not have any references to CQHCI
>
>>
>>  #define DRIVER_NAME "sdhci"
>>
>> @@ -2474,6 +2475,20 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
>>  	}
>>  }
>>
>> +#ifdef CONFIG_MMC_CQ_HCI
>
> This won't work if CQHCI is a module.  If you fix the dependencies then you
> can use:
>
> #if IS_ENABLED(CONFIG_MMC_CQ_HCI)
>
> Or if you select CONFIG_MMC_CQ_HCI in Kconfig then you don't need ifdef at all.
>
>> +static irqreturn_t sdhci_cmdq_irq(struct mmc_host *mmc, u32 intmask)
>> +{
>> +	return cmdq_irq(mmc, intmask);
>
> This will give a linker error if the driver is built-in but CQHCI is a
> module.  Probably drivers that use CQHCI should just select it in Kconfig.
>
> Another issue is that CQHCI won't be able to interpret SDHCI interrupt bits.
>  I suggest you define a generic set of bit flags not dependent on SDHCI or
> CQHCI and then we can create a function to convert the SDHCI interrupt status.
>
>> +}
>> +
>> +#else
>> +static irqreturn_t sdhci_cmdq_irq(struct mmc_host *mmc, u32 intmask)
>> +{
>> +	pr_err("%s: rxd cmdq-irq when disabled !!!!\n", mmc_hostname(mmc));
>> +	return IRQ_NONE;
>> +}
>> +#endif
>> +
>>  static irqreturn_t sdhci_irq(int irq, void *dev_id)
>>  {
>>  	irqreturn_t result = IRQ_NONE;
>> @@ -2495,6 +2510,15 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
>>  	}
>>
>>  	do {
>> +		if (host->mmc->card && mmc_card_cmdq(host->mmc->card) &&
>> +		    !mmc_host_halt(host->mmc)) {
>> +			pr_debug("*** %s: cmdq intr: 0x%08x\n",
>> +					mmc_hostname(host->mmc),
>> +					intmask);
>> +			result = sdhci_cmdq_irq(host->mmc, intmask);
>> +			goto out;
>> +		}
>> +
>
> We need to create a new SDHCI host op to enable individual drivers to
> handle the IRQ if they choose.  Then it is up to individual drivers to call
> into CQHCI.
>
>>  		/* Clear selected interrupts. */
>>  		mask = intmask & (SDHCI_INT_CMD_MASK | SDHCI_INT_DATA_MASK |
>>  				  SDHCI_INT_BUS_POWER);
>> @@ -2823,6 +2847,106 @@ static int sdhci_set_dma_mask(struct sdhci_host *host)
>>  	return ret;
>>  }
>>
>> +#ifdef CONFIG_MMC_CQ_HCI
>> +static void sdhci_cmdq_clear_set_irqs(struct mmc_host *mmc, bool clear)
>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +	u32 ier = 0;
>> +
>> +	ier &= ~SDHCI_INT_ALL_MASK;
>> +
>> +	if (clear) {
>> +		ier = SDHCI_INT_CMDQ_EN | SDHCI_INT_ERROR_MASK;
>
> SDHCI_INT_ERROR_MASK is not ideal here.  I would expect to set only the bits
> we know and want.
>
>> +		sdhci_writel(host, ier, SDHCI_INT_ENABLE);
>> +		sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
>> +	} else {
>> +		ier = SDHCI_INT_BUS_POWER | SDHCI_INT_DATA_END_BIT |
>> +			     SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT |
>> +			     SDHCI_INT_INDEX | SDHCI_INT_END_BIT |
>> +			     SDHCI_INT_CRC | SDHCI_INT_TIMEOUT |
>> +			     SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
>> +			     SDHCI_INT_ACMD12ERR;
>
> We ought to have the default interrupts defined, so that the same ones are
> set here.
>
>> +		sdhci_writel(host, ier, SDHCI_INT_ENABLE);
>> +		sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
>> +	}
>> +}
>> +
>> +static void sdhci_cmdq_set_data_timeout(struct mmc_host *mmc, u32 val)
>
> Do we really need different callbacks for ->clear_set_irqs(),
> ->set_data_timeout(), ->set_block_size() and ->clear_set_dumpregs()?  It
> looks like we could consolidate them all into just 2 i.e. to start and stop
> CQ mode.
>
>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +	sdhci_writeb(host, val, SDHCI_TIMEOUT_CONTROL);
>> +}
>
> We can't expect CQHCI to provide the SDHCI register value.  Ideally we don't
> want to set the highest value but instead calculate the value based on the
> maximum sized transfer.
>
>> +
>> +static void sdhci_cmdq_dump_vendor_regs(struct mmc_host *mmc)
>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +	sdhci_dumpregs(host);
>> +}
>
> Instead you should export sdhci_dumpregs() and let the individual drivers
> connect it to CQHCI.
>
>> +
>> +static int sdhci_cmdq_init(struct sdhci_host *host, struct mmc_host *mmc,
>> +			   bool dma64)
>> +{
>> +	return cmdq_init(host->cq_host, mmc, dma64);
>> +}
>
> Individual drivers should initialize their CQE driver implementation.  It is
> not necessarily CQHCI.
>
>> +
>> +static void sdhci_cmdq_set_block_size(struct mmc_host *mmc)
>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +	sdhci_set_blk_size_reg(host, 512, 0);
>> +}
>> +
>> +static void sdhci_cmdq_clear_set_dumpregs(struct mmc_host *mmc, bool set)
>> +{
>> +	struct sdhci_host *host = mmc_priv(mmc);
>> +
>> +	if (host->ops->clear_set_dumpregs)
>> +		host->ops->clear_set_dumpregs(host, set);
This is vendor specific callback for platform drivers to enable/disable 
(say some TEST bus) registers for debugging purposes.


>> +}
>
> As questioned further below, what is this for?
>
>> +#else
>> +static void sdhci_cmdq_clear_set_irqs(struct mmc_host *mmc, bool clear)
>> +{
>> +
>> +}
>> +
>> +static void sdhci_cmdq_set_data_timeout(struct mmc_host *mmc, u32 val)
>> +{
>> +
>> +}
>> +
>> +static void sdhci_cmdq_dump_vendor_regs(struct mmc_host *mmc)
>> +{
>> +
>> +}
>> +
>> +static int sdhci_cmdq_init(struct sdhci_host *host, struct mmc_host *mmc,
>> +			   bool dma64)
>> +{
>> +	return -ENOSYS;
>> +}
>> +
>> +static void sdhci_cmdq_set_block_size(struct mmc_host *mmc)
>> +{
>> +
>> +}
>> +
>> +static void sdhci_cmdq_clear_set_dumpregs(struct mmc_host *mmc, bool set)
>> +{
>> +
>> +}
>> +
>> +#endif
>> +
>> +static const struct cmdq_host_ops sdhci_cmdq_ops = {
>> +	.clear_set_irqs = sdhci_cmdq_clear_set_irqs,
>> +	.set_data_timeout = sdhci_cmdq_set_data_timeout,
>> +	.dump_vendor_regs = sdhci_cmdq_dump_vendor_regs,
>> +	.set_block_size = sdhci_cmdq_set_block_size,
>> +	.clear_set_dumpregs = sdhci_cmdq_clear_set_dumpregs,
>> +};
>
> It would be up to individual drivers to provide CQHCI ops.
>
>> +
>>  int sdhci_add_host(struct sdhci_host *host)
>>  {
>>  	struct mmc_host *mmc;
>> @@ -3341,11 +3465,25 @@ int sdhci_add_host(struct sdhci_host *host)
>>  	if (ret)
>>  		goto unled;
>>
>> -	pr_info("%s: SDHCI controller on %s [%s] using %s\n",
>> -		mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
>> +	if (mmc->caps2 &  MMC_CAP2_CMD_QUEUE) {
>> +		bool dma64 = (host->flags & SDHCI_USE_ADMA_64BIT) ?
>> +			true : false;
>> +		ret = sdhci_cmdq_init(host, mmc, dma64);
>
> You must initialize before mmc_add_host() because mmc_add_host() also starts
> the host.  If you re-base on top of the SDHCI patches, then individual
> drivers can take advantage of the new sdhci_setup_host() /
> __sdhci_add_host() split.
>
>> +		if (ret)
>> +			pr_err("%s: CMDQ init: failed (%d)\n",
>> +			       mmc_hostname(host->mmc), ret);
>> +		else
>> +			host->cq_host->ops = &sdhci_cmdq_ops;
>> +	}
>> +
>> +	pr_info("%s: SDHCI controller on %s [%s] using %s in %s mode\n",
>> +	mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
>>  		(host->flags & SDHCI_USE_ADMA) ?
>> -		(host->flags & SDHCI_USE_64_BIT_DMA) ? "ADMA 64-bit" : "ADMA" :
>> -		(host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO");
>> +		((host->flags & SDHCI_USE_ADMA_64BIT) ?
>> +		"64-bit ADMA" : "32-bit ADMA") :
>> +		((host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO"),
>> +		((mmc->caps2 &  MMC_CAP2_CMD_QUEUE) && !ret) ?
>> +		"CMDQ" : "legacy");
>
> It is probably better if CQHCI prints its own info.  That way it you get a
> consistent message irrespective of whether the driver uses SDHCI.  Also you
> can print other CQHCI details such as the version.
>
>>
>>  	sdhci_enable_card_detection(host);
>>
>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>> index 609f87c..fccc750 100644
>> --- a/drivers/mmc/host/sdhci.h
>> +++ b/drivers/mmc/host/sdhci.h
>> @@ -150,6 +150,8 @@
>>  		SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_DATA_CRC | \
>>  		SDHCI_INT_DATA_END_BIT | SDHCI_INT_ADMA_ERROR | \
>>  		SDHCI_INT_BLK_GAP)
>> +
>> +#define SDHCI_INT_CMDQ_EN	(0x1 << 14)
>
> We can define this bit for convenience but we need a comment to say that it
> is non-standard.
>
>>  #define SDHCI_INT_ALL_MASK	((unsigned int)-1)
>>
>>  #define SDHCI_ACMD12_ERR	0x3C
>> @@ -446,6 +448,7 @@ struct sdhci_host {
>>  #define SDHCI_PV_ENABLED	(1<<8)	/* Preset value enabled */
>>  #define SDHCI_SDIO_IRQ_ENABLED	(1<<9)	/* SDIO irq enabled */
>>  #define SDHCI_USE_64_BIT_DMA	(1<<12)	/* Use 64-bit DMA */
>> +#define SDHCI_USE_ADMA_64BIT	(1<<12)	/* Host is 64-bit ADMA capable */
>
> SDHCI_USE_ADMA_64BIT looks redundant.
>
>>  #define SDHCI_HS400_TUNING	(1<<13)	/* Tuning for HS400 */
>>
>>  	unsigned int version;	/* SDHCI spec. version */
>> @@ -509,6 +512,8 @@ struct sdhci_host {
>>  	unsigned int		tuning_mode;	/* Re-tuning mode supported by host */
>>  #define SDHCI_TUNING_MODE_1	0
>>
>> +	struct cmdq_host *cq_host;
>> +
>
> The individual drivers will have to have their own reference to CQHCI.
>
>>  	unsigned long private[0] ____cacheline_aligned;
>>  };
>>
>> @@ -544,6 +549,7 @@ struct sdhci_ops {
>>  	void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
>>  	void	(*platform_init)(struct sdhci_host *host);
>>  	void    (*card_event)(struct sdhci_host *host);
>> +	void	(*clear_set_dumpregs)(struct sdhci_host *host, bool set);
>
> What is this for?

This is vendor specific callback to enable/disable some TEST bus 
registers for debugging purposes.

>
>>  	void	(*voltage_switch)(struct sdhci_host *host);
>>  	int	(*select_drive_strength)(struct sdhci_host *host,
>>  					 struct mmc_card *card,
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH RFCv2 10/10] mmc: sdhci: add command queue support to sdhci

[Adrian Hunter]

On 25/07/16 13:24, Ritesh Harjani wrote:
> Sorry about this long delay, was pulled into some release.
> Will be more responsive from now.

Well, I was away, so now it is my delay, sorry.

> We have gone through your comments. We had some queries and wanted to
> discuss more about your initial comments before going onto individual
> comments in the code. It would be great if you could help us understand more
> on your initial comments by answering to our queries.
> 
> 
> On 7/5/2016 4:45 PM, Adrian Hunter wrote:
>> On 27/06/16 16:22, Ritesh Harjani wrote:
>>> From: Asutosh Das <asutoshd@codeaurora.org>
>>>
>>> Adds command-queue support to SDHCi compliant drivers.
>>
>> CQHCI is not recognized in the SDHCI specification,
> Yes. Not sure about future plan though.

SD Association and JEDEC are different bodies and I have never seen a SD
Association Spec. that mentions eMMC, so I would not expect the SDHCI spec.
ever to recognize CQHCI.

> 
>> and SDHCI should
>> facilitate any CQE driver implementation not just CQHCI.  That means there
>> are 2 options:
> 
> Are we aware of any other vendor specific implementation of CQHCI? Or do you
> foresee this coming through?

No

> Actually CQ host controller interface(CQHCI) is what is proposed in emmc
> JEDEC5.1 which is mentioned as extension to HCI in block diagram. (Fig B.110
> in emmc-5.1 JEDEC).
> 
> 
>>
>> 1. Provide minimal support to allow individual host drivers to deal with
>> CQHCI directly.
>>
>> 2. Create explicit support for a CQE companion driver and use that to
>> provide common support for CQHCI.
>>
>> I started looking at option 2 and concluded that it was taking SDHCI in the
>> wrong direction because it made drivers more dependent on SDHCI and gave
>> them less flexibility, and it seemed inconsistent with the aim of making
>> SDHCI more like a library.
>>
>> Consequently, I suggest the following:
>>
>> 1. Individual drivers are responsible for initializing and setting up CQHCI
>> and its callbacks, and shutting it down.
> 
> CQHCI callbacks is what the concern is. Currently the callbacks are placed
> in SDHCI, because these callbacks are doing nothing other than setting up
> SDHCI registers itself for CQHCI. There is nothing platform specific which
> is happening in these.
> So why do you think that these should be implemented in individual platform
> specific drivers?
> Will that not result into same code duplication in all SDHCI compliant
> platform drivers?

Common functions that do not need CQHCI definitions can still be in sdhci.c.
Some amount of code duplication is the cost of allowing drivers to be able
to do whatever the need to.

Linking CQHCI and SDHCI goes in the opposite direction of making SDHCI more
like a library.  It means more callbacks and more potential issues if
drivers need to do anything differently.

> 
> 
>>
>> 2. SDHCI provides a new callback so that individual drivers can process
>> interrupts and pass them to CQHCI.
> This again will depend upon the initial ask on - do you foresee SDHCI
> facilitating other CQE implementations? Or if you are already aware of
> something else other than CQHCI?

No I don't know if there will ever be another implementation, however it is
not impossible.  Obviously it would be much messier for SDHCI to support
different CQE implementations than for individual drivers to support just
the one they use.

> 
> 
>>
>> 3. SDHCI provides and exports any common helper functions that do not depend
>> directly on CQHCI.  For example the functions to set interrupts, timeout,
>> blocks size and dump registers.
>>
>>>
>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>> Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
>>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>> [subhashj@codeaurora.org: fixed trivial merge conflicts and
>>> compilation error]
>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>>> [riteshh@codeaurora.org: fixed merge conflicts]
>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>> ---
>>>  drivers/mmc/host/sdhci.c | 146
>>> +++++++++++++++++++++++++++++++++++++++++++++--
>>>  drivers/mmc/host/sdhci.h |   6 ++
>>>  2 files changed, 148 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index 9f5cdaa..0ed9c45 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -32,6 +32,7 @@
>>>  #include <linux/mmc/slot-gpio.h>
>>>
>>>  #include "sdhci.h"
>>> +#include "cmdq_hci.h"
>>
>> As discussed above, SDHCI should not have any references to CQHCI
>>
>>>
>>>  #define DRIVER_NAME "sdhci"
>>>
>>> @@ -2474,6 +2475,20 @@ static void sdhci_data_irq(struct sdhci_host
>>> *host, u32 intmask)
>>>      }
>>>  }
>>>
>>> +#ifdef CONFIG_MMC_CQ_HCI
>>
>> This won't work if CQHCI is a module.  If you fix the dependencies then you
>> can use:
>>
>> #if IS_ENABLED(CONFIG_MMC_CQ_HCI)
>>
>> Or if you select CONFIG_MMC_CQ_HCI in Kconfig then you don't need ifdef at
>> all.
>>
>>> +static irqreturn_t sdhci_cmdq_irq(struct mmc_host *mmc, u32 intmask)
>>> +{
>>> +    return cmdq_irq(mmc, intmask);
>>
>> This will give a linker error if the driver is built-in but CQHCI is a
>> module.  Probably drivers that use CQHCI should just select it in Kconfig.
>>
>> Another issue is that CQHCI won't be able to interpret SDHCI interrupt bits.
>>  I suggest you define a generic set of bit flags not dependent on SDHCI or
>> CQHCI and then we can create a function to convert the SDHCI interrupt
>> status.
>>
>>> +}
>>> +
>>> +#else
>>> +static irqreturn_t sdhci_cmdq_irq(struct mmc_host *mmc, u32 intmask)
>>> +{
>>> +    pr_err("%s: rxd cmdq-irq when disabled !!!!\n", mmc_hostname(mmc));
>>> +    return IRQ_NONE;
>>> +}
>>> +#endif
>>> +
>>>  static irqreturn_t sdhci_irq(int irq, void *dev_id)
>>>  {
>>>      irqreturn_t result = IRQ_NONE;
>>> @@ -2495,6 +2510,15 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
>>>      }
>>>
>>>      do {
>>> +        if (host->mmc->card && mmc_card_cmdq(host->mmc->card) &&
>>> +            !mmc_host_halt(host->mmc)) {
>>> +            pr_debug("*** %s: cmdq intr: 0x%08x\n",
>>> +                    mmc_hostname(host->mmc),
>>> +                    intmask);
>>> +            result = sdhci_cmdq_irq(host->mmc, intmask);
>>> +            goto out;
>>> +        }
>>> +
>>
>> We need to create a new SDHCI host op to enable individual drivers to
>> handle the IRQ if they choose.  Then it is up to individual drivers to call
>> into CQHCI.
>>
>>>          /* Clear selected interrupts. */
>>>          mask = intmask & (SDHCI_INT_CMD_MASK | SDHCI_INT_DATA_MASK |
>>>                    SDHCI_INT_BUS_POWER);
>>> @@ -2823,6 +2847,106 @@ static int sdhci_set_dma_mask(struct sdhci_host
>>> *host)
>>>      return ret;
>>>  }
>>>
>>> +#ifdef CONFIG_MMC_CQ_HCI
>>> +static void sdhci_cmdq_clear_set_irqs(struct mmc_host *mmc, bool clear)
>>> +{
>>> +    struct sdhci_host *host = mmc_priv(mmc);
>>> +    u32 ier = 0;
>>> +
>>> +    ier &= ~SDHCI_INT_ALL_MASK;
>>> +
>>> +    if (clear) {
>>> +        ier = SDHCI_INT_CMDQ_EN | SDHCI_INT_ERROR_MASK;
>>
>> SDHCI_INT_ERROR_MASK is not ideal here.  I would expect to set only the bits
>> we know and want.
>>
>>> +        sdhci_writel(host, ier, SDHCI_INT_ENABLE);
>>> +        sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
>>> +    } else {
>>> +        ier = SDHCI_INT_BUS_POWER | SDHCI_INT_DATA_END_BIT |
>>> +                 SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT |
>>> +                 SDHCI_INT_INDEX | SDHCI_INT_END_BIT |
>>> +                 SDHCI_INT_CRC | SDHCI_INT_TIMEOUT |
>>> +                 SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
>>> +                 SDHCI_INT_ACMD12ERR;
>>
>> We ought to have the default interrupts defined, so that the same ones are
>> set here.
>>
>>> +        sdhci_writel(host, ier, SDHCI_INT_ENABLE);
>>> +        sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
>>> +    }
>>> +}
>>> +
>>> +static void sdhci_cmdq_set_data_timeout(struct mmc_host *mmc, u32 val)
>>
>> Do we really need different callbacks for ->clear_set_irqs(),
>> ->set_data_timeout(), ->set_block_size() and ->clear_set_dumpregs()?  It
>> looks like we could consolidate them all into just 2 i.e. to start and stop
>> CQ mode.
>>
>>> +{
>>> +    struct sdhci_host *host = mmc_priv(mmc);
>>> +
>>> +    sdhci_writeb(host, val, SDHCI_TIMEOUT_CONTROL);
>>> +}
>>
>> We can't expect CQHCI to provide the SDHCI register value.  Ideally we don't
>> want to set the highest value but instead calculate the value based on the
>> maximum sized transfer.
>>
>>> +
>>> +static void sdhci_cmdq_dump_vendor_regs(struct mmc_host *mmc)
>>> +{
>>> +    struct sdhci_host *host = mmc_priv(mmc);
>>> +
>>> +    sdhci_dumpregs(host);
>>> +}
>>
>> Instead you should export sdhci_dumpregs() and let the individual drivers
>> connect it to CQHCI.
>>
>>> +
>>> +static int sdhci_cmdq_init(struct sdhci_host *host, struct mmc_host *mmc,
>>> +               bool dma64)
>>> +{
>>> +    return cmdq_init(host->cq_host, mmc, dma64);
>>> +}
>>
>> Individual drivers should initialize their CQE driver implementation.  It is
>> not necessarily CQHCI.
>>
>>> +
>>> +static void sdhci_cmdq_set_block_size(struct mmc_host *mmc)
>>> +{
>>> +    struct sdhci_host *host = mmc_priv(mmc);
>>> +
>>> +    sdhci_set_blk_size_reg(host, 512, 0);
>>> +}
>>> +
>>> +static void sdhci_cmdq_clear_set_dumpregs(struct mmc_host *mmc, bool set)
>>> +{
>>> +    struct sdhci_host *host = mmc_priv(mmc);
>>> +
>>> +    if (host->ops->clear_set_dumpregs)
>>> +        host->ops->clear_set_dumpregs(host, set);
> This is vendor specific callback for platform drivers to enable/disable (say
> some TEST bus) registers for debugging purposes.
> 
> 
>>> +}
>>
>> As questioned further below, what is this for?
>>
>>> +#else
>>> +static void sdhci_cmdq_clear_set_irqs(struct mmc_host *mmc, bool clear)
>>> +{
>>> +
>>> +}
>>> +
>>> +static void sdhci_cmdq_set_data_timeout(struct mmc_host *mmc, u32 val)
>>> +{
>>> +
>>> +}
>>> +
>>> +static void sdhci_cmdq_dump_vendor_regs(struct mmc_host *mmc)
>>> +{
>>> +
>>> +}
>>> +
>>> +static int sdhci_cmdq_init(struct sdhci_host *host, struct mmc_host *mmc,
>>> +               bool dma64)
>>> +{
>>> +    return -ENOSYS;
>>> +}
>>> +
>>> +static void sdhci_cmdq_set_block_size(struct mmc_host *mmc)
>>> +{
>>> +
>>> +}
>>> +
>>> +static void sdhci_cmdq_clear_set_dumpregs(struct mmc_host *mmc, bool set)
>>> +{
>>> +
>>> +}
>>> +
>>> +#endif
>>> +
>>> +static const struct cmdq_host_ops sdhci_cmdq_ops = {
>>> +    .clear_set_irqs = sdhci_cmdq_clear_set_irqs,
>>> +    .set_data_timeout = sdhci_cmdq_set_data_timeout,
>>> +    .dump_vendor_regs = sdhci_cmdq_dump_vendor_regs,
>>> +    .set_block_size = sdhci_cmdq_set_block_size,
>>> +    .clear_set_dumpregs = sdhci_cmdq_clear_set_dumpregs,
>>> +};
>>
>> It would be up to individual drivers to provide CQHCI ops.
>>
>>> +
>>>  int sdhci_add_host(struct sdhci_host *host)
>>>  {
>>>      struct mmc_host *mmc;
>>> @@ -3341,11 +3465,25 @@ int sdhci_add_host(struct sdhci_host *host)
>>>      if (ret)
>>>          goto unled;
>>>
>>> -    pr_info("%s: SDHCI controller on %s [%s] using %s\n",
>>> -        mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
>>> +    if (mmc->caps2 &  MMC_CAP2_CMD_QUEUE) {
>>> +        bool dma64 = (host->flags & SDHCI_USE_ADMA_64BIT) ?
>>> +            true : false;
>>> +        ret = sdhci_cmdq_init(host, mmc, dma64);
>>
>> You must initialize before mmc_add_host() because mmc_add_host() also starts
>> the host.  If you re-base on top of the SDHCI patches, then individual
>> drivers can take advantage of the new sdhci_setup_host() /
>> __sdhci_add_host() split.
>>
>>> +        if (ret)
>>> +            pr_err("%s: CMDQ init: failed (%d)\n",
>>> +                   mmc_hostname(host->mmc), ret);
>>> +        else
>>> +            host->cq_host->ops = &sdhci_cmdq_ops;
>>> +    }
>>> +
>>> +    pr_info("%s: SDHCI controller on %s [%s] using %s in %s mode\n",
>>> +    mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
>>>          (host->flags & SDHCI_USE_ADMA) ?
>>> -        (host->flags & SDHCI_USE_64_BIT_DMA) ? "ADMA 64-bit" : "ADMA" :
>>> -        (host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO");
>>> +        ((host->flags & SDHCI_USE_ADMA_64BIT) ?
>>> +        "64-bit ADMA" : "32-bit ADMA") :
>>> +        ((host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO"),
>>> +        ((mmc->caps2 &  MMC_CAP2_CMD_QUEUE) && !ret) ?
>>> +        "CMDQ" : "legacy");
>>
>> It is probably better if CQHCI prints its own info.  That way it you get a
>> consistent message irrespective of whether the driver uses SDHCI.  Also you
>> can print other CQHCI details such as the version.
>>
>>>
>>>      sdhci_enable_card_detection(host);
>>>
>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>> index 609f87c..fccc750 100644
>>> --- a/drivers/mmc/host/sdhci.h
>>> +++ b/drivers/mmc/host/sdhci.h
>>> @@ -150,6 +150,8 @@
>>>          SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_DATA_CRC | \
>>>          SDHCI_INT_DATA_END_BIT | SDHCI_INT_ADMA_ERROR | \
>>>          SDHCI_INT_BLK_GAP)
>>> +
>>> +#define SDHCI_INT_CMDQ_EN    (0x1 << 14)
>>
>> We can define this bit for convenience but we need a comment to say that it
>> is non-standard.
>>
>>>  #define SDHCI_INT_ALL_MASK    ((unsigned int)-1)
>>>
>>>  #define SDHCI_ACMD12_ERR    0x3C
>>> @@ -446,6 +448,7 @@ struct sdhci_host {
>>>  #define SDHCI_PV_ENABLED    (1<<8)    /* Preset value enabled */
>>>  #define SDHCI_SDIO_IRQ_ENABLED    (1<<9)    /* SDIO irq enabled */
>>>  #define SDHCI_USE_64_BIT_DMA    (1<<12)    /* Use 64-bit DMA */
>>> +#define SDHCI_USE_ADMA_64BIT    (1<<12)    /* Host is 64-bit ADMA
>>> capable */
>>
>> SDHCI_USE_ADMA_64BIT looks redundant.
>>
>>>  #define SDHCI_HS400_TUNING    (1<<13)    /* Tuning for HS400 */
>>>
>>>      unsigned int version;    /* SDHCI spec. version */
>>> @@ -509,6 +512,8 @@ struct sdhci_host {
>>>      unsigned int        tuning_mode;    /* Re-tuning mode supported by
>>> host */
>>>  #define SDHCI_TUNING_MODE_1    0
>>>
>>> +    struct cmdq_host *cq_host;
>>> +
>>
>> The individual drivers will have to have their own reference to CQHCI.
>>
>>>      unsigned long private[0] ____cacheline_aligned;
>>>  };
>>>
>>> @@ -544,6 +549,7 @@ struct sdhci_ops {
>>>      void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
>>>      void    (*platform_init)(struct sdhci_host *host);
>>>      void    (*card_event)(struct sdhci_host *host);
>>> +    void    (*clear_set_dumpregs)(struct sdhci_host *host, bool set);
>>
>> What is this for?
> 
> This is vendor specific callback to enable/disable some TEST bus registers
> for debugging purposes.
> 
>>
>>>      void    (*voltage_switch)(struct sdhci_host *host);
>>>      int    (*select_drive_strength)(struct sdhci_host *host,
>>>                       struct mmc_card *card,
>>>
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 
> 
> 

Re: [PATCH RFCv2 10/10] mmc: sdhci: add command queue support to sdhci

[Ritesh Harjani]

Hi Adrian,

Thanks for replying to queries.
I will work on your comments. Mostly after
http://www.spinics.net/lists/linux-mmc/msg38467.html


Regards
Ritesh


On 8/10/2016 4:58 PM, Adrian Hunter wrote:
> On 25/07/16 13:24, Ritesh Harjani wrote:
>> Sorry about this long delay, was pulled into some release.
>> Will be more responsive from now.
>
> Well, I was away, so now it is my delay, sorry.
>
>> We have gone through your comments. We had some queries and wanted to
>> discuss more about your initial comments before going onto individual
>> comments in the code. It would be great if you could help us understand more
>> on your initial comments by answering to our queries.
>>
>>
>> On 7/5/2016 4:45 PM, Adrian Hunter wrote:
>>> On 27/06/16 16:22, Ritesh Harjani wrote:
>>>> From: Asutosh Das <asutoshd@codeaurora.org>
>>>>
>>>> Adds command-queue support to SDHCi compliant drivers.
>>>
>>> CQHCI is not recognized in the SDHCI specification,
>> Yes. Not sure about future plan though.
>
> SD Association and JEDEC are different bodies and I have never seen a SD
> Association Spec. that mentions eMMC, so I would not expect the SDHCI spec.
> ever to recognize CQHCI.
>
>>
>>> and SDHCI should
>>> facilitate any CQE driver implementation not just CQHCI.  That means there
>>> are 2 options:
>>
>> Are we aware of any other vendor specific implementation of CQHCI? Or do you
>> foresee this coming through?
>
> No
>
>> Actually CQ host controller interface(CQHCI) is what is proposed in emmc
>> JEDEC5.1 which is mentioned as extension to HCI in block diagram. (Fig B.110
>> in emmc-5.1 JEDEC).
>>
>>
>>>
>>> 1. Provide minimal support to allow individual host drivers to deal with
>>> CQHCI directly.
>>>
>>> 2. Create explicit support for a CQE companion driver and use that to
>>> provide common support for CQHCI.
>>>
>>> I started looking at option 2 and concluded that it was taking SDHCI in the
>>> wrong direction because it made drivers more dependent on SDHCI and gave
>>> them less flexibility, and it seemed inconsistent with the aim of making
>>> SDHCI more like a library.
>>>
>>> Consequently, I suggest the following:
>>>
>>> 1. Individual drivers are responsible for initializing and setting up CQHCI
>>> and its callbacks, and shutting it down.
>>
>> CQHCI callbacks is what the concern is. Currently the callbacks are placed
>> in SDHCI, because these callbacks are doing nothing other than setting up
>> SDHCI registers itself for CQHCI. There is nothing platform specific which
>> is happening in these.
>> So why do you think that these should be implemented in individual platform
>> specific drivers?
>> Will that not result into same code duplication in all SDHCI compliant
>> platform drivers?
>
> Common functions that do not need CQHCI definitions can still be in sdhci.c.
> Some amount of code duplication is the cost of allowing drivers to be able
> to do whatever the need to.
>
> Linking CQHCI and SDHCI goes in the opposite direction of making SDHCI more
> like a library.  It means more callbacks and more potential issues if
> drivers need to do anything differently.
>
>>
>>
>>>
>>> 2. SDHCI provides a new callback so that individual drivers can process
>>> interrupts and pass them to CQHCI.
>> This again will depend upon the initial ask on - do you foresee SDHCI
>> facilitating other CQE implementations? Or if you are already aware of
>> something else other than CQHCI?
>
> No I don't know if there will ever be another implementation, however it is
> not impossible.  Obviously it would be much messier for SDHCI to support
> different CQE implementations than for individual drivers to support just
> the one they use.
>
>>
>>
>>>
>>> 3. SDHCI provides and exports any common helper functions that do not depend
>>> directly on CQHCI.  For example the functions to set interrupts, timeout,
>>> blocks size and dump registers.
>>>
>>>>
>>>> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
>>>> Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
>>>> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>>>> [subhashj@codeaurora.org: fixed trivial merge conflicts and
>>>> compilation error]
>>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>>>> [riteshh@codeaurora.org: fixed merge conflicts]
>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>> ---
>>>>  drivers/mmc/host/sdhci.c | 146
>>>> +++++++++++++++++++++++++++++++++++++++++++++--
>>>>  drivers/mmc/host/sdhci.h |   6 ++
>>>>  2 files changed, 148 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>>> index 9f5cdaa..0ed9c45 100644
>>>> --- a/drivers/mmc/host/sdhci.c
>>>> +++ b/drivers/mmc/host/sdhci.c
>>>> @@ -32,6 +32,7 @@
>>>>  #include <linux/mmc/slot-gpio.h>
>>>>
>>>>  #include "sdhci.h"
>>>> +#include "cmdq_hci.h"
>>>
>>> As discussed above, SDHCI should not have any references to CQHCI
>>>
>>>>
>>>>  #define DRIVER_NAME "sdhci"
>>>>
>>>> @@ -2474,6 +2475,20 @@ static void sdhci_data_irq(struct sdhci_host
>>>> *host, u32 intmask)
>>>>      }
>>>>  }
>>>>
>>>> +#ifdef CONFIG_MMC_CQ_HCI
>>>
>>> This won't work if CQHCI is a module.  If you fix the dependencies then you
>>> can use:
>>>
>>> #if IS_ENABLED(CONFIG_MMC_CQ_HCI)
>>>
>>> Or if you select CONFIG_MMC_CQ_HCI in Kconfig then you don't need ifdef at
>>> all.
>>>
>>>> +static irqreturn_t sdhci_cmdq_irq(struct mmc_host *mmc, u32 intmask)
>>>> +{
>>>> +    return cmdq_irq(mmc, intmask);
>>>
>>> This will give a linker error if the driver is built-in but CQHCI is a
>>> module.  Probably drivers that use CQHCI should just select it in Kconfig.
>>>
>>> Another issue is that CQHCI won't be able to interpret SDHCI interrupt bits.
>>>  I suggest you define a generic set of bit flags not dependent on SDHCI or
>>> CQHCI and then we can create a function to convert the SDHCI interrupt
>>> status.
>>>
>>>> +}
>>>> +
>>>> +#else
>>>> +static irqreturn_t sdhci_cmdq_irq(struct mmc_host *mmc, u32 intmask)
>>>> +{
>>>> +    pr_err("%s: rxd cmdq-irq when disabled !!!!\n", mmc_hostname(mmc));
>>>> +    return IRQ_NONE;
>>>> +}
>>>> +#endif
>>>> +
>>>>  static irqreturn_t sdhci_irq(int irq, void *dev_id)
>>>>  {
>>>>      irqreturn_t result = IRQ_NONE;
>>>> @@ -2495,6 +2510,15 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
>>>>      }
>>>>
>>>>      do {
>>>> +        if (host->mmc->card && mmc_card_cmdq(host->mmc->card) &&
>>>> +            !mmc_host_halt(host->mmc)) {
>>>> +            pr_debug("*** %s: cmdq intr: 0x%08x\n",
>>>> +                    mmc_hostname(host->mmc),
>>>> +                    intmask);
>>>> +            result = sdhci_cmdq_irq(host->mmc, intmask);
>>>> +            goto out;
>>>> +        }
>>>> +
>>>
>>> We need to create a new SDHCI host op to enable individual drivers to
>>> handle the IRQ if they choose.  Then it is up to individual drivers to call
>>> into CQHCI.
>>>
>>>>          /* Clear selected interrupts. */
>>>>          mask = intmask & (SDHCI_INT_CMD_MASK | SDHCI_INT_DATA_MASK |
>>>>                    SDHCI_INT_BUS_POWER);
>>>> @@ -2823,6 +2847,106 @@ static int sdhci_set_dma_mask(struct sdhci_host
>>>> *host)
>>>>      return ret;
>>>>  }
>>>>
>>>> +#ifdef CONFIG_MMC_CQ_HCI
>>>> +static void sdhci_cmdq_clear_set_irqs(struct mmc_host *mmc, bool clear)
>>>> +{
>>>> +    struct sdhci_host *host = mmc_priv(mmc);
>>>> +    u32 ier = 0;
>>>> +
>>>> +    ier &= ~SDHCI_INT_ALL_MASK;
>>>> +
>>>> +    if (clear) {
>>>> +        ier = SDHCI_INT_CMDQ_EN | SDHCI_INT_ERROR_MASK;
>>>
>>> SDHCI_INT_ERROR_MASK is not ideal here.  I would expect to set only the bits
>>> we know and want.
>>>
>>>> +        sdhci_writel(host, ier, SDHCI_INT_ENABLE);
>>>> +        sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
>>>> +    } else {
>>>> +        ier = SDHCI_INT_BUS_POWER | SDHCI_INT_DATA_END_BIT |
>>>> +                 SDHCI_INT_DATA_CRC | SDHCI_INT_DATA_TIMEOUT |
>>>> +                 SDHCI_INT_INDEX | SDHCI_INT_END_BIT |
>>>> +                 SDHCI_INT_CRC | SDHCI_INT_TIMEOUT |
>>>> +                 SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
>>>> +                 SDHCI_INT_ACMD12ERR;
>>>
>>> We ought to have the default interrupts defined, so that the same ones are
>>> set here.
>>>
>>>> +        sdhci_writel(host, ier, SDHCI_INT_ENABLE);
>>>> +        sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
>>>> +    }
>>>> +}
>>>> +
>>>> +static void sdhci_cmdq_set_data_timeout(struct mmc_host *mmc, u32 val)
>>>
>>> Do we really need different callbacks for ->clear_set_irqs(),
>>> ->set_data_timeout(), ->set_block_size() and ->clear_set_dumpregs()?  It
>>> looks like we could consolidate them all into just 2 i.e. to start and stop
>>> CQ mode.
>>>
>>>> +{
>>>> +    struct sdhci_host *host = mmc_priv(mmc);
>>>> +
>>>> +    sdhci_writeb(host, val, SDHCI_TIMEOUT_CONTROL);
>>>> +}
>>>
>>> We can't expect CQHCI to provide the SDHCI register value.  Ideally we don't
>>> want to set the highest value but instead calculate the value based on the
>>> maximum sized transfer.
>>>
>>>> +
>>>> +static void sdhci_cmdq_dump_vendor_regs(struct mmc_host *mmc)
>>>> +{
>>>> +    struct sdhci_host *host = mmc_priv(mmc);
>>>> +
>>>> +    sdhci_dumpregs(host);
>>>> +}
>>>
>>> Instead you should export sdhci_dumpregs() and let the individual drivers
>>> connect it to CQHCI.
>>>
>>>> +
>>>> +static int sdhci_cmdq_init(struct sdhci_host *host, struct mmc_host *mmc,
>>>> +               bool dma64)
>>>> +{
>>>> +    return cmdq_init(host->cq_host, mmc, dma64);
>>>> +}
>>>
>>> Individual drivers should initialize their CQE driver implementation.  It is
>>> not necessarily CQHCI.
>>>
>>>> +
>>>> +static void sdhci_cmdq_set_block_size(struct mmc_host *mmc)
>>>> +{
>>>> +    struct sdhci_host *host = mmc_priv(mmc);
>>>> +
>>>> +    sdhci_set_blk_size_reg(host, 512, 0);
>>>> +}
>>>> +
>>>> +static void sdhci_cmdq_clear_set_dumpregs(struct mmc_host *mmc, bool set)
>>>> +{
>>>> +    struct sdhci_host *host = mmc_priv(mmc);
>>>> +
>>>> +    if (host->ops->clear_set_dumpregs)
>>>> +        host->ops->clear_set_dumpregs(host, set);
>> This is vendor specific callback for platform drivers to enable/disable (say
>> some TEST bus) registers for debugging purposes.
>>
>>
>>>> +}
>>>
>>> As questioned further below, what is this for?
>>>
>>>> +#else
>>>> +static void sdhci_cmdq_clear_set_irqs(struct mmc_host *mmc, bool clear)
>>>> +{
>>>> +
>>>> +}
>>>> +
>>>> +static void sdhci_cmdq_set_data_timeout(struct mmc_host *mmc, u32 val)
>>>> +{
>>>> +
>>>> +}
>>>> +
>>>> +static void sdhci_cmdq_dump_vendor_regs(struct mmc_host *mmc)
>>>> +{
>>>> +
>>>> +}
>>>> +
>>>> +static int sdhci_cmdq_init(struct sdhci_host *host, struct mmc_host *mmc,
>>>> +               bool dma64)
>>>> +{
>>>> +    return -ENOSYS;
>>>> +}
>>>> +
>>>> +static void sdhci_cmdq_set_block_size(struct mmc_host *mmc)
>>>> +{
>>>> +
>>>> +}
>>>> +
>>>> +static void sdhci_cmdq_clear_set_dumpregs(struct mmc_host *mmc, bool set)
>>>> +{
>>>> +
>>>> +}
>>>> +
>>>> +#endif
>>>> +
>>>> +static const struct cmdq_host_ops sdhci_cmdq_ops = {
>>>> +    .clear_set_irqs = sdhci_cmdq_clear_set_irqs,
>>>> +    .set_data_timeout = sdhci_cmdq_set_data_timeout,
>>>> +    .dump_vendor_regs = sdhci_cmdq_dump_vendor_regs,
>>>> +    .set_block_size = sdhci_cmdq_set_block_size,
>>>> +    .clear_set_dumpregs = sdhci_cmdq_clear_set_dumpregs,
>>>> +};
>>>
>>> It would be up to individual drivers to provide CQHCI ops.
>>>
>>>> +
>>>>  int sdhci_add_host(struct sdhci_host *host)
>>>>  {
>>>>      struct mmc_host *mmc;
>>>> @@ -3341,11 +3465,25 @@ int sdhci_add_host(struct sdhci_host *host)
>>>>      if (ret)
>>>>          goto unled;
>>>>
>>>> -    pr_info("%s: SDHCI controller on %s [%s] using %s\n",
>>>> -        mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
>>>> +    if (mmc->caps2 &  MMC_CAP2_CMD_QUEUE) {
>>>> +        bool dma64 = (host->flags & SDHCI_USE_ADMA_64BIT) ?
>>>> +            true : false;
>>>> +        ret = sdhci_cmdq_init(host, mmc, dma64);
>>>
>>> You must initialize before mmc_add_host() because mmc_add_host() also starts
>>> the host.  If you re-base on top of the SDHCI patches, then individual
>>> drivers can take advantage of the new sdhci_setup_host() /
>>> __sdhci_add_host() split.
>>>
>>>> +        if (ret)
>>>> +            pr_err("%s: CMDQ init: failed (%d)\n",
>>>> +                   mmc_hostname(host->mmc), ret);
>>>> +        else
>>>> +            host->cq_host->ops = &sdhci_cmdq_ops;
>>>> +    }
>>>> +
>>>> +    pr_info("%s: SDHCI controller on %s [%s] using %s in %s mode\n",
>>>> +    mmc_hostname(mmc), host->hw_name, dev_name(mmc_dev(mmc)),
>>>>          (host->flags & SDHCI_USE_ADMA) ?
>>>> -        (host->flags & SDHCI_USE_64_BIT_DMA) ? "ADMA 64-bit" : "ADMA" :
>>>> -        (host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO");
>>>> +        ((host->flags & SDHCI_USE_ADMA_64BIT) ?
>>>> +        "64-bit ADMA" : "32-bit ADMA") :
>>>> +        ((host->flags & SDHCI_USE_SDMA) ? "DMA" : "PIO"),
>>>> +        ((mmc->caps2 &  MMC_CAP2_CMD_QUEUE) && !ret) ?
>>>> +        "CMDQ" : "legacy");
>>>
>>> It is probably better if CQHCI prints its own info.  That way it you get a
>>> consistent message irrespective of whether the driver uses SDHCI.  Also you
>>> can print other CQHCI details such as the version.
>>>
>>>>
>>>>      sdhci_enable_card_detection(host);
>>>>
>>>> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
>>>> index 609f87c..fccc750 100644
>>>> --- a/drivers/mmc/host/sdhci.h
>>>> +++ b/drivers/mmc/host/sdhci.h
>>>> @@ -150,6 +150,8 @@
>>>>          SDHCI_INT_DATA_TIMEOUT | SDHCI_INT_DATA_CRC | \
>>>>          SDHCI_INT_DATA_END_BIT | SDHCI_INT_ADMA_ERROR | \
>>>>          SDHCI_INT_BLK_GAP)
>>>> +
>>>> +#define SDHCI_INT_CMDQ_EN    (0x1 << 14)
>>>
>>> We can define this bit for convenience but we need a comment to say that it
>>> is non-standard.
>>>
>>>>  #define SDHCI_INT_ALL_MASK    ((unsigned int)-1)
>>>>
>>>>  #define SDHCI_ACMD12_ERR    0x3C
>>>> @@ -446,6 +448,7 @@ struct sdhci_host {
>>>>  #define SDHCI_PV_ENABLED    (1<<8)    /* Preset value enabled */
>>>>  #define SDHCI_SDIO_IRQ_ENABLED    (1<<9)    /* SDIO irq enabled */
>>>>  #define SDHCI_USE_64_BIT_DMA    (1<<12)    /* Use 64-bit DMA */
>>>> +#define SDHCI_USE_ADMA_64BIT    (1<<12)    /* Host is 64-bit ADMA
>>>> capable */
>>>
>>> SDHCI_USE_ADMA_64BIT looks redundant.
>>>
>>>>  #define SDHCI_HS400_TUNING    (1<<13)    /* Tuning for HS400 */
>>>>
>>>>      unsigned int version;    /* SDHCI spec. version */
>>>> @@ -509,6 +512,8 @@ struct sdhci_host {
>>>>      unsigned int        tuning_mode;    /* Re-tuning mode supported by
>>>> host */
>>>>  #define SDHCI_TUNING_MODE_1    0
>>>>
>>>> +    struct cmdq_host *cq_host;
>>>> +
>>>
>>> The individual drivers will have to have their own reference to CQHCI.
>>>
>>>>      unsigned long private[0] ____cacheline_aligned;
>>>>  };
>>>>
>>>> @@ -544,6 +549,7 @@ struct sdhci_ops {
>>>>      void    (*adma_workaround)(struct sdhci_host *host, u32 intmask);
>>>>      void    (*platform_init)(struct sdhci_host *host);
>>>>      void    (*card_event)(struct sdhci_host *host);
>>>> +    void    (*clear_set_dumpregs)(struct sdhci_host *host, bool set);
>>>
>>> What is this for?
>>
>> This is vendor specific callback to enable/disable some TEST bus registers
>> for debugging purposes.
>>
>>>
>>>>      void    (*voltage_switch)(struct sdhci_host *host);
>>>>      int    (*select_drive_strength)(struct sdhci_host *host,
>>>>                       struct mmc_card *card,
>>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

[Linus Walleij]

On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:

> From: Asutosh Das <asutoshd@codeaurora.org>
>
> eMMC cards with EXT_CSD version >= 8, optionally support command
> 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>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Even if we don't merge the specific mechanism provided by the
rest of the patches, this patch just make us know more about
the capabilities of the hardware we're running on, which is good.

WIll it be reported properly by the lsmmc command too?

Yours,
Linus Walleij

Re: [PATCH RFCv2 00/10] mmc: Add HW Command Queuing Support

[Linus Walleij]

On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:

> This patch series refreshes the older patch series sent a
> while ago by Asutosh Das[1].
>
> Current set of patch series is only introducing the basic CMDQ
> driver to get more review comments. Based on the reviews, will
> add other features as well to the patch list.

kthread is deprecated since the talks of the last kernel
summit. You should prefer to use workqueues.

The queueing mechanism looks like a duplicate copy/paste
solution built on the "old" processing queue.

Overall this is the problem with the whole patch set: instead
of a "deep" integration by improving the existing request
processing, a separate code path is bolted on the side.

What you should do is improve the existing infrastructure to
accomodate for the new mechanism.

For example: a new processing thread is introduced instead
of augmenting the existing one to handle this case, resulting
in a big slew of duplicated code.

For example: this is not at all integrated with the other feature,
packed command, which is *also* speculatively pulling out ever more
stuff from the block layer, albeit in a totally different manner,
actually integrated with the existing queue mechanism.

Needless to say: these two features (packed command and
command queue) should use the *same codepath* just that the
command queuing is more powerful, so that packed command is
merely a subset of the command queueing.

Now: note that I sent a patch do delete packed command. I do
not know if that will be accepted, but something deeply
integrated is for sure needed, we need a elegant and compact
core of code to maintain, not more copies of the already existing
messy code.

Yours,
Linus Walleij

Re: [PATCH RFCv2 00/10] mmc: Add HW Command Queuing Support

[Arnd Bergmann]

On Monday, November 21, 2016 4:52:37 PM CET Linus Walleij wrote:
> On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
> 
> > This patch series refreshes the older patch series sent a
> > while ago by Asutosh Das[1].
> >
> > Current set of patch series is only introducing the basic CMDQ
> > driver to get more review comments. Based on the reviews, will
> > add other features as well to the patch list.
> 
> kthread is deprecated since the talks of the last kernel
> summit. You should prefer to use workqueues.

It's the kthread freezer that is deprecated, not kthreads. It with
blk_mq, we probably won't want to have this kthread, but that's
a differennt issue, and using workqueues instead won't help.

	Arnd

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

[Adrian Hunter]

On 21/11/16 17:34, Linus Walleij wrote:
> On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
> 
>> From: Asutosh Das <asutoshd@codeaurora.org>
>>
>> eMMC cards with EXT_CSD version >= 8, optionally support command
>> 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>
>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> Even if we don't merge the specific mechanism provided by the
> rest of the patches, this patch just make us know more about
> the capabilities of the hardware we're running on, which is good.

I think SW CMDQ is a better starting point:

	https://marc.info/?l=linux-mmc&m=147729857722285

It cleans up the queue thread a bit:

	https://marc.info/?l=linux-mmc&m=147729857222281&w=2

And introduces queue semantics:

	https://marc.info/?l=linux-mmc&m=147729863322314&w=2

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

[Linus Walleij]

On Tue, Nov 22, 2016 at 8:58 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 21/11/16 17:34, Linus Walleij wrote:
>> On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>>
>>> From: Asutosh Das <asutoshd@codeaurora.org>
>>>
>>> eMMC cards with EXT_CSD version >= 8, optionally support command
>>> 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>
>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> Even if we don't merge the specific mechanism provided by the
>> rest of the patches, this patch just make us know more about
>> the capabilities of the hardware we're running on, which is good.
>
> I think SW CMDQ is a better starting point:

When I supplied the Reviewe-by, as stated below it I am not talking
about the patch set as a whole. I am talking about *this* patch.

As you can see in my reply to 00/10 I have serious concerns about
this patchset overall.

Yours,
Linus Walleij

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

[Adrian Hunter]

On 22/11/16 12:20, Linus Walleij wrote:
> On Tue, Nov 22, 2016 at 8:58 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 21/11/16 17:34, Linus Walleij wrote:
>>> On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>>>
>>>> From: Asutosh Das <asutoshd@codeaurora.org>
>>>>
>>>> eMMC cards with EXT_CSD version >= 8, optionally support command
>>>> 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>
>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>
>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>
>>> Even if we don't merge the specific mechanism provided by the
>>> rest of the patches, this patch just make us know more about
>>> the capabilities of the hardware we're running on, which is good.
>>
>> I think SW CMDQ is a better starting point:
> 
> When I supplied the Reviewe-by, as stated below it I am not talking
> about the patch set as a whole. I am talking about *this* patch.

I should have been more explicit.  SW CMDQ covers some of the same ground
and has basically the same patch:

	https://marc.info/?l=linux-mmc&m=147729859922297&w=2

> 
> As you can see in my reply to 00/10 I have serious concerns about
> this patchset overall.

I didn't mean to imply otherwise.

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

[Linus Walleij]

On Tue, Nov 22, 2016 at 11:31 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 22/11/16 12:20, Linus Walleij wrote:
>> On Tue, Nov 22, 2016 at 8:58 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 21/11/16 17:34, Linus Walleij wrote:
>>>> On Mon, Jun 27, 2016 at 3:22 PM, Ritesh Harjani <riteshh@codeaurora.org> wrote:
>>>>
>>>>> From: Asutosh Das <asutoshd@codeaurora.org>
>>>>>
>>>>> eMMC cards with EXT_CSD version >= 8, optionally support command
>>>>> 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>
>>>>> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
>>>>
>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>
>>>> Even if we don't merge the specific mechanism provided by the
>>>> rest of the patches, this patch just make us know more about
>>>> the capabilities of the hardware we're running on, which is good.
>>>
>>> I think SW CMDQ is a better starting point:
>>
>> When I supplied the Reviewe-by, as stated below it I am not talking
>> about the patch set as a whole. I am talking about *this* patch.
>
> I should have been more explicit.  SW CMDQ covers some of the same ground
> and has basically the same patch:
>
>         https://marc.info/?l=linux-mmc&m=147729859922297&w=2

OK it seems to define more of the constants, I'll go and review it.

Yours,
Linus Walleij

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

[Linus Walleij]

On Tue, Nov 22, 2016 at 1:30 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Nov 22, 2016 at 11:31 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:

>> I should have been more explicit.  SW CMDQ covers some of the same ground
>> and has basically the same patch:
>>
>>         https://marc.info/?l=linux-mmc&m=147729859922297&w=2
>
> OK it seems to define more of the constants, I'll go and review it.

Bah it's not in my INBOX for some reason, maybe I fell off the linux-mmc list
at some point :( I review it here:

Leave out this:

+ cmdq_en Command Queue enabled: 1 => enabled, 0 => not enabled
(...)
+MMC_DEV_ATTR(cmdq_en, "%d\n", card->ext_csd.cmdq_en);
(...)
+ &dev_attr_cmdq_en.attr,
(...)
+ bool cmdq_en; /* Command Queue enabled */

Because that is about enabling the *use* of this mechnism rather than
detecting its presence.

With the above removed it is superior to $SUBJECT so you
can add:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij