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

[Linus Walleij]

On Mon, Apr 17, 2023 at 3:24 PM Ulf Hansson <ulf.hansson at linaro.org> wrote:
> 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?

I break that out as a separate function in patch 9, can you check if
that solves the problem? The end result after all the patches should
be fine.

Yours,
Linus Walleij