[PATCH] mmc: meson-mx-sdio: trigger a soft reset after a timeout or CRC error

[PATCH] mmc: meson-mx-sdio: trigger a soft reset after a timeout or CRC error

[Martin Blumenstingl]

The vendor driver (from the 3.10 kernel) triggers a soft reset every
time before starting a new command. While this fixes a problem where
SDIO cards are not detected at all (because all commands simply
timed out) this hurts SD card read performance a bit (in my tests
between 10% to 20%).

Trigger a soft reset after we got a CRC error or if the previous command
timed out (just like the vendor driver from the same 3.10 kernel for the
newer SDHC controller IP does). This fixes detection of SDIO cards and
doesn't hurt SD card read performance at the same time.

With this patch the initialization of an RTL8723BS SDIO card looks like
this:
  req done (CMD52): -110: 00000000 00000000 00000000 00000000
  clock 400000Hz busmode 2 powermode 2 cs 1 Vdd 21 width 1 timing 0
  starting CMD0 arg 00000000 flags 000000c0
  req done (CMD0): 0: 00000000 00000000 00000000 00000000
  clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 0
  starting CMD8 arg 000001aa flags 000002f5
  req done (CMD8): -110: 00000000 00000000 00000000 00000000
  starting CMD5 arg 00000000 flags 000002e1
  req done (CMD5): 0: 90ff0000 00000000 00000000 00000000
  starting CMD5 arg 00200000 flags 000002e1
  req done (CMD5): 0: 90ff0000 00000000 00000000 00000000
  starting CMD3 arg 00000000 flags 00000075
  req done (CMD3): 0: 00010000 00000000 00000000 00000000
  starting CMD7 arg 00010000 flags 00000015
  req done (CMD7): 0: 00001e00 00000000 00000000 00000000
  starting CMD52 arg 00000000 flags 00000195
  req done (CMD52): 0: 00001032 00000000 00000000 00000000
  [... more CMD52 omitted ...]
  clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 2
  clock 50000000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 2
  starting CMD52 arg 00000e00 flags 00000195
  req done (CMD52): 0: 00001000 00000000 00000000 00000000
  starting CMD52 arg 80000e02 flags 00000195
  req done (CMD52): 0: 00001002 00000000 00000000 00000000
  clock 50000000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 4 timing 2
  starting CMD52 arg 00020000 flags 00000195
  req done (CMD52): 0: 00001007 00000000 00000000 00000000
  [... more CMD52 omitted ...]
  new high speed SDIO card at address 0001

Fixes: ed80a13bb4c4c9 ("mmc: meson-mx-sdio: Add a driver for the Amlogic Meson8 and Meson8b SoCs")
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
Ulf, I consider this non-critical because as long as the meson-mx-sdhc
driver is not merged we only have one MMC driver for these platforms.
I don't know anybody to prefer SDIO wifi over SD card access, so this
can go into -next (in my option at least).


 drivers/mmc/host/meson-mx-sdio.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/meson-mx-sdio.c b/drivers/mmc/host/meson-mx-sdio.c
index 2e58743d83bb..3813b544f571 100644
--- a/drivers/mmc/host/meson-mx-sdio.c
+++ b/drivers/mmc/host/meson-mx-sdio.c
@@ -246,6 +246,9 @@ static void meson_mx_mmc_request_done(struct meson_mx_mmc_host *host)
 
 	mrq = host->mrq;
 
+	if (host->cmd->error)
+		meson_mx_mmc_soft_reset(host);
+
 	host->mrq = NULL;
 	host->cmd = NULL;
 
-- 
2.26.2

Re: [PATCH] mmc: meson-mx-sdio: trigger a soft reset after a timeout or CRC error

[Ulf Hansson]

