On Tue, 9 Jun 2020 12:01:42 +0200
Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> On Tue, 9 Jun 2020 at 10:14, Yue Hu <zbestahu@xxxxxxxxx> wrote:
> >
> > From: Yue Hu <huyue2@xxxxxxxxxx>
> >
> > 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.
>
> >
> > Signed-off-by: Yue Hu <huyue2@xxxxxxxxxx>
> > ---
> > 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
