[RFT] arm64: dts: renesas: salvator-common: enable SDR104 for SD cards

[RFT] arm64: dts: renesas: salvator-common: enable SDR104 for SD cards

[Wolfram Sang]

Tests showed that SDHI driver problems have been solved and SDR104 works
now reliably on both Salvator-X SD card slots, both with an R-Car H3
(r8a7795 ES1.0) and R-Car M3 (r8a7796 ES 1.0). So, finally, enable it.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Awesome, awesome! For the last two days, I did various tests with my boards and
SD cards and SDR104 worked absolutely reliably :D Even the problematic card I
had works flawlessly. I couldn't trigger the known failures anymore. Although I
tried, I can't point to a single patch which "fixed" the issue. It is the
constant work on dealing with smaller issues which makes SDR104 work at the end
of the day. However, two patches are likely a bigger part of the cake:

43b0b361b01700 ("mmc: tmio: always get number of taps")
-> Already upstream. This fixed retuning on card changes.

85b81aa16ec4ed ("mmc: tmio: fix CMD12 (STOP) handling")
-> In mmc/next. This allows to handle tuning errors gracefully and to retune.

With the freshly submitted Gen3 DMA patches, we also get nice transfer speeds :)

This patch is based on renesas-drivers/master as of today. A branch for testing
can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/v8-sdr104

All patches needed for reliable SDR104 are already upstream or in mmc/next. The
above branch only adds another patch to enable DMA on Gen3. It is not strictly
needed, but very nice to have. I think it would be cool to get both patches
into v4.14.

Looking forward to other testers. Simon, do you have some time for this?

Thanks and kind regards,

  a very happy Wolfram


 arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
index a451996f590a51..18e2da9f866684 100644
--- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
+++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
@@ -569,6 +569,7 @@
 	wp-gpios = <&gpio3 13 GPIO_ACTIVE_HIGH>;
 	bus-width = <4>;
 	sd-uhs-sdr50;
+	sd-uhs-sdr104;
 	status = "okay";
 };
 
@@ -597,6 +598,7 @@
 	wp-gpios = <&gpio4 16 GPIO_ACTIVE_HIGH>;
 	bus-width = <4>;
 	sd-uhs-sdr50;
+	sd-uhs-sdr104;
 	status = "okay";
 };
 
-- 
2.11.0

Re: [RFT] arm64: dts: renesas: salvator-common: enable SDR104 for SD cards

[Simon Horman]

On Wed, Aug 02, 2017 at 10:13:11PM +0200, Wolfram Sang wrote:
> Tests showed that SDHI driver problems have been solved and SDR104 works
> now reliably on both Salvator-X SD card slots, both with an R-Car H3
> (r8a7795 ES1.0) and R-Car M3 (r8a7796 ES 1.0). So, finally, enable it.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Awesome, awesome! For the last two days, I did various tests with my boards and
> SD cards and SDR104 worked absolutely reliably :D Even the problematic card I
> had works flawlessly. I couldn't trigger the known failures anymore. Although I
> tried, I can't point to a single patch which "fixed" the issue. It is the
> constant work on dealing with smaller issues which makes SDR104 work at the end
> of the day. However, two patches are likely a bigger part of the cake:
> 
> 43b0b361b01700 ("mmc: tmio: always get number of taps")
> -> Already upstream. This fixed retuning on card changes.
> 
> 85b81aa16ec4ed ("mmc: tmio: fix CMD12 (STOP) handling")
> -> In mmc/next. This allows to handle tuning errors gracefully and to retune.
> 
> With the freshly submitted Gen3 DMA patches, we also get nice transfer speeds :)
> 
> This patch is based on renesas-drivers/master as of today. A branch for testing
> can be found here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/v8-sdr104
> 
> All patches needed for reliable SDR104 are already upstream or in mmc/next. The
> above branch only adds another patch to enable DMA on Gen3. It is not strictly
> needed, but very nice to have. I think it would be cool to get both patches
> into v4.14.
> 
> Looking forward to other testers. Simon, do you have some time for this?

