Re: [PATCH 0/2] Fix V1P8_SIGNAL_ENA and HIGH_SPEED_ENA

[Adrian Hunter <adrian.hunter@intel.com>]

On 16/04/25 22:11, Adrian Hunter wrote:
> On 16/04/25 19:59, Judith Mendez wrote:
>> Hello Adrian,
>>
>> On 4/7/25 5:27 PM, Judith Mendez wrote:
>>> For all TI devices, timing was closed For Legacy and HS modes in
>>> half cycle timing, where data is launched on the negative edge of
>>> clock and latched on the following positive edge of clock. The
>>> switch to full cycle timing happens when any of HIGH_SPEED_ENA,
>>> V1P8_SIGNAL_ENA, or UHS_MODE_SELECT is set.
>>>
>>> Currently HIGH_SPEED_ENA is set for HS modes and violates timing
>>> requirements for TI devices so add a .set_hs_ena callback in
>>> sdhci_am654 driver so that HIGH_SPEED_ENA is not set for this mode.
>>>
>>> There are eMMC boot failures seen with V1P8_SIGNAL_ENA with a
>>> specific Kingston eMMC due to the sequencing when enumerating to
>>> HS200 mode. Since V1P8_SIGNAL_ENA is optional for eMMC, do not
>>> set V1P8_SIGNAL_ENA be default. This fix was previously merged in
>>> the kernel, but was reverted due to the "heuristics for enabling
>>> the quirk"[0]. The new implementation applies the quirk based-off of
>>> bus width, which should not be an issue since there is no internal
>>> LDO for MMC0 8bit wide interface and hence V1P8_SIGNAL_ENA should only
>>> effect timing for MMC0 interface.
>>
>>
>> On this patch series, I am bringing back the fix for V1P8_SIGNAL_ENA,
>> Ulf requested a change [0] which I am planning to do for v2. But I was
>> hoping to get your opinion on whether Hiago's suggestion [1] is doable
>> so I can add that as well to v2. Thanks for your attention.
>>
>>
>> [0] https://lore.kernel.org/linux-mmc/CAPDyKFqx-G4NynanFWrspz7-uXXF74RfjcU-Sw2nq2JhL3LPuQ@mail.gmail.com/
>> [1] https://lore.kernel.org/linux-mmc/20250412132012.xpjywokcpztb4jg4@hiago-nb/
>>
> 
> Sorry for the slow reply - been a bit distracted.
> 
> I'll look at it properly tomorrow, but noticed
> sdhci_am654_write_b() already is dealing with SDHCI_HOST_CONTROL
> and SDHCI_CTRL_HISPD.  Can you make use of that instead of
> a .set_hs_ena callback?

Patch 1 continues to look unneeded because sdhci_am654_write_b()
seems to do the same thing.

WRT patch 2, the suggestion to add a DT property and check the bus
width seems fine to me.  DT properties can be added to identify
"broken" hardware that cannot be identified by the compatible
string.