2019-09-04 07:56:05

by Chaotian Jing

[permalink] [raw]
Subject: fix device in programming state after ioctl()

the user space program may access eMMC by ioctl(), after the ioctl() was
completed, it should ensure that eMMC is in transfer state, or it will
cause other thread which access eMMC got timeout error, as it assume that
card was in transfer state.

this patch add CMD13 polling for R1B command to avoid this issue.

Chaotian Jing (2):
mmc: block: make the card_busy_detect() more generic
mmc: block: add CMD13 polling for ioctl() cmd with R1B response

drivers/mmc/core/block.c | 111 ++++++++++++++++++++-------------------
1 file changed, 57 insertions(+), 54 deletions(-)

--
2.18.0



2019-09-04 07:56:08

by Chaotian Jing

[permalink] [raw]
Subject: [PATCH 1/2] mmc: block: make the card_busy_detect() more generic

to use the card_busy_detect() to wait card levae the programming state,
there may be do not have the "struct request *" argument.

Signed-off-by: Chaotian Jing <[email protected]>
---
drivers/mmc/core/block.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 2c71a434c915..aa7c19f7e298 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -981,7 +981,7 @@ static inline bool mmc_blk_in_tran_state(u32 status)
}

static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
- struct request *req, u32 *resp_errs)
+ u32 *resp_errs)
{
unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
int err = 0;
@@ -992,8 +992,8 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,

err = __mmc_send_status(card, &status, 5);
if (err) {
- pr_err("%s: error %d requesting status\n",
- req->rq_disk->disk_name, err);
+ dev_err(mmc_dev(card->host),
+ "error %d requesting status\n", err);
return err;
}

@@ -1006,9 +1006,9 @@ static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
* leaves the program state.
*/
if (done) {
- pr_err("%s: Card stuck in wrong state! %s %s status: %#x\n",
- mmc_hostname(card->host),
- req->rq_disk->disk_name, __func__, status);
+ dev_err(mmc_dev(card->host),
+ "Card stuck in wrong state! %s status: %#x\n",
+ __func__, status);
return -ETIMEDOUT;
}

@@ -1671,7 +1671,7 @@ static int mmc_blk_fix_state(struct mmc_card *card, struct request *req)

mmc_blk_send_stop(card, timeout);

- err = card_busy_detect(card, timeout, req, NULL);
+ err = card_busy_detect(card, timeout, NULL);

mmc_retune_release(card->host);

@@ -1895,7 +1895,7 @@ static int mmc_blk_card_busy(struct mmc_card *card, struct request *req)
if (mmc_host_is_spi(card->host) || rq_data_dir(req) == READ)
return 0;

- err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, req, &status);
+ err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, &status);

/*
* Do not assume data transferred correctly if there are any error bits
--
2.18.0

2019-09-04 07:57:21

by Chaotian Jing

[permalink] [raw]
Subject: [PATCH 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response

currently there is no CMD13 polling and other code to wait card
change to transfer state after R1B command completed. and this
polling operation cannot do in user space, because other request
may coming before the CMD13 from user space.

Signed-off-by: Chaotian Jing <[email protected]>
---
drivers/mmc/core/block.c | 107 ++++++++++++++++++++-------------------
1 file changed, 55 insertions(+), 52 deletions(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index aa7c19f7e298..9d6f7a5612b5 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -468,6 +468,58 @@ static int ioctl_do_sanitize(struct mmc_card *card)
return err;
}

+static inline bool mmc_blk_in_tran_state(u32 status)
+{
+ /*
+ * Some cards mishandle the status bits, so make sure to check both the
+ * busy indication and the card state.
+ */
+ return status & R1_READY_FOR_DATA &&
+ (R1_CURRENT_STATE(status) == R1_STATE_TRAN);
+}
+
+static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
+ u32 *resp_errs)
+{
+ unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
+ int err = 0;
+ u32 status;
+
+ do {
+ bool done = time_after(jiffies, timeout);
+
+ err = __mmc_send_status(card, &status, 5);
+ if (err) {
+ dev_err(mmc_dev(card->host),
+ "error %d requesting status\n", err);
+ return err;
+ }
+
+ /* Accumulate any response error bits seen */
+ if (resp_errs)
+ *resp_errs |= status;
+
+ /*
+ * Timeout if the device never becomes ready for data and never
+ * leaves the program state.
+ */
+ if (done) {
+ dev_err(mmc_dev(card->host),
+ "Card stuck in wrong state! %s status: %#x\n",
+ __func__, status);
+ return -ETIMEDOUT;
+ }
+
+ /*
+ * Some cards mishandle the status bits,
+ * so make sure to check both the busy
+ * indication and the card state.
+ */
+ } while (!mmc_blk_in_tran_state(status));
+
+ return err;
+}
+
static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
struct mmc_blk_ioc_data *idata)
{
@@ -623,6 +675,9 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
__func__, status, err);
}

