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

Judith Mendez
2025-04-11T21:55:48Z

Hi Hilago,

On 4/11/25 2:48 PM, Hiago De Franco wrote:
> On Fri, Apr 11, 2025 at 11:37:14AM -0500, Judith Mendez wrote:
>>
>> The reason that patches fixes SD init for you is because in original patch,
>> quirk was applied for both SD and eMMC with the exception of SD for am64x
>> SoC. This patch only applies the quirk for eMMC.
>>
>> We cannot use the original implementation because the logic applying the
>> quirk is based off of vmmc/vqmmc nodes in DT. The assumption was that am64x
>> based boards will only have vmmc node since there is an internal LDO that is
>> used to switch IO signal voltage instead of vqmmc. However, SolidRun
>> HimmingBoard-T board has a different implementation on voltage regulator
>> nodes in DT and the quirk applied to them as well and breaks their SD boot.
>> So we now only apply the quirk for eMMC. Without this quirk applied to SD,
>> am62x SD will continue having some stability issues.
>>
>> ~ Judith
>
> I got the idea now, thanks for the explanation, I missed in the original
> patches this was only about the eMMC and *not* the SD card. Is there any
> plans to send patches to the SD card as well?
>
> About that, I was wondering if instead of checking the bus_width to
> apply the fix for the eMMC, we could set a devicetree property to
> explicity apply the quirk. This way, we could also decide to apply it on
> the SD node without checking any bus width. As example:
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi b/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi
> index 1ea8f64b1b3b..c4423c09e809 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-verdin.dtsi
> @@ -1466,6 +1466,7 @@ &sdhci1 {
> vmmc-supply = <&reg_sdhc1_vmmc>;
> vqmmc-supply = <&reg_sdhc1_vqmmc>;
> ti,fails-without-test-cd;
> + ti,suppress-v1p8-ena;
> status = "disabled";
> };
>
> diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
> index 4e1156a2f1b8..a0485c2bb549 100644
> --- a/drivers/mmc/host/sdhci_am654.c
> +++ b/drivers/mmc/host/sdhci_am654.c
> @@ -87,7 +87,6 @@
> #define CLOCK_TOO_SLOW_HZ 50000000
> #define SDHCI_AM654_AUTOSUSPEND_DELAY -1
> #define RETRY_TUNING_MAX 10
> -#define BUS_WIDTH_8 8
>
> /* Command Queue Host Controller Interface Base address */
> #define SDHCI_AM654_CQE_BASE_ADDR 0x200
> @@ -844,7 +843,6 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
> struct device *dev = &pdev->dev;
> int drv_strength;
> int ret;
> - u32 bus_width;
>
> if (sdhci_am654->flags & DLL_PRESENT) {
> ret = device_property_read_u32(dev, "ti,trm-icp",
> @@ -886,9 +884,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_SIGNAL_ENA for eMMC */
> - device_property_read_u32(dev, "bus-width", &bus_width);
> - if (bus_width == BUS_WIDTH_8)
> + /*
> + * Suppress V1P8_SIGNAL_ENA for MMC devices if ti,suppress-v1p8-ena
> + * flag is present.
> + */
> + if (device_property_read_bool(dev, "ti,suppress-v1p8-ena"))
> sdhci_am654->quirks |= SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA;
>
> sdhci_get_of_property(pdev);
>
> Which makes our Kingston SD card work again:
>
> root@verdin-am62-15412807:~# dmesg | grep -i mmc1
> [ 1.897055] mmc1: CQHCI version 5.10
> [ 2.043673] mmc1: SDHCI controller on fa00000.mmc [fa00000.mmc] using ADMA 64-bit
> [ 2.260610] mmc1: new UHS-I speed SDR104 SDHC card at address 0001
> [ 2.268150] mmcblk1: mmc1:0001 SD32G 29.1 GiB
>
> Sorry if I missed something, would also be a valid solution?

Actually this was one of the previous implementations, I should have
that original patch somewhere. My understanding was that we do not like
adding new DT properties if we can find a way to apply the quirk in the
driver.

If this implementation flies with the maintainers, then we can go back
to DT property implementation.

Adrian, is this fine with you?

~ Judith