Re: [PATCH 03/19] mmc: atmel-mci: Respect the cmd->busy_timeout from the mmc core

From: ludovic.desroches@microchip.com
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-mmc@vger.kernel.org,
	"Adrian Hunter" <adrian.hunter@intel.com>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Rui Miguel Silva" <rmfrfs@gmail.com>,
	"Johan Hovold" <johan@kernel.org>,
	"Alex Elder" <elder@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Jonathan Neuschäfer" <j.neuschaefer@gmx.net>,
	"Bruce Chang" <brucechang@via.com.tw>,
	"Harald Welte" <HaraldWelte@viatech.com>,
	"Alex Dubov" <oakad@yahoo.com>,
	"Sascha Sommer" <saschasommer@freenet.de>,
	"Manivannan Sadhasivam" <manivannan.sadhasivam@linaro.org>,
	mirq-linux@rere.qmqm.pl,
	"Jesper Nilsson" <jesper.nilsson@axis.com>,
	"Lars Persson" <lars.persson@axis.com>,
	"Paul Cercueil" <paul@crapouillou.net>,
	"Nicolas Ferre" <nicolas.ferre@microchip.com>
Subject: Re: [PATCH 03/19] mmc: atmel-mci: Respect the cmd->busy_timeout from the mmc core
Date: Fri, 17 Apr 2020 22:44:44 +0200	[thread overview]
Message-ID: <20200417204444.63ggl53sxztdyvln@ROU-LT-M43218B.mchp-main.com> (raw)
In-Reply-To: <20200414161413.3036-4-ulf.hansson@linaro.org>

On Tue, Apr 14, 2020 at 06:13:57PM +0200, Ulf Hansson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Using a fixed 2s timeout for all commands is a bit problematic.
> 
> For some commands it means waiting longer than needed for the timer to
> expire, which may not a big issue, but still. For other commands, like for
> an erase (CMD38) that uses a R1B response, may require longer timeouts than
> 2s. In these cases, we may end up treating the command as it failed, while
> it just needed some more time to complete successfully.
> 
> Fix the problem by respecting the cmd->busy_timeout, which is provided by
> the mmc core.
> 
> Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Far better than a fixed timeout.

This driver covers many versions of the IP. I could test it with only one
version. As I didn't any regression and these three patches make sense

Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>

Thanks

> ---
>  drivers/mmc/host/atmel-mci.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
> index 7292970065b6..5cb692687698 100644
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -809,6 +809,9 @@ static u32 atmci_prepare_command(struct mmc_host *mmc,
>  static void atmci_send_command(struct atmel_mci *host,
>                 struct mmc_command *cmd, u32 cmd_flags)
>  {
> +       unsigned int timeout_ms = cmd->busy_timeout ? cmd->busy_timeout :
> +               ATMCI_CMD_TIMEOUT_MS;
> +
>         WARN_ON(host->cmd);
>         host->cmd = cmd;
> 
> @@ -819,8 +822,7 @@ static void atmci_send_command(struct atmel_mci *host,
>         atmci_writel(host, ATMCI_ARGR, cmd->arg);
>         atmci_writel(host, ATMCI_CMDR, cmd_flags);
> 
> -       mod_timer(&host->timer,
> -                 jiffies + msecs_to_jiffies(ATMCI_CMD_TIMEOUT_MS));
> +       mod_timer(&host->timer, jiffies + msecs_to_jiffies(timeout_ms));
>  }
> 
>  static void atmci_send_stop_cmd(struct atmel_mci *host, struct mmc_data *data)
> --
> 2.20.1
> 