* [PATCH] mmc: sdio: fix clock rate setting for SDR12/SDR25 mode
@ 2020-06-22 8:19 haibo.chen
2020-06-29 8:03 ` Pali Rohár
2020-07-06 14:49 ` Ulf Hansson
0 siblings, 2 replies; 5+ messages in thread
From: haibo.chen @ 2020-06-22 8:19 UTC (permalink / raw)
To: adrian.hunter, ulf.hansson, linux-mmc, pali
Cc: linux-imx, haibo.chen, fugang.duan, dianders, huyue2, mka
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>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] mmc: sdio: fix clock rate setting for SDR12/SDR25 mode
2020-06-22 8:19 [PATCH] mmc: sdio: fix clock rate setting for SDR12/SDR25 mode haibo.chen
@ 2020-06-29 8:03 ` Pali Rohár
2020-07-06 14:49 ` Ulf Hansson
1 sibling, 0 replies; 5+ messages in thread
From: Pali Rohár @ 2020-06-29 8:03 UTC (permalink / raw)
To: haibo.chen
Cc: adrian.hunter, ulf.hansson, linux-mmc, linux-imx, fugang.duan,
dianders, huyue2, mka
On Monday 22 June 2020 16:19:19 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>
Hello! I do not know what should be the correct behavior according to
sdio standards, but I tested this patch with DDR50 card and behavior of
that card was not changed, it is working as before.
Tested-by: Pali Rohár <pali@kernel.org>
> ---
> 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mmc: sdio: fix clock rate setting for SDR12/SDR25 mode
2020-06-22 8:19 [PATCH] mmc: sdio: fix clock rate setting for SDR12/SDR25 mode haibo.chen
2020-06-29 8:03 ` Pali Rohár
@ 2020-07-06 14:49 ` Ulf Hansson
2020-07-07 1:48 ` BOUGH CHEN
1 sibling, 1 reply; 5+ messages in thread
From: Ulf Hansson @ 2020-07-06 14:49 UTC (permalink / raw)
To: Haibo Chen
Cc: Adrian Hunter, linux-mmc, Pali Rohár, dl-linux-imx,
fugang.duan, Doug Anderson, huyue2, Matthias Kaehlcke
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?
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH] mmc: sdio: fix clock rate setting for SDR12/SDR25 mode
2020-07-06 14:49 ` Ulf Hansson
@ 2020-07-07 1:48 ` BOUGH CHEN
2020-07-07 7:04 ` Pali Rohár
0 siblings, 1 reply; 5+ messages in thread
From: BOUGH CHEN @ 2020-07-07 1:48 UTC (permalink / raw)
To: Ulf Hansson, stable
Cc: Adrian Hunter, linux-mmc, Pali Rohár, dl-linux-imx,
Andy Duan, Doug Anderson, huyue2, Matthias Kaehlcke
> -----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
> >
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] mmc: sdio: fix clock rate setting for SDR12/SDR25 mode
2020-07-07 1:48 ` BOUGH CHEN
@ 2020-07-07 7:04 ` Pali Rohár
0 siblings, 0 replies; 5+ messages in thread
From: Pali Rohár @ 2020-07-07 7:04 UTC (permalink / raw)
To: BOUGH CHEN
Cc: Ulf Hansson, stable, Adrian Hunter, linux-mmc, dl-linux-imx,
Andy Duan, Doug Anderson, huyue2, Matthias Kaehlcke
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
> > >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-07-07 7:04 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22 8:19 [PATCH] mmc: sdio: fix clock rate setting for SDR12/SDR25 mode haibo.chen
2020-06-29 8:03 ` Pali Rohár
2020-07-06 14:49 ` Ulf Hansson
2020-07-07 1:48 ` BOUGH CHEN
2020-07-07 7:04 ` Pali Rohár
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.