[PATCH RESEND v3 0/3] Add ti,suppress-v1p8-ena

[PATCH RESEND v3 0/3] Add ti,suppress-v1p8-ena

[Judith Mendez]

Resend patch series to fix cc list

There are MMC boot failures seen with V1P8_SIGNAL_ENA on Kingston eMMC
and Microcenter/Patriot SD cards on Sitara K3 boards due to the HS200
initialization sequence involving V1P8_SIGNAL_ENA. 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 suppress V1P8_SIGNAL_ENA. Add new ti,suppress-v1p8-ena
to am62x, am62ax, and am62px SoC dtsi files since there is no internal LDO
tied to sdhci1 interface so V1P8_SIGNAL_ENA only affects timing.

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
which determines whether to apply the quirk for SD.

Changes since v2:
- Include patch 3/3
- Reword cover letter
- Reword binding patch description
- Add fixes/cc tags to driver patch
- Reorder patches according to binding patch first
- Resend to fix cc list in original v3 series

Link to v2:
https://lore.kernel.org/linux-mmc/20250417182652.3521104-1-jm@ti.com/
Link to v1:
https://lore.kernel.org/linux-mmc/20250407222702.2199047-1-jm@ti.com/

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

Judith Mendez (3):
  dt-bindings: mmc: sdhci-am654: Add ti,suppress-v1p8-ena
  mmc: sdhci_am654: Add sdhci_am654_start_signal_voltage_switch
  arm64: dts: ti: k3-am62*: add ti,suppress-v1p8-ena

 .../devicetree/bindings/mmc/sdhci-am654.yaml  |  5 +++
 arch/arm64/boot/dts/ti/k3-am62-main.dtsi      |  1 +
 arch/arm64/boot/dts/ti/k3-am62a-main.dtsi     |  1 +
 .../dts/ti/k3-am62p-j722s-common-main.dtsi    |  1 +
 drivers/mmc/host/sdhci_am654.c                | 32 +++++++++++++++++++
 5 files changed, 40 insertions(+)


base-commit: 1be38f81251f6d276713c259ecf4414f82f22c29
-- 
2.49.0

[PATCH RESEND v3 1/3] dt-bindings: mmc: sdhci-am654: Add ti,suppress-v1p8-ena

[Judith Mendez]

Some Microcenter/Patriot SD cards and Kingston eMMC are failing init
across Sitara K3 boards. Init failure is due to the sequence when
V1P8_SIGNAL_ENA is set. The V1P8_SIGNAL_ENA has a timing component tied
to it where if set, switch to full-cycle timing happens. The failing
cards do not like change to full-cycle timing before changing bus
width, so add flag to sdhci-am654 binding to suppress V1P8_SIGNAL_ENA
before changing bus width. The switch to full-cycle timing should happen
with HIGH_SPEED_ENA after change of bus width.

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 676a74695389..0f92bbf8e13b 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

[PATCH RESEND v3 2/3] 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>
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
---
 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 f75c31815ab0..d4cafc7af97d 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);
@@ -802,6 +827,7 @@ static int sdhci_am654_get_of_property(struct platform_device *pdev,
 {
 	struct device *dev = &pdev->dev;
 	int drv_strength;
+	u32 bus_width;
 	int ret;
 
 	if (sdhci_am654->flags & DLL_PRESENT) {
@@ -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 */
+	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

[PATCH RESEND v3 3/3] arm64: dts: ti: k3-am62*: add ti,suppress-v1p8-ena

[Judith Mendez]

Add ti,suppress-v1p8-ena flag to sdhci1 nodes to suppress
V1P8_SIGNAL_ENA.

On am62x, am62ax, and am62px SoCs, there is no internal LDO
tied to sdhci1 interface so V1P8_SIGNAL_ENA only affects timing.
Suppress V1P8_SIGNAL_ENA since it causes init failures across
Microcenter/Patriot SD cards and Kingston eMMC on am62* SK boards.

Signed-off-by: Judith Mendez <jm@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62-main.dtsi               | 1 +
 arch/arm64/boot/dts/ti/k3-am62a-main.dtsi              | 1 +
 arch/arm64/boot/dts/ti/k3-am62p-j722s-common-main.dtsi | 1 +
 3 files changed, 3 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
index 7d355aa73ea2..a61153b0af32 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
@@ -586,6 +586,7 @@ sdhci1: mmc@fa00000 {
 		ti,itap-del-sel-sd-hs = <0x0>;
 		ti,itap-del-sel-sdr12 = <0x0>;
 		ti,itap-del-sel-sdr25 = <0x0>;
+		ti,suppress-v1p8-ena;
 		status = "disabled";
 	};
 
diff --git a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
index a1daba7b1fad..44d973caf200 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62a-main.dtsi
@@ -606,6 +606,7 @@ sdhci1: mmc@fa00000 {
 		ti,itap-del-sel-sd-hs = <0x0>;
 		ti,itap-del-sel-sdr12 = <0x0>;
 		ti,itap-del-sel-sdr25 = <0x0>;
+		ti,suppress-v1p8-ena;
 		status = "disabled";
 	};
 
diff --git a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-main.dtsi
index 6e3beb5c2e01..c4304dae9757 100644
--- a/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62p-j722s-common-main.dtsi
@@ -604,6 +604,7 @@ sdhci1: mmc@fa00000 {
 		ti,itap-del-sel-sd-hs = <0x0>;
 		ti,itap-del-sel-sdr12 = <0x0>;
 		ti,itap-del-sel-sdr25 = <0x0>;
+		ti,suppress-v1p8-ena;
 		status = "disabled";
 	};
 
-- 
2.49.0

Re: [PATCH RESEND v3 0/3] Add ti,suppress-v1p8-ena

[Nishanth Menon]

On 17:05-20250422, Judith Mendez wrote:
> Resend patch series to fix cc list
> 
> There are MMC boot failures seen with V1P8_SIGNAL_ENA on Kingston eMMC
> and Microcenter/Patriot SD cards on Sitara K3 boards due to the HS200
> initialization sequence involving V1P8_SIGNAL_ENA. 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 suppress V1P8_SIGNAL_ENA. Add new ti,suppress-v1p8-ena
> to am62x, am62ax, and am62px SoC dtsi files since there is no internal LDO
> tied to sdhci1 interface so V1P8_SIGNAL_ENA only affects timing.
> 
> 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
> which determines whether to apply the quirk for SD.
[...]
> 
> [0] https://lore.kernel.org/linux-mmc/20250127-am654-mmc-regression-v2-1-9bb39fb12810@solid-run.com/

Why cant we use compatible to enable the quirk instead of blindly
enabling for all IPs including ones that have onchip LDOs? That was the
reason it failed in the first place for am64x.

This is very much like a quirk that seems to go hand-in-hand with the
compatible for am62-sdhci ?

Is it worth exploring that option in the driver thread? from where I
stand, this sounds very much like an issue that AM62x IP has, and should
be handled by the driver instead of punting to dts to select where to
use and not to use the quirk.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH RESEND v3 0/3] Add ti,suppress-v1p8-ena

[Judith Mendez]

Hi Nishanth,

On 4/23/25 1:08 PM, Nishanth Menon wrote:
> On 17:05-20250422, Judith Mendez wrote:
>> Resend patch series to fix cc list
>>
>> There are MMC boot failures seen with V1P8_SIGNAL_ENA on Kingston eMMC
>> and Microcenter/Patriot SD cards on Sitara K3 boards due to the HS200
>> initialization sequence involving V1P8_SIGNAL_ENA. 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 suppress V1P8_SIGNAL_ENA. Add new ti,suppress-v1p8-ena
>> to am62x, am62ax, and am62px SoC dtsi files since there is no internal LDO
>> tied to sdhci1 interface so V1P8_SIGNAL_ENA only affects timing.
>>
>> 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
>> which determines whether to apply the quirk for SD.
> [...]
>>
>> [0] https://lore.kernel.org/linux-mmc/20250127-am654-mmc-regression-v2-1-9bb39fb12810@solid-run.com/
> 
> Why cant we use compatible to enable the quirk instead of blindly
> enabling for all IPs including ones that have onchip LDOs? That was the
> reason it failed in the first place for am64x.

We made an assumption that did not work out.

> 
> This is very much like a quirk that seems to go hand-in-hand with the
> compatible for am62-sdhci ?
> 
> Is it worth exploring that option in the driver thread? from where I
> stand, this sounds very much like an issue that AM62x IP has, and should
> be handled by the driver instead of punting to dts to select where to
> use and not to use the quirk.
> 

Sure, I can test this out and respin the series. It does seem like a
more clean solution, thanks for reviewing.

~ Judith

Re: [PATCH RESEND v3 1/3] dt-bindings: mmc: sdhci-am654: Add ti,suppress-v1p8-ena

[Krzysztof Kozlowski]

On Tue, Apr 22, 2025 at 05:05:10PM GMT, Judith Mendez wrote:
> Some Microcenter/Patriot SD cards and Kingston eMMC are failing init
> across Sitara K3 boards. Init failure is due to the sequence when
> V1P8_SIGNAL_ENA is set. The V1P8_SIGNAL_ENA has a timing component tied
> to it where if set, switch to full-cycle timing happens. The failing
> cards do not like change to full-cycle timing before changing bus
> width, so add flag to sdhci-am654 binding to suppress V1P8_SIGNAL_ENA
> before changing bus width. The switch to full-cycle timing should happen
> with HIGH_SPEED_ENA after change of bus width.
> 
> 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 676a74695389..0f92bbf8e13b 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:

Do not tell what the drivers should do, but tell what is the issue with
the hardware, e.g. some cards do not like full-cycle.... and this will
also hint you that it should be most likely generic, not specific to
this device.

> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:

Best regards,
Krzysztof

Re: [PATCH RESEND v3 1/3] dt-bindings: mmc: sdhci-am654: Add ti,suppress-v1p8-ena

[Judith Mendez]

Hi Krzysztof,

On 4/25/25 2:48 AM, Krzysztof Kozlowski wrote:
> On Tue, Apr 22, 2025 at 05:05:10PM GMT, Judith Mendez wrote:
>> Some Microcenter/Patriot SD cards and Kingston eMMC are failing init
>> across Sitara K3 boards. Init failure is due to the sequence when
>> V1P8_SIGNAL_ENA is set. The V1P8_SIGNAL_ENA has a timing component tied
>> to it where if set, switch to full-cycle timing happens. The failing
>> cards do not like change to full-cycle timing before changing bus
>> width, so add flag to sdhci-am654 binding to suppress V1P8_SIGNAL_ENA
>> before changing bus width. The switch to full-cycle timing should happen
>> with HIGH_SPEED_ENA after change of bus width.
>>
>> 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 676a74695389..0f92bbf8e13b 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:
> 
> Do not tell what the drivers should do, but tell what is the issue with
> the hardware, e.g. some cards do not like full-cycle.... and this will
> also hint you that it should be most likely generic, not specific to
> this device.
> 

Thanks for your review, but this patch has been dropped in v4 since
we adopted a new implementation [0] using compatible string.

[0] 
https://lore.kernel.org/linux-devicetree/20250423180809.l3l6sfbwquaaazar@shrank/

~ Judith