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

Josua Mayer
2025-01-19T20:15:19Z

Hi Judith,

Am 13.09.24 um 20:54 schrieb Judith Mendez:
> The sdhci_start_signal_voltage_switch function sets
> V1P8_SIGNAL_ENA by default after switching to 1v8 signaling.
> V1P8_SIGNAL_ENA determines whether to launch cmd/data on neg
> edge or pos edge of clock.
>
> Due to some eMMC and SD failures seen across am62x platform,
> do not set V1P8_SIGNAL_ENA by default, only enable the bit
> for devices that require this bit in order to switch to 1v8
> voltage for uhs modes.
I have just tested this patch on downstream TI Linux fork tagged 10.01.10
(v6.6.58) and it breaks sd-card detection on SolidRun AM642 HummingBoard-T
supported in-tree: arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dts since v6.11.

The board fails during boot from sdcard:
mmc1: error -110 whilst initialising SD card

Hence I suspect something is not okay with the heuristics enabling this quirk.
Kindly note I have not tested it with pure 6.13-rc1 yet.

My downstream workaround in dts is to link a fixed regulator vmmc-supply,
but the upstream dts specifies neither vmmc- nor vqmmc supplies.

>
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
> Changes since v1:
> - Invert quirk logic
> - Simplify sdhci_am654_start_signal_voltage_switch() and call
> sdhci_start_signal_voltage_switch() when the quirk does not apply
> - Simply logic when detecting when quirk should be applied
> ---
> drivers/mmc/host/sdhci_am654.c | 30 ++++++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index 0aa3c40ea6ed8..9ff07aadb2d91 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
cut
> @@ -844,6 +868,11 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
> if (device_property_read_bool(dev, "ti,fails-without-test-cd"))
> sdhci_am654->quirks |= SDHCI_AM654_QUIRK_FORCE_CDTEST;
>
> + /* Suppress v1p8 ena for eMMC and SD with vqmmc supply */
> + if (!!of_parse_phandle(dev->of_node, "vmmc-supply", 0) ==
> + !!of_parse_phandle(dev->of_node, "vqmmc-supply", 0))
> + sdhci_am654->quirks |= SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA;
> +
Was it intentional to apply the quirk both when neither,
and when both supplies are specified?