Re: [PATCH v2 3/3] mmc: core: Re-work HW reset for SDIO cards

[Ulf Hansson <ulf.hansson@linaro.org>]

On Tue, 12 Nov 2019 at 01:33, Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Sat, Nov 9, 2019 at 2:31 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> > index 6f8342702c73..abf8f5eb0a1c 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -1469,8 +1469,7 @@ void mmc_detach_bus(struct mmc_host *host)
> >         mmc_bus_put(host);
> >  }
> >
> > -static void _mmc_detect_change(struct mmc_host *host, unsigned long delay,
> > -                               bool cd_irq)
> > +void _mmc_detect_change(struct mmc_host *host, unsigned long delay, bool cd_irq)
> >  {
> >         /*
> >          * If the device is configured as wakeup, we prevent a new sleep for
> > @@ -2129,7 +2128,7 @@ int mmc_hw_reset(struct mmc_host *host)
> >         ret = host->bus_ops->hw_reset(host);
> >         mmc_bus_put(host);
> >
> > -       if (ret)
> > +       if (ret < 0)
> >                 pr_warn("%s: tried to HW reset card, got error %d\n",
> >                         mmc_hostname(host), ret);
>
> Other callers besides marvell need to be updated?  In theory only SDIO
> should have positive return values so I guess we don't care about the
> caller in drivers/mmc/core/block.c, right?

Correct, but maybe I should add some more information about that in a
function header of mmc_hw_reset(). Let me consider doing that as a
change on top.

>  What about:
>
> drivers/net/wireless/ath/ath10k/sdio.c
>
> ...I guess I don't know if there is more than one function probed
> there.  Maybe there's not and thus we're fine here too?

Well, honestly I don't know.

In any case, that would mean the driver is broken anyways and needs to
be fixed. At least that's my approach to doing this change.

>
>
> I didn't spend massive amounts of time looking for potential problems,
> but in general seems workable to me.  Thanks!
>
> Reviewed-by: Douglas Anderson <dianders@chromium.org>

Thanks!

Kind regards
Uffe