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

["Pali Rohár" <pali@kernel.org>]

On Tuesday 07 July 2020 01:48:18 BOUGH CHEN wrote:
> > -----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>

Hello! I think you can add Fixes line, e.g.:

Fixes: a303c5319c8e ("mmc: sdio: support SDIO UHS cards")

> 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
> > >