[RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning cmds

[RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning cmds

[Wolfram Sang]

Some SD cards failed to tune SDR104 occasionally. Adding a delay between
tuning commands makes the cards work although it is not yet known why.
It seems to be host dependent, only a few need such a delay (I found AMD
SDHCI and Freescale eSDHC so far). 25us is not enough, 50us never failed
up to now, so let's double that value until we know what is going on.
Extra thank you to Adam for sending me a problematic SD card to test.

Reported-by: Adam Ford <aford173@gmail.com>
Closes: https://lore.kernel.org/all/CAHCN7xK_fr_gREVsOzN=atcS08mwufr-=7q1JAN=CCyVk_k-dA@mail.gmail.com/
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

This is one of the patches which look simple but took quite a while to
come up with. Part of it was identifying the issue, part of it was
trying to understand the issue. But I am stuck now, so I need help:

@Adam: could you kindly test this patch with your setup? First, we need
to make sure it helps your case as well.

@Shimoda: If it helps for Adam's case, maybe we could ask the HW team if
they see a reason for this delay? I already scanned the SD/MMC/SDHI specs
without finding any trace, Marek helped as well. My gut feeling says it
is host controller specific. But first, let's hope it works for Adam.

 drivers/mmc/host/renesas_sdhi_core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 345934e4f59e..6088cf94e1d3 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -711,6 +711,9 @@ static int renesas_sdhi_execute_tuning(struct mmc_host *mmc, u32 opcode)
 
 		if (cmd_error)
 			mmc_send_abort_tuning(mmc, opcode);
+
+		/* FIXME: Needed for some SD cards. The reason is not known yet */
+		usleep_range(100, 250);
 	}
 
 	ret = renesas_sdhi_select_tuning(host);
-- 
2.30.2

Re: [RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning cmds

[Adam Ford]

On Wed, May 31, 2023 at 2:00 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> Some SD cards failed to tune SDR104 occasionally. Adding a delay between
> tuning commands makes the cards work although it is not yet known why.
> It seems to be host dependent, only a few need such a delay (I found AMD
> SDHCI and Freescale eSDHC so far). 25us is not enough, 50us never failed
> up to now, so let's double that value until we know what is going on.
> Extra thank you to Adam for sending me a problematic SD card to test.
>
> Reported-by: Adam Ford <aford173@gmail.com>
> Closes: https://lore.kernel.org/all/CAHCN7xK_fr_gREVsOzN=atcS08mwufr-=7q1JAN=CCyVk_k-dA@mail.gmail.com/
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> This is one of the patches which look simple but took quite a while to
> come up with. Part of it was identifying the issue, part of it was
> trying to understand the issue. But I am stuck now, so I need help:
>
> @Adam: could you kindly test this patch with your setup? First, we need
> to make sure it helps your case as well.

I am testing this on a 5.10 kernel based on the Renesas RZ linux
kernel branch.  The good news is that the card doesn't hang like it
used to on the older 4.19 kernel, but it still does not enumerate to
SDR104 speeds.

[    5.222863] mmc0: tuning execution failed: -5
[    5.308026] mmc0: new high speed SDHC card at address 59b4
[    5.314397] mmcblk0: mmc0:59b4 USD00 29.5 GiB
[    5.320745]  mmcblk0: p1 p2

# cat /sys/kernel/debug/mmc0/ios

clock: 50000000 Hz
actual clock: 49999998 Hz
vdd: 21 (3.3 ~ 3.4 V)
bus mode: 2 (push-pull)
chip select: 0 (don't care)
power mode: 2 (on)
bus width: 2 (4 bits)
timing spec: 2 (sd high-speed)
signal voltage: 0 (3.30 V)
driver type: 0 (driver type B)

On other platforms, this same card registers 200MHz at SDR104 speeds.

I'll try again on the latest 6.4 RC branch to see if there is some
progress there.

adam
>
> @Shimoda: If it helps for Adam's case, maybe we could ask the HW team if
> they see a reason for this delay? I already scanned the SD/MMC/SDHI specs
> without finding any trace, Marek helped as well. My gut feeling says it
> is host controller specific. But first, let's hope it works for Adam.
>
>  drivers/mmc/host/renesas_sdhi_core.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 345934e4f59e..6088cf94e1d3 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -711,6 +711,9 @@ static int renesas_sdhi_execute_tuning(struct mmc_host *mmc, u32 opcode)
>
>                 if (cmd_error)
>                         mmc_send_abort_tuning(mmc, opcode);
> +
> +               /* FIXME: Needed for some SD cards. The reason is not known yet */
> +               usleep_range(100, 250);
>         }
>
>         ret = renesas_sdhi_select_tuning(host);
> --
> 2.30.2
>

Re: [RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning cmds

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]


> I am testing this on a 5.10 kernel based on the Renesas RZ linux
> kernel branch.  The good news is that the card doesn't hang like it
> used to on the older 4.19 kernel, but it still does not enumerate to
> SDR104 speeds.

U-Boot has a 1ms delay. Could you increase the delay to 1ms, please?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning cmds

[Adam Ford]

On Wed, May 31, 2023 at 12:45 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
>
> > I am testing this on a 5.10 kernel based on the Renesas RZ linux
> > kernel branch.  The good news is that the card doesn't hang like it
> > used to on the older 4.19 kernel, but it still does not enumerate to
> > SDR104 speeds.
>
> U-Boot has a 1ms delay. Could you increase the delay to 1ms, please?

Sorry for the delay, but I increased this to  usleep_range(1000, 2500)
and it still hangs.

I don't know if the issue is dependent on the variant.  For what it's
worth, I am testing an RZ/G2M, but I can do the same thing on an
RZ/G2N or RZ/G2H if you want.

adam
>

RE: [RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning cmds

[Biju Das]

Hi Adam,

> -----Original Message-----
> From: Adam Ford <aford173@gmail.com>
> Sent: Friday, June 16, 2023 3:35 PM
> To: Wolfram Sang <wsa+renesas@sang-engineering.com>; Adam Ford
> <aford173@gmail.com>; linux-mmc@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> Subject: Re: [RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning
> cmds
> 
> On Wed, May 31, 2023 at 12:45 PM Wolfram Sang <wsa+renesas@sang-
> engineering.com> wrote:
> >
> >
> > > I am testing this on a 5.10 kernel based on the Renesas RZ linux
> > > kernel branch.  The good news is that the card doesn't hang like it
> > > used to on the older 4.19 kernel, but it still does not enumerate to
> > > SDR104 speeds.
> >
> > U-Boot has a 1ms delay. Could you increase the delay to 1ms, please?
> 
> Sorry for the delay, but I increased this to  usleep_range(1000, 2500) and
> it still hangs.
> 
> I don't know if the issue is dependent on the variant.  For what it's
> worth, I am testing an RZ/G2M, but I can do the same thing on an RZ/G2N or
> RZ/G2H if you want.

I am doing 6.1.y testing[1] with all RZ board variants,
[1] https://git.kernel.org/pub/scm/linux/kernel/git/cip/linux-cip.git/log/?h=linux-6.1.y-cip

I have done SDHI testing with RZ/G2M 1.3 HiHope board and I didn't face this issue.

Can you please let me know how to reproduce this issue?

Cheers,
Biju

Re: [RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning cmds

[Adam Ford]

On Fri, Jun 16, 2023 at 9:39 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Adam,
>
> > -----Original Message-----
> > From: Adam Ford <aford173@gmail.com>
> > Sent: Friday, June 16, 2023 3:35 PM
> > To: Wolfram Sang <wsa+renesas@sang-engineering.com>; Adam Ford
> > <aford173@gmail.com>; linux-mmc@vger.kernel.org; linux-renesas-
> > soc@vger.kernel.org; Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> > Subject: Re: [RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning
> > cmds
> >
> > On Wed, May 31, 2023 at 12:45 PM Wolfram Sang <wsa+renesas@sang-
> > engineering.com> wrote:
> > >
> > >
> > > > I am testing this on a 5.10 kernel based on the Renesas RZ linux
> > > > kernel branch.  The good news is that the card doesn't hang like it
> > > > used to on the older 4.19 kernel, but it still does not enumerate to
> > > > SDR104 speeds.
> > >
> > > U-Boot has a 1ms delay. Could you increase the delay to 1ms, please?
> >
> > Sorry for the delay, but I increased this to  usleep_range(1000, 2500) and
> > it still hangs.
> >
> > I don't know if the issue is dependent on the variant.  For what it's
> > worth, I am testing an RZ/G2M, but I can do the same thing on an RZ/G2N or
> > RZ/G2H if you want.
>
> I am doing 6.1.y testing[1] with all RZ board variants,
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/cip/linux-cip.git/log/?h=linux-6.1.y-cip
>
> I have done SDHI testing with RZ/G2M 1.3 HiHope board and I didn't face this issue.
>
> Can you please let me know how to reproduce this issue?

It's not easy to reproduce, and it only happens on a small subset of
cards.  I sent one to Wolfram sometime last year.  I can also  take
these same cards that hang when booting Linux, and they enumerate just
fine in U-Boot at SRD104 speeds.  I can use the same RZ/G2 board with
other cards, and other cards work fine.  I then tested these same
'failing' cards on other platforms and they work fine, so it seems
like there is a tuning combination in Linux that is different from
U-Boot that is unique to these certain cards which prevents them from
working.

adam


>
> Cheers,
> Biju

Re: [RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning cmds

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 388 bytes --]

Hi Adam,

> > U-Boot has a 1ms delay. Could you increase the delay to 1ms, please?
> 
> Sorry for the delay, but I increased this to  usleep_range(1000, 2500)
> and it still hangs.

Sigh, back to the drawing board... But still thanks for trying! I'll try
to come up with something better after EOSS in Prague (will you be there
by any chance?)

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning cmds

[Adam Ford]

On Mon, Jun 19, 2023 at 1:02 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> Hi Adam,
>
> > > U-Boot has a 1ms delay. Could you increase the delay to 1ms, please?
> >
> > Sorry for the delay, but I increased this to  usleep_range(1000, 2500)
> > and it still hangs.
>
> Sigh, back to the drawing board... But still thanks for trying! I'll try
> to come up with something better after EOSS in Prague (will you be there
> by any chance?)

I really tried to go, but my employer declined this time.  I was going
to go on my own, butI didn't have enough personal time off available
to fly out there.

adam
>
> Happy hacking,
>
>    Wolfram
>

Re: [RFC/RFT PATCH] mmc: renesas_sdhi: add delay between tuning cmds

[Adam Ford]

On Mon, Jun 19, 2023 at 5:39 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Mon, Jun 19, 2023 at 1:02 AM Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
> >
> > Hi Adam,
> >
> > > > U-Boot has a 1ms delay. Could you increase the delay to 1ms, please?
> > >
> > > Sorry for the delay, but I increased this to  usleep_range(1000, 2500)
> > > and it still hangs.
> >
> > Sigh, back to the drawing board... But still thanks for trying! I'll try
> > to come up with something better after EOSS in Prague (will you be there
> > by any chance?)

Wolfram,

I wanted to test the 6.7-rc1 on the microSD cards to see if there has
been any improvement.  In the process, I found yet another card which
appears to have a similar issue where U-Boot can initialize a microSD
card in SDR104 mode, but Linux fails.  I think this brings the total
to 3 cards.  These same cards work on other platforms, so I don't
think there is anything wrong with the cards, especially since they
work on this platform in U-Boot.

 I added some debug code to indicate when when the priv->taps and
priv->smpcmp bits are set when executing the timing command.  FWICT,
the priv-taps never get set, and smpcmp appears to always be
successful.

[    3.741462] priv->smpcmp set bit 0
[    3.747404] mmc_send_tuning opcode 19
[    3.751173] priv->smpcmp set bit 1
[    3.757129] mmc_send_tuning opcode 19
[    3.760860] priv->smpcmp set bit 2
[    3.766815] mmc_send_tuning opcode 19
[    3.770564] priv->smpcmp set bit 3
[    3.776514] mmc_send_tuning opcode 19
[    3.780262] priv->smpcmp set bit 4
[    3.786213] mmc_send_tuning opcode 19
[    3.789966] priv->smpcmp set bit 5
[    3.795915] mmc_send_tuning opcode 19
[    3.799662] priv->smpcmp set bit 6
[    3.805611] mmc_send_tuning opcode 19
[    3.809364] priv->smpcmp set bit 7
[    3.815294] mmc_send_tuning opcode 19
[    3.819040] priv->smpcmp set bit 8
[    3.824988] mmc_send_tuning opcode 19
[    3.828735] priv->smpcmp set bit 9
[    3.834685] mmc_send_tuning opcode 19
[    3.838432] priv->smpcmp set bit 10
[    3.844466] mmc_send_tuning opcode 19
[    3.848214] priv->smpcmp set bit 11
[    3.854251] mmc_send_tuning opcode 19
[    3.857997] priv->smpcmp set bit 12
[    3.864032] mmc_send_tuning opcode 19
[    3.867778] priv->smpcmp set bit 13
[    3.873820] mmc_send_tuning opcode 19
[    3.877566] priv->smpcmp set bit 14
[    3.883582] mmc_send_tuning opcode 19
[    3.887328] priv->smpcmp set bit 15
[    3.893361] priv->tap_num = 8
[    3.896332] taps_size = 16
[    3.899059] tap_cnt = 0
[    3.901518] min_tap_row = 3
[    3.904312] renesas_sdhi_select_tuning failed
[    3.908732] mmc1: tuning execution failed: -5

When I compare the values of taps and smpcmp in U-Boot vs linux, I get
U-Boot Shows:
taps = dfdf
priv->smpcmp = dfdf
Sometimes,  priv->smpcmp = df9f

Linux shows:
priv->taps = 0
priv->smpcmp = ffff

And the Linux values are consistent.

Another thing that is interesting to me is that U-Boot explicitly
forces PIO when tuning and it only happens when executing "ret =
mmc_send_tuning(mmc, opcode, NULL);"  Is there a proper way to
temporarily switch to PIO in the Linux driver to see if I can
accomplish the same thing?

If I eject the card before it's mounted, it waits for the card to be
inserted.  If I insert the card, it enumerates:

[    3.701926] renesas_sdhi_start_signal_voltage_switch: 0
[    3.869834] renesas_sdhi_start_signal_voltage_switch: 1
[    3.898030] mmc_send_tuning opcode 19
[    3.907006] priv->taps set bit 0
[    3.910232] priv->smpcmp set bit 0
[    3.916160] mmc_send_tuning opcode 19
[    3.919901] priv->taps set bit 1
[    3.923142] priv->smpcmp set bit 1
[    3.929054] mmc_send_tuning opcode 19
[    3.932793] priv->taps set bit 2
[    3.936034] priv->smpcmp set bit 2
[    3.941953] mmc_send_tuning opcode 19
[    3.945697] priv->taps set bit 3
[    3.948938] priv->smpcmp set bit 3
[    3.954857] mmc_send_tuning opcode 19
[    3.958596] priv->taps set bit 4
[    3.961836] priv->smpcmp set bit 4
[    3.967756] mmc_send_tuning opcode 19
[    3.974009] mmc_send_tuning opcode 19
[    3.977749] priv->taps set bit 6
[    3.980987] priv->smpcmp set bit 6
[    3.986910] mmc_send_tuning opcode 19
[    3.990636] priv->taps set bit 7
[    3.993876] priv->smpcmp set bit 7
[    3.999797] mmc_send_tuning opcode 19
[    4.003535] priv->taps set bit 8
[    4.006760] priv->smpcmp set bit 8
[    4.012688] mmc_send_tuning opcode 19
[    4.016427] priv->taps set bit 9
[    4.019668] priv->smpcmp set bit 9
[    4.025587] mmc_send_tuning opcode 19
[    4.029326] priv->taps set bit 10
[    4.032654] priv->smpcmp set bit 10
[    4.038689] mmc_send_tuning opcode 19
[    4.042422] priv->taps set bit 11
[    4.045745] priv->smpcmp set bit 11
[    4.051748] mmc_send_tuning opcode 19
[    4.055484] priv->taps set bit 12
[    4.058795] priv->smpcmp set bit 12
[    4.064803] mmc_send_tuning opcode 19
[    4.071044] mmc_send_tuning opcode 19
[    4.074752] priv->taps set bit 14
[    4.078075] priv->smpcmp set bit 14
[    4.084077] mmc_send_tuning opcode 19
[    4.087832] priv->taps set bit 15
[    4.091157] priv->smpcmp set bit 15
[    4.097152] priv->tap_num = 8
[    4.100127] taps_size = 16
[    4.102828] priv->taps = dfdf
[    4.105800] priv->smpcmp = dfdf
[    4.108947] tap_cnt = 7
[    4.111395] min_tap_row = 3
[    4.116003] mmc1: new ultra high speed SDR104 SDHC card at address 59b4
[    4.123361] mmcblk1: mmc1:59b4 USD00 29.5 GiB
[    4.129577]  mmcblk1: p1 p2


Is there  some sort of reset or power-cycle that needs to be better configured?

thanks,

adam
>
> I really tried to go, but my employer declined this time.  I was going
> to go on my own, butI didn't have enough personal time off available
> to fly out there.
>
> adam
> >
> > Happy hacking,
> >
> >    Wolfram
> >