Yes, I should be able to run this through my - until now thought to be
broken - test case this week.

It would be very nice to be able to get closure on this :)

> 
> Thanks and kind regards,
> 
>   a very happy Wolfram
> 
> 
>  arch/arm64/boot/dts/renesas/salvator-common.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/salvator-common.dtsi b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> index a451996f590a51..18e2da9f866684 100644
> --- a/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> +++ b/arch/arm64/boot/dts/renesas/salvator-common.dtsi
> @@ -569,6 +569,7 @@
>  	wp-gpios = <&gpio3 13 GPIO_ACTIVE_HIGH>;
>  	bus-width = <4>;
>  	sd-uhs-sdr50;
> +	sd-uhs-sdr104;
>  	status = "okay";
>  };
>  
> @@ -597,6 +598,7 @@
>  	wp-gpios = <&gpio4 16 GPIO_ACTIVE_HIGH>;
>  	bus-width = <4>;
>  	sd-uhs-sdr50;
> +	sd-uhs-sdr104;
>  	status = "okay";
>  };
>  
> -- 
> 2.11.0
> 

Re: [RFT] arm64: dts: renesas: salvator-common: enable SDR104 for SD cards

[Simon Horman]

On Thu, Aug 03, 2017 at 09:43:51AM +0200, Simon Horman wrote:
> On Wed, Aug 02, 2017 at 10:13:11PM +0200, Wolfram Sang wrote:
> > Tests showed that SDHI driver problems have been solved and SDR104 works
> > now reliably on both Salvator-X SD card slots, both with an R-Car H3
> > (r8a7795 ES1.0) and R-Car M3 (r8a7796 ES 1.0). So, finally, enable it.
> > 
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> > 
> > Awesome, awesome! For the last two days, I did various tests with my boards and
> > SD cards and SDR104 worked absolutely reliably :D Even the problematic card I
> > had works flawlessly. I couldn't trigger the known failures anymore. Although I
> > tried, I can't point to a single patch which "fixed" the issue. It is the
> > constant work on dealing with smaller issues which makes SDR104 work at the end
> > of the day. However, two patches are likely a bigger part of the cake:
> > 
> > 43b0b361b01700 ("mmc: tmio: always get number of taps")
> > -> Already upstream. This fixed retuning on card changes.
> > 
> > 85b81aa16ec4ed ("mmc: tmio: fix CMD12 (STOP) handling")
> > -> In mmc/next. This allows to handle tuning errors gracefully and to retune.
> > 
> > With the freshly submitted Gen3 DMA patches, we also get nice transfer speeds :)
> > 
> > This patch is based on renesas-drivers/master as of today. A branch for testing
> > can be found here:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/v8-sdr104
> > 
> > All patches needed for reliable SDR104 are already upstream or in mmc/next. The
> > above branch only adds another patch to enable DMA on Gen3. It is not strictly
> > needed, but very nice to have. I think it would be cool to get both patches
> > into v4.14.
> > 
> > Looking forward to other testers. Simon, do you have some time for this?
> 
> Yes, I should be able to run this through my - until now thought to be
> broken - test case this week.
> 
> It would be very nice to be able to get closure on this :)

Awesome indeed!

I have tested this using my test-case which is to remove and then
re-insert a Samsung MB-SD32D/EU card. Slot SDHI3 was used although I expect
that is not an important parameter.

A bisection indicates the above test works since
85b81aa16ec4ed ("mmc: tmio: fix CMD12 (STOP) handling")
which was included in v4.12 and highlighted by you above.

Tested-by: Simon Horman <horms@verge.net.au>

I will apply this for v4.14 in the near future.

Re: [RFT] arm64: dts: renesas: salvator-common: enable SDR104 for SD cards

[Simon Horman]

