[PATCH v4 0/2] Add SDHCI_QUIRK2_SUPPRESS_V1P8_ENA

[PATCH v4 0/2] Add SDHCI_QUIRK2_SUPPRESS_V1P8_ENA

[Judith Mendez]

There are MMC boot failures seen with V1P8_SIGNAL_ENA on Kingston eMMC and
Microcenter/Patriot SD cards on am62* Sitara K3 boards due to the HS200
initialization sequence involving V1P8_SIGNAL_ENA. Since V1P8_SIGNAL_ENA
is optional for eMMC and only affects timing for host controllers using
ti,am62-sdhci compatible so far, add a new platform data structure for am62
compatible and append the new SDHCI_QUIRK2_SUPPRESS_V1P8_ENA 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 the quirk based on compatible string,
ensuring the quirk is never applied to devices with internal LDOs, then
V1P8_SIGNAL_ENA also has a voltage component tied to it.

Changes since v3 RESEND:
- apply the quirk based on compatible string instead of based on bus
  width or DT property
- rephrase commit descriptions and cover-letter descriptions based on
  new implementation.

Link to v3 RESEND:
https://lore.kernel.org/linux-mmc/8678d284-db12-451a-b789-2b75f9932f9f@ti.com
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 (2):
  mmc: sdhci: Add a quirk for suppressing V1P8_SIGNAL_ENA
  mmc: sdhci_am654: Add SDHCI_QUIRK2_SUPPRESS_V1P8_ENA quirk to am62
    compatible

 drivers/mmc/host/sdhci.c       |  8 +++++---
 drivers/mmc/host/sdhci.h       |  2 ++
 drivers/mmc/host/sdhci_am654.c | 14 +++++++++++++-
 3 files changed, 20 insertions(+), 4 deletions(-)


base-commit: 1be38f81251f6d276713c259ecf4414f82f22c29
-- 
2.49.0

[PATCH v4 1/2] mmc: sdhci: Add a quirk for suppressing V1P8_SIGNAL_ENA

[Judith Mendez]

There are init failures across Kingston eMMC and SD cards like Microcenter
and Patriot due to the sequence of when V1P8_SIGNAL_ENA is set. Since it
is not always required, add a quirk SDHCI_QUIRK2_SUPPRESS_V1P8_ENA which
allows users with init failures to suppress V1P8_SIGNAL_ENA.

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
Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/mmc/host/sdhci.c | 8 +++++---
 drivers/mmc/host/sdhci.h | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index fd5681d1e31f..ef66dede8d09 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2698,8 +2698,10 @@ int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 		 * Enable 1.8V Signal Enable in the Host Control2
 		 * register
 		 */
-		ctrl |= SDHCI_CTRL_VDD_180;
-		sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+		if (!(host->quirks2 & SDHCI_QUIRK2_SUPPRESS_V1P8_ENA)) {
+			ctrl |= SDHCI_CTRL_VDD_180;
+			sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
+		}
 
 		/* Some controller need to do more when switching */
 		if (host->ops->voltage_switch)
