linux-kernel - Re: [PATCH 0/2] Fix V1P8_SIGNAL_ENA and HIGH_SPEED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250411194813.c4ft2uxgdiuza5cm@hiago-nb>
Date: Fri, 11 Apr 2025 16:48:13 -0300
From: Hiago De Franco <hiagofranco@...il.com>
To: Judith Mendez <jm@...com>
Cc: Josua Mayer <josua@...id-run.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Ulf Hansson <ulf.hansson@...aro.org>, linux-mmc@...r.kernel.org,
	linux-kernel@...r.kernel.org, Moteen Shah <m-shah@...com>,
	Hiago De Franco <hiago.franco@...adex.com>
Subject: Re: [PATCH 0/2] Fix V1P8_SIGNAL_ENA and HIGH_SPEED_ENA

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@...din-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?

Cheers,
Hiago.