* [PATCH v1 0/2] two minor changes of eMMC BKOPS
@ 2021-08-17 22:42 Bean Huo
2021-08-17 22:42 ` [PATCH v1 1/2] mmc: core: Issue HPI in case the BKOPS timed out Bean Huo
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Bean Huo @ 2021-08-17 22:42 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter; +Cc: linux-mmc, linux-kernel, Bean Huo
From: Bean Huo <beanhuo@micron.com>
Bean Huo (2):
mmc: core: Issue HPI in case the BKOPS timed out
mmc: core: Let BKOPS timeout readable/writable via sysfs
drivers/mmc/core/mmc.c | 32 ++++++++++++++++++++++++++++++++
drivers/mmc/core/mmc_ops.c | 14 ++++++++++----
include/linux/mmc/card.h | 1 +
3 files changed, 43 insertions(+), 4 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v1 1/2] mmc: core: Issue HPI in case the BKOPS timed out
2021-08-17 22:42 [PATCH v1 0/2] two minor changes of eMMC BKOPS Bean Huo
@ 2021-08-17 22:42 ` Bean Huo
2021-08-24 14:56 ` Ulf Hansson
2021-08-17 22:42 ` [PATCH v1 2/2] mmc: core: Let BKOPS timeout readable/writable via sysfs Bean Huo
2021-08-23 7:53 ` [PATCH v1 0/2] two minor changes of eMMC BKOPS Bean Huo
2 siblings, 1 reply; 7+ messages in thread
From: Bean Huo @ 2021-08-17 22:42 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter; +Cc: linux-mmc, linux-kernel, Bean Huo
From: Bean Huo <beanhuo@micron.com>
If the BKOPS timed out, the card is probably still busy in the
R1_STATE_PRG. Rather than let application in the userland continue
to wait, let's try to abort it with a HPI command to get back into
R1_STATE_TRAN.
Signed-off-by: Bean Huo <beanhuo@micron.com>
---
drivers/mmc/core/mmc_ops.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 90d213a2203f..0c54858e89c0 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -959,8 +959,15 @@ void mmc_run_bkops(struct mmc_card *card)
*/
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
EXT_CSD_BKOPS_START, 1, MMC_BKOPS_TIMEOUT_MS);
- if (err)
- pr_warn("%s: Error %d starting bkops\n",
+ /*
+ * If the BKOPS timed out, the card is probably still busy in the
+ * R1_STATE_PRG. Rather than continue to wait, let's try to abort
+ * it with a HPI command to get back into R1_STATE_TRAN.
+ */
+ if (err == -ETIMEDOUT && !mmc_interrupt_hpi(card))
+ pr_warn("%s: BKOPS aborted\n", mmc_hostname(card->host));
+ else if (err)
+ pr_warn("%s: Error %d running bkops\n",
mmc_hostname(card->host), err);
mmc_retune_release(card->host);
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v1 2/2] mmc: core: Let BKOPS timeout readable/writable via sysfs
2021-08-17 22:42 [PATCH v1 0/2] two minor changes of eMMC BKOPS Bean Huo
2021-08-17 22:42 ` [PATCH v1 1/2] mmc: core: Issue HPI in case the BKOPS timed out Bean Huo
@ 2021-08-17 22:42 ` Bean Huo
2021-08-24 14:33 ` Ulf Hansson
2021-08-23 7:53 ` [PATCH v1 0/2] two minor changes of eMMC BKOPS Bean Huo
2 siblings, 1 reply; 7+ messages in thread
From: Bean Huo @ 2021-08-17 22:42 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter; +Cc: linux-mmc, linux-kernel, Bean Huo
From: Bean Huo <beanhuo@micron.com>
For special cases, the application in the userspace wants to change
BKOPS operation timeout value, also, wants eMMC back to R1_STATE_TRAN
after BKOPS timeouts. A fixed BKOPS timeout value(120s) is no longer
feasible, therefore, it is better to let the user controls its timeout
value.
Signed-off-by: Bean Huo <beanhuo@micron.com>
---
drivers/mmc/core/mmc.c | 32 ++++++++++++++++++++++++++++++++
drivers/mmc/core/mmc_ops.c | 3 +--
include/linux/mmc/card.h | 1 +
3 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 838726b68ff3..617ff18b5b0e 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -29,6 +29,7 @@
#define DEFAULT_CMD6_TIMEOUT_MS 500
#define MIN_CACHE_EN_TIMEOUT_MS 1600
#define CACHE_FLUSH_TIMEOUT_MS 30000 /* 30s */
+#define MMC_BKOPS_TIMEOUT_MS (120 * 1000) /* 120s */
static const unsigned int tran_exp[] = {
10000, 100000, 1000000, 10000000,
@@ -836,6 +837,35 @@ static ssize_t mmc_dsr_show(struct device *dev,
static DEVICE_ATTR(dsr, S_IRUGO, mmc_dsr_show, NULL);
+static ssize_t bkops_timeout_ms_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct mmc_card *card = mmc_dev_to_card(dev);
+ return sysfs_emit(buf, "%d\n", card->bkops_timeout_ms);
+}
+
+static ssize_t bkops_timeout_ms_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t len)
+{
+ struct mmc_card *card = mmc_dev_to_card(dev);
+ unsigned int new;
+ int ret;
+
+ ret = kstrtouint(buf, 0, &new);
+ if (ret < 0)
+ return ret;
+
+ if (new == 0)
+ return -EINVAL;
+
+ card->bkops_timeout_ms = new;
+
+ return len;
+}
+
+static DEVICE_ATTR_RW(bkops_timeout_ms);
+
static struct attribute *mmc_std_attrs[] = {
&dev_attr_cid.attr,
&dev_attr_csd.attr,
@@ -862,6 +892,7 @@ static struct attribute *mmc_std_attrs[] = {
&dev_attr_rca.attr,
&dev_attr_dsr.attr,
&dev_attr_cmdq_en.attr,
+ &dev_attr_bkops_timeout_ms.attr,
NULL,
};
ATTRIBUTE_GROUPS(mmc_std);
@@ -1624,6 +1655,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
card->ocr = ocr;
card->type = MMC_TYPE_MMC;
card->rca = 1;
+ card->bkops_timeout_ms = MMC_BKOPS_TIMEOUT_MS;
memcpy(card->raw_cid, cid, sizeof(card->raw_cid));
}
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 0c54858e89c0..9af5e4671de2 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -19,7 +19,6 @@
#include "host.h"
#include "mmc_ops.h"
-#define MMC_BKOPS_TIMEOUT_MS (120 * 1000) /* 120s */
#define MMC_SANITIZE_TIMEOUT_MS (240 * 1000) /* 240s */
static const u8 tuning_blk_pattern_4bit[] = {
@@ -958,7 +957,7 @@ void mmc_run_bkops(struct mmc_card *card)
* urgent levels by using an asynchronous background task, when idle.
*/
err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
- EXT_CSD_BKOPS_START, 1, MMC_BKOPS_TIMEOUT_MS);
+ EXT_CSD_BKOPS_START, 1, card->bkops_timeout_ms);
/*
* If the BKOPS timed out, the card is probably still busy in the
* R1_STATE_PRG. Rather than continue to wait, let's try to abort
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 74e6c0624d27..9e038d212067 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -294,6 +294,7 @@ struct mmc_card {
bool reenable_cmdq; /* Re-enable Command Queue */
+ unsigned int bkops_timeout_ms;
unsigned int erase_size; /* erase size in sectors */
unsigned int erase_shift; /* if erase unit is power 2 */
unsigned int pref_erase; /* in sectors */
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 0/2] two minor changes of eMMC BKOPS
2021-08-17 22:42 [PATCH v1 0/2] two minor changes of eMMC BKOPS Bean Huo
2021-08-17 22:42 ` [PATCH v1 1/2] mmc: core: Issue HPI in case the BKOPS timed out Bean Huo
2021-08-17 22:42 ` [PATCH v1 2/2] mmc: core: Let BKOPS timeout readable/writable via sysfs Bean Huo
@ 2021-08-23 7:53 ` Bean Huo
2 siblings, 0 replies; 7+ messages in thread
From: Bean Huo @ 2021-08-23 7:53 UTC (permalink / raw)
To: ulf.hansson, adrian.hunter; +Cc: linux-mmc, linux-kernel, Bean Huo
Hi Ullf,
Any thought about this patch?
Thanks
Bean
On Wed, 2021-08-18 at 00:42 +0200, Bean Huo wrote:
> From: Bean Huo <beanhuo@micron.com>
>
>
> Bean Huo (2):
> mmc: core: Issue HPI in case the BKOPS timed out
> mmc: core: Let BKOPS timeout readable/writable via sysfs
>
> drivers/mmc/core/mmc.c | 32 ++++++++++++++++++++++++++++++++
> drivers/mmc/core/mmc_ops.c | 14 ++++++++++----
> include/linux/mmc/card.h | 1 +
> 3 files changed, 43 insertions(+), 4 deletions(-)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/2] mmc: core: Let BKOPS timeout readable/writable via sysfs
2021-08-17 22:42 ` [PATCH v1 2/2] mmc: core: Let BKOPS timeout readable/writable via sysfs Bean Huo
@ 2021-08-24 14:33 ` Ulf Hansson
2021-08-25 8:43 ` Bean Huo
0 siblings, 1 reply; 7+ messages in thread
From: Ulf Hansson @ 2021-08-24 14:33 UTC (permalink / raw)
To: Bean Huo; +Cc: Adrian Hunter, linux-mmc, Linux Kernel Mailing List, Bean Huo
On Wed, 18 Aug 2021 at 00:42, Bean Huo <huobean@gmail.com> wrote:
>
> From: Bean Huo <beanhuo@micron.com>
>
> For special cases, the application in the userspace wants to change
> BKOPS operation timeout value, also, wants eMMC back to R1_STATE_TRAN
> after BKOPS timeouts. A fixed BKOPS timeout value(120s) is no longer
> feasible, therefore, it is better to let the user controls its timeout
> value.
I am not fond of exporting tweakable timeout values through sysfs.
Primarily because it's ABI and it becomes difficult to change.
Can you perhaps explain in more detail when you want to have a
different timeout?
Perhaps we can do something similar as we currently do for
mmc_santize(), where we allow userspace to pass the cmd-timeout?
Kind regards
Uffe
>
> Signed-off-by: Bean Huo <beanhuo@micron.com>
> ---
> drivers/mmc/core/mmc.c | 32 ++++++++++++++++++++++++++++++++
> drivers/mmc/core/mmc_ops.c | 3 +--
> include/linux/mmc/card.h | 1 +
> 3 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 838726b68ff3..617ff18b5b0e 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -29,6 +29,7 @@
> #define DEFAULT_CMD6_TIMEOUT_MS 500
> #define MIN_CACHE_EN_TIMEOUT_MS 1600
> #define CACHE_FLUSH_TIMEOUT_MS 30000 /* 30s */
> +#define MMC_BKOPS_TIMEOUT_MS (120 * 1000) /* 120s */
>
> static const unsigned int tran_exp[] = {
> 10000, 100000, 1000000, 10000000,
> @@ -836,6 +837,35 @@ static ssize_t mmc_dsr_show(struct device *dev,
>
> static DEVICE_ATTR(dsr, S_IRUGO, mmc_dsr_show, NULL);
>
> +static ssize_t bkops_timeout_ms_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct mmc_card *card = mmc_dev_to_card(dev);
> + return sysfs_emit(buf, "%d\n", card->bkops_timeout_ms);
> +}
> +
> +static ssize_t bkops_timeout_ms_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + struct mmc_card *card = mmc_dev_to_card(dev);
> + unsigned int new;
> + int ret;
> +
> + ret = kstrtouint(buf, 0, &new);
> + if (ret < 0)
> + return ret;
> +
> + if (new == 0)
> + return -EINVAL;
> +
> + card->bkops_timeout_ms = new;
> +
> + return len;
> +}
> +
> +static DEVICE_ATTR_RW(bkops_timeout_ms);
> +
> static struct attribute *mmc_std_attrs[] = {
> &dev_attr_cid.attr,
> &dev_attr_csd.attr,
> @@ -862,6 +892,7 @@ static struct attribute *mmc_std_attrs[] = {
> &dev_attr_rca.attr,
> &dev_attr_dsr.attr,
> &dev_attr_cmdq_en.attr,
> + &dev_attr_bkops_timeout_ms.attr,
> NULL,
> };
> ATTRIBUTE_GROUPS(mmc_std);
> @@ -1624,6 +1655,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
> card->ocr = ocr;
> card->type = MMC_TYPE_MMC;
> card->rca = 1;
> + card->bkops_timeout_ms = MMC_BKOPS_TIMEOUT_MS;
> memcpy(card->raw_cid, cid, sizeof(card->raw_cid));
> }
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 0c54858e89c0..9af5e4671de2 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -19,7 +19,6 @@
> #include "host.h"
> #include "mmc_ops.h"
>
> -#define MMC_BKOPS_TIMEOUT_MS (120 * 1000) /* 120s */
> #define MMC_SANITIZE_TIMEOUT_MS (240 * 1000) /* 240s */
>
> static const u8 tuning_blk_pattern_4bit[] = {
> @@ -958,7 +957,7 @@ void mmc_run_bkops(struct mmc_card *card)
> * urgent levels by using an asynchronous background task, when idle.
> */
> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> - EXT_CSD_BKOPS_START, 1, MMC_BKOPS_TIMEOUT_MS);
> + EXT_CSD_BKOPS_START, 1, card->bkops_timeout_ms);
> /*
> * If the BKOPS timed out, the card is probably still busy in the
> * R1_STATE_PRG. Rather than continue to wait, let's try to abort
> diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
> index 74e6c0624d27..9e038d212067 100644
> --- a/include/linux/mmc/card.h
> +++ b/include/linux/mmc/card.h
> @@ -294,6 +294,7 @@ struct mmc_card {
>
> bool reenable_cmdq; /* Re-enable Command Queue */
>
> + unsigned int bkops_timeout_ms;
> unsigned int erase_size; /* erase size in sectors */
> unsigned int erase_shift; /* if erase unit is power 2 */
> unsigned int pref_erase; /* in sectors */
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] mmc: core: Issue HPI in case the BKOPS timed out
2021-08-17 22:42 ` [PATCH v1 1/2] mmc: core: Issue HPI in case the BKOPS timed out Bean Huo
@ 2021-08-24 14:56 ` Ulf Hansson
0 siblings, 0 replies; 7+ messages in thread
From: Ulf Hansson @ 2021-08-24 14:56 UTC (permalink / raw)
To: Bean Huo; +Cc: Adrian Hunter, linux-mmc, Linux Kernel Mailing List, Bean Huo
On Wed, 18 Aug 2021 at 00:42, Bean Huo <huobean@gmail.com> wrote:
>
> From: Bean Huo <beanhuo@micron.com>
>
> If the BKOPS timed out, the card is probably still busy in the
> R1_STATE_PRG. Rather than let application in the userland continue
> to wait, let's try to abort it with a HPI command to get back into
> R1_STATE_TRAN.
>
> Signed-off-by: Bean Huo <beanhuo@micron.com>
Applied for next, thanks! I took the liberty of updating the commit
message a bit to clarify.
Kind regards
Uffe
> ---
> drivers/mmc/core/mmc_ops.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 90d213a2203f..0c54858e89c0 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -959,8 +959,15 @@ void mmc_run_bkops(struct mmc_card *card)
> */
> err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> EXT_CSD_BKOPS_START, 1, MMC_BKOPS_TIMEOUT_MS);
> - if (err)
> - pr_warn("%s: Error %d starting bkops\n",
> + /*
> + * If the BKOPS timed out, the card is probably still busy in the
> + * R1_STATE_PRG. Rather than continue to wait, let's try to abort
> + * it with a HPI command to get back into R1_STATE_TRAN.
> + */
> + if (err == -ETIMEDOUT && !mmc_interrupt_hpi(card))
> + pr_warn("%s: BKOPS aborted\n", mmc_hostname(card->host));
> + else if (err)
> + pr_warn("%s: Error %d running bkops\n",
> mmc_hostname(card->host), err);
>
> mmc_retune_release(card->host);
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/2] mmc: core: Let BKOPS timeout readable/writable via sysfs
2021-08-24 14:33 ` Ulf Hansson
@ 2021-08-25 8:43 ` Bean Huo
0 siblings, 0 replies; 7+ messages in thread
From: Bean Huo @ 2021-08-25 8:43 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Adrian Hunter, linux-mmc, Linux Kernel Mailing List, Bean Huo
On Tue, 2021-08-24 at 16:33 +0200, Ulf Hansson wrote:
> On Wed, 18 Aug 2021 at 00:42, Bean Huo <huobean@gmail.com> wrote:
> > From: Bean Huo <beanhuo@micron.com>
> >
> > For special cases, the application in the userspace wants to change
> > BKOPS operation timeout value, also, wants eMMC back to
> > R1_STATE_TRAN
> > after BKOPS timeouts. A fixed BKOPS timeout value(120s) is no
> > longer
> > feasible, therefore, it is better to let the user controls its
> > timeout
> > value.
>
> I am not fond of exporting tweakable timeout values through sysfs.
> Primarily because it's ABI and it becomes difficult to change.
>
> Can you perhaps explain in more detail when you want to have a
> different timeout?
>
> Perhaps we can do something similar as we currently do for
> mmc_santize(), where we allow userspace to pass the cmd-timeout?
>
> Kind regards
> Uffe
>
> > Signed-off-by: Bean Huo <beanhuo@micron.com>
> >
Hi Ullf,
Thanks for your review.
Actually, taking the first patch "MMC: core: Issue HPI in case the
BKOPS timed out" is good enough in the most of cases. And the current
120s timeout for the manual BKOPS is also enough, we don't need to
change this timeout value through sysfs.
Kind regards,
Bean
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-25 8:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-17 22:42 [PATCH v1 0/2] two minor changes of eMMC BKOPS Bean Huo
2021-08-17 22:42 ` [PATCH v1 1/2] mmc: core: Issue HPI in case the BKOPS timed out Bean Huo
2021-08-24 14:56 ` Ulf Hansson
2021-08-17 22:42 ` [PATCH v1 2/2] mmc: core: Let BKOPS timeout readable/writable via sysfs Bean Huo
2021-08-24 14:33 ` Ulf Hansson
2021-08-25 8:43 ` Bean Huo
2021-08-23 7:53 ` [PATCH v1 0/2] two minor changes of eMMC BKOPS Bean Huo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.