[PATCH v2 0/2] Fix V1P8_SIGNAL_ENA

[PATCH v2 0/2] Fix V1P8_SIGNAL_ENA

[Judith Mendez]

There are eMMC boot failures seen with V1P8_SIGNAL_ENA on Kingston
eMMC and variouse types of SD cards on Sitara K3 SoCs due to the
sequencing when enumerating to HS200 mode. Since V1P8_SIGNAL_ENA is
optional for eMMC, do not set V1P8_SIGNAL_ENA by default for eMMC.
For SD cards we shall parse DT for ti,suppress-v1p8-ena property
to determine whether to apply the quirk.

This fix was previously merged in the kernel, but was reverted due
to the "heuristics for enabling the quirk"[0]. This issue is adressed
in this patch series by adding optional ti,suppress-v1p8-ena DT property
to apply the quirk for SD.

Changes since v1:
- Drop patch for High_Speed_ENA
- Add ti,suppress-v1p8-ena for SD cards
- Add binding patch for ti,suppress-v1p8-ena
- Update cover-letter/patch descriptions according to new changes

[0] https://lore.kernel.org/linux-mmc/20250127-am654-mmc-regression-v2-1-9bb39fb12810@solid-run.com/

Judith Mendez (2):
  mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch
  dt-bindings: mmc: sdhci-am654: Add ti,suppress-v1p8-ena

 .../devicetree/bindings/mmc/sdhci-am654.yaml  |  5 +++
 drivers/mmc/host/sdhci_am654.c                | 32 +++++++++++++++++++
 2 files changed, 37 insertions(+)

-- 
2.49.0

[PATCH v2 1/2] mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch

[Judith Mendez]

The sdhci_start_signal_voltage_switch function sets
V1P8_SIGNAL_ENA by default after switching to 1v8 signaling.
V1P8_SIGNAL_ENA has a timing component where it determines
whether to launch cmd/data on neg edge (half cycle timing)
or pos edge (full cycle timing) of clock. V1P8_SIGNAL_ENA also
has a voltage switch component where if there exists an internal
LDO, for SD this bit is used to switch from 3.3V to 1.8V IO
signal voltage.

The sequence is to switch to 1.8 IO voltage, set V1P8_SIGNAL_ENA,
change bus width, then update HIGH_SPEED_ENA & UHS_MODE_SELECT.
During bus width change is when eMMC failures are seen with
Kingston eMMC and various types SD cards across Sitara K3 SoCs.

So, add a quirk to suppress V1P8_SIGNAL_ENA and do not enable by
default for eMMC since it is anyways optional for this interface
and parse DT property: ti,fails-without-test-cd to apply the quirk
for SD cards.

Signed-off-by: Judith Mendez <jm@ti.com>
Suggested-by: Hiago De Franco <hiago.franco@toradex.com>
---
 drivers/mmc/host/sdhci_am654.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index f75c31815ab00..2d9f1406d06eb 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -87,6 +87,7 @@
 #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
@@ -155,6 +156,7 @@ struct sdhci_am654_data {
 	u32 tuning_loop;
 
 #define SDHCI_AM654_QUIRK_FORCE_CDTEST BIT(0)
+#define SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA BIT(1)
 };
 
 struct window {
@@ -356,6 +358,29 @@ static void sdhci_j721e_4bit_set_clock(struct sdhci_host *host,
 	sdhci_set_clock(host, clock);
 }
 
+static int sdhci_am654_start_signal_voltage_switch(struct mmc_host *mmc, struct mmc_ios *ios)
+{
+	struct sdhci_host *host = mmc_priv(mmc);
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct sdhci_am654_data *sdhci_am654 = sdhci_pltfm_priv(pltfm_host);
+	int ret;
+
+	if ((sdhci_am654->quirks & SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA) &&
+	    ios->signal_voltage == MMC_SIGNAL_VOLTAGE_180) {
+		if (!IS_ERR(mmc->supply.vqmmc)) {
+			ret = mmc_regulator_set_vqmmc(mmc, ios);
+			if (ret < 0) {
+				pr_err("%s: Switching to 1.8V signalling voltage failed,\n",
+				       mmc_hostname(mmc));
+				return -EIO;
+			}
+		}
+		return 0;
+	}
+
+	return sdhci_start_signal_voltage_switch(mmc, ios);
+}
+
 static u8 sdhci_am654_write_power_on(struct sdhci_host *host, u8 val, int reg)
 {
 	writeb(val, host->ioaddr + reg);
@@ -803,6 +828,7 @@ 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",
@@ -844,6 +870,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 || device_property_read_bool(dev, "ti,suppress-v1p8-ena"))
+		sdhci_am654->quirks |= SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA;
+
 	sdhci_get_of_property(pdev);
 
 	return 0;
@@ -940,6 +971,7 @@ static int sdhci_am654_probe(struct platform_device *pdev)
 		goto err_pltfm_free;
 	}
 
+	host->mmc_host_ops.start_signal_voltage_switch = sdhci_am654_start_signal_voltage_switch;
 	host->mmc_host_ops.execute_tuning = sdhci_am654_execute_tuning;
 
 	pm_runtime_get_noresume(dev);
-- 
2.49.0

Re: [PATCH v2 1/2] mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch

[Francesco Dolcini]

Hello Judith,

