MIME-Version: 1.0
References: <20230213133707.27857-1-ulf.hansson@linaro.org>
In-Reply-To: <20230213133707.27857-1-ulf.hansson@linaro.org>
From:   Ulf Hansson <ulf.hansson@linaro.org>
Date:   Tue, 14 Feb 2023 13:18:35 +0100
Message-ID: <CAPDyKFoSpY_yGGu4jmYJm6qVYju2TmdN9DA3yvTLAwF59an_Ww@mail.gmail.com>
Subject: Re: [PATCH] mmc: core: Align to common busy polling behaviour for mmc ioctls
To:     linux-mmc@vger.kernel.org, Christian Lohle <cloehle@hyperstone.com>
Cc:     Adrian Hunter <adrian.hunter@intel.com>,
        Avri Altman <avri.altman@wdc.com>,
        linux-kernel@vger.kernel.org, Ulf Hansson <ulf.hansson@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, 13 Feb 2023 at 14:37, Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> Let's align to the common busy polling behaviour for mmc ioctls, by
> updating the below two corresponding parts, that comes into play when using
> an R1B response for a command.
>
> *) A command with an R1B response should be prepared by calling
> mmc_prepare_busy_cmd(), which make us respects the host's busy timeout
> constraints.
> **) When an R1B response is being used and the host also supports HW busy
> detection, we should skip to poll for busy completion.
>
> Suggested-by: Christian Loehle <cloehle@hyperstone.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

I have now applied this for next and by adding your reviewed-by tag, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/core/block.c   | 25 +++++++++++++++++--------
>  drivers/mmc/core/mmc_ops.c |  1 +
>  2 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 20da7ed43e6d..672ab90c4b2d 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -470,6 +470,8 @@ 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;
> +       unsigned int busy_timeout_ms;
>         int err;
>         unsigned int target_part;
>
> @@ -545,6 +547,13 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
>             (cmd.opcode == MMC_SWITCH))
>                 return mmc_sanitize(card, idata->ic.cmd_timeout_ms);
>
> +       /* If it's an R1B response we need some more preparations. */
> +       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_wait_for_req(card->host, &mrq);
>         memcpy(&idata->ic.response, cmd.resp, sizeof(cmd.resp));
>
> @@ -596,14 +605,14 @@ 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);
>
> -       if (idata->rpmb || (cmd.flags & MMC_RSP_R1B) == MMC_RSP_R1B) {
> -               /*
> -                * Ensure RPMB/R1B command has completed by polling CMD13 "Send Status". Here we
> -                * allow to override the default timeout value if a custom timeout is specified.
> -                */
> -               err = mmc_poll_for_busy(card, idata->ic.cmd_timeout_ms ? : MMC_BLK_TIMEOUT_MS,
> -                                       false, MMC_BUSY_IO);
> -       }
> +       /* No need to poll when using HW busy detection. */
> +       if ((card->host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp)
> +               return 0;
> +
> +       /* 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);
>
>         return err;
>  }
> diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
> index 81c55bfd6e0c..3b3adbddf664 100644
> --- a/drivers/mmc/core/mmc_ops.c
> +++ b/drivers/mmc/core/mmc_ops.c
> @@ -575,6 +575,7 @@ bool mmc_prepare_busy_cmd(struct mmc_host *host, struct mmc_command *cmd,
>         cmd->busy_timeout = timeout_ms;
>         return true;
>  }
> +EXPORT_SYMBOL_GPL(mmc_prepare_busy_cmd);
>
>  /**
>   *     __mmc_switch - modify EXT_CSD register
> --
> 2.34.1
>