On Thu, Aug 03, 2017 at 05:54:00PM +0200, Simon Horman wrote:
> On Thu, Aug 03, 2017 at 09:43:51AM +0200, Simon Horman wrote:
> > On Wed, Aug 02, 2017 at 10:13:11PM +0200, Wolfram Sang wrote:
> > > Tests showed that SDHI driver problems have been solved and SDR104 works
> > > now reliably on both Salvator-X SD card slots, both with an R-Car H3
> > > (r8a7795 ES1.0) and R-Car M3 (r8a7796 ES 1.0). So, finally, enable it.
> > > 
> > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > ---
> > > 
> > > Awesome, awesome! For the last two days, I did various tests with my boards and
> > > SD cards and SDR104 worked absolutely reliably :D Even the problematic card I
> > > had works flawlessly. I couldn't trigger the known failures anymore. Although I
> > > tried, I can't point to a single patch which "fixed" the issue. It is the
> > > constant work on dealing with smaller issues which makes SDR104 work at the end
> > > of the day. However, two patches are likely a bigger part of the cake:
> > > 
> > > 43b0b361b01700 ("mmc: tmio: always get number of taps")
> > > -> Already upstream. This fixed retuning on card changes.
> > > 
> > > 85b81aa16ec4ed ("mmc: tmio: fix CMD12 (STOP) handling")
> > > -> In mmc/next. This allows to handle tuning errors gracefully and to retune.
> > > 
> > > With the freshly submitted Gen3 DMA patches, we also get nice transfer speeds :)
> > > 
> > > This patch is based on renesas-drivers/master as of today. A branch for testing
> > > can be found here:
> > > 
> > > git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/v8-sdr104
> > > 
> > > All patches needed for reliable SDR104 are already upstream or in mmc/next. The
> > > above branch only adds another patch to enable DMA on Gen3. It is not strictly
> > > needed, but very nice to have. I think it would be cool to get both patches
> > > into v4.14.
> > > 
> > > Looking forward to other testers. Simon, do you have some time for this?
> > 
> > Yes, I should be able to run this through my - until now thought to be
> > broken - test case this week.
> > 
> > It would be very nice to be able to get closure on this :)
> 
> Awesome indeed!
> 
> I have tested this using my test-case which is to remove and then
> re-insert a Samsung MB-SD32D/EU card. Slot SDHI3 was used although I expect
> that is not an important parameter.
> 
> A bisection indicates the above test works since
> 85b81aa16ec4ed ("mmc: tmio: fix CMD12 (STOP) handling")
> which was included in v4.12 and highlighted by you above.
> 
> Tested-by: Simon Horman <horms@verge.net.au>
> 
> I will apply this for v4.14 in the near future.

Oops, I got a little excited there.
As it is an "RFT" I'm happy to apply it if you repost
it or otherwise indicate that is what you would like to happen.

Re: [RFT] arm64: dts: renesas: salvator-common: enable SDR104 for SD cards

[Wolfram Sang]

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


> As it is an "RFT" I'm happy to apply it if you repost
> it or otherwise indicate that is what you would like to happen.

As discussed in a chat, I tried SDR104 with my SDIO WiFi cards:

H3:
- one slot worked flawlessly
- one slot could load FW but failed to read status
(with SDR50: both slots work)

M3-W:
- both slots could not load the firmware
(with SDR50: one slot works, one fails to load FW)

The cards, however, were correctly identified as SDR104 and there were
no tuning errors and no other SDHI related warnings.

Changing TDSEL in the PFC did not change anything.

