Re: [PATCH 0/2] Fix V1P8_SIGNAL_ENA and HIGH_SPEED_ENA
From: Judith Mendez
Date: Fri Apr 11 2025 - 17:55:59 EST
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 = <®_sdhc1_vmmc>;
vqmmc-supply = <®_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