[v3,06/10] mmc: mmci: Retry the busy start condition

This makes the ux500 ->busy_complete() callback re-read
the status register 10 times while waiting for the busy
signal to assert in the status register.

If this does not happen, we bail out regarding the
command completed already, i.e. before we managed to
start to check the busy status.

There is a comment in the code about this, let's just
implement it to be certain that we can catch this glitch
if it happens.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- Rebased.
ChangeLog v1->v2:
- Move over the initial saving of host->busy_status from
  an unrelated patch to this one: it is clear what we are
  doing: we don't want to miss any transient
  (MCI_CMDSENT | MCI_CMDRESPEND) in the status register.
---
 drivers/mmc/host/mmci.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

On Sun, 11 Jun 2023 at 21:41, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> This makes the ux500 ->busy_complete() callback re-read
> the status register 10 times while waiting for the busy
> signal to assert in the status register.
>
> If this does not happen, we bail out regarding the
> command completed already, i.e. before we managed to
> start to check the busy status.
>
> There is a comment in the code about this, let's just
> implement it to be certain that we can catch this glitch
> if it happens.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v2->v3:
> - Rebased.
> ChangeLog v1->v2:
> - Move over the initial saving of host->busy_status from
>   an unrelated patch to this one: it is clear what we are
>   doing: we don't want to miss any transient
>   (MCI_CMDSENT | MCI_CMDRESPEND) in the status register.
> ---
>  drivers/mmc/host/mmci.c | 40 ++++++++++++++++++++++++++++------------
>  1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index edc68fc7b642..2858d8d67129 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -664,6 +664,7 @@ static u32 ux500v2_get_dctrl_cfg(struct mmci_host *host)
>  static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
>  {
>         void __iomem *base = host->base;
> +       int retries = 10;
>
>         if (status & err_msk) {
>                 /* Stop any ongoing busy detection if an error occurs */
> @@ -684,21 +685,36 @@ static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
>          * Note that, the card may need a couple of clock cycles before
>          * it starts signaling busy on DAT0, hence re-read the
>          * MMCISTATUS register here, to allow the busy bit to be set.
> -        * Potentially we may even need to poll the register for a
> -        * while, to allow it to be set, but tests indicates that it
> -        * isn't needed.
>          */
>         if (host->busy_state == MMCI_BUSY_DONE) {
> -               status = readl(base + MMCISTATUS);
> -               if (status & host->variant->busy_detect_flag) {
> -                       writel(readl(base + MMCIMASK0) |
> -                              host->variant->busy_detect_mask,
> -                              base + MMCIMASK0);
> -
> -                       host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND);
> -                       host->busy_state = MMCI_BUSY_WAITING_FOR_START_IRQ;
> -                       return false;
> +               /*
> +                * Save the first status register read to be sure to catch
> +                * all bits that may be lost will retrying. If the command
> +                * is still busy this will result in assigning 0 to
> +                * host->busy_status, which is what it should be in IDLE.
> +                */
> +               host->busy_status = status & (MCI_CMDSENT | MCI_CMDRESPEND);
> +               while (retries) {
> +                       status = readl(base + MMCISTATUS);
> +                       if (status & host->variant->busy_detect_flag) {
> +                               writel(readl(base + MMCIMASK0) |
> +                                      host->variant->busy_detect_mask,
> +                                      base + MMCIMASK0);
> +
> +                               /* Keep accumulating status bits */
> +                               host->busy_status |= status & (MCI_CMDSENT | MCI_CMDRESPEND);

Looks like we should accumulate the status, no matter whether the
host->variant->busy_detect_flag is set?

Perhaps move this just before the if-clause above?

> +                               host->busy_state = MMCI_BUSY_WAITING_FOR_START_IRQ;
> +                               return false;
> +                       }
> +                       retries--;
>                 }
> +               dev_dbg(mmc_dev(host->mmc), "no busy signalling in time\n");
> +               writel(host->variant->busy_detect_mask, base + MMCICLEAR);
> +               writel(readl(base + MMCIMASK0) &
> +                      ~host->variant->busy_detect_mask, base + MMCIMASK0);
> +               host->busy_state = MMCI_BUSY_DONE;
> +               host->busy_status = 0;
> +               return true;
>         }
>

Kind regards
Uffe