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

From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
To: "stefan@agner.ch" <stefan@agner.ch>,
	"ulf.hansson@linaro.org" <ulf.hansson@linaro.org>,
	"adrian.hunter@intel.com" <adrian.hunter@intel.com>
Cc: "jonathanh@nvidia.com" <jonathanh@nvidia.com>,
	"linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"thierry.reding@gmail.com" <thierry.reding@gmail.com>,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH 1/3] mmc: tegra: prevent HS200 on Tegra 3
Date: Fri, 13 Jul 2018 15:39:24 +0000	[thread overview]
Message-ID: <1531496362.5479.34.camel@toradex.com> (raw)
In-Reply-To: <20180712073904.4705-1-stefan@agner.ch>

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