Re: [PATCH v2] Revert "mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch"

[Sverdlin, Alexander]

Hi Judith,

On Wed, 2025-03-19 at 11:25 -0500, Judith Mendez 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.

I've just noticed that 14afef2333af mentioned the reverted 941a7abd4666
in a way that one may think of it as a dependency:
---
    Now that we have fixed timing issues for am62x [1], lets
    change the otap/itap values back according to the device
    datasheet.
    
    [1] https://lore.kernel.org/linux-mmc/20240913185403.1339115-1-jm@xxxxxx/
---

that why I wanted to double check with you. But if you say they are actually
independent, that's fine for me!

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com