On Mon, 4 May 2020 at 00:28, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> The vendor driver (from the 3.10 kernel) triggers a soft reset every
> time before starting a new command. While this fixes a problem where
> SDIO cards are not detected at all (because all commands simply
> timed out) this hurts SD card read performance a bit (in my tests
> between 10% to 20%).
>
> Trigger a soft reset after we got a CRC error or if the previous command
> timed out (just like the vendor driver from the same 3.10 kernel for the
> newer SDHC controller IP does). This fixes detection of SDIO cards and
> doesn't hurt SD card read performance at the same time.
>
> With this patch the initialization of an RTL8723BS SDIO card looks like
> this:
>   req done (CMD52): -110: 00000000 00000000 00000000 00000000
>   clock 400000Hz busmode 2 powermode 2 cs 1 Vdd 21 width 1 timing 0
>   starting CMD0 arg 00000000 flags 000000c0
>   req done (CMD0): 0: 00000000 00000000 00000000 00000000
>   clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 0
>   starting CMD8 arg 000001aa flags 000002f5
>   req done (CMD8): -110: 00000000 00000000 00000000 00000000
>   starting CMD5 arg 00000000 flags 000002e1
>   req done (CMD5): 0: 90ff0000 00000000 00000000 00000000
>   starting CMD5 arg 00200000 flags 000002e1
>   req done (CMD5): 0: 90ff0000 00000000 00000000 00000000
>   starting CMD3 arg 00000000 flags 00000075
>   req done (CMD3): 0: 00010000 00000000 00000000 00000000
>   starting CMD7 arg 00010000 flags 00000015
>   req done (CMD7): 0: 00001e00 00000000 00000000 00000000
>   starting CMD52 arg 00000000 flags 00000195
>   req done (CMD52): 0: 00001032 00000000 00000000 00000000
>   [... more CMD52 omitted ...]
>   clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 2
>   clock 50000000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 2
>   starting CMD52 arg 00000e00 flags 00000195
>   req done (CMD52): 0: 00001000 00000000 00000000 00000000
>   starting CMD52 arg 80000e02 flags 00000195
>   req done (CMD52): 0: 00001002 00000000 00000000 00000000
>   clock 50000000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 4 timing 2
>   starting CMD52 arg 00020000 flags 00000195
>   req done (CMD52): 0: 00001007 00000000 00000000 00000000
>   [... more CMD52 omitted ...]
>   new high speed SDIO card at address 0001
>
> Fixes: ed80a13bb4c4c9 ("mmc: meson-mx-sdio: Add a driver for the Amlogic Meson8 and Meson8b SoCs")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> Ulf, I consider this non-critical because as long as the meson-mx-sdhc
> driver is not merged we only have one MMC driver for these platforms.
> I don't know anybody to prefer SDIO wifi over SD card access, so this
> can go into -next (in my option at least).
>

Alright, applied for next, thanks!

Kind regards
Uffe


>
>  drivers/mmc/host/meson-mx-sdio.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/meson-mx-sdio.c b/drivers/mmc/host/meson-mx-sdio.c
> index 2e58743d83bb..3813b544f571 100644
> --- a/drivers/mmc/host/meson-mx-sdio.c
> +++ b/drivers/mmc/host/meson-mx-sdio.c
> @@ -246,6 +246,9 @@ static void meson_mx_mmc_request_done(struct meson_mx_mmc_host *host)
>
>         mrq = host->mrq;
>
> +       if (host->cmd->error)
> +               meson_mx_mmc_soft_reset(host);
> +
>         host->mrq = NULL;
>         host->cmd = NULL;
>
> --
> 2.26.2
>

Re: [PATCH] mmc: meson-mx-sdio: trigger a soft reset after a timeout or CRC error

[Martin Blumenstingl]

Hi Ulf,

On Thu, May 7, 2020 at 11:29 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Mon, 4 May 2020 at 00:28, Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
> >
> > The vendor driver (from the 3.10 kernel) triggers a soft reset every
> > time before starting a new command. While this fixes a problem where
> > SDIO cards are not detected at all (because all commands simply
> > timed out) this hurts SD card read performance a bit (in my tests
> > between 10% to 20%).
> >
> > Trigger a soft reset after we got a CRC error or if the previous command
> > timed out (just like the vendor driver from the same 3.10 kernel for the
> > newer SDHC controller IP does). This fixes detection of SDIO cards and
> > doesn't hurt SD card read performance at the same time.
> >
> > With this patch the initialization of an RTL8723BS SDIO card looks like
> > this:
> >   req done (CMD52): -110: 00000000 00000000 00000000 00000000
> >   clock 400000Hz busmode 2 powermode 2 cs 1 Vdd 21 width 1 timing 0
> >   starting CMD0 arg 00000000 flags 000000c0
> >   req done (CMD0): 0: 00000000 00000000 00000000 00000000
> >   clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 0
> >   starting CMD8 arg 000001aa flags 000002f5
> >   req done (CMD8): -110: 00000000 00000000 00000000 00000000
> >   starting CMD5 arg 00000000 flags 000002e1
> >   req done (CMD5): 0: 90ff0000 00000000 00000000 00000000
> >   starting CMD5 arg 00200000 flags 000002e1
> >   req done (CMD5): 0: 90ff0000 00000000 00000000 00000000
> >   starting CMD3 arg 00000000 flags 00000075
> >   req done (CMD3): 0: 00010000 00000000 00000000 00000000
> >   starting CMD7 arg 00010000 flags 00000015
> >   req done (CMD7): 0: 00001e00 00000000 00000000 00000000
> >   starting CMD52 arg 00000000 flags 00000195
> >   req done (CMD52): 0: 00001032 00000000 00000000 00000000
> >   [... more CMD52 omitted ...]
> >   clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 2
> >   clock 50000000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 2
> >   starting CMD52 arg 00000e00 flags 00000195
> >   req done (CMD52): 0: 00001000 00000000 00000000 00000000
> >   starting CMD52 arg 80000e02 flags 00000195
> >   req done (CMD52): 0: 00001002 00000000 00000000 00000000
> >   clock 50000000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 4 timing 2
> >   starting CMD52 arg 00020000 flags 00000195
> >   req done (CMD52): 0: 00001007 00000000 00000000 00000000
> >   [... more CMD52 omitted ...]
> >   new high speed SDIO card at address 0001
> >
> > Fixes: ed80a13bb4c4c9 ("mmc: meson-mx-sdio: Add a driver for the Amlogic Meson8 and Meson8b SoCs")
> > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > ---
> > Ulf, I consider this non-critical because as long as the meson-mx-sdhc
> > driver is not merged we only have one MMC driver for these platforms.
> > I don't know anybody to prefer SDIO wifi over SD card access, so this
> > can go into -next (in my option at least).
> >
>
> Alright, applied for next, thanks!
thank you for taking this patch!

