linux-kernel - Re: [PATCH 3/3] mmc: tegra: prevent ACMD23 on Tegra 3

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1553c44e99b96a813af129d1c4169222@agner.ch>
Date:   Thu, 26 Jul 2018 16:47:12 +0200
From:   Stefan Agner <stefan@...er.ch>
To:     Peter Geis <pgwipeout@...il.com>
Cc:     adrian.hunter@...el.com, ulf.hansson@...aro.org,
        thierry.reding@...il.com, jonathanh@...dia.com,
        marcel.ziswiler@...adex.com, linux-mmc@...r.kernel.org,
        linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] mmc: tegra: prevent ACMD23 on Tegra 3

On 26.07.2018 15:56, Peter Geis wrote:
> On 07/12/2018 03:39 AM, Stefan Agner wrote:
>> It seems that SD3.0 advertisement needs to be set for higher eMMC
>> speed modes (namely DDR52) as well. The TRM states that the SD3.0
>> advertisement bit should be set for all controller instances, even
>> for those not supporting UHS-I mode...
>>
>> When specifying vqmmc-supply as a fixed 1.8V regulator on a Tegra
>> SD/MMC instance which is connected to a eMMC device, the stack
>> enables SD3.0. However, enabling it has consequences: If SDHCI 3.0
>> support is advertised the stack enables Auto-CMD23. Unfortunately
>> Auto-CMD23 seems not to work well with Tegra 3 currently. It leads
>> to regular warnings:
>>    mmc2: Got command interrupt 0x00010000 even though no command operation was in progress.
>>
>> It is not entirely clear why those errors happens. It seems that
>> a Linux 3.1 based downstream kernel which has Auto-CMD23 support
>> does not show those warnings.
>>
>> Use quirk SDHCI_QUIRK2_ACMD23_BROKEN to prevent Auto-CMD23 being
>> used for now. With this the eMMC works stable on high-speed mode
>> while still announcing SD3.0.
>>
>> This allows to use mmc-ddr-1_8v to enables DDR52 mode. In DDR52
>> mode read speed improves from about 42MiB/s to 72MiB/s on an
>> Apalis T30.
>>
>> Signed-off-by: Stefan Agner <stefan@...er.ch>
>> ---
>>   drivers/mmc/host/sdhci-tegra.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-tegra.c b/drivers/mmc/host/sdhci-tegra.c
>> index 888a1ad511db..11c0b2069c7c 100644
>> --- a/drivers/mmc/host/sdhci-tegra.c
>> +++ b/drivers/mmc/host/sdhci-tegra.c
>> @@ -336,7 +336,15 @@ static const struct sdhci_pltfm_data sdhci_tegra30_pdata = {
>>   		  SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC |
>>   		  SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
>>   	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
>> -		   SDHCI_QUIRK2_BROKEN_HS200,
>> +		   SDHCI_QUIRK2_BROKEN_HS200 |
>> +		   /*
>> +		    * Auto-CMD23 leads to "Got command interrupt 0x00010000 even
>> +		    * though no command operation was in progress."
>> +		    *
>> +		    * The exact reason is unknown, as the same hardware seems
>> +		    * to support Auto CMD23 on a downstream 3.1 kernel.
>> +		    */
>> +		   SDHCI_QUIRK2_ACMD23_BROKEN,
>>   	.ops  = &tegra_sdhci_ops,
>>   };
>>
> 
> I finally got around to testing this on the Ouya (Tegra 3).

Thanks for testing!

> 
> I found that the "Got command interrupt 0x00010000 even though no
> command operation was in progress." error occurred when the interface
> is unstable.
> I've had a lot of problems with sdmmc4 stability on the Ouya above 34
> Mhz, probably due to the fact that they are using the internal cmd and
> clock pull-up resistors, against the TRM's instruction.
> At 39Mhz, I saw the error this patch corrects.
> With the patch, the error went away, but the interface is still
> unstable under load.

How does this instability manifest exactly?

> 
> Lowering down to 32Mhz, without the patch there are no errors.

So the patch does not make it less stable right?

--
Stefan