Re: [PATCH] mmc: sdio: Fix 1-bit mode for SD-combo cards during suspend

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

On Tue, 9 Jun 2020 at 12:40, Yue Hu <zbestahu@gmail.com> wrote:
>
> On Tue, 9 Jun 2020 12:01:42 +0200
> Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> > On Tue, 9 Jun 2020 at 10:14, Yue Hu <zbestahu@gmail.com> wrote:
> > >
> > > From: Yue Hu <huyue2@yulong.com>
> > >
> > > Commit 6b5eda369ac3 ("sdio: put active devices into 1-bit mode during
> > > suspend") disabled 4-bit mode during system suspend. After this patch,
> > > commit 7310ece86ad7 ("mmc: implement SD-combo (IO+mem) support") used
> > > new sdio_enable_4bit_bus() instead of sdio_enable_wide() to support
> > > SD-combo cards, also for card resume. However, no corresponding support
> > > added during suspend. That is not correct. Let's fix it.
> >
> > I believe the change makes sense to me.
> >
> > However, the commit 6b5eda369ac3 that you refer to is from v2.6.34,
> > which is more than ten years ago. That makes me wonder, are these
> > cards really being used?
>
> Current code logic will switch to 1-bit/4-bit mode in suspend/resume.
>
> >
> > Did you test this with a combo card?
>
> No, i have no real environment to test it. I'm just reading the code.
> Obviously, sdio_disable_wide() used in suspend is for SDIO cards only.
> However, sdio_enable_4bit_mode() used in resume is for SD-combo cards.
> We should also add the support in suspend to avoid the potential issue.
>
> Thank you.

Alright, let's give this a try and hopefully there is no regression reported.

Applied for next, thanks!

Kind regards
Uffe


>
> >
> > >
> > > Signed-off-by: Yue Hu <huyue2@yulong.com>
> > > ---
> > >  drivers/mmc/core/sdio.c | 23 ++++++++++++++++++++++-
> > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> > > index ebb387a..2d2ae35 100644
> > > --- a/drivers/mmc/core/sdio.c
> > > +++ b/drivers/mmc/core/sdio.c
> > > @@ -285,6 +285,27 @@ static int sdio_disable_wide(struct mmc_card *card)
> > >         return 0;
> > >  }
> > >
> > > +static int sdio_disable_4bit_bus(struct mmc_card *card)
> > > +{
> > > +       int err;
> > > +
> > > +       if (card->type == MMC_TYPE_SDIO)
> > > +               goto out;
> > > +
> > > +       if (!(card->host->caps & MMC_CAP_4_BIT_DATA))
> > > +               return 0;
> > > +
> > > +       if (!(card->scr.bus_widths & SD_SCR_BUS_WIDTH_4))
> > > +               return 0;
> > > +
> > > +       err = mmc_app_set_bus_width(card, MMC_BUS_WIDTH_1);
> > > +       if (err)
> > > +               return err;
> > > +
> > > +out:
> > > +       return sdio_disable_wide(card);
> > > +}
> > > +
> > >
> > >  static int sdio_enable_4bit_bus(struct mmc_card *card)
> > >  {
> > > @@ -960,7 +981,7 @@ static int mmc_sdio_suspend(struct mmc_host *host)
> > >         mmc_claim_host(host);
> > >
> > >         if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host))
> > > -               sdio_disable_wide(host->card);
> > > +               sdio_disable_4bit_bus(host->card);
> > >
> > >         if (!mmc_card_keep_power(host)) {
> > >                 mmc_power_off(host);
> > > --
> > > 1.9.1
> > >
> >
> > Kind regards
> > Uffe
>