The manual of the WiFi card (u-blox EMMY-W1) mentions a maximum line
length of 100mm. Measuring a direct line from the SoC to the slots,
I'd think we are very much on the edge of that. And given the length
of the SDIO adapter, we are surely exceeding that :(

Maybe we should do more tests with more boards? On the other hand, it
seems the WiFi card is running out of the specs.

So much for now. I'll sleep over it and try to get more data tomorrow.

Regards,

   Wolfram


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

Re: [RFT] arm64: dts: renesas: salvator-common: enable SDR104 for SD cards

[Simon Horman]

On Tue, Aug 08, 2017 at 08:54:45PM +0200, Wolfram Sang wrote:
> 
> > As it is an "RFT" I'm happy to apply it if you repost
> > it or otherwise indicate that is what you would like to happen.
> 
> As discussed in a chat, I tried SDR104 with my SDIO WiFi cards:
> 
> H3:
> - one slot worked flawlessly
> - one slot could load FW but failed to read status
> (with SDR50: both slots work)
> 
> M3-W:
> - both slots could not load the firmware
> (with SDR50: one slot works, one fails to load FW)
> 
> The cards, however, were correctly identified as SDR104 and there were
> no tuning errors and no other SDHI related warnings.
> 
> Changing TDSEL in the PFC did not change anything.
> 
> The manual of the WiFi card (u-blox EMMY-W1) mentions a maximum line
> length of 100mm. Measuring a direct line from the SoC to the slots,
> I'd think we are very much on the edge of that. And given the length
> of the SDIO adapter, we are surely exceeding that :(
> 
> Maybe we should do more tests with more boards? On the other hand, it
> seems the WiFi card is running out of the specs.
> 
> So much for now. I'll sleep over it and try to get more data tomorrow.

Thanks. Unfortunately my H3 is out of arms length but I can arrange
some testing if it is helpful.

Is one possibility to enable it for the slots listed above that are now
thought to work but not for the other one?

The SDR50 failure on M3-W seems particularly troubling as that
has been enabled in upstream for a while, right?

Re: [RFT] arm64: dts: renesas: salvator-common: enable SDR104 for SD cards

[Wolfram Sang]

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

Hi Simon,

On Mon, Aug 14, 2017 at 07:10:23AM +0200, Simon Horman wrote:
> On Tue, Aug 08, 2017 at 08:54:45PM +0200, Wolfram Sang wrote:
> > 
> > > As it is an "RFT" I'm happy to apply it if you repost
> > > it or otherwise indicate that is what you would like to happen.
> > 
> > As discussed in a chat, I tried SDR104 with my SDIO WiFi cards:
> > 
> > H3:
> > - one slot worked flawlessly
> > - one slot could load FW but failed to read status
> > (with SDR50: both slots work)
> > 
> > M3-W:
> > - both slots could not load the firmware
> > (with SDR50: one slot works, one fails to load FW)
> > 
> > The cards, however, were correctly identified as SDR104 and there were
> > no tuning errors and no other SDHI related warnings.
> > 
> > Changing TDSEL in the PFC did not change anything.
> > 
> > The manual of the WiFi card (u-blox EMMY-W1) mentions a maximum line
> > length of 100mm. Measuring a direct line from the SoC to the slots,
> > I'd think we are very much on the edge of that. And given the length
> > of the SDIO adapter, we are surely exceeding that :(
> > 
> > Maybe we should do more tests with more boards? On the other hand, it
> > seems the WiFi card is running out of the specs.
> > 
> > So much for now. I'll sleep over it and try to get more data tomorrow.
> 
> Thanks. Unfortunately my H3 is out of arms length but I can arrange
> some testing if it is helpful.

Do you have UHS-SDIO cards? Otherwise, some more testing in Spain will
be good.

> Is one possibility to enable it for the slots listed above that are now
> thought to work but not for the other one?

I don't think so. We'd need more test coverage. My gut feeling is that
it varies from board to board. Or more precisely: from environment
around the board to environment around the board. I'd like to test a)
more boards) and b) a resoldered WLAN card to skip the extra cabling
there. I still think the wire length of ~23cm (exceeding the spec of max
10cm) is a factor here, so resoldering would save ~6cm. That all sounds
to me like a hacking sprint for our next meeting.

Maybe ULCB has less line lengths? Would be interesting to check, too.

I could also try to apply more shielding here at home, too.

> The SDR50 failure on M3-W seems particularly troubling as that
> has been enabled in upstream for a while, right?

Not nice, yes. Yet, until this thread, there have been no issues
reported. So, it doesn't look like a substantial fault to me (famous
last words?)

