* Re: [PATCH v3] mmc: block: delete packed command support
From: Linus Walleij @ 2016-11-29 14:25 UTC (permalink / raw)
To: Alex Lemberg
Cc: Ulf Hansson, linux-mmc@vger.kernel.org, Chunyan Zhang,
Baolin Wang, Namjae Jeon, Maya Erez, Christoph Hellwig
In-Reply-To: <50128E8A-D7B3-49E3-9E18-4AD240F1EBD0@sandisk.com>
On Sun, Nov 27, 2016 at 3:25 PM, Alex Lemberg <Alex.Lemberg@sandisk.com> wrote:
> I understand your concerns about the current packed commands
> support and the problem of transition to blk_mq.
It may be possible to re-implement the feature later, but then
only if we get some queue inspection features in blk-mq.
Do you agree with my assessment in the thread that this
feature can only really make an improvement on random writes,
i.e. it will not improve reads (even random reads) and it will not
improve sequential writes?
> But as I mentioned in previous email thread, the packed commands
> feature is still in use on eMMC4.5 or eMMC5.1 devices on hosts
> without SW/HW CMDQ support.
True. In the market place of devices, sure.
But noone is maintaining or testing the code in the Linux
kernel. Noone bothered to enable it for their platform so we can
test it.
> Taking this feature out meaning stop supporting one of important
> performance features of eMMC devices.
> Looking forward this feature is replaced with a CMDQ, but should we
> kill it for those who doesn’t have a CMDQ?
What we need from memory card vendors like you (SanDisk)
is a target test rig.
That is: a board with an SoC that we developers can get hold
of (for reasonable cost) and where both the host controller and
the eMMC card has the desired features so we can test it.
Also that this board has support in the upstream kernel.
There are too many vendor trees with too much forked code
out there, I know this is common, but we cannot develop
important core features in the kernel that have no in-kernel
user and no way for us to test it if we need to re-write the
code, it is just unmaintainable.
Yours,
Linus Walleij
^ permalink raw reply
* [PATCH v3 1/1] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP
From: Thierry Escande @ 2016-11-29 14:24 UTC (permalink / raw)
To: Ulf Hansson, Shawn Lin; +Cc: bhthompson, linux-mmc
In-Reply-To: <1480429488-22289-1-git-send-email-thierry.escande@collabora.com>
From: zhaojohn <john.zhao@intel.com>
Hynix eMMC devices sometimes take 50% longer to resume from sleep. This
occurs on Braswell based chromebook with acpi sdhci controller.
Based on a recommendation from Hynix engineers, this patch sends a
Power-Off Notification using mmc_poweroff_notify() before going to S3 to
have a resume time consistently within spec. No voltage regulator are
being cut through this notification but merely tells the eMMC firmware
that we're about to do so and get it to behave better on resume.
Signed-off-by: zhaojohn <john.zhao@intel.com>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
drivers/mmc/card/block.c | 8 ++++++++
drivers/mmc/core/mmc.c | 8 +++++++-
include/linux/mmc/card.h | 2 ++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 709a872..0066a66 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -2573,6 +2573,14 @@ static const struct mmc_fixup blk_fixups[] =
MMC_FIXUP("V10016", CID_MANFID_KINGSTON, CID_OEMID_ANY, add_quirk_mmc,
MMC_QUIRK_TRIM_BROKEN),
+ /*
+ * Hynix eMMC devices sometimes take 50% longer to resume from sleep.
+ * Based on a recommendation from Hynix, send a Power-Off Notification
+ * before going to S3 to restore a resume time consistently within spec.
+ */
+ MMC_FIXUP("HBG4e\005", CID_MANFID_HYNIX, 0x014a, add_quirk_mmc,
+ MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP),
+
END_FIXUP
};
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index df19777..774d478 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1941,8 +1941,14 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
if (mmc_can_poweroff_notify(host->card) &&
((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
err = mmc_poweroff_notify(host->card, notify_type);
- else if (mmc_can_sleep(host->card))
+ else if (mmc_can_sleep(host->card)) {
+ if (host->card->quirks & MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
+ err = mmc_poweroff_notify(host->card, notify_type);
+ if (err)
+ goto out;
+ }
err = mmc_sleep(host);
+ }
else if (!mmc_host_is_spi(host))
err = mmc_deselect_cards(host);
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 73fad83..e4940f4 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -281,6 +281,8 @@ struct mmc_card {
#define MMC_QUIRK_BROKEN_IRQ_POLLING (1<<11) /* Polling SDIO_CCCR_INTx could create a fake interrupt */
#define MMC_QUIRK_TRIM_BROKEN (1<<12) /* Skip trim */
#define MMC_QUIRK_BROKEN_HPI (1<<13) /* Disable broken HPI support */
+#define MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP \
+ (1<<14) /* Poweroff notification*/
unsigned int erase_size; /* erase size in sectors */
--
2.7.4
^ permalink raw reply related
* [PATCH v3 0/1] Hynix quirk on sleep notifcation
From: Thierry Escande @ 2016-11-29 14:24 UTC (permalink / raw)
To: Ulf Hansson, Shawn Lin; +Cc: bhthompson, linux-mmc
Hi Ulf, Hi Shawn,
I added the cid device name and the oemid to the MMC_FIXUP macro call.
There is an issue in the CID product name: The 6th byte contains a non
printable character and I had to add it to the compared string. Please
tell me if this is ok for you.
Changes in v2:
- Add a few more details in the commit message
Changes in v3:
- Install the quirk only on HBG4e controller and 0x014a oemid
zhaojohn (1):
mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP
drivers/mmc/card/block.c | 8 ++++++++
drivers/mmc/core/mmc.c | 8 +++++++-
include/linux/mmc/card.h | 2 ++
3 files changed, 17 insertions(+), 1 deletion(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH 8/8] mmc: mmc_test: remove BUG_ONs and deploy error handling
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <1478071617-12984-1-git-send-email-shawn.lin@rock-chips.com>
On 2 November 2016 at 08:26, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> It is unnecessary to panic the kernel when testing mmc. Instead,
> cast a warning for folkz to debug and return the error code to
> the caller to indicate the failure of this test should be enough.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/card/mmc_test.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
> index 5a8dc5a..db1a7ac 100644
> --- a/drivers/mmc/card/mmc_test.c
> +++ b/drivers/mmc/card/mmc_test.c
> @@ -214,7 +214,8 @@ static void mmc_test_prepare_mrq(struct mmc_test_card *test,
> struct mmc_request *mrq, struct scatterlist *sg, unsigned sg_len,
> unsigned dev_addr, unsigned blocks, unsigned blksz, int write)
> {
> - BUG_ON(!mrq || !mrq->cmd || !mrq->data || !mrq->stop);
> + if (WARN_ON(!mrq || !mrq->cmd || !mrq->data || !mrq->stop))
> + return;
>
> if (blocks > 1) {
> mrq->cmd->opcode = write ?
> @@ -694,7 +695,8 @@ static int mmc_test_cleanup(struct mmc_test_card *test)
> static void mmc_test_prepare_broken_mrq(struct mmc_test_card *test,
> struct mmc_request *mrq, int write)
> {
> - BUG_ON(!mrq || !mrq->cmd || !mrq->data);
> + if (WARN_ON(!mrq || !mrq->cmd || !mrq->data))
> + return;
>
> if (mrq->data->blocks > 1) {
> mrq->cmd->opcode = write ?
> @@ -714,7 +716,8 @@ static int mmc_test_check_result(struct mmc_test_card *test,
> {
> int ret;
>
> - BUG_ON(!mrq || !mrq->cmd || !mrq->data);
> + if (WARN_ON(!mrq || !mrq->cmd || !mrq->data))
> + return -EINVAL;
>
> ret = 0;
>
> @@ -755,7 +758,8 @@ static int mmc_test_check_broken_result(struct mmc_test_card *test,
> {
> int ret;
>
> - BUG_ON(!mrq || !mrq->cmd || !mrq->data);
> + if (WARN_ON(!mrq || !mrq->cmd || !mrq->data))
> + return -EINVAL;
>
> ret = 0;
>
> --
> 2.3.7
>
>
^ permalink raw reply
* Re: [PATCH 7/8] mmc: queue: remove BUG_ON for bounce_sg
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <1478071609-12941-1-git-send-email-shawn.lin@rock-chips.com>
On 2 November 2016 at 08:26, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> bounce_sg for mqrq_cur and mqrq_pre are proper
> allocated when initializing the queue and will not
> be freed before explicitly cleaning the queue. So from
> the code itself it should be quite confident to remove
> this check. If that BUG_ON take effects, it is mostly
> likely the memory is randomly oopsing.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/card/queue.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
> index 8037f73..6c8978a 100644
> --- a/drivers/mmc/card/queue.c
> +++ b/drivers/mmc/card/queue.c
> @@ -505,8 +505,6 @@ unsigned int mmc_queue_map_sg(struct mmc_queue *mq, struct mmc_queue_req *mqrq)
> return blk_rq_map_sg(mq->queue, mqrq->req, mqrq->sg);
> }
>
> - BUG_ON(!mqrq->bounce_sg);
> -
> if (mmc_packed_cmd(cmd_type))
> sg_len = mmc_queue_packed_map_sg(mq, mqrq->packed,
> mqrq->bounce_sg, cmd_type);
> --
> 2.3.7
>
>
> --
> 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
^ permalink raw reply
* Re: [PATCH 6/8] mmc: sdio_uart: remove meaningless BUG_ON
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <1478071585-12898-1-git-send-email-shawn.lin@rock-chips.com>
On 2 November 2016 at 08:26, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> The code seems quite simple to maintain the sdio_uart_table,
> and the insert/remove port from the table are symmetric. If
> the BUG_ON occurs, which means serial_core modify the index
> or mess up the port sequence anyway.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/card/sdio_uart.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/mmc/card/sdio_uart.c b/drivers/mmc/card/sdio_uart.c
> index 5af6fb9..491c187 100644
> --- a/drivers/mmc/card/sdio_uart.c
> +++ b/drivers/mmc/card/sdio_uart.c
> @@ -135,8 +135,6 @@ static void sdio_uart_port_remove(struct sdio_uart_port *port)
> {
> struct sdio_func *func;
>
> - BUG_ON(sdio_uart_table[port->index] != port);
> -
> spin_lock(&sdio_uart_table_lock);
> sdio_uart_table[port->index] = NULL;
> spin_unlock(&sdio_uart_table_lock);
> --
> 2.3.7
>
>
> --
> 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
^ permalink raw reply
* Re: [PATCH 5/8] mmc: core: remove BUG_ONs from core.c
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <1478071576-12857-1-git-send-email-shawn.lin@rock-chips.com>
On 2 November 2016 at 08:26, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> BUG_ONs doesn't help anything except for stop the system from
> running. If it occurs, it implies we should deploy proper error
> handling for that. So this patch is gonna discard these meaningless
> BUG_ONs and deploy error handling if needed.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/core/core.c | 34 +++++++++-------------------------
> 1 file changed, 9 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 2553d90..7fc4814 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -306,16 +306,16 @@ static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
> mrq->sbc->mrq = mrq;
> }
> if (mrq->data) {
> - 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);
> -
> + if (mrq->data->blksz > host->max_blk_size ||
> + mrq->data->blocks > host->max_blk_count ||
> + mrq->data->blocks * mrq->data->blksz > host->max_req_size)
> + return -EINVAL;
> #ifdef CONFIG_MMC_DEBUG
> sz = 0;
> for_each_sg(mrq->data->sg, sg, mrq->data->sg_len, i)
> sz += sg->length;
> - BUG_ON(sz != mrq->data->blocks * mrq->data->blksz);
> + if (sz != mrq->data->blocks * mrq->data->blksz)
> + return -EINVAL;
> #endif
>
> mrq->cmd->data = mrq->data;
> @@ -349,8 +349,6 @@ void mmc_start_bkops(struct mmc_card *card, bool from_exception)
> int timeout;
> bool use_busy_signal;
>
> - BUG_ON(!card);
> -
> if (!card->ext_csd.man_bkops_en || mmc_card_doing_bkops(card))
> return;
>
> @@ -754,8 +752,6 @@ int mmc_interrupt_hpi(struct mmc_card *card)
> u32 status;
> unsigned long prg_wait;
>
> - BUG_ON(!card);
> -
> if (!card->ext_csd.hpi_en) {
> pr_info("%s: HPI enable bit unset\n", mmc_hostname(card->host));
> return 1;
> @@ -850,7 +846,6 @@ int mmc_stop_bkops(struct mmc_card *card)
> {
> int err = 0;
>
> - BUG_ON(!card);
> err = mmc_interrupt_hpi(card);
>
> /*
> @@ -1666,8 +1661,6 @@ int mmc_set_signal_voltage(struct mmc_host *host, int signal_voltage, u32 ocr)
> int err = 0;
> u32 clock;
>
> - BUG_ON(!host);
> -
> /*
> * Send CMD11 only if the request is to switch the card to
> * 1.8V signalling.
> @@ -1884,9 +1877,7 @@ void mmc_power_cycle(struct mmc_host *host, u32 ocr)
> */
> static void __mmc_release_bus(struct mmc_host *host)
> {
> - BUG_ON(!host);
> - BUG_ON(host->bus_refs);
> - BUG_ON(!host->bus_dead);
> + WARN_ON(!host->bus_dead);
>
> host->bus_ops = NULL;
> }
> @@ -1926,15 +1917,12 @@ void mmc_attach_bus(struct mmc_host *host, const struct mmc_bus_ops *ops)
> {
> unsigned long flags;
>
> - BUG_ON(!host);
> - BUG_ON(!ops);
> -
> WARN_ON(!host->claimed);
>
> spin_lock_irqsave(&host->lock, flags);
>
> - BUG_ON(host->bus_ops);
> - BUG_ON(host->bus_refs);
> + WARN_ON(host->bus_ops);
> + WARN_ON(host->bus_refs);
>
> host->bus_ops = ops;
> host->bus_refs = 1;
> @@ -1950,8 +1938,6 @@ void mmc_detach_bus(struct mmc_host *host)
> {
> unsigned long flags;
>
> - BUG_ON(!host);
> -
> WARN_ON(!host->claimed);
> WARN_ON(!host->bus_ops);
>
> @@ -2865,8 +2851,6 @@ void mmc_stop_host(struct mmc_host *host)
> }
> mmc_bus_put(host);
>
> - BUG_ON(host->card);
> -
> mmc_claim_host(host);
> mmc_power_off(host);
> mmc_release_host(host);
> --
> 2.3.7
>
>
^ permalink raw reply
* Re: [PATCH 4/8] mmc: core: remove BUG_ONs from sd
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <1478071548-12816-1-git-send-email-shawn.lin@rock-chips.com>
On 2 November 2016 at 08:25, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> BUG_ONs doesn't help anything except for stop the system from
> running. If it occurs, it implies we should deploy proper error
> handling for that. So this patch is gonna discard these meaningless
> BUG_ONs and deploy error handling if needed.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/core/sd.c | 14 --------------
> drivers/mmc/core/sd_ops.c | 27 ++++-----------------------
> 2 files changed, 4 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
> index 73c762a..deb90c2 100644
> --- a/drivers/mmc/core/sd.c
> +++ b/drivers/mmc/core/sd.c
> @@ -927,7 +927,6 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
> u32 cid[4];
> u32 rocr = 0;
>
> - BUG_ON(!host);
> WARN_ON(!host->claimed);
>
> err = mmc_sd_get_cid(host, ocr, cid, &rocr);
> @@ -1043,9 +1042,6 @@ static int mmc_sd_init_card(struct mmc_host *host, u32 ocr,
> */
> static void mmc_sd_remove(struct mmc_host *host)
> {
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_remove_card(host->card);
> host->card = NULL;
> }
> @@ -1065,9 +1061,6 @@ static void mmc_sd_detect(struct mmc_host *host)
> {
> int err;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_get_card(host->card);
>
> /*
> @@ -1091,9 +1084,6 @@ static int _mmc_sd_suspend(struct mmc_host *host)
> {
> int err = 0;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_claim_host(host);
>
> if (mmc_card_suspended(host->card))
> @@ -1136,9 +1126,6 @@ static int _mmc_sd_resume(struct mmc_host *host)
> {
> int err = 0;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_claim_host(host);
>
> if (!mmc_card_suspended(host->card))
> @@ -1221,7 +1208,6 @@ int mmc_attach_sd(struct mmc_host *host)
> int err;
> u32 ocr, rocr;
>
> - BUG_ON(!host);
> WARN_ON(!host->claimed);
>
> err = mmc_send_app_op_cond(host, 0, &ocr);
> diff --git a/drivers/mmc/core/sd_ops.c b/drivers/mmc/core/sd_ops.c
> index 16b774c..de125a4 100644
> --- a/drivers/mmc/core/sd_ops.c
> +++ b/drivers/mmc/core/sd_ops.c
> @@ -27,8 +27,8 @@ int mmc_app_cmd(struct mmc_host *host, struct mmc_card *card)
> int err;
> struct mmc_command cmd = {0};
>
> - BUG_ON(!host);
> - BUG_ON(card && (card->host != host));
> + if (WARN_ON(card && card->host != host))
> + return -EINVAL;
>
> cmd.opcode = MMC_APP_CMD;
>
> @@ -72,8 +72,8 @@ int mmc_wait_for_app_cmd(struct mmc_host *host, struct mmc_card *card,
>
> int i, err;
>
> - BUG_ON(!cmd);
> - BUG_ON(retries < 0);
> + if (retries < 0)
> + retries = MMC_CMD_RETRIES;
>
> err = -EIO;
>
> @@ -122,9 +122,6 @@ int mmc_app_set_bus_width(struct mmc_card *card, int width)
> {
> struct mmc_command cmd = {0};
>
> - BUG_ON(!card);
> - BUG_ON(!card->host);
> -
> cmd.opcode = SD_APP_SET_BUS_WIDTH;
> cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>
> @@ -147,8 +144,6 @@ int mmc_send_app_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
> struct mmc_command cmd = {0};
> int i, err = 0;
>
> - BUG_ON(!host);
> -
> cmd.opcode = SD_APP_OP_COND;
> if (mmc_host_is_spi(host))
> cmd.arg = ocr & (1 << 30); /* SPI only defines one bit */
> @@ -224,9 +219,6 @@ int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca)
> int err;
> struct mmc_command cmd = {0};
>
> - BUG_ON(!host);
> - BUG_ON(!rca);
> -
> cmd.opcode = SD_SEND_RELATIVE_ADDR;
> cmd.arg = 0;
> cmd.flags = MMC_RSP_R6 | MMC_CMD_BCR;
> @@ -249,10 +241,6 @@ int mmc_app_send_scr(struct mmc_card *card, u32 *scr)
> struct scatterlist sg;
> void *data_buf;
>
> - BUG_ON(!card);
> - BUG_ON(!card->host);
> - BUG_ON(!scr);
> -
> /* NOTE: caller guarantees scr is heap-allocated */
>
> err = mmc_app_cmd(card->host, card);
> @@ -307,9 +295,6 @@ int mmc_sd_switch(struct mmc_card *card, int mode, int group,
> struct mmc_data data = {0};
> struct scatterlist sg;
>
> - BUG_ON(!card);
> - BUG_ON(!card->host);
> -
> /* NOTE: caller guarantees resp is heap-allocated */
>
> mode = !!mode;
> @@ -352,10 +337,6 @@ int mmc_app_sd_status(struct mmc_card *card, void *ssr)
> struct mmc_data data = {0};
> struct scatterlist sg;
>
> - BUG_ON(!card);
> - BUG_ON(!card->host);
> - BUG_ON(!ssr);
> -
> /* NOTE: caller guarantees ssr is heap-allocated */
>
> err = mmc_app_cmd(card->host, card);
> --
> 2.3.7
>
>
> --
> 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
^ permalink raw reply
* Re: [PATCH 3/8] mmc: core: remove BUG_ONs from mmc
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <1478071533-12775-1-git-send-email-shawn.lin@rock-chips.com>
On 2 November 2016 at 08:25, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> BUG_ONs doesn't help anything except for stop the system from
> running. If it occurs, it implies we should deploy proper error
> handling for that. So this patch is gonna discard these meaningless
> BUG_ONs and deploy error handling if needed.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/core/mmc.c | 14 --------------
> drivers/mmc/core/mmc_ops.c | 17 -----------------
> 2 files changed, 31 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index e811bd9..4763a35 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1464,7 +1464,6 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> u32 cid[4];
> u32 rocr;
>
> - BUG_ON(!host);
> WARN_ON(!host->claimed);
>
> /* Set correct bus mode for MMC before attempting init */
> @@ -1854,9 +1853,6 @@ static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
> */
> static void mmc_remove(struct mmc_host *host)
> {
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_remove_card(host->card);
> host->card = NULL;
> }
> @@ -1876,9 +1872,6 @@ static void mmc_detect(struct mmc_host *host)
> {
> int err;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_get_card(host->card);
>
> /*
> @@ -1904,9 +1897,6 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
> unsigned int notify_type = is_suspend ? EXT_CSD_POWER_OFF_SHORT :
> EXT_CSD_POWER_OFF_LONG;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_claim_host(host);
>
> if (mmc_card_suspended(host->card))
> @@ -1963,9 +1953,6 @@ static int _mmc_resume(struct mmc_host *host)
> {
> int err = 0;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_claim_host(host);
>
> if (!mmc_card_suspended(host->card))
> @@ -2098,7 +2085,6 @@ int mmc_attach_mmc(struct mmc_host *host)
> int err;
> u32 ocr, rocr;
>
> - BUG_ON(!host);
> WARN_ON(!host->claimed);
>
> /* Set correct bus mode for MMC before attempting attach */
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 481bbdb..2bc4780 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -60,9 +60,6 @@ static inline int __mmc_send_status(struct mmc_card *card, u32 *status,
> int err;
> struct mmc_command cmd = {0};
>
> - BUG_ON(!card);
> - BUG_ON(!card->host);
> -
> cmd.opcode = MMC_SEND_STATUS;
> if (!mmc_host_is_spi(card->host))
> cmd.arg = card->rca << 16;
> @@ -92,8 +89,6 @@ static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
> {
> struct mmc_command cmd = {0};
>
> - BUG_ON(!host);
> -
> cmd.opcode = MMC_SELECT_CARD;
>
> if (card) {
> @@ -109,7 +104,6 @@ static int _mmc_select_card(struct mmc_host *host, struct mmc_card *card)
>
> int mmc_select_card(struct mmc_card *card)
> {
> - BUG_ON(!card);
>
> return _mmc_select_card(card->host, card);
> }
> @@ -181,8 +175,6 @@ int mmc_send_op_cond(struct mmc_host *host, u32 ocr, u32 *rocr)
> struct mmc_command cmd = {0};
> int i, err = 0;
>
> - BUG_ON(!host);
> -
> cmd.opcode = MMC_SEND_OP_COND;
> cmd.arg = mmc_host_is_spi(host) ? 0 : ocr;
> cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R3 | MMC_CMD_BCR;
> @@ -221,9 +213,6 @@ int mmc_all_send_cid(struct mmc_host *host, u32 *cid)
> int err;
> struct mmc_command cmd = {0};
>
> - BUG_ON(!host);
> - BUG_ON(!cid);
> -
> cmd.opcode = MMC_ALL_SEND_CID;
> cmd.arg = 0;
> cmd.flags = MMC_RSP_R2 | MMC_CMD_BCR;
> @@ -241,9 +230,6 @@ int mmc_set_relative_addr(struct mmc_card *card)
> {
> struct mmc_command cmd = {0};
>
> - BUG_ON(!card);
> - BUG_ON(!card->host);
> -
> cmd.opcode = MMC_SET_RELATIVE_ADDR;
> cmd.arg = card->rca << 16;
> cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
> @@ -257,9 +243,6 @@ mmc_send_cxd_native(struct mmc_host *host, u32 arg, u32 *cxd, int opcode)
> int err;
> struct mmc_command cmd = {0};
>
> - BUG_ON(!host);
> - BUG_ON(!cxd);
> -
> cmd.opcode = opcode;
> cmd.arg = arg;
> cmd.flags = MMC_RSP_R2 | MMC_CMD_AC;
> --
> 2.3.7
>
>
^ permalink raw reply
* Re: [PATCH 2/8] mmc: debugfs: remove BUG_ON from mmc_ext_csd_open
From: Ulf Hansson @ 2016-11-29 12:41 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <1478071479-12730-1-git-send-email-shawn.lin@rock-chips.com>
On 2 November 2016 at 08:24, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> Return error value for file_operations callback instead
> of triggering BUG_ON which is meaningless. Personally I
> don't believe n != EXT_CSD_STR_LEN could happen. Anyway,
> propagate the error to the caller.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/core/debugfs.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
> index c8451ce..30623b8 100644
> --- a/drivers/mmc/core/debugfs.c
> +++ b/drivers/mmc/core/debugfs.c
> @@ -321,7 +321,11 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
> for (i = 0; i < 512; i++)
> n += sprintf(buf + n, "%02x", ext_csd[i]);
> n += sprintf(buf + n, "\n");
> - BUG_ON(n != EXT_CSD_STR_LEN);
> +
> + if (n != EXT_CSD_STR_LEN) {
> + err = -EINVAL;
> + goto out_free;
> + }
>
> filp->private_data = buf;
> kfree(ext_csd);
> --
> 2.3.7
>
>
> --
> 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
^ permalink raw reply
* Re: [PATCH 1/8] mmc: core: remove BUG_ONs from sdio
From: Ulf Hansson @ 2016-11-29 12:40 UTC (permalink / raw)
To: Shawn Lin; +Cc: linux-mmc@vger.kernel.org
In-Reply-To: <1478071440-12689-1-git-send-email-shawn.lin@rock-chips.com>
On 2 November 2016 at 08:24, Shawn Lin <shawn.lin@rock-chips.com> wrote:
> BUG_ONs doesn't help anything except for stop the system from
> running. If it occurs, it implies we should deploy proper error
> handling for that. So this patch is gonna discard these meaningless
> BUG_ONs and deploy error handling if needed.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
>
> drivers/mmc/core/sdio.c | 17 ++---------------
> drivers/mmc/core/sdio_cis.c | 3 ++-
> drivers/mmc/core/sdio_irq.c | 12 +++++++-----
> 3 files changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index bd44ba8..ecbc529 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -63,7 +63,8 @@ static int sdio_init_func(struct mmc_card *card, unsigned int fn)
> int ret;
> struct sdio_func *func;
>
> - BUG_ON(fn > SDIO_MAX_FUNCS);
> + if (WARN_ON(fn > SDIO_MAX_FUNCS))
> + return -EINVAL;
>
> func = sdio_alloc_func(card);
> if (IS_ERR(func))
> @@ -555,7 +556,6 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
> u32 rocr = 0;
> u32 ocr_card = ocr;
>
> - BUG_ON(!host);
> WARN_ON(!host->claimed);
>
> /* to query card if 1.8V signalling is supported */
> @@ -791,9 +791,6 @@ static void mmc_sdio_remove(struct mmc_host *host)
> {
> int i;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> for (i = 0;i < host->card->sdio_funcs;i++) {
> if (host->card->sdio_func[i]) {
> sdio_remove_func(host->card->sdio_func[i]);
> @@ -820,9 +817,6 @@ static void mmc_sdio_detect(struct mmc_host *host)
> {
> int err;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> /* Make sure card is powered before detecting it */
> if (host->caps & MMC_CAP_POWER_OFF_CARD) {
> err = pm_runtime_get_sync(&host->card->dev);
> @@ -916,9 +910,6 @@ static int mmc_sdio_resume(struct mmc_host *host)
> {
> int err = 0;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> /* Basic card reinitialization. */
> mmc_claim_host(host);
>
> @@ -970,9 +961,6 @@ static int mmc_sdio_power_restore(struct mmc_host *host)
> {
> int ret;
>
> - BUG_ON(!host);
> - BUG_ON(!host->card);
> -
> mmc_claim_host(host);
>
> /*
> @@ -1063,7 +1051,6 @@ int mmc_attach_sdio(struct mmc_host *host)
> u32 ocr, rocr;
> struct mmc_card *card;
>
> - BUG_ON(!host);
> WARN_ON(!host->claimed);
>
> err = mmc_send_io_op_cond(host, 0, &ocr);
> diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c
> index dcb3dee..f8c3728 100644
> --- a/drivers/mmc/core/sdio_cis.c
> +++ b/drivers/mmc/core/sdio_cis.c
> @@ -262,7 +262,8 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func)
> else
> prev = &card->tuples;
>
> - BUG_ON(*prev);
> + if (*prev)
> + return -EINVAL;
>
> do {
> unsigned char tpl_code, tpl_link;
> diff --git a/drivers/mmc/core/sdio_irq.c b/drivers/mmc/core/sdio_irq.c
> index 91bbbfb..f1faf9a 100644
> --- a/drivers/mmc/core/sdio_irq.c
> +++ b/drivers/mmc/core/sdio_irq.c
> @@ -214,7 +214,9 @@ static int sdio_card_irq_put(struct mmc_card *card)
> struct mmc_host *host = card->host;
>
> WARN_ON(!host->claimed);
> - BUG_ON(host->sdio_irqs < 1);
> +
> + if (host->sdio_irqs < 1)
> + return -EINVAL;
>
> if (!--host->sdio_irqs) {
> if (!(host->caps2 & MMC_CAP2_SDIO_IRQ_NOTHREAD)) {
> @@ -261,8 +263,8 @@ int sdio_claim_irq(struct sdio_func *func, sdio_irq_handler_t *handler)
> int ret;
> unsigned char reg;
>
> - BUG_ON(!func);
> - BUG_ON(!func->card);
> + if (!func)
> + return -EINVAL;
>
> pr_debug("SDIO: Enabling IRQ for %s...\n", sdio_func_id(func));
>
> @@ -304,8 +306,8 @@ int sdio_release_irq(struct sdio_func *func)
> int ret;
> unsigned char reg;
>
> - BUG_ON(!func);
> - BUG_ON(!func->card);
> + if (!func)
> + return -EINVAL;
>
> pr_debug("SDIO: Disabling IRQ for %s...\n", sdio_func_id(func));
>
> --
> 2.3.7
>
>
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-29 12:00 UTC (permalink / raw)
To: Ulf Hansson
Cc: Gregory CLEMENT, Adrian Hunter,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jason Cooper,
Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Thomas Petazzoni,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Jimmy Xu, Jisheng Zhang, Nadav Haklai, Ryan Gao, Doug Jones,
Victor Gu, Wei(SOCP) Liu, Wilson Ding
In-Reply-To: <CAPDyKFp=KHYogJE9WkJUYKphJhsrMfLjxxvNKmiAB+35bER4FQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Ulf,
On 2016/11/29 19:11, Ulf Hansson wrote:
> [...]
>
>>>>>
>>>>
>>>> Sorry that I didn't make myself clear.
>>>>
>>>> Our host PHY delay line consists of hundreds of sampling points.
>>>> Each sampling point represents a different phase shift.
>>>>
>>>> In lower speed mode, our host driver will scan the delay line.
>>>> It will select and test multiple sampling points, other than testing
>>>> only single sampling point.
>>>>
>>>> If a sampling point fails to transfer cmd/data, our host driver will
>>>> move to test next sampling point, until we find out a group of successful
>>>> sampling points which can transfer cmd/data. At last we will select
>>>> a perfect one from them.
>>>
>>> Ahh, I see. Unfortunate, this is going to be very hard to implement properly.
>>>
>>> The main problem is that the host driver has *no* knowledge about the
>>> internal state of the card, as that is the responsibility of the mmc
>>> core to keep track of.
>>>
>>> If the host driver would send a command during every update of the
>>> "ios" setting, from ->set_ios(), for sure it would lead to commands
>>> being sent that are "forbidden" in the current internal state of the
>>> card.
>>> This would lead to that the card initialization sequence fails,
>>> because the card may move to an unknown internal state and the mmc
>>> core would have no knowledge about what happened.
>>>
>>
>> Yes. In theory, host layer should not initiate a command by itself.
>>
>> We assume that bus is idle and card is stable in Tran state, when core layer
>> asks host to switch "ios".
>
> Understand, but this is a wrong assumption. The card may very well in
> another state than Tran state.
>
Could you please provide an example that card might not be in Tran state?
It seems that card should be in Tran state after CMD6 succeed.
If CMD6 fails, mmc driver will not execute ios setting. Thus ->set_ios()
will not be called.
>> Besides, we only select the commands which is valid in the whole procedure,
>> such as CMD8 for eMMC.
>> Those test commands are actually like read operations to card registers.
>> The card will return to Tran state even if transfer fails. It is also easy
>> for host to recover.
>
> For example, I would recommend you to investigate in detail the
> sequence for when a CMD6 command is sent to the card.
> The host must *not* start sending commands from ->set_ios() during a
> CMD6 sequence. For example a CMD8 is not allowed.
>
> Moreover, due to this, I wonder if it is even possible to get this HW
> to work properly.
>
In my very own opinion, ->set_ios() is only executed after CMD6 sequence
succeeds, based on current mmc.c/sd.c/sdio.c.
I personally think that it should not interfere CMD6 sequence.
I'm afraid that HW cannot help and SW driver has to take care of this.
>>
>>> Hmm..
>>>
>>> Can you specify, *exactly*, under which "ios updates" you need to
>>> verify updated PHY setting changes by sending a cmd/data? Also, please
>>> specify if it's enough to only test the CMD line or also DATA lines.
>>>
>>
>> When one of the three parameters in below changes, our host driver needs
>> to adjust PHY in lower speed mode.
>> 1. Speed Mode (timing): like legacy mode --> HS DDR
>> 2. Bus Clock: like 400KHz --> 50MHz
>> 3. Bus Width: like 1-bit --> 4-bit/8-bit
>>
>> For eMMC, we use CMD8 to test sampling point.
>> For SD, we use CMD13.
>> For SDIO, currently CMD52 is used to read a register from CCCR.
>> Those commands in above are all valid during the whole procedure to switch
>> to high speed mode from legacy mode.
>>
>> It is the best case if the test command can transfer both on CMD and DAT lines.
>> CMD8 for eMMC can test both CMD line and DAT lines. CMD13 and CMD52 only test
>> CMD line. We might use ACMD51 for SD and CMD53 for SDIO later thus DAT lines
>> are also under test.
>
> Thanks for sharing these details!
>
> So, if possible, I would recommend you to discuss these issues with
> some of the HW designers. Perhaps you can figure out an alternative
> method of confirming/testing PHY setting changes? Sending commands to
> the card just doesn't work well for all cases.
>
Thanks a lot for you patience.
Actually, we, including HW engineers, have been working on this for
a very long time. We also test a lot on many actual products. It is
quiet stable in real use scenarios.
I know it is still not good enough. It seems to be impossible to find
another practical and reliable solution, based on our tests.
Could you please provide some suggestions thus we can try our best to improve it
to meet your requirement?
Thank you.
Best regards,
Hu Ziji
> Kind regards
> Uffe
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V8 00/20] mmc: mmc: Add Software Command Queuing
From: Ulf Hansson @ 2016-11-29 11:41 UTC (permalink / raw)
To: Adrian Hunter
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala, Harjani Ritesh,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480414167-15577-1-git-send-email-adrian.hunter@intel.com>
On 29 November 2016 at 11:09, Adrian Hunter <adrian.hunter@intel.com> wrote:
> Hi
>
> Here is an updated version of the Software Command Queuing patches,
> re-based on next, with some changes - refer changes in V8 below.
> It would be good to move at least a few of these patches: for example,
> patches 2-7 could be considered to be tidy-ups. Patch 1 could be
> rolled into the Packed Commands removal patch.
Hi Adrian,
I decided to move a little further than patch 7, so I have queued up
patch 1 to and including patch 9 for next.
I didn't fold patch 1 into the "packed-command removal patch", but
just put it on top.
Thanks!
Kind regards
Uffe
>
> Performance results (not updated since V5):
>
> Results can vary from run to run, but here are some results showing 1, 2 or 4
> processes with 4k and 32k record sizes. They show up to 40% improvement in
> read performance when there are multiple processes.
>
> iozone -s 8192k -r 4k -i 0 -i 1 -i 2 -i 8 -I -t 1 -F /mnt/mmc/iozone1.tmp
>
> Children see throughput for 1 initial writers = 27909.87 kB/sec 24204.14 kB/sec -13.28 %
> Children see throughput for 1 rewriters = 28839.28 kB/sec 25531.92 kB/sec -11.47 %
> Children see throughput for 1 readers = 25889.65 kB/sec 24883.23 kB/sec -3.89 %
> Children see throughput for 1 re-readers = 25558.23 kB/sec 24679.89 kB/sec -3.44 %
> Children see throughput for 1 random readers = 25571.48 kB/sec 24689.52 kB/sec -3.45 %
> Children see throughput for 1 mixed workload = 25758.59 kB/sec 24487.52 kB/sec -4.93 %
> Children see throughput for 1 random writers = 24787.51 kB/sec 19368.99 kB/sec -21.86 %
>
> iozone -s 8192k -r 32k -i 0 -i 1 -i 2 -i 8 -I -t 1 -F /mnt/mmc/iozone1.tmp
>
> Children see throughput for 1 initial writers = 91344.61 kB/sec 102008.56 kB/sec 11.67 %
> Children see throughput for 1 rewriters = 87932.36 kB/sec 96630.44 kB/sec 9.89 %
> Children see throughput for 1 readers = 134879.82 kB/sec 110292.79 kB/sec -18.23 %
> Children see throughput for 1 re-readers = 147632.13 kB/sec 109053.33 kB/sec -26.13 %
> Children see throughput for 1 random readers = 93547.37 kB/sec 112225.50 kB/sec 19.97 %
> Children see throughput for 1 mixed workload = 93560.04 kB/sec 110515.21 kB/sec 18.12 %
> Children see throughput for 1 random writers = 92841.84 kB/sec 81153.81 kB/sec -12.59 %
>
> iozone -s 8192k -r 4k -i 0 -i 1 -i 2 -i 8 -I -t 2 -F /mnt/mmc/iozone1.tmp /mnt/mmc/iozone2.tmp
>
> Children see throughput for 2 initial writers = 31145.43 kB/sec 33771.25 kB/sec 8.43 %
> Children see throughput for 2 rewriters = 30592.57 kB/sec 35916.46 kB/sec 17.40 %
> Children see throughput for 2 readers = 31669.83 kB/sec 37460.13 kB/sec 18.28 %
> Children see throughput for 2 re-readers = 32079.94 kB/sec 37373.33 kB/sec 16.50 %
> Children see throughput for 2 random readers = 27731.19 kB/sec 37601.65 kB/sec 35.59 %
> Children see throughput for 2 mixed workload = 13927.50 kB/sec 14617.06 kB/sec 4.95 %
> Children see throughput for 2 random writers = 31250.00 kB/sec 33106.72 kB/sec 5.94 %
>
> iozone -s 8192k -r 32k -i 0 -i 1 -i 2 -i 8 -I -t 2 -F /mnt/mmc/iozone1.tmp /mnt/mmc/iozone2.tmp
>
> Children see throughput for 2 initial writers = 123255.84 kB/sec 131252.22 kB/sec 6.49 %
> Children see throughput for 2 rewriters = 115234.91 kB/sec 107225.74 kB/sec -6.95 %
> Children see throughput for 2 readers = 128921.86 kB/sec 148562.71 kB/sec 15.23 %
> Children see throughput for 2 re-readers = 127815.24 kB/sec 149304.32 kB/sec 16.81 %
> Children see throughput for 2 random readers = 125600.46 kB/sec 148406.56 kB/sec 18.16 %
> Children see throughput for 2 mixed workload = 44006.94 kB/sec 50937.36 kB/sec 15.75 %
> Children see throughput for 2 random writers = 120623.95 kB/sec 103969.05 kB/sec -13.81 %
>
> iozone -s 8192k -r 4k -i 0 -i 1 -i 2 -i 8 -I -t 4 -F /mnt/mmc/iozone1.tmp /mnt/mmc/iozone2.tmp /mnt/mmc/iozone3.tmp /mnt/mmc/iozone4.tmp
>
> Children see throughput for 4 initial writers = 24100.96 kB/sec 33336.58 kB/sec 38.32 %
> Children see throughput for 4 rewriters = 31650.20 kB/sec 33091.53 kB/sec 4.55 %
> Children see throughput for 4 readers = 33276.92 kB/sec 41799.89 kB/sec 25.61 %
> Children see throughput for 4 re-readers = 31786.96 kB/sec 41501.74 kB/sec 30.56 %
> Children see throughput for 4 random readers = 31991.65 kB/sec 40973.93 kB/sec 28.08 %
> Children see throughput for 4 mixed workload = 15804.80 kB/sec 13581.32 kB/sec -14.07 %
> Children see throughput for 4 random writers = 31231.42 kB/sec 34537.03 kB/sec 10.58 %
>
> iozone -s 8192k -r 32k -i 0 -i 1 -i 2 -i 8 -I -t 4 -F /mnt/mmc/iozone1.tmp /mnt/mmc/iozone2.tmp /mnt/mmc/iozone3.tmp /mnt/mmc/iozone4.tmp
>
> Children see throughput for 4 initial writers = 116567.42 kB/sec 119280.35 kB/sec 2.33 %
> Children see throughput for 4 rewriters = 115010.96 kB/sec 120864.34 kB/sec 5.09 %
> Children see throughput for 4 readers = 130700.29 kB/sec 177834.21 kB/sec 36.06 %
> Children see throughput for 4 re-readers = 125392.58 kB/sec 175975.28 kB/sec 40.34 %
> Children see throughput for 4 random readers = 132194.57 kB/sec 176630.46 kB/sec 33.61 %
> Children see throughput for 4 mixed workload = 56464.98 kB/sec 54140.61 kB/sec -4.12 %
> Children see throughput for 4 random writers = 109128.36 kB/sec 85359.80 kB/sec -21.78 %
>
>
> The current block driver supports 2 requests on the go at a time. Patches
> 3 - 8 make preparations for an arbitrary sized queue. Patches 9 - 12
> introduce Command Queue definitions and helpers. Patches 13 - 16
> complete the job of making the block driver use a queue. Patches 17 - 20
> finally add Software Command Queuing. Most of the Software Command Queuing
> functionality is added in patch 19.
>
> As noted below, the patches can also be found here:
>
> http://git.infradead.org/users/ahunter/linux-sdhci.git/shortlog/refs/heads/swcmdq
>
> which also includes a debug-only patch to help debug stuck queues:
> mmc: block: Add debugfs state file for debugging stuck queues
>
> Changes in V8:
>
> Re-based on next, dropping references to Packed Commands.
>
> mmc: block: Restore line inadvertently removed with packed commands
> New patch
>
> mmc: block: Fix 4K native sector check
> Moved to be the 2nd patch
> Added Reviewed-by: Linus Walleij
>
> mmc: queue: Fix queue thread wake-up
> Added Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Added Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
>
> mmc: queue: Factor out mmc_queue_alloc_bounce_bufs()
> Added Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Added Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
>
> mmc: queue: Factor out mmc_queue_alloc_bounce_sgs()
> Added Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Added Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
>
> mmc: queue: Factor out mmc_queue_alloc_sgs()
> Added Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Added Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
>
> mmc: queue: Factor out mmc_queue_reqs_free_bufs()
> Added Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Added Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
>
> mmc: queue: Introduce queue depth
> Drop chunk referring to mmc_packed_init().
> Combined into new patch "mmc: queue: Introduce queue depth and use it to allocate and free"
>
> mmc: queue: Use queue depth to allocate and free
> Combined into new patch "mmc: queue: Introduce queue depth and use it to allocate and free"
>
> mmc: queue: Allocate queue of size qdepth
> Combined into new patch "mmc: queue: Introduce queue depth and use it to allocate and free"
>
> mmc: queue: Introduce queue depth and use it to allocate and free
> New patch from combining 3 patches above.
>
> mmc: mmc: Add Command Queue definitions
> Add comment about excluding qdepths of 1 or 2.
>
> mmc: mmc: Add functions to enable / disable the Command Queue
> Change mmc_cmdq_switch() 'enable' parameter from 'int' to 'bool'.
>
> mmc: mmc_test: Disable Command Queue while mmc_test is used
> Added Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
>
> mmc: block: Disable Command Queue while RPMB is used
> As per Ritesh, assign 'ret' to 0 and return 'ret'.
> Added Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
>
> mmc: core: Do not prepare a new request twice
> Added Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
>
> mmc: core: Export mmc_retune_hold() and mmc_retune_release()
> Added Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
>
> mmc: block: Factor out mmc_blk_requeue()
> Dropped because it only affected "packed commands" code.
>
> mmc: block: Use local var for mqrq_cur
> Dropped chucks referring to Packed Commands.
> Added Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>
> mmc: block: Pass mqrq to mmc_blk_prep_packed_list()
> Dropped because it only affected "packed commands" code.
>
> mmc: block: Introduce queue semantics
> Add mmc_blk_requeue() lost from dropped patches.
>
> mmc: queue: Share mmc request array between partitions
> Dropped chucks referring to Packed Commands.
> Change mqrq_ref_cnt from 'int' to 'unsigned int'
> Add a comment about synchronisation of mqrq_ref_cnt.
>
> mmc: mmc: Enable Software Command Queuing
> Get rid of MMC_CAP_SWCMDQ and use MMC_CAP_CMD_DURING_TFR instead
>
> mmc: sdhci-pci: Enable Software Command Queuing for some Intel controllers
> Dropped because MMC_CAP_SWCMDQ removed.
>
> mmc: sdhci-acpi: Enable Software Command Queuing for some Intel controllers
> Dropped because MMC_CAP_SWCMDQ removed.
>
> Changes in V7:
>
> Re-based on next.
>
> mmc: mmc: Add Command Queue definitions
> Remove cmdq_en flag and add Linus Walleij's Reviewed-by.
>
> mmc: mmc: Add functions to enable / disable the Command
> Add cmdq_en flag.
>
> Changes in V6:
>
> mmc: core: Do not prepare a new request twice
> Ensure struct mmc_async_req is always initialized to zero
>
> Changes in V5:
>
> Patches 1-5 dropped because they have been applied.
>
> Re-based on next.
>
> Fixed use of blk_end_request_cur() when it should have been
> blk_end_request_all() to error out requests during error recovery.
>
> Fixed unpaired retune_hold / retune_release in the error recovery path.
>
> Changes in V4:
>
> Re-based on next + v4.8-rc2 + "block: Fix secure erase" patch
>
> Changes in V3:
>
> Patches 1-25 dropped because they have been applied.
>
> Re-based on next.
>
> mmc: queue: Allocate queue of size qdepth
> Free queue during cleanup
>
> mmc: mmc: Add Command Queue definitions
> Add cmdq_en to mmc-dev-attrs.txt documentation
>
> mmc: queue: Share mmc request array between partitions
> New patch
>
> Changes in V2:
>
> Added 5 patches already sent here:
>
> http://marc.info/?l=linux-mmc&m=146712062816835
>
> Added 3 more new patches:
>
> mmc: sdhci-pci: Do not runtime suspend at the end of sdhci_pci_probe()
> mmc: sdhci: Avoid STOP cmd triggering warning in sdhci_send_command()
> mmc: sdhci: sdhci_execute_tuning() must delete timer
>
> Carried forward the V2 fix to:
>
> mmc: mmc_test: Disable Command Queue while mmc_test is used
>
> Also reset the cmd circuit for data timeout if it is processing the data
> cmd, in patch:
>
> mmc: sdhci: Do not reset cmd or data circuits that are in use
>
> There wasn't much comment on the RFC so there have been few changes.
> Venu Byravarasu commented that it may be more efficient to use Software
> Command Queuing only when there is more than 1 request queued - it isn't
> obvious how well that would work in practice, but it could be added later
> if it could be shown to be beneficial.
>
> Original Cover Letter:
>
> Chuanxiao Dong sent some patches last year relating to eMMC 5.1 Software
> Command Queuing. He did not follow-up but I have contacted him and he says
> it is OK if I take over upstreaming the patches.
>
> eMMC Command Queuing is a feature added in version 5.1. The card maintains
> a queue of up to 32 data transfers. Commands CMD45/CMD45 are sent to queue
> up transfers in advance, and then one of the transfers is selected to
> "execute" by CMD46/CMD47 at which point data transfer actually begins.
>
> The advantage of command queuing is that the card can prepare for transfers
> in advance. That makes a big difference in the case of random reads because
> the card can start reading into its cache in advance.
>
> A v5.1 host controller can manage the command queue itself, but it is also
> possible for software to manage the queue using an non-v5.1 host controller
> - that is what Software Command Queuing is.
>
> Refer to the JEDEC (http://www.jedec.org/) eMMC v5.1 Specification for more
> information about Command Queuing.
>
> While these patches are heavily based on Dong's patches, there are some
> changes:
>
> SDHCI has been amended to support commands during transfer. That is a
> generic change added in patches 1 - 5. [Those patches have now been applied]
> In principle, that would also support SDIO's CMD52 during data transfer.
>
> The original approach added multiple commands into the same request for
> sending CMD44, CMD45 and CMD13. That is not strictly necessary and has
> been omitted for now.
>
> The original approach also called blk_end_request() from the mrq->done()
> function, which means the upper layers learnt of completed requests
> slightly earlier. That is not strictly related to Software Command Queuing
> and is something that could potentially be done for all data requests.
> That has been omitted for now.
>
> The current block driver supports 2 requests on the go at a time. Patches
> 1 - 8 make preparations for an arbitrary sized queue. Patches 9 - 12
> introduce Command Queue definitions and helpers. Patches 13 - 19
> complete the job of making the block driver use a queue. Patches 20 - 23
> finally add Software Command Queuing, and 24 - 25 enable it for Intel eMMC
> controllers. Most of the Software Command Queuing functionality is added
> in patch 22.
>
> The patches can also be found here:
>
> http://git.infradead.org/users/ahunter/linux-sdhci.git/shortlog/refs/heads/swcmdq
>
> The patches have only had basic testing so far. Ad-hoc testing shows a
> degradation in sequential read performance of about 10% but an increase in
> throughput for mixed workload of multiple processes of about 90%. The
> reduction in sequential performance is due to the need to read the Queue
> Status register between each transfer.
>
> These patches should not conflict with Hardware Command Queuing which
> handles the queue in a completely different way and thus does not need
> to share code with Software Command Queuing. The exceptions being the
> Command Queue definitions and queue allocation which should be able to be
> used.
>
>
> Adrian Hunter (20):
> mmc: block: Restore line inadvertently removed with packed commands
> mmc: block: Fix 4K native sector check
> mmc: queue: Fix queue thread wake-up
> mmc: queue: Factor out mmc_queue_alloc_bounce_bufs()
> mmc: queue: Factor out mmc_queue_alloc_bounce_sgs()
> mmc: queue: Factor out mmc_queue_alloc_sgs()
> mmc: queue: Factor out mmc_queue_reqs_free_bufs()
> mmc: queue: Introduce queue depth and use it to allocate and free
> mmc: mmc: Add Command Queue definitions
> mmc: mmc: Add functions to enable / disable the Command Queue
> mmc: mmc_test: Disable Command Queue while mmc_test is used
> mmc: block: Disable Command Queue while RPMB is used
> mmc: core: Do not prepare a new request twice
> mmc: core: Export mmc_retune_hold() and mmc_retune_release()
> mmc: block: Use local var for mqrq_cur
> mmc: block: Introduce queue semantics
> mmc: queue: Share mmc request array between partitions
> mmc: queue: Add a function to control wake-up on new requests
> mmc: block: Add Software Command Queuing
> mmc: mmc: Enable Software Command Queuing
>
> Documentation/mmc/mmc-dev-attrs.txt | 1 +
> drivers/mmc/card/block.c | 712 +++++++++++++++++++++++++++++++++---
> drivers/mmc/card/mmc_test.c | 21 +-
> drivers/mmc/card/queue.c | 328 +++++++++++------
> drivers/mmc/card/queue.h | 27 +-
> drivers/mmc/core/core.c | 18 +-
> drivers/mmc/core/host.c | 2 +
> drivers/mmc/core/host.h | 2 -
> drivers/mmc/core/mmc.c | 44 ++-
> drivers/mmc/core/mmc_ops.c | 28 ++
> include/linux/mmc/card.h | 8 +
> include/linux/mmc/core.h | 6 +
> include/linux/mmc/host.h | 3 +-
> include/linux/mmc/mmc.h | 17 +
> 14 files changed, 1035 insertions(+), 182 deletions(-)
>
>
> Regards
> Adrian
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ulf Hansson @ 2016-11-29 11:11 UTC (permalink / raw)
To: Ziji Hu
Cc: Gregory CLEMENT, Adrian Hunter, linux-mmc@vger.kernel.org,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree@vger.kernel.org, Thomas Petazzoni,
linux-arm-kernel@lists.infradead.org, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Victor Gu, Wei(SOCP) Liu,
Wilson Ding
In-Reply-To: <02725a0f-c061-e7f2-9a01-8975c62ab5a7@marvell.com>
[...]
>>>>
>>>
>>> Sorry that I didn't make myself clear.
>>>
>>> Our host PHY delay line consists of hundreds of sampling points.
>>> Each sampling point represents a different phase shift.
>>>
>>> In lower speed mode, our host driver will scan the delay line.
>>> It will select and test multiple sampling points, other than testing
>>> only single sampling point.
>>>
>>> If a sampling point fails to transfer cmd/data, our host driver will
>>> move to test next sampling point, until we find out a group of successful
>>> sampling points which can transfer cmd/data. At last we will select
>>> a perfect one from them.
>>
>> Ahh, I see. Unfortunate, this is going to be very hard to implement properly.
>>
>> The main problem is that the host driver has *no* knowledge about the
>> internal state of the card, as that is the responsibility of the mmc
>> core to keep track of.
>>
>> If the host driver would send a command during every update of the
>> "ios" setting, from ->set_ios(), for sure it would lead to commands
>> being sent that are "forbidden" in the current internal state of the
>> card.
>> This would lead to that the card initialization sequence fails,
>> because the card may move to an unknown internal state and the mmc
>> core would have no knowledge about what happened.
>>
>
> Yes. In theory, host layer should not initiate a command by itself.
>
> We assume that bus is idle and card is stable in Tran state, when core layer
> asks host to switch "ios".
Understand, but this is a wrong assumption. The card may very well in
another state than Tran state.
> Besides, we only select the commands which is valid in the whole procedure,
> such as CMD8 for eMMC.
> Those test commands are actually like read operations to card registers.
> The card will return to Tran state even if transfer fails. It is also easy
> for host to recover.
For example, I would recommend you to investigate in detail the
sequence for when a CMD6 command is sent to the card.
The host must *not* start sending commands from ->set_ios() during a
CMD6 sequence. For example a CMD8 is not allowed.
Moreover, due to this, I wonder if it is even possible to get this HW
to work properly.
>
>> Hmm..
>>
>> Can you specify, *exactly*, under which "ios updates" you need to
>> verify updated PHY setting changes by sending a cmd/data? Also, please
>> specify if it's enough to only test the CMD line or also DATA lines.
>>
>
> When one of the three parameters in below changes, our host driver needs
> to adjust PHY in lower speed mode.
> 1. Speed Mode (timing): like legacy mode --> HS DDR
> 2. Bus Clock: like 400KHz --> 50MHz
> 3. Bus Width: like 1-bit --> 4-bit/8-bit
>
> For eMMC, we use CMD8 to test sampling point.
> For SD, we use CMD13.
> For SDIO, currently CMD52 is used to read a register from CCCR.
> Those commands in above are all valid during the whole procedure to switch
> to high speed mode from legacy mode.
>
> It is the best case if the test command can transfer both on CMD and DAT lines.
> CMD8 for eMMC can test both CMD line and DAT lines. CMD13 and CMD52 only test
> CMD line. We might use ACMD51 for SD and CMD53 for SDIO later thus DAT lines
> are also under test.
Thanks for sharing these details!
So, if possible, I would recommend you to discuss these issues with
some of the HW designers. Perhaps you can figure out an alternative
method of confirming/testing PHY setting changes? Sending commands to
the card just doesn't work well for all cases.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 7/10] mmc: sdhci-xenon: Add support to PHYs of Marvell Xenon SDHC
From: Ziji Hu @ 2016-11-29 10:33 UTC (permalink / raw)
To: Ulf Hansson
Cc: Gregory CLEMENT, Adrian Hunter, linux-mmc@vger.kernel.org,
Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Rob Herring,
devicetree@vger.kernel.org, Thomas Petazzoni,
linux-arm-kernel@lists.infradead.org, Jimmy Xu, Jisheng Zhang,
Nadav Haklai, Ryan Gao, Doug Jones, Victor Gu, Wei(SOCP) Liu,
Wilson Ding
In-Reply-To: <CAPDyKFqjNy2oJiHHt+8CkTaiiExFKaR0i4GXGKdOMMRX-swPRg@mail.gmail.com>
Hi Ulf,
On 2016/11/29 15:49, Ulf Hansson wrote:
> On 29 November 2016 at 03:53, Ziji Hu <huziji@marvell.com> wrote:
>> Hi Ulf,
>>
>> On 2016/11/28 23:16, Ulf Hansson wrote:
>>> On 28 November 2016 at 12:38, Ziji Hu <huziji@marvell.com> wrote:
>>>> Hi Ulf,
>>>>
>>>> On 2016/11/28 19:13, Ulf Hansson wrote:
>>>>>>
>>>>>> As you suggest, I replace mmc_wait_for_cmd() with mmc_send_tuning(), to
>>>>>> send commands for testing current sampling point set in our host PHY.
>>>>>>
>>>>>> According to my test result, it shows that mmc_send_tuning() can only support
>>>>>> tuning command (CMD21/CMD19).
>>>>>> As a result, we cannot use mmc_send_tuning() when card is in the speed modes
>>>>>> which doesn't support tuning, such as eMMC HS SDR, eMMC HS DRR and
>>>>>> SD SDR 12/SDR25/DDR50. Card will not response to tuning commands in those
>>>>>> speed modes.
>>>>>>
>>>>>> Could you please provide suggestions for the speed mode in which tuning is
>>>>>> not available?
>>>>>>
>>>>>
>>>>> Normally the mmc host driver shouldn't have to care about what the
>>>>> card supports, as that is the responsibility of the mmc core to
>>>>> manage.
>>>>>
>>>>> The host should only need to implement the ->execute_tuning() ops,
>>>>> which gets called when the card supports tuning (CMD19/21). Does it
>>>>> make sense?
>>>>>
>>>> I think it is irrelevant to tuning procedure.
>>>>
>>>> Our host requires to adjust PHY setting after each time ios setting
>>>> (SDCLK/bus width/speed mode) is changed.
>>>> The simplified sequence is:
>>>> mmc change ios --> mmc_set_ios() --> ->set_ios() --> after sdhci_set_ios(),
>>>> adjust PHY setting.
>>>> During PHY setting adjustment, out host driver has to send commands to
>>>> test current sampling point. Tuning is another independent step.
>>>
>>> For those speed modes (or other ios changes) that *don't* requires
>>> tuning, then what will you do when you send the command to confirm the
>>> change of PHY setting and it fails?
>>>
>>> My assumption is that you will fail anyway, by propagating the error
>>> to the mmc core. At least that what was my understanding from your
>>> earlier replies, right!?
>>>
>>> Then, I think there are no point having the host driver sending a
>>> command to confirm the PHY settings, as the mmc core will anyway
>>> discover if something goes wrong when the next command is sent.
>>>
>>> Please correct me if I am wrong!
>>>
>>
>> Sorry that I didn't make myself clear.
>>
>> Our host PHY delay line consists of hundreds of sampling points.
>> Each sampling point represents a different phase shift.
>>
>> In lower speed mode, our host driver will scan the delay line.
>> It will select and test multiple sampling points, other than testing
>> only single sampling point.
>>
>> If a sampling point fails to transfer cmd/data, our host driver will
>> move to test next sampling point, until we find out a group of successful
>> sampling points which can transfer cmd/data. At last we will select
>> a perfect one from them.
>
> Ahh, I see. Unfortunate, this is going to be very hard to implement properly.
>
> The main problem is that the host driver has *no* knowledge about the
> internal state of the card, as that is the responsibility of the mmc
> core to keep track of.
>
> If the host driver would send a command during every update of the
> "ios" setting, from ->set_ios(), for sure it would lead to commands
> being sent that are "forbidden" in the current internal state of the
> card.
> This would lead to that the card initialization sequence fails,
> because the card may move to an unknown internal state and the mmc
> core would have no knowledge about what happened.
>
Yes. In theory, host layer should not initiate a command by itself.
We assume that bus is idle and card is stable in Tran state, when core layer
asks host to switch "ios".
Besides, we only select the commands which is valid in the whole procedure,
such as CMD8 for eMMC.
Those test commands are actually like read operations to card registers.
The card will return to Tran state even if transfer fails. It is also easy
for host to recover.
> Hmm..
>
> Can you specify, *exactly*, under which "ios updates" you need to
> verify updated PHY setting changes by sending a cmd/data? Also, please
> specify if it's enough to only test the CMD line or also DATA lines.
>
When one of the three parameters in below changes, our host driver needs
to adjust PHY in lower speed mode.
1. Speed Mode (timing): like legacy mode --> HS DDR
2. Bus Clock: like 400KHz --> 50MHz
3. Bus Width: like 1-bit --> 4-bit/8-bit
For eMMC, we use CMD8 to test sampling point.
For SD, we use CMD13.
For SDIO, currently CMD52 is used to read a register from CCCR.
Those commands in above are all valid during the whole procedure to switch
to high speed mode from legacy mode.
It is the best case if the test command can transfer both on CMD and DAT lines.
CMD8 for eMMC can test both CMD line and DAT lines. CMD13 and CMD52 only test
CMD line. We might use ACMD51 for SD and CMD53 for SDIO later thus DAT lines
are also under test.
> Kind regards
> Uffe
>
^ permalink raw reply
* Re: [PATCH V7 20/25] mmc: queue: Share mmc request array between partitions
From: Adrian Hunter @ 2016-11-29 10:14 UTC (permalink / raw)
To: Linus Walleij
Cc: Ulf Hansson, linux-mmc, Alex Lemberg, Mateusz Nowak,
Yuliy Izrailov, Jaehoon Chung, Dong Aisheng, Das Asutosh,
Zhangfei Gao, Dorfman Konstantin, David Griego, Sahitya Tummala,
Harjani Ritesh, Venu Byravarasu
In-Reply-To: <CACRpkdYfVUgh2kx97HKTcG9zcGX=3b9MQsUi-0MNj6KTkfmCCw@mail.gmail.com>
On 25/11/16 17:01, Linus Walleij wrote:
> On Fri, Nov 25, 2016 at 11:07 AM, Adrian Hunter <adrian.hunter@intel.com> wrote:
>
>> eMMC can have multiple internal partitions that are represented as separate
>> disks / queues. However the card has only 1 command queue which must be
>> empty when switching partitions. Consequently the array of mmc requests
>> that are queued can be shared between partitions saving memory.
>>
>> Keep a pointer to the mmc request queue on the card, and use that instead
>> of allocating a new one for each partition. Use a reference count to keep
>> track of when to free it.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>
> This is a good refactoring no matter how we proceed with command
> queueuing. Some comments.
>
>> @@ -1480,6 +1480,9 @@ static int mmc_packed_init(struct mmc_queue *mq, struct mmc_card *card)
>> if (mq->qdepth != 2)
>> return -EINVAL;
>>
>> + if (mqrq_cur->packed)
>> + goto out;
>
> Well packed command is gone so this goes away.
>
>> +++ b/drivers/mmc/card/queue.c
>> @@ -200,10 +200,17 @@ static struct mmc_queue_req *mmc_queue_alloc_mqrqs(struct mmc_queue *mq,
>> struct mmc_queue_req *mqrq;
>> int i;
>>
>> + if (mq->card->mqrq) {
>> + mq->card->mqrq_ref_cnt += 1;
>> + return mq->card->mqrq;
>> + }
>> +
>> mqrq = kcalloc(qdepth, sizeof(*mqrq), GFP_KERNEL);
>> if (mqrq) {
>> for (i = 0; i < mq->qdepth; i++)
>> mqrq[i].task_id = i;
>> + mq->card->mqrq = mqrq;
>> + mq->card->mqrq_ref_cnt = 1;
>> }
>
> OK
>
>> + if (mq->card->mqrq_ref_cnt > 1)
>> + return !!mq->mqrq[0].bounce_buf;
>
> Hm that seems inseparable from the other changes.
>
> Decrease of refcount seems correct.
>
>> + struct mmc_queue_req *mqrq; /* Shared queue structure */
>> + int mqrq_ref_cnt; /* Shared queue ref. count */
>
> I'm not smart enough to see if we're always increasing/decreasing
> this under a lock or otherwise exclusive context, or if it would be
> better to use an atomic type for counting, like kref does?
Queues are allocated and deallocated via mmc_blk_probe() and mmc_blk_probe()
so no other synchronization is needed for the reference count. I have
updated the commit message anf put a comment in the code to reflect that.
>
> Well maybe the whole thing could use kref I dunno.
>
> I guess it should be an unsigned int atleast.
Ok
^ permalink raw reply
* [PATCH V8 20/20] mmc: mmc: Enable Software Command Queuing
From: Adrian Hunter @ 2016-11-29 10:09 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala, Harjani Ritesh,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480414167-15577-1-git-send-email-adrian.hunter@intel.com>
Enable the Command Queue if the host controller supports Software Command
Queuing. It is not compatible with Packed Commands, so do not enable that
at the same time.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/core/mmc.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 720759a81296..7b1a92d79148 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1755,6 +1755,20 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
}
}
+ /* Enable Command Queue if supported */
+ card->ext_csd.cmdq_en = false;
+ if (card->ext_csd.cmdq_support && host->caps & MMC_CAP_CMD_DURING_TFR) {
+ err = mmc_cmdq_enable(card);
+ if (err && err != -EBADMSG)
+ goto free_card;
+ if (err) {
+ pr_warn("%s: Enabling CMDQ failed\n",
+ mmc_hostname(card->host));
+ card->ext_csd.cmdq_support = false;
+ card->ext_csd.cmdq_depth = 0;
+ err = 0;
+ }
+ }
/*
* In some cases (e.g. RPMB or mmc_test), the Command Queue must be
* disabled for a time, so a flag is needed to indicate to re-enable the
@@ -1768,7 +1782,8 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
*/
if (card->ext_csd.max_packed_writes >= 3 &&
card->ext_csd.max_packed_reads >= 5 &&
- host->caps2 & MMC_CAP2_PACKED_CMD) {
+ host->caps2 & MMC_CAP2_PACKED_CMD &&
+ !card->ext_csd.cmdq_en) {
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_EXP_EVENTS_CTRL,
EXT_CSD_PACKED_EVENT_EN,
--
1.9.1
^ permalink raw reply related
* [PATCH V8 19/20] mmc: block: Add Software Command Queuing
From: Adrian Hunter @ 2016-11-29 10:09 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala, Harjani Ritesh,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480414167-15577-1-git-send-email-adrian.hunter@intel.com>
eMMC Command Queuing is a feature added in version 5.1. The card maintains
a queue of up to 32 data transfers. Commands CMD44/CMD45 are sent to queue
up transfers in advance, and then one of the transfers is selected to
"execute" by CMD46/CMD47 at which point data transfer actually begins.
The advantage of command queuing is that the card can prepare for transfers
in advance. That makes a big difference in the case of random reads because
the card can start reading into its cache in advance.
A v5.1 host controller can manage the command queue itself, but it is also
possible for software to manage the queue using an non-v5.1 host controller
- that is what Software Command Queuing is.
Refer to the JEDEC (http://www.jedec.org/) eMMC v5.1 Specification for more
information about Command Queuing.
Two important aspects of Command Queuing that affect the implementation
are:
- only read/write requests are queued
- the queue must be empty to send other commands, including re-tuning
To support Software Command Queuing a separate function is provided to
issue read/write requests (i.e. mmc_swcmdq_issue_rw_rq()) and the
mmc_blk_request structure amended to cater for additional commands CMD44
and CMD45. There is a separate function (mmc_swcmdq_prep()) to prepare the
needed commands, but transfers are started by mmc_start_req() like normal.
mmc_swcmdq_issue_rw_rq() enqueues the new request and then executes tasks
until the queue is empty or mmc_swcmdq_execute() asks for a new request.
This puts mmc_swcmdq_execute() in control of the decision whether to queue
more requests or wait for the active one.
Recovery is invoked if anything goes wrong. Recovery has 2 options:
1. Discard the queue and re-queue all requests. If that fails, fall back
to option 2.
2. Reset and re-queue all requests. If that fails, error out all the
requests.
In either case, re-tuning will be done if needed after the queue becomes
empty because re-tuning is released at that point.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/card/block.c | 591 ++++++++++++++++++++++++++++++++++++++++++++++-
drivers/mmc/card/queue.c | 6 +-
drivers/mmc/card/queue.h | 11 +-
include/linux/mmc/core.h | 1 +
4 files changed, 606 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 6ead9f64cd1c..60193ff47926 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -108,6 +108,7 @@ struct mmc_blk_data {
#define MMC_BLK_WRITE BIT(1)
#define MMC_BLK_DISCARD BIT(2)
#define MMC_BLK_SECDISCARD BIT(3)
+#define MMC_BLK_SWCMDQ BIT(4)
/*
* Only set in main mmc_blk_data associated
@@ -1630,7 +1631,584 @@ static int mmc_blk_cmd_err(struct mmc_blk_data *md, struct mmc_card *card,
return ret;
}
-static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
+static enum mmc_blk_status mmc_swcmdq_err_check(struct mmc_card *card,
+ struct mmc_async_req *areq)
+{
+ struct mmc_queue_req *mqrq = container_of(areq, struct mmc_queue_req,
+ mmc_active);
+ struct mmc_blk_request *brq = &mqrq->brq;
+ struct request *req = mqrq->req;
+ int err;
+
+ err = brq->data.error;
+ /* In the case of data errors, send stop */
+ if (err)
+ mmc_wait_for_cmd(card->host, &brq->stop, 0);
+ else
+ err = brq->cmd.error;
+
+ /* In the case of CRC errors when re-tuning is needed, retry */
+ if (err == -EILSEQ && card->host->need_retune)
+ return MMC_BLK_RETRY;
+
+ /* For other errors abort */
+ if (err)
+ return MMC_BLK_ABORT;
+
+ if (blk_rq_bytes(req) != brq->data.bytes_xfered)
+ return MMC_BLK_PARTIAL;
+
+ return MMC_BLK_SUCCESS;
+}
+
+static void mmc_swcmdq_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq)
+{
+ struct mmc_blk_data *md = mq->blkdata;
+ struct mmc_card *card = md->queue.card;
+ struct mmc_blk_request *brq = &mqrq->brq;
+ struct request *req = mqrq->req;
+ bool do_data_tag;
+
+ /*
+ * Reliable writes are used to implement Forced Unit Access and
+ * are supported only on MMCs.
+ */
+ bool do_rel_wr = (req->cmd_flags & REQ_FUA) &&
+ (rq_data_dir(req) == WRITE) &&
+ (md->flags & MMC_BLK_REL_WR);
+
+ memset(brq, 0, sizeof(struct mmc_blk_request));
+ brq->mrq.cmd = &brq->cmd;
+ brq->mrq.data = &brq->data;
+ brq->mrq.cap_cmd_during_tfr = true;
+
+ if (rq_data_dir(req) == READ) {
+ brq->cmd.opcode = MMC_EXECUTE_READ_TASK;
+ brq->data.flags = MMC_DATA_READ;
+ brq->stop.flags = MMC_RSP_R1 | MMC_CMD_AC;
+ } else {
+ brq->cmd.opcode = MMC_EXECUTE_WRITE_TASK;
+ brq->data.flags = MMC_DATA_WRITE;
+ brq->stop.flags = MMC_RSP_R1B | MMC_CMD_AC;
+ }
+ brq->cmd.arg = mqrq->task_id << 16;
+ brq->cmd.flags = MMC_RSP_R1 | MMC_CMD_ADTC;
+
+ brq->data.blksz = 512;
+ brq->data.blocks = blk_rq_sectors(req);
+
+ brq->stop.opcode = MMC_STOP_TRANSMISSION;
+ brq->stop.arg = 0;
+
+ /*
+ * The block layer doesn't support all sector count
+ * restrictions, so we need to be prepared for too big
+ * requests.
+ */
+ if (brq->data.blocks > card->host->max_blk_count)
+ brq->data.blocks = card->host->max_blk_count;
+
+ if (do_rel_wr)
+ mmc_apply_rel_rw(brq, card, req);
+
+ /*
+ * Data tag is used only during writing meta data to speed
+ * up write and any subsequent read of this meta data
+ */
+ do_data_tag = (card->ext_csd.data_tag_unit_size) &&
+ (req->cmd_flags & REQ_META) &&
+ (rq_data_dir(req) == WRITE) &&
+ ((brq->data.blocks * brq->data.blksz) >=
+ card->ext_csd.data_tag_unit_size);
+
+ brq->cmd44.opcode = MMC_QUE_TASK_PARAMS;
+ brq->cmd44.arg = brq->data.blocks |
+ (do_rel_wr ? (1 << 31) : 0) |
+ ((rq_data_dir(req) == READ) ? (1 << 30) : 0) |
+ (do_data_tag ? (1 << 29) : 0) |
+ mqrq->task_id << 16;
+ brq->cmd44.flags = MMC_RSP_R1 | MMC_CMD_AC;
+
+ brq->cmd45.opcode = MMC_QUE_TASK_ADDR;
+ brq->cmd45.arg = blk_rq_pos(req);
+
+ mmc_set_data_timeout(&brq->data, card);
+
+ brq->data.sg = mqrq->sg;
+ brq->data.sg_len = mmc_queue_map_sg(mq, mqrq);
+
+ /*
+ * Adjust the sg list so it is the same size as the
+ * request.
+ */
+ if (brq->data.blocks != blk_rq_sectors(req)) {
+ int i, data_size = brq->data.blocks << 9;
+ struct scatterlist *sg;
+
+ for_each_sg(brq->data.sg, sg, brq->data.sg_len, i) {
+ data_size -= sg->length;
+ if (data_size <= 0) {
+ sg->length += data_size;
+ i++;
+ break;
+ }
+ }
+ brq->data.sg_len = i;
+ }
+
+ mqrq->mmc_active.mrq = &brq->mrq;
+ mqrq->mmc_active.err_check = mmc_swcmdq_err_check;
+
+ mmc_queue_bounce_pre(mqrq);
+}
+
+static int mmc_swcmdq_blk_err(struct mmc_card *card, int err)
+{
+ /* Re-try after CRC errors when re-tuning is needed */
+ if (err == -EILSEQ && card->host->need_retune)
+ return MMC_BLK_RETRY;
+
+ if (err)
+ return MMC_BLK_ABORT;
+
+ return 0;
+}
+
+#define SWCMDQ_ENQUEUE_ERR ( \
+ R1_OUT_OF_RANGE | \
+ R1_ADDRESS_ERROR | \
+ R1_BLOCK_LEN_ERROR | \
+ R1_WP_VIOLATION | \
+ R1_CC_ERROR | \
+ R1_ERROR | \
+ R1_COM_CRC_ERROR | \
+ R1_ILLEGAL_COMMAND)
+
+static int __mmc_swcmdq_enqueue(struct mmc_queue *mq,
+ struct mmc_queue_req *mqrq)
+{
+ struct mmc_card *card = mq->card;
+ int err;
+
+ mmc_swcmdq_prep(mq, mqrq);
+
+ err = mmc_wait_for_cmd(card->host, &mqrq->brq.cmd44, 0);
+ if (err)
+ goto out;
+
+ err = mmc_wait_for_cmd(card->host, &mqrq->brq.cmd45, 0);
+ if (err)
+ goto out;
+
+ /*
+ * Don't assume the task is queued if there are any error bits set in
+ * the response.
+ */
+ if (mqrq->brq.cmd45.resp[0] & SWCMDQ_ENQUEUE_ERR)
+ return MMC_BLK_ABORT;
+out:
+ return mmc_swcmdq_blk_err(card, err);
+}
+
+static int mmc_swcmdq_enqueue(struct mmc_queue *mq, struct request *req)
+{
+ struct mmc_queue_req *mqrq;
+
+ mqrq = mmc_queue_req_find(mq, req);
+ if (!mqrq) {
+ WARN_ON(1);
+ mmc_blk_requeue(mq->queue, req);
+ return 0;
+ }
+
+ /* Need to hold re-tuning so long as the queue is not empty */
+ if (mq->qcnt == 1)
+ mmc_retune_hold(mq->card->host);
+
+ return __mmc_swcmdq_enqueue(mq, mqrq);
+}
+
+static struct mmc_async_req *mmc_swcmdq_next(struct mmc_queue *mq)
+{
+ int i = __ffs(mq->qsr);
+
+ __clear_bit(i, &mq->qsr);
+
+ if (i >= mq->qdepth)
+ return NULL;
+
+ return &mq->mqrq[i].mmc_active;
+}
+
+static int mmc_get_qsr(struct mmc_card *card, u32 *qsr)
+{
+ struct mmc_command cmd = {0};
+ int err, retries = 3;
+
+ cmd.opcode = MMC_SEND_STATUS;
+ cmd.arg = card->rca << 16 | 1 << 15;
+ cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
+ err = mmc_wait_for_cmd(card->host, &cmd, retries);
+ if (err)
+ return err;
+
+ *qsr = cmd.resp[0];
+
+ return 0;
+}
+
+static int mmc_await_qsr(struct mmc_card *card, u32 *qsr)
+{
+ unsigned long timeout;
+ u32 status = 0;
+ int err;
+
+ timeout = jiffies + msecs_to_jiffies(10 * 1000);
+ while (!status) {
+ err = mmc_get_qsr(card, &status);
+ if (err)
+ return err;
+ if (time_after(jiffies, timeout)) {
+ pr_err("%s: Card stuck with queued tasks\n",
+ mmc_hostname(card->host));
+ return -ETIMEDOUT;
+ }
+ }
+
+ *qsr = status;
+
+ return 0;
+}
+
+static int mmc_swcmdq_await_qsr(struct mmc_queue *mq, struct mmc_card *card,
+ bool wait)
+{
+ struct mmc_queue_req *mqrq;
+ u32 qsr;
+ int err;
+
+ if (wait)
+ err = mmc_await_qsr(card, &qsr);
+ else
+ err = mmc_get_qsr(card, &qsr);
+ if (err)
+ goto out_err;
+
+ mq->qsr = qsr;
+
+ if (card->host->areq) {
+ /*
+ * The active request remains in the QSR until completed. Remove
+ * it so that mq->qsr only contains ones that are ready but not
+ * executed.
+ */
+ mqrq = container_of(card->host->areq, struct mmc_queue_req,
+ mmc_active);
+ __clear_bit(mqrq->task_id, &mq->qsr);
+ }
+
+ if (mq->qsr)
+ mq->qsr_err = false;
+out_err:
+ if (err) {
+ /* Don't repeatedly retry if no progress is made */
+ if (mq->qsr_err)
+ return MMC_BLK_ABORT;
+ mq->qsr_err = true;
+ }
+
+ return mmc_swcmdq_blk_err(card, err);
+}
+
+static int mmc_swcmdq_execute(struct mmc_queue *mq, bool flush, bool requeuing,
+ bool new_req)
+{
+ struct mmc_card *card = mq->card;
+ struct mmc_async_req *next = NULL, *prev;
+ struct mmc_blk_request *brq;
+ struct mmc_queue_req *mqrq;
+ enum mmc_blk_status status;
+ struct request *req;
+ int active = card->host->areq ? 1 : 0;
+ int ret;
+
+ if (mq->prepared_areq) {
+ /*
+ * A request that has been prepared before (i.e. passed to
+ * mmc_start_req()) but not started because another new request
+ * turned up.
+ */
+ next = mq->prepared_areq;
+ } else if (requeuing) {
+ /* Just finish the active request */
+ next = NULL;
+ } else if (mq->qsr) {
+ /* Get the next task from the Queue Status Register */
+ next = mmc_swcmdq_next(mq);
+ } else if (mq->qcnt > active) {
+ /*
+ * There are queued tasks so read the Queue Status Register to
+ * see if any are ready. Wait for a ready task only if there is
+ * no active request and no new request.
+ */
+ ret = mmc_swcmdq_await_qsr(mq, card, !active && !new_req);
+ if (ret)
+ return ret;
+ if (mq->qsr)
+ next = mmc_swcmdq_next(mq);
+ }
+
+ if (next) {
+ /*
+ * Don't wake for a new request when waiting for the active
+ * request if there is another request ready to start.
+ */
+ if (active)
+ mmc_queue_set_wake(mq, false);
+ } else {
+ if (!active)
+ return 0;
+ /*
+ * Don't wake for a new request when flushing or the queue is
+ * full.
+ */
+ if (flush || mq->qcnt == mq->qdepth)
+ mmc_queue_set_wake(mq, false);
+ else
+ mmc_queue_set_wake(mq, true);
+ }
+
+ prev = mmc_start_req(card->host, next, &status);
+
+ if (status == MMC_BLK_NEW_REQUEST) {
+ mq->prepared_areq = next;
+ return status;
+ }
+
+ mq->prepared_areq = NULL;
+
+ if (!prev)
+ return 0;
+
+ mqrq = container_of(prev, struct mmc_queue_req, mmc_active);
+ brq = &mqrq->brq;
+ req = mqrq->req;
+
+ mmc_queue_bounce_post(mqrq);
+
+ switch (status) {
+ case MMC_BLK_SUCCESS:
+ case MMC_BLK_PARTIAL:
+ case MMC_BLK_SUCCESS_ERR:
+ mmc_blk_reset_success(mq->blkdata, MMC_BLK_SWCMDQ);
+ ret = blk_end_request(req, 0, brq->data.bytes_xfered);
+ if (ret) {
+ if (!requeuing)
+ return __mmc_swcmdq_enqueue(mq, mqrq);
+ return 0;
+ }
+ break;
+ case MMC_BLK_NEW_REQUEST:
+ return status;
+ default:
+ if (mqrq->retry_cnt++) {
+ blk_end_request_all(req, -EIO);
+ break;
+ }
+ return status;
+ }
+
+ mmc_queue_req_free(mq, mqrq);
+
+ /* Release re-tuning when queue is empty */
+ if (!mq->qcnt)
+ mmc_retune_release(card->host);
+
+ return 0;
+}
+
+static enum mmc_blk_status mmc_swcmdq_requeue_check(struct mmc_card *card,
+ struct mmc_async_req *areq)
+{
+ enum mmc_blk_status ret = mmc_swcmdq_err_check(card, areq);
+
+ /*
+ * In the case of success, prevent mmc_start_req() from starting
+ * another request by returning MMC_BLK_SUCCESS_ERR.
+ */
+ return ret == MMC_BLK_SUCCESS ? MMC_BLK_SUCCESS_ERR : ret;
+}
+
+static int mmc_swcmdq_await_active(struct mmc_queue *mq)
+{
+ struct mmc_async_req *areq = mq->card->host->areq;
+ int err;
+
+ if (!areq)
+ return 0;
+
+ areq->err_check = mmc_swcmdq_requeue_check;
+
+ err = mmc_swcmdq_execute(mq, true, true, false);
+
+ /* The request will be requeued anyway, so ignore 'retry' */
+ if (err == MMC_BLK_RETRY)
+ err = 0;
+
+ return err;
+}
+
+static int mmc_swcmdq_discard_queue(struct mmc_queue *mq)
+{
+ struct mmc_command cmd = {0};
+
+ if (!mq->qcnt)
+ return 0;
+
+ mq->qsr = 0;
+
+ cmd.opcode = MMC_CMDQ_TASK_MGMT;
+ cmd.arg = 1; /* Discard entire queue */
+ cmd.flags = MMC_RSP_R1B | MMC_CMD_AC;
+ /* This is for recovery and the response is not needed, so ignore CRC */
+ cmd.flags &= ~MMC_RSP_CRC;
+
+ return mmc_wait_for_cmd(mq->card->host, &cmd, 0);
+}
+
+static int __mmc_swcmdq_requeue(struct mmc_queue *mq)
+{
+ unsigned long i, qslots = mq->qslots;
+ int err;
+
+ if (qslots) {
+ /* Cause re-tuning before next command, if needed */
+ mmc_retune_release(mq->card->host);
+ mmc_retune_hold(mq->card->host);
+ }
+
+ while (qslots) {
+ i = __ffs(qslots);
+ err = __mmc_swcmdq_enqueue(mq, &mq->mqrq[i]);
+ if (err)
+ return err;
+ __clear_bit(i, &qslots);
+ }
+
+ return 0;
+}
+
+static void __mmc_swcmdq_error_out(struct mmc_queue *mq)
+{
+ unsigned long i, qslots = mq->qslots;
+ struct request *req;
+
+ if (qslots)
+ mmc_retune_release(mq->card->host);
+
+ while (qslots) {
+ i = __ffs(qslots);
+ req = mq->mqrq[i].req;
+ blk_end_request_all(req, -EIO);
+ mq->mqrq[i].req = NULL;
+ __clear_bit(i, &qslots);
+ }
+
+ mq->qslots = 0;
+ mq->qcnt = 0;
+}
+
+static int mmc_swcmdq_requeue(struct mmc_queue *mq)
+{
+ int err;
+
+ /* Wait for active request */
+ err = mmc_swcmdq_await_active(mq);
+ if (err)
+ return err;
+
+ err = mmc_swcmdq_discard_queue(mq);
+ if (err)
+ return err;
+
+ return __mmc_swcmdq_requeue(mq);
+}
+
+static void mmc_swcmdq_reset(struct mmc_queue *mq)
+{
+ /* Wait for active request ignoring errors */
+ mmc_swcmdq_await_active(mq);
+
+ /* Ensure the queue is discarded */
+ mmc_swcmdq_discard_queue(mq);
+
+ /* Reset and requeue else error out all requests */
+ if (mmc_blk_reset(mq->blkdata, mq->card->host, MMC_BLK_SWCMDQ) ||
+ __mmc_swcmdq_requeue(mq))
+ __mmc_swcmdq_error_out(mq);
+}
+
+/*
+ * Recovery has 2 options:
+ * 1. Discard the queue and re-queue all requests. If that fails, fall back to
+ * option 2.
+ * 2. Reset and re-queue all requests. If that fails, error out all the
+ * requests.
+ * In either case, re-tuning will be done if needed after the queue becomes
+ * empty because re-tuning is released at that point.
+ */
+static void mmc_swcmdq_recovery(struct mmc_queue *mq, int err)
+{
+ switch (err) {
+ case MMC_BLK_RETRY:
+ err = mmc_swcmdq_requeue(mq);
+ if (!err)
+ break;
+ /* Fall through */
+ default:
+ mmc_swcmdq_reset(mq);
+ }
+}
+
+static int mmc_swcmdq_issue_rw_rq(struct mmc_queue *mq, struct request *req)
+{
+ struct mmc_context_info *cntx = &mq->card->host->context_info;
+ bool flush = !req && !cntx->is_waiting_last_req;
+ int err;
+
+ /* Enqueue new requests */
+ if (req) {
+ err = mmc_swcmdq_enqueue(mq, req);
+ if (err)
+ mmc_swcmdq_recovery(mq, err);
+ }
+
+ /*
+ * Keep executing queued requests until the queue is empty or
+ * mmc_swcmdq_execute() asks for new requests by returning
+ * MMC_BLK_NEW_REQUEST.
+ */
+ while (mq->qcnt) {
+ /*
+ * Re-tuning can only be done when the queue is empty. Recovery
+ * for MMC_BLK_RETRY will discard the queue and re-queue all
+ * requests. At the point the queue is empty, re-tuning is
+ * released and will be done automatically before the next
+ * mmc_request.
+ */
+ if (mq->card->host->need_retune)
+ mmc_swcmdq_recovery(mq, MMC_BLK_RETRY);
+ err = mmc_swcmdq_execute(mq, flush, false, !!req);
+ if (err == MMC_BLK_NEW_REQUEST)
+ return 0;
+ if (err)
+ mmc_swcmdq_recovery(mq, err);
+ }
+
+ return 0;
+}
+
+static int __mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
{
struct mmc_blk_data *md = mq->blkdata;
struct mmc_card *card = md->queue.card;
@@ -1802,6 +2380,17 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
return 0;
}
+static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *req)
+{
+ struct mmc_blk_data *md = mq->blkdata;
+ struct mmc_card *card = md->queue.card;
+
+ if (card->ext_csd.cmdq_en)
+ return mmc_swcmdq_issue_rw_rq(mq, req);
+ else
+ return __mmc_blk_issue_rw_rq(mq, req);
+}
+
int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
{
int ret;
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 3f9a229a480e..c55a64e5b76a 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -64,6 +64,7 @@ struct mmc_queue_req *mmc_queue_req_find(struct mmc_queue *mq,
mqrq->req = req;
mq->qcnt += 1;
__set_bit(mqrq->task_id, &mq->qslots);
+ mqrq->retry_cnt = 0;
return mqrq;
}
@@ -358,7 +359,10 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
if (!mq->queue)
return -ENOMEM;
- mq->qdepth = 2;
+ if (card->ext_csd.cmdq_en)
+ mq->qdepth = card->ext_csd.cmdq_depth;
+ else
+ mq->qdepth = 2;
mq->mqrq = mmc_queue_alloc_mqrqs(mq, mq->qdepth);
if (!mq->mqrq)
goto blk_cleanup;
diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
index 23e115f4c78d..6c00307ac159 100644
--- a/drivers/mmc/card/queue.h
+++ b/drivers/mmc/card/queue.h
@@ -15,9 +15,13 @@ static inline bool mmc_req_is_special(struct request *req)
struct mmc_blk_request {
struct mmc_request mrq;
- struct mmc_command sbc;
+ union {
+ struct mmc_command sbc;
+ struct mmc_command cmd44;
+ };
struct mmc_command cmd;
struct mmc_command stop;
+ struct mmc_command cmd45;
struct mmc_data data;
int retune_retry_done;
};
@@ -31,6 +35,7 @@ struct mmc_queue_req {
unsigned int bounce_sg_len;
struct mmc_async_req mmc_active;
int task_id;
+ unsigned int retry_cnt;
};
struct mmc_queue {
@@ -47,6 +52,10 @@ struct mmc_queue {
int qdepth;
int qcnt;
unsigned long qslots;
+ /* Following are defined for Software Command Queuing */
+ unsigned long qsr;
+ struct mmc_async_req *prepared_areq;
+ bool qsr_err;
};
extern int mmc_init_queue(struct mmc_queue *, struct mmc_card *, spinlock_t *,
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index d8f46e1ae7f2..03a013c83e31 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -25,6 +25,7 @@ enum mmc_blk_status {
MMC_BLK_ECC_ERR,
MMC_BLK_NOMEDIUM,
MMC_BLK_NEW_REQUEST,
+ MMC_BLK_SUCCESS_ERR, /* Success but prevent starting another request */
};
struct mmc_command {
--
1.9.1
^ permalink raw reply related
* [PATCH V8 18/20] mmc: queue: Add a function to control wake-up on new requests
From: Adrian Hunter @ 2016-11-29 10:09 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala, Harjani Ritesh,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480414167-15577-1-git-send-email-adrian.hunter@intel.com>
Add a function to control wake-up on new requests. This will enable
Software Command Queuing to choose whether or not to queue new
requests immediately or wait for the current task to complete.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/card/queue.c | 16 ++++++++++++++++
drivers/mmc/card/queue.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 63daf53b7246..3f9a229a480e 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -97,6 +97,7 @@ static int mmc_queue_thread(void *d)
cntx->is_waiting_last_req = false;
cntx->is_new_req = false;
if (!req) {
+ mq->is_new_req = false;
/*
* Dispatch queue is empty so set flags for
* mmc_request_fn() to wake us up.
@@ -147,6 +148,8 @@ static void mmc_request_fn(struct request_queue *q)
return;
}
+ mq->is_new_req = true;
+
cntx = &mq->card->host->context_info;
if (cntx->is_waiting_last_req) {
@@ -158,6 +161,19 @@ static void mmc_request_fn(struct request_queue *q)
wake_up_process(mq->thread);
}
+void mmc_queue_set_wake(struct mmc_queue *mq, bool wake_me)
+{
+ struct mmc_context_info *cntx = &mq->card->host->context_info;
+ struct request_queue *q = mq->queue;
+
+ if (cntx->is_waiting_last_req != wake_me) {
+ spin_lock_irq(q->queue_lock);
+ cntx->is_waiting_last_req = wake_me;
+ cntx->is_new_req = wake_me && mq->is_new_req;
+ spin_unlock_irq(q->queue_lock);
+ }
+}
+
static struct scatterlist *mmc_alloc_sg(int sg_len, int *err)
{
struct scatterlist *sg;
diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
index 95ca33098e1f..23e115f4c78d 100644
--- a/drivers/mmc/card/queue.h
+++ b/drivers/mmc/card/queue.h
@@ -40,6 +40,7 @@ struct mmc_queue {
unsigned int flags;
#define MMC_QUEUE_SUSPENDED (1 << 0)
bool asleep;
+ bool is_new_req;
struct mmc_blk_data *blkdata;
struct request_queue *queue;
struct mmc_queue_req *mqrq;
@@ -64,5 +65,6 @@ extern unsigned int mmc_queue_map_sg(struct mmc_queue *,
extern struct mmc_queue_req *mmc_queue_req_find(struct mmc_queue *,
struct request *);
extern void mmc_queue_req_free(struct mmc_queue *, struct mmc_queue_req *);
+extern void mmc_queue_set_wake(struct mmc_queue *, bool);
#endif
--
1.9.1
^ permalink raw reply related
* [PATCH V8 17/20] mmc: queue: Share mmc request array between partitions
From: Adrian Hunter @ 2016-11-29 10:09 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala, Harjani Ritesh,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480414167-15577-1-git-send-email-adrian.hunter@intel.com>
eMMC can have multiple internal partitions that are represented as separate
disks / queues. However the card has only 1 command queue which must be
empty when switching partitions. Consequently the array of mmc requests
that are queued can be shared between partitions saving memory.
Keep a pointer to the mmc request queue on the card, and use that instead
of allocating a new one for each partition. Use a reference count to keep
track of when to free it. Queues are allocated and deallocated via
mmc_blk_probe() and mmc_blk_probe() so no other synchronization is needed
for the reference count.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/card/queue.c | 42 ++++++++++++++++++++++++++++++++++++------
include/linux/mmc/card.h | 4 ++++
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 34e5be6b94ae..63daf53b7246 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -200,10 +200,22 @@ static struct mmc_queue_req *mmc_queue_alloc_mqrqs(struct mmc_queue *mq,
struct mmc_queue_req *mqrq;
int i;
+ if (mq->card->mqrq) {
+ /*
+ * Queues are allocated and deallocated via mmc_blk_probe() and
+ * mmc_blk_probe() so no other synchronization is needed for
+ * mqrq_ref_cnt.
+ */
+ mq->card->mqrq_ref_cnt += 1;
+ return mq->card->mqrq;
+ }
+
mqrq = kcalloc(qdepth, sizeof(*mqrq), GFP_KERNEL);
if (mqrq) {
for (i = 0; i < mq->qdepth; i++)
mqrq[i].task_id = i;
+ mq->card->mqrq = mqrq;
+ mq->card->mqrq_ref_cnt = 1;
}
return mqrq;
@@ -214,6 +226,9 @@ static bool mmc_queue_alloc_bounce_bufs(struct mmc_queue *mq,
{
int i;
+ if (mq->card->mqrq_ref_cnt > 1)
+ return !!mq->mqrq[0].bounce_buf;
+
for (i = 0; i < mq->qdepth; i++) {
mq->mqrq[i].bounce_buf = kmalloc(bouncesz, GFP_KERNEL);
if (!mq->mqrq[i].bounce_buf)
@@ -237,6 +252,9 @@ static int mmc_queue_alloc_bounce_sgs(struct mmc_queue *mq,
{
int i, ret;
+ if (mq->card->mqrq_ref_cnt > 1)
+ return 0;
+
for (i = 0; i < mq->qdepth; i++) {
mq->mqrq[i].sg = mmc_alloc_sg(1, &ret);
if (ret)
@@ -254,6 +272,9 @@ static int mmc_queue_alloc_sgs(struct mmc_queue *mq, int max_segs)
{
int i, ret;
+ if (mq->card->mqrq_ref_cnt > 1)
+ return 0;
+
for (i = 0; i < mq->qdepth; i++) {
mq->mqrq[i].sg = mmc_alloc_sg(max_segs, &ret);
if (ret)
@@ -283,6 +304,19 @@ static void mmc_queue_reqs_free_bufs(struct mmc_queue *mq)
mmc_queue_req_free_bufs(&mq->mqrq[i]);
}
+static void mmc_queue_free_mqrqs(struct mmc_queue *mq)
+{
+ if (!mq->mqrq)
+ return;
+
+ if (!--mq->card->mqrq_ref_cnt) {
+ mmc_queue_reqs_free_bufs(mq);
+ kfree(mq->card->mqrq);
+ mq->card->mqrq = NULL;
+ }
+ mq->mqrq = NULL;
+}
+
/**
* mmc_init_queue - initialise a queue structure.
* @mq: mmc queue
@@ -373,9 +407,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
return 0;
cleanup_queue:
- mmc_queue_reqs_free_bufs(mq);
- kfree(mq->mqrq);
- mq->mqrq = NULL;
+ mmc_queue_free_mqrqs(mq);
blk_cleanup:
blk_cleanup_queue(mq->queue);
return ret;
@@ -398,9 +430,7 @@ void mmc_cleanup_queue(struct mmc_queue *mq)
blk_start_queue(q);
spin_unlock_irqrestore(q->queue_lock, flags);
- mmc_queue_reqs_free_bufs(mq);
- kfree(mq->mqrq);
- mq->mqrq = NULL;
+ mmc_queue_free_mqrqs(mq);
mq->card = NULL;
}
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 5ac2243bc5a9..47399bcab659 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -207,6 +207,7 @@ struct sdio_cis {
struct mmc_ios;
struct sdio_func;
struct sdio_func_tuple;
+struct mmc_queue_req;
#define SDIO_MAX_FUNCS 7
@@ -308,6 +309,9 @@ struct mmc_card {
struct dentry *debugfs_root;
struct mmc_part part[MMC_NUM_PHY_PARTITION]; /* physical partitions */
unsigned int nr_parts;
+
+ struct mmc_queue_req *mqrq; /* Shared queue structure */
+ unsigned int mqrq_ref_cnt; /* Shared queue ref. count */
};
/*
--
1.9.1
^ permalink raw reply related
* [PATCH V8 16/20] mmc: block: Introduce queue semantics
From: Adrian Hunter @ 2016-11-29 10:09 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala, Harjani Ritesh,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480414167-15577-1-git-send-email-adrian.hunter@intel.com>
Change from viewing the requests in progress as 'current' and 'previous',
to viewing them as a queue. The current request is allocated to the first
free slot. The presence of incomplete requests is determined from the
count (mq->qcnt) of entries in the queue. Non-read-write requests (i.e.
discards and flushes) are not added to the queue at all and require no
special handling. Also no special handling is needed for the
MMC_BLK_NEW_REQUEST case.
As well as allowing an arbitrarily sized queue, the queue thread function
is significantly simpler.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/mmc/card/block.c | 49 +++++++++++++++++++------------
drivers/mmc/card/queue.c | 76 ++++++++++++++++++++++++++++++------------------
drivers/mmc/card/queue.h | 10 +++++--
3 files changed, 85 insertions(+), 50 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index fdeb3f335fd5..6ead9f64cd1c 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -129,6 +129,13 @@ 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 void mmc_blk_requeue(struct request_queue *q, struct request *req)
+{
+ spin_lock_irq(q->queue_lock);
+ blk_requeue_request(q, req);
+ spin_unlock_irq(q->queue_lock);
+}
+
static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
{
struct mmc_blk_data *md;
@@ -1630,12 +1637,21 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
struct mmc_blk_request *brq;
int ret = 1, disable_multi = 0, retry = 0, type, retune_retry_done = 0;
enum mmc_blk_status status;
- struct mmc_queue_req *mqrq_cur = mq->mqrq_cur;
+ struct mmc_queue_req *mqrq_cur = NULL;
struct mmc_queue_req *mq_rq;
struct request *req;
struct mmc_async_req *areq;
- if (!rqc && !mq->mqrq_prev->req)
+ if (rqc) {
+ mqrq_cur = mmc_queue_req_find(mq, rqc);
+ if (!mqrq_cur) {
+ WARN_ON(1);
+ mmc_blk_requeue(mq->queue, rqc);
+ rqc = NULL;
+ }
+ }
+
+ if (!mq->qcnt)
return 0;
do {
@@ -1659,11 +1675,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
} else
areq = NULL;
areq = mmc_start_req(card->host, areq, &status);
- if (!areq) {
- if (status == MMC_BLK_NEW_REQUEST)
- mq->flags |= MMC_QUEUE_NEW_REQUEST;
+ if (!areq)
return 0;
- }
mq_rq = container_of(areq, struct mmc_queue_req, mmc_active);
brq = &mq_rq->brq;
@@ -1760,6 +1773,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
}
} while (ret);
+ mmc_queue_req_free(mq, mq_rq);
+
return 1;
cmd_abort:
@@ -1774,6 +1789,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
if (mmc_card_removed(card)) {
rqc->cmd_flags |= REQ_QUIET;
blk_end_request_all(rqc, -EIO);
+ mmc_queue_req_free(mq, mqrq_cur);
} else {
mmc_blk_rw_rq_prep(mqrq_cur, card, 0, mq);
mmc_start_req(card->host,
@@ -1781,6 +1797,8 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
}
}
+ mmc_queue_req_free(mq, mq_rq);
+
return 0;
}
@@ -1789,9 +1807,8 @@ int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
int ret;
struct mmc_blk_data *md = mq->blkdata;
struct mmc_card *card = md->queue.card;
- bool req_is_special = mmc_req_is_special(req);
- if (req && !mq->mqrq_prev->req)
+ if (req && !mq->qcnt)
/* claim host only for the first request */
mmc_get_card(card);
@@ -1804,20 +1821,19 @@ int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
goto out;
}
- mq->flags &= ~MMC_QUEUE_NEW_REQUEST;
if (req && req_op(req) == REQ_OP_DISCARD) {
/* complete ongoing async transfer before issuing discard */
- if (card->host->areq)
+ if (mq->qcnt)
mmc_blk_issue_rw_rq(mq, NULL);
ret = mmc_blk_issue_discard_rq(mq, req);
} else if (req && req_op(req) == REQ_OP_SECURE_ERASE) {
/* complete ongoing async transfer before issuing secure erase*/
- if (card->host->areq)
+ if (mq->qcnt)
mmc_blk_issue_rw_rq(mq, NULL);
ret = mmc_blk_issue_secdiscard_rq(mq, req);
} else if (req && req_op(req) == REQ_OP_FLUSH) {
/* complete ongoing async transfer before issuing flush */
- if (card->host->areq)
+ if (mq->qcnt)
mmc_blk_issue_rw_rq(mq, NULL);
ret = mmc_blk_issue_flush(mq, req);
} else {
@@ -1825,13 +1841,8 @@ int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
}
out:
- if ((!req && !(mq->flags & MMC_QUEUE_NEW_REQUEST)) || req_is_special)
- /*
- * Release host when there are no more requests
- * and after special request(discard, flush) is done.
- * In case sepecial request, there is no reentry to
- * the 'mmc_blk_issue_rq' with 'mqrq_prev->req'.
- */
+ /* Release host when there are no more requests */
+ if (!mq->qcnt)
mmc_put_card(card);
return ret;
}
diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 48197545b539..34e5be6b94ae 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -49,6 +49,35 @@ static int mmc_prep_request(struct request_queue *q, struct request *req)
return BLKPREP_OK;
}
+struct mmc_queue_req *mmc_queue_req_find(struct mmc_queue *mq,
+ struct request *req)
+{
+ struct mmc_queue_req *mqrq;
+ int i = ffz(mq->qslots);
+
+ if (i >= mq->qdepth)
+ return NULL;
+
+ mqrq = &mq->mqrq[i];
+ WARN_ON(mqrq->req || mq->qcnt >= mq->qdepth ||
+ test_bit(mqrq->task_id, &mq->qslots));
+ mqrq->req = req;
+ mq->qcnt += 1;
+ __set_bit(mqrq->task_id, &mq->qslots);
+
+ return mqrq;
+}
+
+void mmc_queue_req_free(struct mmc_queue *mq,
+ struct mmc_queue_req *mqrq)
+{
+ WARN_ON(!mqrq->req || mq->qcnt < 1 ||
+ !test_bit(mqrq->task_id, &mq->qslots));
+ mqrq->req = NULL;
+ mq->qcnt -= 1;
+ __clear_bit(mqrq->task_id, &mq->qslots);
+}
+
static int mmc_queue_thread(void *d)
{
struct mmc_queue *mq = d;
@@ -59,7 +88,7 @@ static int mmc_queue_thread(void *d)
down(&mq->thread_sem);
do {
- struct request *req = NULL;
+ struct request *req;
spin_lock_irq(q->queue_lock);
set_current_state(TASK_INTERRUPTIBLE);
@@ -72,38 +101,17 @@ static int mmc_queue_thread(void *d)
* Dispatch queue is empty so set flags for
* mmc_request_fn() to wake us up.
*/
- if (mq->mqrq_prev->req)
+ if (mq->qcnt)
cntx->is_waiting_last_req = true;
else
mq->asleep = true;
}
- mq->mqrq_cur->req = req;
spin_unlock_irq(q->queue_lock);
- if (req || mq->mqrq_prev->req) {
- bool req_is_special = mmc_req_is_special(req);
-
+ if (req || mq->qcnt) {
set_current_state(TASK_RUNNING);
mmc_blk_issue_rq(mq, req);
cond_resched();
- if (mq->flags & MMC_QUEUE_NEW_REQUEST) {
- mq->flags &= ~MMC_QUEUE_NEW_REQUEST;
- continue; /* fetch again */
- }
-
- /*
- * Current request becomes previous request
- * and vice versa.
- * In case of special requests, current request
- * has been finished. Do not assign it to previous
- * request.
- */
- if (req_is_special)
- mq->mqrq_cur->req = NULL;
-
- mq->mqrq_prev->brq.mrq.data = NULL;
- mq->mqrq_prev->req = NULL;
- swap(mq->mqrq_prev, mq->mqrq_cur);
} else {
if (kthread_should_stop()) {
set_current_state(TASK_RUNNING);
@@ -186,6 +194,21 @@ static void mmc_queue_setup_discard(struct request_queue *q,
queue_flag_set_unlocked(QUEUE_FLAG_SECERASE, q);
}
+static struct mmc_queue_req *mmc_queue_alloc_mqrqs(struct mmc_queue *mq,
+ int qdepth)
+{
+ struct mmc_queue_req *mqrq;
+ int i;
+
+ mqrq = kcalloc(qdepth, sizeof(*mqrq), GFP_KERNEL);
+ if (mqrq) {
+ for (i = 0; i < mq->qdepth; i++)
+ mqrq[i].task_id = i;
+ }
+
+ return mqrq;
+}
+
static bool mmc_queue_alloc_bounce_bufs(struct mmc_queue *mq,
unsigned int bouncesz)
{
@@ -286,12 +309,9 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card,
return -ENOMEM;
mq->qdepth = 2;
- mq->mqrq = kcalloc(mq->qdepth, sizeof(struct mmc_queue_req),
- GFP_KERNEL);
+ mq->mqrq = mmc_queue_alloc_mqrqs(mq, mq->qdepth);
if (!mq->mqrq)
goto blk_cleanup;
- mq->mqrq_cur = &mq->mqrq[0];
- mq->mqrq_prev = &mq->mqrq[1];
mq->queue->queuedata = mq;
blk_queue_prep_rq(mq->queue, mmc_prep_request);
diff --git a/drivers/mmc/card/queue.h b/drivers/mmc/card/queue.h
index dac8c3d010dd..95ca33098e1f 100644
--- a/drivers/mmc/card/queue.h
+++ b/drivers/mmc/card/queue.h
@@ -30,6 +30,7 @@ struct mmc_queue_req {
struct scatterlist *bounce_sg;
unsigned int bounce_sg_len;
struct mmc_async_req mmc_active;
+ int task_id;
};
struct mmc_queue {
@@ -38,14 +39,13 @@ struct mmc_queue {
struct semaphore thread_sem;
unsigned int flags;
#define MMC_QUEUE_SUSPENDED (1 << 0)
-#define MMC_QUEUE_NEW_REQUEST (1 << 1)
bool asleep;
struct mmc_blk_data *blkdata;
struct request_queue *queue;
struct mmc_queue_req *mqrq;
- struct mmc_queue_req *mqrq_cur;
- struct mmc_queue_req *mqrq_prev;
int qdepth;
+ int qcnt;
+ unsigned long qslots;
};
extern int mmc_init_queue(struct mmc_queue *, struct mmc_card *, spinlock_t *,
@@ -61,4 +61,8 @@ extern unsigned int mmc_queue_map_sg(struct mmc_queue *,
extern int mmc_access_rpmb(struct mmc_queue *);
+extern struct mmc_queue_req *mmc_queue_req_find(struct mmc_queue *,
+ struct request *);
+extern void mmc_queue_req_free(struct mmc_queue *, struct mmc_queue_req *);
+
#endif
--
1.9.1
^ permalink raw reply related
* [PATCH V8 15/20] mmc: block: Use local var for mqrq_cur
From: Adrian Hunter @ 2016-11-29 10:09 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala, Harjani Ritesh,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480414167-15577-1-git-send-email-adrian.hunter@intel.com>
A subsequent patch will remove 'mq->mqrq_cur'. Prepare for that by
assigning it to a local variable.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/mmc/card/block.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index e84e2d8f6e31..fdeb3f335fd5 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -1630,6 +1630,7 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
struct mmc_blk_request *brq;
int ret = 1, disable_multi = 0, retry = 0, type, retune_retry_done = 0;
enum mmc_blk_status status;
+ struct mmc_queue_req *mqrq_cur = mq->mqrq_cur;
struct mmc_queue_req *mq_rq;
struct request *req;
struct mmc_async_req *areq;
@@ -1647,14 +1648,14 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
!IS_ALIGNED(blk_rq_sectors(rqc), 8)) {
pr_err("%s: Transfer size is not 4KB sector size aligned\n",
rqc->rq_disk->disk_name);
- mq_rq = mq->mqrq_cur;
+ mq_rq = mqrq_cur;
req = rqc;
rqc = NULL;
goto cmd_abort;
}
- mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
- areq = &mq->mqrq_cur->mmc_active;
+ mmc_blk_rw_rq_prep(mqrq_cur, card, 0, mq);
+ areq = &mqrq_cur->mmc_active;
} else
areq = NULL;
areq = mmc_start_req(card->host, areq, &status);
@@ -1774,9 +1775,9 @@ static int mmc_blk_issue_rw_rq(struct mmc_queue *mq, struct request *rqc)
rqc->cmd_flags |= REQ_QUIET;
blk_end_request_all(rqc, -EIO);
} else {
- mmc_blk_rw_rq_prep(mq->mqrq_cur, card, 0, mq);
+ mmc_blk_rw_rq_prep(mqrq_cur, card, 0, mq);
mmc_start_req(card->host,
- &mq->mqrq_cur->mmc_active, NULL);
+ &mqrq_cur->mmc_active, NULL);
}
}
--
1.9.1
^ permalink raw reply related
* [PATCH V8 14/20] mmc: core: Export mmc_retune_hold() and mmc_retune_release()
From: Adrian Hunter @ 2016-11-29 10:09 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala, Harjani Ritesh,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480414167-15577-1-git-send-email-adrian.hunter@intel.com>
Re-tuning can only be done when the Command Queue is empty, when means
holding and releasing re-tuning from the block driver, so export those
functions.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
---
drivers/mmc/core/host.c | 2 ++
drivers/mmc/core/host.h | 2 --
include/linux/mmc/core.h | 3 +++
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 98f25ffb4258..878405e28bdb 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -112,6 +112,7 @@ void mmc_retune_hold(struct mmc_host *host)
host->retune_now = 1;
host->hold_retune += 1;
}
+EXPORT_SYMBOL(mmc_retune_hold);
void mmc_retune_release(struct mmc_host *host)
{
@@ -120,6 +121,7 @@ void mmc_retune_release(struct mmc_host *host)
else
WARN_ON(1);
}
+EXPORT_SYMBOL(mmc_retune_release);
int mmc_retune(struct mmc_host *host)
{
diff --git a/drivers/mmc/core/host.h b/drivers/mmc/core/host.h
index 992bf5397633..0787b3002481 100644
--- a/drivers/mmc/core/host.h
+++ b/drivers/mmc/core/host.h
@@ -17,8 +17,6 @@
void mmc_retune_enable(struct mmc_host *host);
void mmc_retune_disable(struct mmc_host *host);
-void mmc_retune_hold(struct mmc_host *host);
-void mmc_retune_release(struct mmc_host *host);
int mmc_retune(struct mmc_host *host);
#endif
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index d045b06fc7ea..d8f46e1ae7f2 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -220,6 +220,9 @@ extern int mmc_set_blockcount(struct mmc_card *card, unsigned int blockcount,
extern int mmc_detect_card_removed(struct mmc_host *host);
+extern void mmc_retune_hold(struct mmc_host *host);
+extern void mmc_retune_release(struct mmc_host *host);
+
/**
* mmc_claim_host - exclusively claim a host
* @host: mmc host to claim
--
1.9.1
^ permalink raw reply related
* [PATCH V8 13/20] mmc: core: Do not prepare a new request twice
From: Adrian Hunter @ 2016-11-29 10:09 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala, Harjani Ritesh,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480414167-15577-1-git-send-email-adrian.hunter@intel.com>
mmc_start_req() assumes it is never called with the new request already
prepared. That is true if the queue consists of only 2 requests, but is not
true for a longer queue. e.g. mmc_start_req() has a current and previous
request but still exits to queue a new request if the queue size is
greater than 2. In that case, when mmc_start_req() is called again, the
current request will have been prepared already. Fix by flagging if the
request has been prepared.
That also means ensuring that struct mmc_async_req is always initialized
to zero, which wasn't the case in mmc_test.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
---
drivers/mmc/card/mmc_test.c | 8 ++++----
drivers/mmc/core/core.c | 12 +++++++++---
include/linux/mmc/host.h | 1 +
3 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/mmc/card/mmc_test.c b/drivers/mmc/card/mmc_test.c
index b42c23665104..81a71d28239a 100644
--- a/drivers/mmc/card/mmc_test.c
+++ b/drivers/mmc/card/mmc_test.c
@@ -826,7 +826,10 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
struct mmc_command stop2;
struct mmc_data data2;
- struct mmc_test_async_req test_areq[2];
+ struct mmc_test_async_req test_areq[2] = {
+ { .test = test },
+ { .test = test },
+ };
struct mmc_async_req *done_areq;
struct mmc_async_req *cur_areq = &test_areq[0].areq;
struct mmc_async_req *other_areq = &test_areq[1].areq;
@@ -834,9 +837,6 @@ static int mmc_test_nonblock_transfer(struct mmc_test_card *test,
int i;
int ret = RESULT_OK;
- test_areq[0].test = test;
- test_areq[1].test = test;
-
mmc_test_nonblock_reset(&mrq1, &cmd1, &stop1, &data1);
mmc_test_nonblock_reset(&mrq2, &cmd2, &stop2, &data2);
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index dc1f27ee50b8..28e1495ac903 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -658,8 +658,10 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
struct mmc_async_req *data = host->areq;
/* Prepare a new request */
- if (areq)
+ if (areq && !areq->pre_req_done) {
+ areq->pre_req_done = true;
mmc_pre_req(host, areq->mrq);
+ }
if (host->areq) {
status = mmc_wait_for_data_req_done(host, host->areq->mrq, areq);
@@ -695,12 +697,16 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
if (status == MMC_BLK_SUCCESS && areq)
start_err = __mmc_start_data_req(host, areq->mrq);
- if (host->areq)
+ if (host->areq) {
+ host->areq->pre_req_done = false;
mmc_post_req(host, host->areq->mrq, 0);
+ }
/* Cancel a prepared request if it was not started. */
- if ((status != MMC_BLK_SUCCESS || start_err) && areq)
+ if ((status != MMC_BLK_SUCCESS || start_err) && areq) {
+ areq->pre_req_done = false;
mmc_post_req(host, areq->mrq, -EINVAL);
+ }
if (status != MMC_BLK_SUCCESS)
host->areq = NULL;
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 8bc884121465..69f796d21c76 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -173,6 +173,7 @@ struct mmc_async_req {
* Returns 0 if success otherwise non zero.
*/
enum mmc_blk_status (*err_check)(struct mmc_card *, struct mmc_async_req *);
+ bool pre_req_done;
};
/**
--
1.9.1
^ permalink raw reply related
* [PATCH V8 12/20] mmc: block: Disable Command Queue while RPMB is used
From: Adrian Hunter @ 2016-11-29 10:09 UTC (permalink / raw)
To: Ulf Hansson
Cc: linux-mmc, Alex Lemberg, Mateusz Nowak, Yuliy Izrailov,
Jaehoon Chung, Dong Aisheng, Das Asutosh, Zhangfei Gao,
Dorfman Konstantin, David Griego, Sahitya Tummala, Harjani Ritesh,
Venu Byravarasu, Linus Walleij
In-Reply-To: <1480414167-15577-1-git-send-email-adrian.hunter@intel.com>
RPMB does not allow Command Queue commands. Disable and re-enable the
Command Queue when switching.
Note that the driver only switches partitions when the queue is empty.
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Harjani Ritesh <riteshh@codeaurora.org>
---
drivers/mmc/card/block.c | 46 ++++++++++++++++++++++++++++++++++++++--------
1 file changed, 38 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 646d1a1fa6ca..e84e2d8f6e31 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -725,10 +725,41 @@ static int mmc_blk_compat_ioctl(struct block_device *bdev, fmode_t mode,
#endif
};
+static int mmc_blk_part_switch_pre(struct mmc_card *card,
+ unsigned int part_type)
+{
+ int ret = 0;
+
+ if (part_type == EXT_CSD_PART_CONFIG_ACC_RPMB) {
+ if (card->ext_csd.cmdq_en) {
+ ret = mmc_cmdq_disable(card);
+ if (ret)
+ return ret;
+ }
+ mmc_retune_pause(card->host);
+ }
+
+ return ret;
+}
+
+static int mmc_blk_part_switch_post(struct mmc_card *card,
+ unsigned int part_type)
+{
+ int ret = 0;
+
+ if (part_type == EXT_CSD_PART_CONFIG_ACC_RPMB) {
+ mmc_retune_unpause(card->host);
+ if (card->reenable_cmdq && !card->ext_csd.cmdq_en)
+ ret = mmc_cmdq_enable(card);
+ }
+
+ return ret;
+}
+
static inline int mmc_blk_part_switch(struct mmc_card *card,
struct mmc_blk_data *md)
{
- int ret;
+ int ret = 0;
struct mmc_blk_data *main_md = dev_get_drvdata(&card->dev);
if (main_md->part_curr == md->part_type)
@@ -737,8 +768,9 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
if (mmc_card_mmc(card)) {
u8 part_config = card->ext_csd.part_config;
- if (md->part_type == EXT_CSD_PART_CONFIG_ACC_RPMB)
- mmc_retune_pause(card->host);
+ ret = mmc_blk_part_switch_pre(card, md->part_type);
+ if (ret)
+ return ret;
part_config &= ~EXT_CSD_PART_CONFIG_ACC_MASK;
part_config |= md->part_type;
@@ -747,19 +779,17 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
EXT_CSD_PART_CONFIG, part_config,
card->ext_csd.part_time);
if (ret) {
- if (md->part_type == EXT_CSD_PART_CONFIG_ACC_RPMB)
- mmc_retune_unpause(card->host);
+ mmc_blk_part_switch_post(card, md->part_type);
return ret;
}
card->ext_csd.part_config = part_config;
- if (main_md->part_curr == EXT_CSD_PART_CONFIG_ACC_RPMB)
- mmc_retune_unpause(card->host);
+ ret = mmc_blk_part_switch_post(card, main_md->part_curr);
}
main_md->part_curr = md->part_type;
- return 0;
+ return ret;
}
static u32 mmc_sd_num_wr_blocks(struct mmc_card *card)
--
1.9.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox