* [PATCH v2] fix device in programming state after ioctl() @ 2019-09-05 7:53 Chaotian Jing 2019-09-05 7:53 ` [PATCH v2 1/2] mmc: block: make the card_busy_detect() more generic Chaotian Jing 2019-09-05 7:53 ` [PATCH v2 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response Chaotian Jing 0 siblings, 2 replies; 6+ messages in thread From: Chaotian Jing @ 2019-09-05 7:53 UTC (permalink / raw) To: Ulf Hansson Cc: Matthias Brugger, Jens Axboe, Hannes Reinecke, Chaotian Jing, Avri Altman, YueHaibing, Wolfram Sang, Ming Lei, Chris Boot, Zachary Hays, linux-mmc, linux-kernel, linux-arm-kernel, linux-mediatek, srv_heupstream v2 vs v1: base on Avri's comment, drop the ioctl_rpmb_card_status_poll() 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 | 150 +++++++++++++++------------------------ 1 file changed, 57 insertions(+), 93 deletions(-) -- 2.18.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] mmc: block: make the card_busy_detect() more generic 2019-09-05 7:53 [PATCH v2] fix device in programming state after ioctl() Chaotian Jing @ 2019-09-05 7:53 ` Chaotian Jing 2019-09-05 8:43 ` Avri Altman 2019-09-05 7:53 ` [PATCH v2 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response Chaotian Jing 1 sibling, 1 reply; 6+ messages in thread From: Chaotian Jing @ 2019-09-05 7:53 UTC (permalink / raw) To: Ulf Hansson Cc: Matthias Brugger, Jens Axboe, Hannes Reinecke, Chaotian Jing, Avri Altman, YueHaibing, Wolfram Sang, Ming Lei, Chris Boot, Zachary Hays, linux-mmc, linux-kernel, linux-arm-kernel, linux-mediatek, srv_heupstream 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 <chaotian.jing@mediatek.com> --- 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH v2 1/2] mmc: block: make the card_busy_detect() more generic 2019-09-05 7:53 ` [PATCH v2 1/2] mmc: block: make the card_busy_detect() more generic Chaotian Jing @ 2019-09-05 8:43 ` Avri Altman 0 siblings, 0 replies; 6+ messages in thread From: Avri Altman @ 2019-09-05 8:43 UTC (permalink / raw) To: Chaotian Jing, Ulf Hansson Cc: Matthias Brugger, Jens Axboe, Hannes Reinecke, YueHaibing, Wolfram Sang, Ming Lei, Chris Boot, Zachary Hays, linux-mmc, linux-kernel, linux-arm-kernel, linux-mediatek, srv_heupstream > > to use the card_busy_detect() to wait card levae the programming state, > there may be do not have the "struct request *" argument. Maybe reword the commit log to make it more clear: A tad optimization, removing the "struct request *" argument from card_busy_detect(). It's not really needed there, and will prove its worth in the next patch, Where we'll use it in __mmc_blk_ioctl_cmd where struct request is not available. > > Signed-off-by: Chaotian Jing <chaotian.jing@mediatek.com> Reviewed-by: Avri Altman <avri.altman@wdc.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response 2019-09-05 7:53 [PATCH v2] fix device in programming state after ioctl() Chaotian Jing 2019-09-05 7:53 ` [PATCH v2 1/2] mmc: block: make the card_busy_detect() more generic Chaotian Jing @ 2019-09-05 7:53 ` Chaotian Jing 2019-09-05 8:44 ` Avri Altman 2019-10-21 11:43 ` Ulf Hansson 1 sibling, 2 replies; 6+ messages in thread From: Chaotian Jing @ 2019-09-05 7:53 UTC (permalink / raw) To: Ulf Hansson Cc: Matthias Brugger, Jens Axboe, Hannes Reinecke, Chaotian Jing, Avri Altman, YueHaibing, Wolfram Sang, Ming Lei, Chris Boot, Zachary Hays, linux-mmc, linux-kernel, linux-arm-kernel, linux-mediatek, srv_heupstream 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 <chaotian.jing@mediatek.com> --- drivers/mmc/core/block.c | 146 +++++++++++++++------------------------ 1 file changed, 55 insertions(+), 91 deletions(-) diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index aa7c19f7e298..ee1fd7df4ec8 100644 --- a/drivers/mmc/core/block.c +++ b/drivers/mmc/core/block.c @@ -408,38 +408,6 @@ static int mmc_blk_ioctl_copy_to_user(struct mmc_ioc_cmd __user *ic_ptr, return 0; } -static int ioctl_rpmb_card_status_poll(struct mmc_card *card, u32 *status, - u32 retries_max) -{ - int err; - u32 retry_count = 0; - - if (!status || !retries_max) - return -EINVAL; - - do { - err = __mmc_send_status(card, status, 5); - if (err) - break; - - if (!R1_STATUS(*status) && - (R1_CURRENT_STATE(*status) != R1_STATE_PRG)) - break; /* RPMB programming operation complete */ - - /* - * Rechedule to give the MMC device a chance to continue - * processing the previous command without being polled too - * frequently. - */ - usleep_range(1000, 5000); - } while (++retry_count < retries_max); - - if (retry_count == retries_max) - err = -EPERM; - - return err; -} - static int ioctl_do_sanitize(struct mmc_card *card) { int err; @@ -468,6 +436,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) { @@ -611,16 +631,12 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md, memcpy(&(idata->ic.response), cmd.resp, sizeof(cmd.resp)); - if (idata->rpmb) { + if (idata->rpmb || (cmd.flags & MMC_RSP_R1B)) { /* - * Ensure RPMB command has completed by polling CMD13 + * Ensure RPMB/R1B command has completed by polling CMD13 * "Send Status". */ - err = ioctl_rpmb_card_status_poll(card, &status, 5); - if (err) - dev_err(mmc_dev(card->host), - "%s: Card Status=0x%08X, error %d\n", - __func__, status, err); + err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, NULL); } return err; @@ -970,58 +986,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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH v2 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response 2019-09-05 7:53 ` [PATCH v2 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response Chaotian Jing @ 2019-09-05 8:44 ` Avri Altman 2019-10-21 11:43 ` Ulf Hansson 1 sibling, 0 replies; 6+ messages in thread From: Avri Altman @ 2019-09-05 8:44 UTC (permalink / raw) To: Chaotian Jing, Ulf Hansson Cc: Matthias Brugger, Jens Axboe, Hannes Reinecke, YueHaibing, Wolfram Sang, Ming Lei, Chris Boot, Zachary Hays, linux-mmc, linux-kernel, linux-arm-kernel, linux-mediatek, srv_heupstream > > > 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 <chaotian.jing@mediatek.com> Reviewed-by: Avri Altman <avri.altman@wdc.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response 2019-09-05 7:53 ` [PATCH v2 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response Chaotian Jing 2019-09-05 8:44 ` Avri Altman @ 2019-10-21 11:43 ` Ulf Hansson 1 sibling, 0 replies; 6+ messages in thread From: Ulf Hansson @ 2019-10-21 11:43 UTC (permalink / raw) To: Chaotian Jing Cc: Matthias Brugger, Jens Axboe, Hannes Reinecke, Avri Altman, YueHaibing, Wolfram Sang, Ming Lei, Chris Boot, Zachary Hays, linux-mmc, Linux Kernel Mailing List, Linux ARM, linux-mediatek, srv_heupstream On Thu, 5 Sep 2019 at 09:53, Chaotian Jing <chaotian.jing@mediatek.com> wrote: > > 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 <chaotian.jing@mediatek.com> Both patch 1 and patch2, looks like material for stable, so unless someone objects I am adding a tag for that. Moreover, I updated the changelogs, also according to suggestions from Avri and then applied both patches for next, thanks! Kind regards Uffe > --- > drivers/mmc/core/block.c | 146 +++++++++++++++------------------------ > 1 file changed, 55 insertions(+), 91 deletions(-) > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c > index aa7c19f7e298..ee1fd7df4ec8 100644 > --- a/drivers/mmc/core/block.c > +++ b/drivers/mmc/core/block.c > @@ -408,38 +408,6 @@ static int mmc_blk_ioctl_copy_to_user(struct mmc_ioc_cmd __user *ic_ptr, > return 0; > } > > -static int ioctl_rpmb_card_status_poll(struct mmc_card *card, u32 *status, > - u32 retries_max) > -{ > - int err; > - u32 retry_count = 0; > - > - if (!status || !retries_max) > - return -EINVAL; > - > - do { > - err = __mmc_send_status(card, status, 5); > - if (err) > - break; > - > - if (!R1_STATUS(*status) && > - (R1_CURRENT_STATE(*status) != R1_STATE_PRG)) > - break; /* RPMB programming operation complete */ > - > - /* > - * Rechedule to give the MMC device a chance to continue > - * processing the previous command without being polled too > - * frequently. > - */ > - usleep_range(1000, 5000); > - } while (++retry_count < retries_max); > - > - if (retry_count == retries_max) > - err = -EPERM; > - > - return err; > -} > - > static int ioctl_do_sanitize(struct mmc_card *card) > { > int err; > @@ -468,6 +436,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) > { > @@ -611,16 +631,12 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md, > > memcpy(&(idata->ic.response), cmd.resp, sizeof(cmd.resp)); > > - if (idata->rpmb) { > + if (idata->rpmb || (cmd.flags & MMC_RSP_R1B)) { > /* > - * Ensure RPMB command has completed by polling CMD13 > + * Ensure RPMB/R1B command has completed by polling CMD13 > * "Send Status". > */ > - err = ioctl_rpmb_card_status_poll(card, &status, 5); > - if (err) > - dev_err(mmc_dev(card->host), > - "%s: Card Status=0x%08X, error %d\n", > - __func__, status, err); > + err = card_busy_detect(card, MMC_BLK_TIMEOUT_MS, NULL); > } > > return err; > @@ -970,58 +986,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 > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-21 11:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-05 7:53 [PATCH v2] fix device in programming state after ioctl() Chaotian Jing 2019-09-05 7:53 ` [PATCH v2 1/2] mmc: block: make the card_busy_detect() more generic Chaotian Jing 2019-09-05 8:43 ` Avri Altman 2019-09-05 7:53 ` [PATCH v2 2/2] mmc: block: add CMD13 polling for ioctl() cmd with R1B response Chaotian Jing 2019-09-05 8:44 ` Avri Altman 2019-10-21 11:43 ` Ulf Hansson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).