RE: [PATCH] mmc: sdio: fix clock rate setting for SDR12/SDR25 mode

[BOUGH CHEN <haibo.chen@nxp.com>]

> -----Original Message-----
> From: Ulf Hansson [mailto:ulf.hansson@linaro.org]
> Sent: 2020年7月6日 22:49
> To: BOUGH CHEN <haibo.chen@nxp.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>; linux-mmc@vger.kernel.org;
> Pali Rohár <pali@kernel.org>; dl-linux-imx <linux-imx@nxp.com>; Andy Duan
> <fugang.duan@nxp.com>; Doug Anderson <dianders@chromium.org>;
> huyue2@yulong.com; Matthias Kaehlcke <mka@chromium.org>
> Subject: Re: [PATCH] mmc: sdio: fix clock rate setting for SDR12/SDR25 mode
> 
> On Mon, 22 Jun 2020 at 10:30, <haibo.chen@nxp.com> wrote:
> >
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > In current code logic, when work in SDR12/SDR25 mode, the final clock
> > rate is incorrect, just the legancy 400KHz, because the
> > card->sw_caps.sd3_bus_mode do not has the flag SD_MODE_UHS_SDR12 or
> > SD_MODE_UHS_SDR25. Besides, SDIO_SPEED_SDR12 is actually value 0, and
> > every mode need to config the timing and clock rate, so remove the
> > ‘if’ operator.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> 
> This looks like a rather serious error, should we tag this for stable?

Yes, need to do that.

Cc: <stable@vger.kernel.org>

Best Regards
Haibo Chen
> 
> In the meantime, I have applied this for next to get it tested, thanks!
> 
> Kind regards
> Uffe
> 
> 
> 
> > ---
> >  drivers/mmc/core/sdio.c | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index
> > 0e32ca7b9488..7b40553d3934 100644
> > --- a/drivers/mmc/core/sdio.c
> > +++ b/drivers/mmc/core/sdio.c
> > @@ -176,15 +176,18 @@ static int sdio_read_cccr(struct mmc_card *card,
> u32 ocr)
> >                         if (mmc_host_uhs(card->host)) {
> >                                 if (data & SDIO_UHS_DDR50)
> >
> card->sw_caps.sd3_bus_mode
> > -                                               |=
> SD_MODE_UHS_DDR50;
> > +                                               |=
> SD_MODE_UHS_DDR50 | SD_MODE_UHS_SDR50
> > +                                                       |
> > + SD_MODE_UHS_SDR25 | SD_MODE_UHS_SDR12;
> >
> >                                 if (data & SDIO_UHS_SDR50)
> >
> card->sw_caps.sd3_bus_mode
> > -                                               |=
> SD_MODE_UHS_SDR50;
> > +                                               |=
> SD_MODE_UHS_SDR50 | SD_MODE_UHS_SDR25
> > +                                                       |
> > + SD_MODE_UHS_SDR12;
> >
> >                                 if (data & SDIO_UHS_SDR104)
> >
> card->sw_caps.sd3_bus_mode
> > -                                               |=
> SD_MODE_UHS_SDR104;
> > +                                               |=
> SD_MODE_UHS_SDR104 | SD_MODE_UHS_SDR50
> > +                                                       |
> > + SD_MODE_UHS_SDR25 | SD_MODE_UHS_SDR12;
> >                         }
> >
> >                         ret = mmc_io_rw_direct(card, 0, 0, @@
> -537,10
> > +540,8 @@ static int sdio_set_bus_speed_mode(struct mmc_card *card)
> >         max_rate = min_not_zero(card->quirk_max_rate,
> >                                 card->sw_caps.uhs_max_dtr);
> >
> > -       if (bus_speed) {
> > -               mmc_set_timing(card->host, timing);
> > -               mmc_set_clock(card->host, max_rate);
> > -       }
> > +       mmc_set_timing(card->host, timing);
> > +       mmc_set_clock(card->host, max_rate);
> >
> >         return 0;
> >  }
> > --
> > 2.17.1
> >