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

Shawn Lin
2017-07-22T09:40:35Z

Hi Adrian,

On 2017/7/21 17:49, Adrian Hunter wrote:
> From: Venkat Gopalakrishnan <venkatg@codeaurora.org>
>
> This patch adds CMDQ support for command-queue compatible
> hosts.
>
> Command queue is added in eMMC-5.1 specification. This
> enables the controller to process upto 32 requests at
> a time.
>
> Adrian Hunter contributed renaming to cqhci, recovery, suspend
> and resume, cqhci_off, cqhci_wait_for_idle, and external timeout
> handling.
>
> Signed-off-by: Asutosh Das <asutoshd@codeaurora.org>
> Signed-off-by: Sujit Reddy Thumma <sthumma@codeaurora.org>
> Signed-off-by: Konstantin Dorfman <kdorfman@codeaurora.org>
> Signed-off-by: Venkat Gopalakrishnan <venkatg@codeaurora.org>
> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> Signed-off-by: Ritesh Harjani <riteshh@codeaurora.org>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/Kconfig | 13 +
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/cqhci.c | 1146 +++++++++++++++++++++++++++++++++++++++++++++
> drivers/mmc/host/cqhci.h | 240 ++++++++++
> 4 files changed, 1400 insertions(+)
> create mode 100644 drivers/mmc/host/cqhci.c
> create mode 100644 drivers/mmc/host/cqhci.h
>

[...]

> +
> +static void cqhci_prep_task_desc(struct mmc_request *mrq,
> + u64 *data, bool intr)
> +{
> + u32 req_flags = mrq->data->flags;
> +
> + *data = CQHCI_VALID(1) |
> + CQHCI_END(1) |
> + CQHCI_INT(intr) |
> + CQHCI_ACT(0x5) |
> + CQHCI_FORCED_PROG(!!(req_flags & MMC_DATA_FORCED_PRG)) |
> + CQHCI_DATA_TAG(!!(req_flags & MMC_DATA_DAT_TAG)) |
> + CQHCI_DATA_DIR(!!(req_flags & MMC_DATA_READ)) |
> + CQHCI_PRIORITY(!!(req_flags & MMC_DATA_PRIO)) |
> + CQHCI_QBAR(!!(req_flags & MMC_DATA_QBR)) |
> + CQHCI_REL_WRITE(!!(req_flags & MMC_DATA_REL_WR)) |
> + CQHCI_BLK_COUNT(mrq->data->blocks) |
> + CQHCI_BLK_ADDR((u64)mrq->data->blk_addr);
> +

I don't find where MMC_DATA_FORCED_PRG is set and MMC_DATA_QBR is set
only for DCMD(erase, flush etc). So I have some silly questions :)

(1) Should MMC_DATA_FORCED_PRG be used for secure erase/trim, etc?
(2) How could the overall code guarantee the read-after-write issue? for
instance, is it possible that two TDLs contain one read for LBA x and a
write for same LBA x, so how to make sure the devices' execute order?

> +int cqhci_resume(struct mmc_host *mmc);
> +
> +#endif
>

--
Best Regards
Shawn Lin