linux-kernel - Re: [PATCH v2] Revert "mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage

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: <5cd9da69-95f1-4cf0-b6ee-cffcf984a390@ti.com>
Date: Wed, 19 Mar 2025 12:07:01 -0500
From: Judith Mendez <jm@...com>
To: "Sverdlin, Alexander" <alexander.sverdlin@...mens.com>,
        "josua@...id-run.com" <josua@...id-run.com>
CC: "rabeeh@...id-run.com" <rabeeh@...id-run.com>,
        "stable@...r.kernel.org"
	<stable@...r.kernel.org>,
        "linux-mmc@...r.kernel.org"
	<linux-mmc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        "jon@...id-run.com" <jon@...id-run.com>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        "adrian.hunter@...el.com"
	<adrian.hunter@...el.com>
Subject: Re: [PATCH v2] Revert "mmc: sdhci_am654: Add
 sdhci_am654_start_signal_voltage_switch"

Hi, Alexander, Joshua,


Let me correct who I direct my questions/responses for.

On 3/19/25 11:25 AM, Judith Mendez wrote:
> Hi, Alexander,
> 
> On 3/19/25 5:22 AM, Sverdlin, Alexander wrote:
>> Hi Judith, Ulf,
>>
>> On Wed, 2025-02-05 at 13:39 -0600, Judith Mendez wrote:
>>> Hi all,
>>>
>>> On 1/27/25 2:12 PM, Josua Mayer wrote:
>>>> This reverts commit 941a7abd4666912b84ab209396fdb54b0dae685d.
>>>>
>>>> This commit uses presence of device-tree properties vmmc-supply and
>>>> vqmmc-supply for deciding whether to enable a quirk affecting timing of
>>>> clock and data.
>>>> The intention was to address issues observed with eMMC and SD on AM62
>>>> platforms.
>>>>
>>>> This new quirk is however also enabled for AM64 breaking microSD access
>>>> on the SolidRun HimmingBoard-T which is supported in-tree since v6.11,
>>>> causing a regression. During boot microSD initialization now fails with
>>>> the error below:
>>>>
>>>> [    2.008520] mmc1: SDHCI controller on fa00000.mmc [fa00000.mmc] 
>>>> using ADMA 64-bit
>>>> [    2.115348] mmc1: error -110 whilst initialising SD card
>>>>
>>>> The heuristics for enabling the quirk are clearly not correct as they
>>>> break at least one but potentially many existing boards.
>>>>
>>>> Revert the change and restore original behaviour until a more
>>>> appropriate method of selecting the quirk is derived.
>>>
>>>
>>> Somehow I missed these emails, apologies.
>>>
>>> Thanks for reporting this issue Josua.
>>>
>>> We do need this patch for am62x devices since it fixes timing issues
>>> with a variety of SD cards on those boards, but if there is a
>>> regression, too bad, patch had to be reverted.
>>>
>>> I will look again into how to implement this quirk, I think using the
>>> voltage regulator nodes to discover if we need this quirk might not have
>>> been a good idea, based on your explanation. I believe I did test the
>>> patch on am64x SK and am64x EVM boards and saw no boot issue there,
>>> so the issue seems related to the voltage regulator nodes existing in DT
>>> (the heuristics for enabling the quirk) as you call it.
>>>
>>> Again, thanks for reporting, will look into fixing this issue for am62x
>>> again soon.
>>
>> does it mean, that 14afef2333af
>> ("arm64: dts: ti: k3-am62-main: Update otap/itap values") has to be 
>> reverted
>> as well, for the time being?
> 
> So sorry for the delay in response.
> 
> Does this fix: ("arm64: dts: ti: k3-am62-main: Update otap/itap values")
> cause any issues for you?
> 
> The otap/itap fix is actually setting tap settings according to the
> device datasheet since they were wrong in the first place.
> 
> The values in the datasheet are the optimal tap settings for our
> boards based off of bench characterization results. If these values
> provide issues for you, please let me know.

This first part is for Alexander.

> 
> 
> Changing topic:
> 
> Going back to the reverted patch. What the patch does is that it
> tries to switch data launch from the rising clock edge to the
> falling clock edge if we find two voltage supplies for SD/SDIO, one
> for powering the SD/SDIO and another for IO voltage switch, or for
> the case that no voltage supplies exist (eMMC).
> 
> (this was based off-of some internal debug that resulted with a
> request to unset V1P8_SIGNAL_ENA to fix timing issues)
> 
> However, if you had one voltage supply, the patch should not have
> affected you at all and I am really confused why you see an issue
> downstream with only one voltage supply.
> 
> That being said, I have dug up more information on V1P8_SIGNAL_ENA.
> If HIGH_SPEED_ENA is set or if V1P8_SIGNAL_ENA is set, these two bits
> are OR'd and if any of the two is set, then data launch always happens
> on rising clock edge. This should be the case for any of the UHS modes
> or > mmc_hs mode for MMC.
> 
> If this is true, we should be setting HIGH_SPEED_ENA anyways for UHS
> modes and thus this patch should have done nothing in this sense, since
> data launch should still be happening on the rising clock edge.
> 
> I am still digging up more information to make sure disabling
> V1P8_SIGNAL_ENA has no other implications.
> 

This second part is to Joshua.

> 
> ~ Judith
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>