+ if (!err && (cmd.flags & MMC_RSP_R1B))
+ err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, NULL);
+
return err;
}

@@ -970,58 +1025,6 @@ static unsigned int mmc_blk_data_timeout_ms(struct mmc_host *host,
return ms;
}

-static inline bool mmc_blk_in_tran_state(u32 status)
-{
- /*
- * Some cards mishandle the status bits, so make sure to check both the
- * busy indication and the card state.
- */
- return status & R1_READY_FOR_DATA &&
- (R1_CURRENT_STATE(status) == R1_STATE_TRAN);
-}
-
-static int card_busy_detect(struct mmc_card *card, unsigned int timeout_ms,
- u32 *resp_errs)
-{
- unsigned long timeout = jiffies + msecs_to_jiffies(timeout_ms);
- int err = 0;
- u32 status;
-
- do {
- bool done = time_after(jiffies, timeout);
-
- err = __mmc_send_status(card, &status, 5);
- if (err) {
- dev_err(mmc_dev(card->host),
- "error %d requesting status\n", err);
- return err;
- }
-
- /* Accumulate any response error bits seen */
- if (resp_errs)
- *resp_errs |= status;
-
- /*
- * Timeout if the device never becomes ready for data and never
- * leaves the program state.
- */
- if (done) {
- dev_err(mmc_dev(card->host),
- "Card stuck in wrong state! %s status: %#x\n",
- __func__, status);
- return -ETIMEDOUT;
- }
-
- /*
- * Some cards mishandle the status bits,
- * so make sure to check both the busy
- * indication and the card state.
- */
- } while (!mmc_blk_in_tran_state(status));
-
- return err;
-}
-
static int mmc_blk_reset(struct mmc_blk_data *md, struct mmc_host *host,
int type)
{
--
2.18.0

2019-09-04 14:13:14

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response

> static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct
> mmc_blk_data *md,
> struct mmc_blk_ioc_data *idata)
> {
> @@ -623,6 +675,9 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card
> *card, struct mmc_blk_data *md,
> __func__, status, err);
> }
>
> + if (!err && (cmd.flags & MMC_RSP_R1B))
> + err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, NULL);
> +
> return err;
> }
You have both the R1B flag check, and status poll (for rpmb) few line above.
Maybe you could re-use it.
It will both simplify this patch, and save the tad optimization of your first patch.

Thanks,
Avri

2019-09-05 03:10:39

by Chaotian Jing

[permalink] [raw]
Subject: Re: [PATCH 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response

On Wed, 2019-09-04 at 14:11 +0000, Avri Altman wrote:
> > static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct
> > mmc_blk_data *md,
> > struct mmc_blk_ioc_data *idata)
> > {
> > @@ -623,6 +675,9 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card
> > *card, struct mmc_blk_data *md,
> > __func__, status, err);
> > }
> >
> > + if (!err && (cmd.flags & MMC_RSP_R1B))
> > + err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, NULL);
> > +
> > return err;
> > }
> You have both the R1B flag check, and status poll (for rpmb) few line above.
> Maybe you could re-use it.
> It will both simplify this patch, and save the tad optimization of your first patch.
>
> Thanks,
> Avri

So that we can drop the ioctl_rpmb_card_status_poll() as it do almost
the same thing with card_busy_detect().


2019-09-05 06:22:36

by Avri Altman

[permalink] [raw]
Subject: RE: [PATCH 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response


> On Wed, 2019-09-04 at 14:11 +0000, Avri Altman wrote:
> > > static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct
> > > mmc_blk_data *md,
> > > struct mmc_blk_ioc_data *idata)
> > > {
> > > @@ -623,6 +675,9 @@ static int __mmc_blk_ioctl_cmd(struct
> mmc_card
> > > *card, struct mmc_blk_data *md,
> > > __func__, status, err);
> > > }
> > >
> > > + if (!err && (cmd.flags & MMC_RSP_R1B))
> > > + err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS,
> NULL);
> > > +
> > > return err;
> > > }
> > You have both the R1B flag check, and status poll (for rpmb) few line
> above.
> > Maybe you could re-use it.
> > It will both simplify this patch, and save the tad optimization of your
> first patch.
> >
> > Thanks,
> > Avri
>
> So that we can drop the ioctl_rpmb_card_status_poll() as it do almost
> the same thing with card_busy_detect().
>
right