I received a confirmation (off-list) saying that this patch works as it should.
Tobias was not Cc'ed on the original mail so he gave me to permission
(again, off-list - he's Cc'ed now) to add his:
Tested-by: Tobias Baumann <017623705678@o2online.de>


Martin

Re: [PATCH] mmc: meson-mx-sdio: trigger a soft reset after a timeout or CRC error

[Ulf Hansson]

On Thu, 7 May 2020 at 23:14, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Ulf,
>
> On Thu, May 7, 2020 at 11:29 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >
> > On Mon, 4 May 2020 at 00:28, Martin Blumenstingl
> > <martin.blumenstingl@googlemail.com> wrote:
> > >
> > > The vendor driver (from the 3.10 kernel) triggers a soft reset every
> > > time before starting a new command. While this fixes a problem where
> > > SDIO cards are not detected at all (because all commands simply
> > > timed out) this hurts SD card read performance a bit (in my tests
> > > between 10% to 20%).
> > >
> > > Trigger a soft reset after we got a CRC error or if the previous command
> > > timed out (just like the vendor driver from the same 3.10 kernel for the
> > > newer SDHC controller IP does). This fixes detection of SDIO cards and
> > > doesn't hurt SD card read performance at the same time.
> > >
> > > With this patch the initialization of an RTL8723BS SDIO card looks like
> > > this:
> > >   req done (CMD52): -110: 00000000 00000000 00000000 00000000
> > >   clock 400000Hz busmode 2 powermode 2 cs 1 Vdd 21 width 1 timing 0
> > >   starting CMD0 arg 00000000 flags 000000c0
> > >   req done (CMD0): 0: 00000000 00000000 00000000 00000000
> > >   clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 0
> > >   starting CMD8 arg 000001aa flags 000002f5
> > >   req done (CMD8): -110: 00000000 00000000 00000000 00000000
> > >   starting CMD5 arg 00000000 flags 000002e1
> > >   req done (CMD5): 0: 90ff0000 00000000 00000000 00000000
> > >   starting CMD5 arg 00200000 flags 000002e1
> > >   req done (CMD5): 0: 90ff0000 00000000 00000000 00000000
> > >   starting CMD3 arg 00000000 flags 00000075
> > >   req done (CMD3): 0: 00010000 00000000 00000000 00000000
> > >   starting CMD7 arg 00010000 flags 00000015
> > >   req done (CMD7): 0: 00001e00 00000000 00000000 00000000
> > >   starting CMD52 arg 00000000 flags 00000195
> > >   req done (CMD52): 0: 00001032 00000000 00000000 00000000
> > >   [... more CMD52 omitted ...]
> > >   clock 400000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 2
> > >   clock 50000000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 1 timing 2
> > >   starting CMD52 arg 00000e00 flags 00000195
> > >   req done (CMD52): 0: 00001000 00000000 00000000 00000000
> > >   starting CMD52 arg 80000e02 flags 00000195
> > >   req done (CMD52): 0: 00001002 00000000 00000000 00000000
> > >   clock 50000000Hz busmode 2 powermode 2 cs 0 Vdd 21 width 4 timing 2
> > >   starting CMD52 arg 00020000 flags 00000195
> > >   req done (CMD52): 0: 00001007 00000000 00000000 00000000
> > >   [... more CMD52 omitted ...]
> > >   new high speed SDIO card at address 0001
> > >
> > > Fixes: ed80a13bb4c4c9 ("mmc: meson-mx-sdio: Add a driver for the Amlogic Meson8 and Meson8b SoCs")
> > > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> > > ---
> > > Ulf, I consider this non-critical because as long as the meson-mx-sdhc
> > > driver is not merged we only have one MMC driver for these platforms.
> > > I don't know anybody to prefer SDIO wifi over SD card access, so this
> > > can go into -next (in my option at least).
> > >
> >
> > Alright, applied for next, thanks!
> thank you for taking this patch!
>
> I received a confirmation (off-list) saying that this patch works as it should.
> Tobias was not Cc'ed on the original mail so he gave me to permission
> (again, off-list - he's Cc'ed now) to add his:
> Tested-by: Tobias Baumann <017623705678@o2online.de>

I will add it next time I rebase my branch, probably on Monday, thanks!

Kind regards
Uffe