On Wed, 13 Sept 2023 at 13:29, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>
> Userspace has currently no way of checking the internal R1 response error
> bits for some commands. This is a problem for some commands, like RPMB for
> example. Typically, we may detect that the busy completion has successfully
> ended, while in fact the card did not complete the requested operation.
>
> To fix the problem, let's always poll with CMD13 for these commands and
> during the polling, let's also aggregate the R1 response bits. Before
> completing the ioctl request, let's propagate the R1 response bits too.
>
> Reviewed-by: Avri Altman <avri.altman@xxxxxxx>
> Co-developed-by: Christian Loehle <christian.loehle@xxxxxxx>
> Signed-off-by: Christian Loehle <christian.loehle@xxxxxxx>
> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
This one is now applied for fixes and has a stable tag added to it.
Please let me know if there is anyone has objections to this.
Kind regards
Uffe
> ---
>
> Changes in v2:
> - Updated Christian's email.
> - Updated the commit message.
> - Initialize mmc_blk_busy_data struct directly.
>
> ---
> drivers/mmc/core/block.c | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index b5b414a71e0b..3a8f27c3e310 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -179,6 +179,7 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
> struct mmc_queue *mq);
> static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
> static int mmc_spi_err_check(struct mmc_card *card);
> +static int mmc_blk_busy_cb(void *cb_data, bool *busy);
>
> static struct mmc_blk_data *mmc_blk_get(struct gendisk *disk)
> {
> @@ -470,7 +471,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
> struct mmc_data data = {};
> struct mmc_request mrq = {};
> struct scatterlist sg;
> - bool r1b_resp, use_r1b_resp = false;
> + bool r1b_resp;
> unsigned int busy_timeout_ms;
> int err;
> unsigned int target_part;
> @@ -551,8 +552,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
> busy_timeout_ms = idata->ic.cmd_timeout_ms ? : MMC_BLK_TIMEOUT_MS;
> r1b_resp = (cmd.flags & MMC_RSP_R1B) == MMC_RSP_R1B;
> if (r1b_resp)
> - use_r1b_resp = mmc_prepare_busy_cmd(card->host, &cmd,
> - busy_timeout_ms);
> + mmc_prepare_busy_cmd(card->host, &cmd, busy_timeout_ms);
>
> mmc_wait_for_req(card->host, &mrq);
> memcpy(&idata->ic.response, cmd.resp, sizeof(cmd.resp));
> @@ -605,19 +605,28 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
> if (idata->ic.postsleep_min_us)
> usleep_range(idata->ic.postsleep_min_us, idata->ic.postsleep_max_us);
>
> - /* No need to poll when using HW busy detection. */
> - if ((card->host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp)
> - return 0;
> -
> if (mmc_host_is_spi(card->host)) {
> if (idata->ic.write_flag || r1b_resp || cmd.flags & MMC_RSP_SPI_BUSY)
> return mmc_spi_err_check(card);
> return err;
> }
> - /* Ensure RPMB/R1B command has completed by polling with CMD13. */
> - if (idata->rpmb || r1b_resp)
> - err = mmc_poll_for_busy(card, busy_timeout_ms, false,
> - MMC_BUSY_IO);
> +
> + /*
> + * Ensure RPMB, writes and R1B responses are completed by polling with
> + * CMD13. Note that, usually we don't need to poll when using HW busy
> + * detection, but here it's needed since some commands may indicate the
> + * error through the R1 status bits.
> + */
> + if (idata->rpmb || idata->ic.write_flag || r1b_resp) {
> + struct mmc_blk_busy_data cb_data = {
> + .card = card,
> + };
> +
> + err = __mmc_poll_for_busy(card->host, 0, busy_timeout_ms,
> + &mmc_blk_busy_cb, &cb_data);
> +
> + idata->ic.response[0] = cb_data.status;
> + }
>
> return err;
> }
> --
> 2.34.1
>