Regards,

   Wolfram


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

Re: [RFT] arm64: dts: renesas: salvator-common: enable SDR104 for SD cards

[Simon Horman]

On Mon, Aug 14, 2017 at 05:46:54PM +0200, Wolfram Sang wrote:
> Hi Simon,
> 
> On Mon, Aug 14, 2017 at 07:10:23AM +0200, Simon Horman wrote:
> > On Tue, Aug 08, 2017 at 08:54:45PM +0200, Wolfram Sang wrote:
> > > 
> > > > As it is an "RFT" I'm happy to apply it if you repost
> > > > it or otherwise indicate that is what you would like to happen.
> > > 
> > > As discussed in a chat, I tried SDR104 with my SDIO WiFi cards:
> > > 
> > > H3:
> > > - one slot worked flawlessly
> > > - one slot could load FW but failed to read status
> > > (with SDR50: both slots work)
> > > 
> > > M3-W:
> > > - both slots could not load the firmware
> > > (with SDR50: one slot works, one fails to load FW)
> > > 
> > > The cards, however, were correctly identified as SDR104 and there were
> > > no tuning errors and no other SDHI related warnings.
> > > 
> > > Changing TDSEL in the PFC did not change anything.
> > > 
> > > The manual of the WiFi card (u-blox EMMY-W1) mentions a maximum line
> > > length of 100mm. Measuring a direct line from the SoC to the slots,
> > > I'd think we are very much on the edge of that. And given the length
> > > of the SDIO adapter, we are surely exceeding that :(
> > > 
> > > Maybe we should do more tests with more boards? On the other hand, it
> > > seems the WiFi card is running out of the specs.
> > > 
> > > So much for now. I'll sleep over it and try to get more data tomorrow.
> > 
> > Thanks. Unfortunately my H3 is out of arms length but I can arrange
> > some testing if it is helpful.
> 
> Do you have UHS-SDIO cards? Otherwise, some more testing in Spain will
> be good.

I am unable to locate my SDIO card :(

> > Is one possibility to enable it for the slots listed above that are now
> > thought to work but not for the other one?
> 
> I don't think so. We'd need more test coverage. My gut feeling is that
> it varies from board to board. Or more precisely: from environment
> around the board to environment around the board. I'd like to test a)
> more boards) and b) a resoldered WLAN card to skip the extra cabling
> there. I still think the wire length of ~23cm (exceeding the spec of max
> 10cm) is a factor here, so resoldering would save ~6cm. That all sounds
> to me like a hacking sprint for our next meeting.
> 
> Maybe ULCB has less line lengths? Would be interesting to check, too.
> 
> I could also try to apply more shielding here at home, too.

More testing is fine by me.

> > The SDR50 failure on M3-W seems particularly troubling as that
> > has been enabled in upstream for a while, right?
> 
> Not nice, yes. Yet, until this thread, there have been no issues
> reported. So, it doesn't look like a substantial fault to me (famous
> last words?)

More testing need here too :)

Re: [RFT] arm64: dts: renesas: salvator-common: enable SDR104 for SD cards

[Wolfram Sang]

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


> As discussed in a chat, I tried SDR104 with my SDIO WiFi cards:
> 
> H3:
> - one slot worked flawlessly
> - one slot could load FW but failed to read status
> (with SDR50: both slots work)
> 
> M3-W:
> - both slots could not load the firmware
> (with SDR50: one slot works, one fails to load FW)

Update for my H3-ES2.0:

 - both slots could not load the firmware with SDR104
 (with SDR50: both slots work)

> The cards, however, were correctly identified as SDR104 and there were
> no tuning errors and no other SDHI related warnings.

This still holds true.


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