[Linux-stm32] [PATCH v2 04/12] mmc: mmci: Break out error check in busy detect

[Ulf Hansson]

On Sun, 9 Apr 2023 at 00:00, Linus Walleij <linus.walleij at linaro.org> wrote:
>
> The busy detect callback for Ux500 checks for an error
> in the status in the first if() clause. The only practical
> reason is that if an error occurs, the if()-clause is not
> executed, and the code falls through to the last
> if()-clause if (host->busy_status) which will clear and
> disable the irq. Make this explicit instead: it is easier
> to read.
>
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
> ---
> ChangeLog v1->v2:
> - No changes
> ---
>  drivers/mmc/host/mmci.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index e742dedaca1a..7d42625f2356 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -665,6 +665,15 @@ static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
>  {
>         void __iomem *base = host->base;
>
> +       if (status & err_msk) {
> +               /* Stop any ongoing busy detection if an error occurs */
> +               writel(host->variant->busy_detect_mask, base + MMCICLEAR);
> +               writel(readl(base + MMCIMASK0) &
> +                      ~host->variant->busy_detect_mask, base + MMCIMASK0);
> +               host->busy_status = 0;
> +               return true;

I agree that this makes the code more explicit, but unfortunately it
also means duplicating the code from the bottom of this function.

Can you instead add a helper function and call it from here and from
the part at the bottom?

> +       }
> +
>         /*
>          * Before unmasking for the busy end IRQ, confirm that the
>          * command was sent successfully. To keep track of having a
> @@ -678,7 +687,7 @@ static bool ux500_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
>          * while, to allow it to be set, but tests indicates that it
>          * isn't needed.
>          */
> -       if (!host->busy_status && !(status & err_msk)) {
> +       if (!host->busy_status) {
>                 status = readl(base + MMCISTATUS);
>                 if (status & host->variant->busy_detect_flag) {
>                         writel(readl(base + MMCIMASK0) |
>

Kind regards
Uffe