Hi Shawn,
Is the intention in this patch to stop using MANUAL_BKOPS for all eMMC5.1+
devices, and only use AUTO_BKOPS?
Please see several questions inline.
On 6/6/16, 6:07 AM, "linux-mmc-ow...@vger.kernel.org on behalf of Shawn Lin"
<linux-mmc-ow...@vger.kernel.org on behalf of shawn....@rock-chips.com> wrote:
>JEDEC eMMC v5.1 introduce an autonomously initiated method
>for background operations.
>
>Host that wants to enable the device to perform background
>operations during device idle time, should signal the device
>by setting AUTO_EN in BKOPS_EN field EXT_CSD[163] to 1b. When
>this bit is set, the device may start or stop background operations
>whenever it sees fit, without any notification to the host.
>
>When AUTO_EN bit is set, the host should keep the device power
>active. The host may set or clear this bit at any time based on
>its power constraints or other considerations.
>
>Currently the manual bkops is only be used under the async req
>circumstances and it's a bit complicated to be controlled as the
>perfect method is that we should do some idle monitor just as rpm
>and send HPI each time if receiving rd/wr req. But it will impact
>performance significantly, especially for random iops since the
>weight of executing HPI against r/w small piece of LBAs is
>nonnegligible.
>
>So we now prefer to select the auto one unconditionally if supported
>which makes it as simple as possible. It should really good enough
>for devices to manage its internal policy for bkops rather than the
>host, which makes us believe that we could achieve the best
>performance for all the devices implementing auto bkops and the only
>thing we should do is to disable it when cutting off the power.
>
>Signed-off-by: Shawn Lin <shawn....@rock-chips.com>
>---
>
> drivers/mmc/core/core.c | 127 +++++++++++++++++++++++++++++++++++++----------
> drivers/mmc/core/mmc.c | 30 ++++++++++-
> include/linux/mmc/card.h | 1 +
> include/linux/mmc/core.h | 4 +-
> include/linux/mmc/mmc.h | 1 +
> 5 files changed, 133 insertions(+), 30 deletions(-)
>
>diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>index e864187..c2e5a66 100644
>--- a/drivers/mmc/core/core.c
>+++ b/drivers/mmc/core/core.c
>@@ -297,24 +297,13 @@ static int mmc_start_request(struct mmc_host *host,
>struct mmc_request *mrq)
> return 0;
> }
>
>-/**
>- * mmc_start_bkops - start BKOPS for supported cards
>- * @card: MMC card to start BKOPS
>- * @form_exception: A flag to indicate if this function was
>- * called due to an exception raised by the card
>- *
>- * Start background operations whenever requested.
>- * When the urgent BKOPS bit is set in a R1 command response
>- * then background operations should be started immediately.
>-*/
>-void mmc_start_bkops(struct mmc_card *card, bool from_exception)
>+static void mmc_start_man_bkops(struct mmc_card *card,
>+ bool from_exception)
> {
> int err;
> int timeout;
> bool use_busy_signal;
>
>- BUG_ON(!card);
>-
> if (!card->ext_csd.man_bkops_en || mmc_card_doing_bkops(card))
> return;
>
>@@ -347,7 +336,7 @@ void mmc_start_bkops(struct mmc_card *card, bool
>from_exception)
> EXT_CSD_BKOPS_START, 1, timeout,
> use_busy_signal, true, false);
> if (err) {
>- pr_warn("%s: Error %d starting bkops\n",
>+ pr_warn("%s: Error %d starting manual bkops\n",
> mmc_hostname(card->host), err);
> mmc_retune_release(card->host);
> goto out;
>@@ -365,6 +354,55 @@ void mmc_start_bkops(struct mmc_card *card, bool
>from_exception)
> out:
> mmc_release_host(card->host);
> }
>+
>+static void mmc_start_auto_bkops(struct mmc_card *card)
>+{
>+ int err;
>+
>+ /* If it's already enable auto_bkops, nothing to do */
>+ if (card->ext_csd.auto_bkops_en)
>+ return;
>+
>+ mmc_claim_host(card->host);
>+
>+ mmc_retune_hold(card->host);
>+
>+ err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>+ EXT_CSD_BKOPS_EN, 2,
>+ card->ext_csd.generic_cmd6_time);
>+ if (err)
>+ pr_warn("%s: Error %d starting auto bkops\n",
>+ mmc_hostname(card->host), err);
>+
>+ card->ext_csd.auto_bkops_en = true;
>+ mmc_card_set_doing_bkops(card);
>+ mmc_retune_release(card->host);
>+ mmc_release_host(card->host);
>+}
>+
>+
>+/**
>+ * mmc_start_bkops - start BKOPS for supported cards
>+ * @card: MMC card to start BKOPS
>+ * @form_exception: A flag to indicate if this function was
>+ * called due to an exception raised by the card
>+ * @is_auto: A flag to indicate if we should use auto bkops
>+ *
>+ * Start background operations whenever requested.
>+ * When the urgent BKOPS bit is set in a R1 command response
>+ * then background operations should be started immediately, which is
>+ * only needed for man_bkops.
>+*/
>+void mmc_start_bkops(struct mmc_card *card, bool from_exception,
>+ bool is_auto)
>+{
>+ BUG_ON(!card);
>+
>+ if (is_auto)
As far as I understand, the AUTO_BKOPS is enabled during “mmc_init_card()”.
If its correct, why the “mmc_start_auto_bkops()” function is needed?
>+ return mmc_start_auto_bkops(card);
>+ else
>+ return mmc_start_man_bkops(card, from_exception);
>+}
> EXPORT_SYMBOL(mmc_start_bkops);
>
> /*
>@@ -610,7 +648,9 @@ struct mmc_async_req *mmc_start_req(struct mmc_host *host,
> if (areq)
> mmc_post_req(host, areq->mrq, -EINVAL);
>
>- mmc_start_bkops(host->card, true);
>+ /* Prefer to use auto bkops for eMMC 5.1 or later */
If the decision is to use AUTO_BKOPS for all eMMC5.1+ devices, why you need to
enable AUTO_BKOPS bit
on every R1_EXCEPTION_EVENT?
AUTO_BKOPS should be enabled only once...
>+ mmc_start_bkops(host->card, true,
>+ host->card->ext_csd.rev >= 8);
>
> /* prepare the request again */
> if (areq)
>@@ -751,20 +791,10 @@ int mmc_wait_for_cmd(struct mmc_host *host, struct
>mmc_command *cmd, int retries
>
> EXPORT_SYMBOL(mmc_wait_for_cmd);
>
>-/**
>- * mmc_stop_bkops - stop ongoing BKOPS
>- * @card: MMC card to check BKOPS
>- *
>- * Send HPI command to stop ongoing background operations to
>- * allow rapid servicing of foreground operations, e.g. read/
>- * writes. Wait until the card comes out of the programming state
>- * to avoid errors in servicing read/write requests.
>- */
>-int mmc_stop_bkops(struct mmc_card *card)
>+static int mmc_stop_man_bkops(struct mmc_card *card)
> {
> int err = 0;
>
>- BUG_ON(!card);
> err = mmc_interrupt_hpi(card);
>
> /*
>@@ -779,6 +809,51 @@ int mmc_stop_bkops(struct mmc_card *card)
>
> return err;
> }
>+
>+static int mmc_stop_auto_bkops(struct mmc_card *card)
>+{
>+ int err = 0;
>+
>+ if (!card->ext_csd.auto_bkops_en)
>+ return 0;
>+
Shouldn’t the BKOPS_STATUS be checked prior to disabling the BKOPS activity of
the device?
>+ err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>+ EXT_CSD_BKOPS_EN, 0, MMC_BKOPS_MAX_TIMEOUT,
>+ true, true, false);
>+ if (err)
>+ pr_warn("%s: Error %d stoping auto bkops\n",
>+ mmc_hostname(card->host), err);
>+
>+ card->ext_csd.auto_bkops_en = false;
>+ mmc_card_clr_doing_bkops(card);
>+ mmc_retune_release(card->host);
>+
>+ return err;
>+}
>+
>+/**
>+ * mmc_stop_bkops - stop ongoing BKOPS
>+ * @card: MMC card to check BKOPS
>+ * @is_auto: A flag to indicate if we should use auto bkops
>+ *
>+ * Send HPI command to stop ongoing background operations to
>+ * allow rapid servicing of foreground operations, e.g. read/
>+ * writes. Wait until the card comes out of the programming state
>+ * to avoid errors in servicing read/write requests.
>+ *
>+ * But for auto bkops, we could only disable AUTO_EN instead of
>+ * sending HPI. And the firmware could do it automatically.
>+ *
>+ */
>+int mmc_stop_bkops(struct mmc_card *card, bool is_auto)
>+{
>+ BUG_ON(!card);
>+
>+ if (is_auto)
>+ return mmc_stop_auto_bkops(card);
>+ else
>+ return mmc_stop_man_bkops(card);
>+}
> EXPORT_SYMBOL(mmc_stop_bkops);
>
> int mmc_read_bkops_status(struct mmc_card *card)
>diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>index 61d6b34..c65bea7 100644
>--- a/drivers/mmc/core/mmc.c
>+++ b/drivers/mmc/core/mmc.c
>@@ -508,6 +508,13 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8
>*ext_csd)
> /* check whether the eMMC card supports BKOPS */
> if (ext_csd[EXT_CSD_BKOPS_SUPPORT] & 0x1) {
> card->ext_csd.bkops = 1;
>+
>+ /* for eMMC v5.1 or later, we could use auto_bkops */
>+ if (card->ext_csd.rev >= 8)
>+ card->ext_csd.auto_bkops_en =
>+ (ext_csd[EXT_CSD_BKOPS_EN] &
>+ EXT_CSD_AUTO_BKOPS_MASK);
>+
> card->ext_csd.man_bkops_en =
> (ext_csd[EXT_CSD_BKOPS_EN] &
> EXT_CSD_MANUAL_BKOPS_MASK);
>@@ -516,6 +523,9 @@ static int mmc_decode_ext_csd(struct mmc_card *card, u8
>*ext_csd)
> if (!card->ext_csd.man_bkops_en)
> pr_debug("%s: MAN_BKOPS_EN bit is not set\n",
> mmc_hostname(card->host));
>+ if (!card->ext_csd.auto_bkops_en)
>+ pr_debug("%s: AUTO_BKOPS_EN bit is not set\n",
>+ mmc_hostname(card->host));
> }
>
> /* check whether the eMMC card supports HPI */
>@@ -1241,7 +1251,7 @@ static int mmc_select_hs400es(struct mmc_card *card)
> }
>
> err = mmc_select_bus_width(card);
>- if (IS_ERR_VALUE(err))
>+ if (err < 0)
> goto out_err;
>
> /* Switch card to HS mode */
>@@ -1676,6 +1686,21 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> mmc_select_powerclass(card);
>
> /*
>+ * Enable auto bkops feature which is mandatory for
>+ * eMMC v5.1 or later
>+ */
>+ if (card->ext_csd.rev >= 8) {
>+ err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>+ EXT_CSD_BKOPS_EN, 2,
>+ card->ext_csd.generic_cmd6_time);
>+ if (err)
>+ pr_warn("%s: Error %d starting auto bkops\n",
>+ mmc_hostname(card->host), err);
>+
>+ card->ext_csd.auto_bkops_en = true;
>+ }
>+
>+ /*
> * Enable HPI feature (if supported)
> */
> if (card->ext_csd.hpi) {
>@@ -1898,7 +1923,8 @@ static int _mmc_suspend(struct mmc_host *host, bool
>is_suspend)
> goto out;
>
> if (mmc_card_doing_bkops(host->card)) {
>- err = mmc_stop_bkops(host->card);
>+ err = mmc_stop_bkops(host->card,
>+ host->card->ext_csd.rev >= 8);
> if (err)
> goto out;
> }
>diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
>index 22defc2..f82f9be 100644
>--- a/include/linux/mmc/card.h
>+++ b/include/linux/mmc/card.h
>@@ -84,6 +84,7 @@ struct mmc_ext_csd {
> unsigned int hpi_cmd; /* cmd used as HPI */
> bool bkops; /* background support bit */
> bool man_bkops_en; /* manual bkops enable bit */
>+ bool auto_bkops_en; /* auto bkops enbale bit */
> unsigned int data_sector_size; /* 512 bytes or 4KB */
> unsigned int data_tag_unit_size; /* DATA TAG UNIT size */
> unsigned int boot_ro_lock; /* ro lock support */
>diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
>index b01e77d..45d53ef 100644
>--- a/include/linux/mmc/core.h
>+++ b/include/linux/mmc/core.h
>@@ -140,7 +140,7 @@ struct mmc_request {
> struct mmc_card;
> struct mmc_async_req;
>
>-extern int mmc_stop_bkops(struct mmc_card *);
>+extern int mmc_stop_bkops(struct mmc_card *, bool);
> extern int mmc_read_bkops_status(struct mmc_card *);
> extern struct mmc_async_req *mmc_start_req(struct mmc_host *,
> struct mmc_async_req *, int *);
>@@ -150,7 +150,7 @@ extern int mmc_wait_for_cmd(struct mmc_host *, struct
>mmc_command *, int);
> extern int mmc_app_cmd(struct mmc_host *, struct mmc_card *);
> extern int mmc_wait_for_app_cmd(struct mmc_host *, struct mmc_card *,
> struct mmc_command *, int);
>-extern void mmc_start_bkops(struct mmc_card *card, bool from_exception);
>+extern void mmc_start_bkops(struct mmc_card *, bool, bool);
> extern int mmc_switch(struct mmc_card *, u8, u8, u8, unsigned int);
> extern int mmc_send_tuning(struct mmc_host *host, u32 opcode, int *cmd_error);
> extern int mmc_get_ext_csd(struct mmc_card *card, u8 **new_ext_csd);
>diff --git a/include/linux/mmc/mmc.h b/include/linux/mmc/mmc.h
>index c376209..a1cced9 100644
>--- a/include/linux/mmc/mmc.h
>+++ b/include/linux/mmc/mmc.h
>@@ -436,6 +436,7 @@ struct _mmc_csd {
> * BKOPS modes
> */
> #define EXT_CSD_MANUAL_BKOPS_MASK 0x01
>+#define EXT_CSD_AUTO_BKOPS_MASK 0x02
>
> /*
> * MMC_SWITCH access modes
>--
>2.3.7
>
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>the body of a message to majord...@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