On Thu, Apr 17, 2025 at 01:26:51PM -0500, Judith Mendez wrote:
> The sdhci_start_signal_voltage_switch function sets
> V1P8_SIGNAL_ENA by default after switching to 1v8 signaling.
> V1P8_SIGNAL_ENA has a timing component where it determines
> whether to launch cmd/data on neg edge (half cycle timing)
> or pos edge (full cycle timing) of clock. V1P8_SIGNAL_ENA also
> has a voltage switch component where if there exists an internal
> LDO, for SD this bit is used to switch from 3.3V to 1.8V IO
> signal voltage.
> 
> The sequence is to switch to 1.8 IO voltage, set V1P8_SIGNAL_ENA,
> change bus width, then update HIGH_SPEED_ENA & UHS_MODE_SELECT.
> During bus width change is when eMMC failures are seen with
> Kingston eMMC and various types SD cards across Sitara K3 SoCs.
> 
> So, add a quirk to suppress V1P8_SIGNAL_ENA and do not enable by
> default for eMMC since it is anyways optional for this interface
> and parse DT property: ti,fails-without-test-cd to apply the quirk
> for SD cards.
> 
> Signed-off-by: Judith Mendez <jm@ti.com>
> Suggested-by: Hiago De Franco <hiago.franco@toradex.com>

Fixes: ac5a41b472b4 ("Revert "mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch"")
Fixes: 941a7abd4666 ("mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch")
Cc:stable@vger.kernel.org

[PATCH v2 2/2] dt-bindings: mmc: sdhci-am654: Add ti,suppress-v1p8-ena

[Judith Mendez]

This patch documents ti,suppress-v1p8-ena which is a flag
used to suppress V1P8_SIGNAL_ENA in sdhci_am654 driver. This
quirk is necessary to fix fail init issues across various
types of SD cards tested on Sitara K3 SoCs.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 Documentation/devicetree/bindings/mmc/sdhci-am654.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml b/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
index 676a746953893..0f92bbf8e13b3 100644
--- a/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
+++ b/Documentation/devicetree/bindings/mmc/sdhci-am654.yaml
@@ -201,6 +201,11 @@ properties:
       and the controller is required to be forced into Test mode
       to set the TESTCD bit.
 
+  ti,suppress-v1p8-ena:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      When present, V1P8_SIGNAL_ENA shall be suppressed.
+
 required:
   - compatible
   - reg
-- 
2.49.0

Re: [PATCH v2 2/2] dt-bindings: mmc: sdhci-am654: Add ti,suppress-v1p8-ena

[Francesco Dolcini]

Hello Judith,
thanks for the patch.

On Thu, Apr 17, 2025 at 01:26:52PM -0500, Judith Mendez wrote:
> This patch documents ti,suppress-v1p8-ena which is a flag
> used to suppress V1P8_SIGNAL_ENA in sdhci_am654 driver. This
> quirk is necessary to fix fail init issues across various
> types of SD cards tested on Sitara K3 SoCs.

bindings are supposed to describe the hardware, not the driver.

You should rephrase the commit message and the description of the property with
this in mind.

In addition, I think that the dt-bindings is supposed to be before the driver
patch in the series.

Re: [PATCH v2 2/2] dt-bindings: mmc: sdhci-am654: Add ti,suppress-v1p8-ena

[Judith Mendez]

Hi Francesco,

On 4/17/25 2:19 PM, Francesco Dolcini wrote:
> Hello Judith,
> thanks for the patch.
> 
> On Thu, Apr 17, 2025 at 01:26:52PM -0500, Judith Mendez wrote:
>> This patch documents ti,suppress-v1p8-ena which is a flag
>> used to suppress V1P8_SIGNAL_ENA in sdhci_am654 driver. This
>> quirk is necessary to fix fail init issues across various
>> types of SD cards tested on Sitara K3 SoCs.
> 
> bindings are supposed to describe the hardware, not the driver.
> 
> You should rephrase the commit message and the description of the property with
> this in mind.
> 
> In addition, I think that the dt-bindings is supposed to be before the driver
> patch in the series.
> 

Right, will fix for v3 along with adding fixes tags.

~ Judith

Re: [PATCH v2 0/2] Fix V1P8_SIGNAL_ENA

[Francesco Dolcini]

On Thu, Apr 17, 2025 at 01:26:50PM -0500, Judith Mendez wrote:
> There are eMMC boot failures seen with V1P8_SIGNAL_ENA on Kingston
> eMMC and variouse types of SD cards on Sitara K3 SoCs due to the
> sequencing when enumerating to HS200 mode. Since V1P8_SIGNAL_ENA is
> optional for eMMC, do not set V1P8_SIGNAL_ENA by default for eMMC.
> For SD cards we shall parse DT for ti,suppress-v1p8-ena property
> to determine whether to apply the quirk.

I assume this ti,suppress-v1p8-ena should be added to some SoC dtsi, am I
wrong?

Francesco

Re: [PATCH v2 0/2] Fix V1P8_SIGNAL_ENA

[Judith Mendez]

Hi Francesco,

On 4/17/25 2:22 PM, Francesco Dolcini wrote:
> On Thu, Apr 17, 2025 at 01:26:50PM -0500, Judith Mendez wrote:
>> There are eMMC boot failures seen with V1P8_SIGNAL_ENA on Kingston
>> eMMC and variouse types of SD cards on Sitara K3 SoCs due to the
>> sequencing when enumerating to HS200 mode. Since V1P8_SIGNAL_ENA is
>> optional for eMMC, do not set V1P8_SIGNAL_ENA by default for eMMC.
>> For SD cards we shall parse DT for ti,suppress-v1p8-ena property
>> to determine whether to apply the quirk.
> 
> I assume this ti,suppress-v1p8-ena should be added to some SoC dtsi, am I
> wrong?
> 

I was planning to add in a separate series once this was merged but I
can add to v3 no problem, thanks for reviewing.

~ Judith