On Mon, Apr 17, 2023 at 3:24 PM Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> On Sun, 9 Apr 2023 at 00:00, Linus Walleij <linus.walleij@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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