@@ -2707,7 +2709,7 @@ int sdhci_start_signal_voltage_switch(struct mmc_host *mmc,
 
 		/* 1.8V regulator output should be stable within 5 ms */
 		ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
-		if (ctrl & SDHCI_CTRL_VDD_180)
+		if (ctrl & SDHCI_CTRL_VDD_180 || (host->quirks2 & SDHCI_QUIRK2_SUPPRESS_V1P8_ENA))
 			return 0;
 
 		pr_warn("%s: 1.8V regulator output did not become stable\n",
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index cd0e35a80542..60e180456ccc 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -536,6 +536,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK2_USE_32BIT_BLK_CNT			(1<<18)
 /* Issue CMD and DATA reset together */
 #define SDHCI_QUIRK2_ISSUE_CMD_DAT_RESET_TOGETHER	(1<<19)
+/* Suppress setting V1P8_SIGNAL_ENA */
+#define SDHCI_QUIRK2_SUPPRESS_V1P8_ENA			(1<<20)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
2.49.0

[PATCH v4 2/2] mmc: sdhci_am654: Add SDHCI_QUIRK2_SUPPRESS_V1P8_ENA quirk to am62 compatible

[Judith Mendez]

Add a new struct for platform data for the ti,am62-sdhci compatible to
apply additional quirks, namely "SDHCI_QUIRK2_SUPPRESS_V1P8_ENA", to
host controllers with am62 compatible.

This fixes MMC init failures seen across am62x boards.

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
Suggested-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/mmc/host/sdhci_am654.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index f75c31815ab0..dd550587d48b 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -650,6 +650,18 @@ static const struct sdhci_am654_driver_data sdhci_j721e_4bit_drvdata = {
 	.flags = IOMUX_PRESENT,
 };
 
+static const struct sdhci_pltfm_data sdhci_am62_4bit_pdata = {
+	.ops = &sdhci_j721e_4bit_ops,
+	.quirks = SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
+	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+		   SDHCI_QUIRK2_SUPPRESS_V1P8_ENA,
+};
+
+static const struct sdhci_am654_driver_data sdhci_am62_4bit_drvdata = {
+	.pdata = &sdhci_am62_4bit_pdata,
+	.flags = IOMUX_PRESENT,
+};
+
 static const struct soc_device_attribute sdhci_am654_devices[] = {
 	{ .family = "AM65X",
 	  .revision = "SR1.0",
@@ -872,7 +884,7 @@ static const struct of_device_id sdhci_am654_of_match[] = {
 	},
 	{
 		.compatible = "ti,am62-sdhci",
-		.data = &sdhci_j721e_4bit_drvdata,
+		.data = &sdhci_am62_4bit_drvdata,
 	},
 	{ /* sentinel */ }
 };
-- 
2.49.0

Re: [PATCH v4 0/2] Add SDHCI_QUIRK2_SUPPRESS_V1P8_ENA

[Mendez, Judith]

Hi all,

On 4/24/2025 1:00 PM, Judith Mendez wrote:
> There are MMC boot failures seen with V1P8_SIGNAL_ENA on Kingston eMMC and
> Microcenter/Patriot SD cards on am62* Sitara K3 boards due to the HS200
> initialization sequence involving V1P8_SIGNAL_ENA. Since V1P8_SIGNAL_ENA
> is optional for eMMC and only affects timing for host controllers using
> ti,am62-sdhci compatible so far, add a new platform data structure for am62
> compatible and append the new SDHCI_QUIRK2_SUPPRESS_V1P8_ENA 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 the quirk based on compatible string,
> ensuring the quirk is never applied to devices with internal LDOs, then
> V1P8_SIGNAL_ENA also has a voltage component tied to it.

Gentle ping on this, are there any comments or any issues with this
type of implementation?

Thanks,

~ Judith

Re: [PATCH v4 0/2] Add SDHCI_QUIRK2_SUPPRESS_V1P8_ENA

[Ulf Hansson]

On Mon, 5 May 2025 at 23:24, Mendez, Judith <jm@ti.com> wrote:
>
> Hi all,
>
> On 4/24/2025 1:00 PM, Judith Mendez wrote:
> > There are MMC boot failures seen with V1P8_SIGNAL_ENA on Kingston eMMC and
> > Microcenter/Patriot SD cards on am62* Sitara K3 boards due to the HS200
> > initialization sequence involving V1P8_SIGNAL_ENA. Since V1P8_SIGNAL_ENA
> > is optional for eMMC and only affects timing for host controllers using
> > ti,am62-sdhci compatible so far, add a new platform data structure for am62
> > compatible and append the new SDHCI_QUIRK2_SUPPRESS_V1P8_ENA 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 the quirk based on compatible string,
> > ensuring the quirk is never applied to devices with internal LDOs, then
> > V1P8_SIGNAL_ENA also has a voltage component tied to it.
>
> Gentle ping on this, are there any comments or any issues with this
> type of implementation?

It looks reasonable to me. Although, in general I think we are trying
to avoid adding new sdhci quirks, perhaps there are good reasons to do
it in this case.

I am deferring to Adrian to make the decision.

Kind regards
Uffe

Re: [PATCH v4 0/2] Add SDHCI_QUIRK2_SUPPRESS_V1P8_ENA

[Adrian Hunter]

On 07/05/2025 13:14, Ulf Hansson wrote:
> On Mon, 5 May 2025 at 23:24, Mendez, Judith <jm@ti.com> wrote:
>>
>> Hi all,
>>
>> On 4/24/2025 1:00 PM, Judith Mendez wrote:
>>> There are MMC boot failures seen with V1P8_SIGNAL_ENA on Kingston eMMC and
>>> Microcenter/Patriot SD cards on am62* Sitara K3 boards due to the HS200
>>> initialization sequence involving V1P8_SIGNAL_ENA. Since V1P8_SIGNAL_ENA
>>> is optional for eMMC and only affects timing for host controllers using
>>> ti,am62-sdhci compatible so far, add a new platform data structure for am62
>>> compatible and append the new SDHCI_QUIRK2_SUPPRESS_V1P8_ENA 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 the quirk based on compatible string,
>>> ensuring the quirk is never applied to devices with internal LDOs, then
>>> V1P8_SIGNAL_ENA also has a voltage component tied to it.
>>
>> Gentle ping on this, are there any comments or any issues with this
>> type of implementation?
> 
> It looks reasonable to me. Although, in general I think we are trying
> to avoid adding new sdhci quirks, perhaps there are good reasons to do
> it in this case.

Yes, we want to avoid new quirks in sdhci.c.

Judith, can you do it like in V3? i.e. in sdhci_am654.c with
SDHCI_AM654_QUIRK_SUPPRESS_V1P8_ENA and sdhci_am654_start_signal_voltage_switch()