[1/3] mmc: tegra: prevent HS200 on Tegra 3

Message ID 20180712073904.4705-1-stefan@agner.ch
State New
Headers show
Series
  • [1/3] mmc: tegra: prevent HS200 on Tegra 3
Related show

Commit Message

Stefan Agner July 12, 2018, 7:39 a.m.
The stack assumes that SDHC controller which support SD3.0 (SDR104) do
support HS200. This is not the case for Tegra 3, which does support SD
3.0
but only supports eMMC spec 4.41.

Use SDHCI_QUIRK2_BROKEN_HS200 to indicate that the controller does not
support HS200.

Note that commit 156e14b126ff ("mmc: sdhci: fix caps2 for HS200") added
the tie between SD3.0 (SDR104) and HS200. I don't think that this is
necessarly true. It is fully legitimate to support SD3.0 and not support
HS200. The quirk naming suggests something is broken in the controller,
but this is not the case: The controller simply does not support HS200.

Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/mmc/host/sdhci-tegra.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Marcel Ziswiler July 13, 2018, 3:39 p.m. | #1
On Thu, 2018-07-12 at 09:39 +0200, Stefan Agner wrote:
> The stack assumes that SDHC controller which support SD3.0 (SDR104)
> do
> support HS200. This is not the case for Tegra 3, which does support
> SD
> 3.0
> but only supports eMMC spec 4.41.
> 
> Use SDHCI_QUIRK2_BROKEN_HS200 to indicate that the controller does
> not
> support HS200.
> 
> Note that commit 156e14b126ff ("mmc: sdhci: fix caps2 for HS200")
> added
> the tie between SD3.0 (SDR104) and HS200. I don't think that this is
> necessarly true. It is fully legitimate to support SD3.0 and not
> support
> HS200. The quirk naming suggests something is broken in the
> controller,
> but this is not the case: The controller simply does not support
> HS200.
> 
> Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
> Signed-off-by: Stefan Agner <stefan@agner.ch>

The whole series stress-tested overnight on various Apalis T30 as well
as Colibri T30 modules:

Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

> ---
>  drivers/mmc/host/sdhci-tegra.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-
> tegra.c
> index 970d38f68939..137df06b9b6e 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -334,7 +334,8 @@ static const struct sdhci_pltfm_data
> sdhci_tegra30_pdata = {
>  		  SDHCI_QUIRK_NO_HISPD_BIT |
>  		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
>  		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> -	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> +		   SDHCI_QUIRK2_BROKEN_HS200,
>  	.ops  = &tegra_sdhci_ops,
>  };

Works quite nicely on today's next once [1] is also applied:

root@apalis-t30:~# cat /sys/kernel/debug/mmc1/ios 
clock:          52000000 Hz
actual clock:   51000000 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:      3 (8 bits)
timing spec:    8 (mmc DDR52)
signal voltage: 1 (1.80 V)
driver type:    0 (driver type B)
root@apalis-t30:~# hdparm -t /dev/mmcblk1

/dev/mmcblk1:
 Timing buffered disk reads: 236 MB in  3.01 seconds =  78.42 MB/sec

root@colibri-t30:~# cat /sys/kernel/debug/mmc0/ios 
clock:          52000000 Hz
actual clock:   51000000 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:      3 (8 bits)
timing spec:    8 (mmc DDR52)
signal voltage: 1 (1.80 V)
driver type:    0 (driver type B)
root@colibri-t30:~# hdparm -t /dev/mmcblk0

/dev/mmcblk0:
 Timing buffered disk reads: 168 MB in  3.00 seconds =  55.99 MB/sec

[1] https://lore.kernel.org/lkml/1531487865-17427-1-git-send-email-avie
namo@nvidia.com
Ulf Hansson July 16, 2018, 10:11 a.m. | #2
On 12 July 2018 at 09:39, Stefan Agner <stefan@agner.ch> wrote:
> The stack assumes that SDHC controller which support SD3.0 (SDR104) do
> support HS200. This is not the case for Tegra 3, which does support SD
> 3.0
> but only supports eMMC spec 4.41.
>
> Use SDHCI_QUIRK2_BROKEN_HS200 to indicate that the controller does not
> support HS200.
>
> Note that commit 156e14b126ff ("mmc: sdhci: fix caps2 for HS200") added
> the tie between SD3.0 (SDR104) and HS200. I don't think that this is
> necessarly true. It is fully legitimate to support SD3.0 and not support
> HS200. The quirk naming suggests something is broken in the controller,
> but this is not the case: The controller simply does not support HS200.
>
> Fixes: 7ad2ed1dfcbe ("mmc: tegra: enable UHS-I modes")
> Signed-off-by: Stefan Agner <stefan@agner.ch>

Thanks, applied all three patches for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/sdhci-tegra.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
> index 970d38f68939..137df06b9b6e 100644
> --- a/drivers/mmc/host/sdhci-tegra.c
> +++ b/drivers/mmc/host/sdhci-tegra.c
> @@ -334,7 +334,8 @@ static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
>                   SDHCI_QUIRK_NO_HISPD_BIT |
>                   SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
>                   SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> -       .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
> +       .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> +                  SDHCI_QUIRK2_BROKEN_HS200,
>         .ops  = &tegra_sdhci_ops,
>  };
>
> --
> 2.18.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
index 970d38f68939..137df06b9b6e 100644
--- a/drivers/mmc/host/sdhci-tegra.c
+++ b/drivers/mmc/host/sdhci-tegra.c
@@ -334,7 +334,8 @@  static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
 		  SDHCI_QUIRK_NO_HISPD_BIT |
 		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
 		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
-	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+		   SDHCI_QUIRK2_BROKEN_HS200,
 	.ops  = &tegra_sdhci_ops,
 };