[PATCH v2] mmc: cv1800b_sdhci: configure SDHCI PHY

[PATCH v2] mmc: cv1800b_sdhci: configure SDHCI PHY

[Andrei Lalaev]

Some samples of the Milk-V Duo and Duo 256M have issues with SD card
communication. As a result, the SD card is not detected, or the correct
mode is not selected.

Configure SDHCI PHY in the same way as in downstream, to ensure
that the PHY is initialized properly.

Fixes: eb36f28ff721 ("mmc: cv1800b: Add sdhci driver support for cv1800b SoC")
Signed-off-by: Andrei Lalaev <andrey.lalaev@gmail.com>
Link: https://lore.kernel.org/u-boot/20260822091510.3253162-1-andrey.lalaev@gmail.com/
---
Changes in v2:
- rename cv1800b_deferred_probe() -> cv1800b_configure_phy()
- call the function from probe() instead of relying on deferred_probe
- Link to v1: https://patch.msgid.link/20260907-milkv-duo-sdhci-configure-v1-1-606b59eaafa0@gmail.com
---
 drivers/mmc/cv1800b_sdhci.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/cv1800b_sdhci.c b/drivers/mmc/cv1800b_sdhci.c
index b756649f90f3..440421813e35 100644
--- a/drivers/mmc/cv1800b_sdhci.c
+++ b/drivers/mmc/cv1800b_sdhci.c
@@ -8,11 +8,17 @@
 #include <sdhci.h>
 #include <linux/delay.h>
 
+#define CV18XX_SDHCI_MSHC_CTRL  0x200
+#define CV18XX_SDHCI_PHY_CONFIG 0x24c
 #define SDHCI_PHY_TX_RX_DLY  0x240
 #define MMC_MAX_CLOCK        375000000
 #define TUNE_MAX_PHCODE      128
 
 #define PHY_TX_SRC_INVERT  BIT(8)
+#define PHY_RX_SRC_INVERT  BIT(24)
+
+#define CV18XX_LATANCY_1T BIT(1)
+#define CV18XX_PHY_TX_BPS BIT(0)
 
 struct cv1800b_sdhci_plat {
 	struct mmc_config cfg;
@@ -64,6 +70,24 @@ static int cv1800b_execute_tuning(struct mmc *mmc, u8 opcode)
 }
 #endif
 
+static int cv1800b_configure_phy(struct sdhci_host *host)
+{
+	u32 val;
+
+	val = sdhci_readl(host, CV18XX_SDHCI_MSHC_CTRL);
+	val |= CV18XX_LATANCY_1T;
+	sdhci_writel(host, val, CV18XX_SDHCI_MSHC_CTRL);
+
+	val = sdhci_readl(host, CV18XX_SDHCI_PHY_CONFIG);
+	val |= CV18XX_PHY_TX_BPS;
+	sdhci_writel(host, val, CV18XX_SDHCI_PHY_CONFIG);
+
+	val = PHY_TX_SRC_INVERT | PHY_RX_SRC_INVERT;
+	sdhci_writel(host, val, SDHCI_PHY_TX_RX_DLY);
+
+	return 0;
+}
+
 const struct sdhci_ops cv1800b_sdhci_sd_ops = {
 #if CONFIG_IS_ENABLED(MMC_SUPPORTS_TUNING)
 	.platform_execute_tuning = cv1800b_execute_tuning,
@@ -105,7 +129,11 @@ static int cv1800b_sdhci_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	return sdhci_probe(dev);
+	ret = sdhci_probe(dev);
+	if (ret)
+		return ret;
+
+	return cv1800b_configure_phy(host);
 }
 
 static const struct udevice_id cv1800b_sdhci_match[] = {

---
base-commit: 509312dc5386ff4c5adabf53a573abca0aac4ce5
change-id: 20260907-milkv-duo-sdhci-configure-aa9790c73d0b

Best regards,
--  
Andrei Lalaev <andrey.lalaev@gmail.com>

Re: [PATCH v2] mmc: cv1800b_sdhci: configure SDHCI PHY

[Hiago De Franco]

Hi Andrei,

On Wed, Sep 09, 2026 at 05:21:12PM +0200, Andrei Lalaev wrote:
> Some samples of the Milk-V Duo and Duo 256M have issues with SD card
> communication. As a result, the SD card is not detected, or the correct
> mode is not selected.
> 
> Configure SDHCI PHY in the same way as in downstream, to ensure
> that the PHY is initialized properly.

Thanks for the v2 patch.

I tested this locally with my Milk-V Duo 256MB, same tests as before, and
it works without issues. So:

Tested-by: Hiago De Franco <hfranco@baylibre.com> # Milk-V Duo 256M

My only minor comment would be related to the 'CV18XX_*' defines, the
others do not see to have this prefix, so I would drop them and keep it
like 'SDHCI_MSHC_CTRL'.

However, as usual, I also asked and AI agent (Claude Sonnet 5 - high, in
this case) to review this patch and looks like it found something
useful:

```
This always sets PHY_TX_BPS and writes TX_SRC_INVERT | RX_SRC_INVERT,
and it only runs once, from probe(). Sophgo's own driver treats this
as mode-dependent, not a fixed value - cvi_general_reset() in their
tree branches on the live SDHCI_HOST_CONTROL2 UHS bits:

    if SDR104:
        PHY_CONFIG    &= ~BIT(0)                 /* TX_BPS off */
        PHY_TX_RX_DLY  = 0x00000100              /* TX_SRC_INVERT only, tap=0 */
    else:
        PHY_CONFIG    |= BIT(0)                  /* TX_BPS on */
        PHY_TX_RX_DLY  = 0x01000100              /* TX_SRC_INVERT | RX_SRC_INVERT */

and it does this on every reset, not once. With this patch, a board
that actually negotiates SDR104 (LicheeRV Nano, or any cv1800b board
without no-1-8-v in the DT) would go into the tuning sweep with
TX_BPS stuck on - cv1800b_execute_tuning() never touches offset 0x24c.

I don't think this affects Duo or Duo 256M today - both set no-1-8-v,
so they stay in HS and only ever want the else-branch values, which is
exactly what's tested above. But since cv1800b_sdhci.c is shared with
LicheeRV Nano, it seemed worth flagging rather than silently baking in
a value that's only correct for one mode."
```

I can't test this since I do not have other boards apart from Milk-V duo
with me, maybe you can take a look at this review to check if it is
valid or not?

According to the agent, a possible fix would be:

```
diff --git i/drivers/mmc/cv1800b_sdhci.c w/drivers/mmc/cv1800b_sdhci.c
index 440421813e3..27203613e5f 100644
--- i/drivers/mmc/cv1800b_sdhci.c
+++ w/drivers/mmc/cv1800b_sdhci.c
@@ -70,25 +70,30 @@ static int cv1800b_execute_tuning(struct mmc *mmc, u8 opcode)
 }
 #endif
 
-static int cv1800b_configure_phy(struct sdhci_host *host)
+static int cv1800b_sdhci_set_ios_post(struct sdhci_host *host)
 {
-	u32 val;
+	u32 mshc_ctrl = sdhci_readl(host, CV18XX_SDHCI_MSHC_CTRL);
+	u32 phy_config = sdhci_readl(host, CV18XX_SDHCI_PHY_CONFIG);
 
-	val = sdhci_readl(host, CV18XX_SDHCI_MSHC_CTRL);
-	val |= CV18XX_LATANCY_1T;
-	sdhci_writel(host, val, CV18XX_SDHCI_MSHC_CTRL);
+	if (host->mmc->selected_mode == UHS_SDR104) {
+		mshc_ctrl &= ~CV18XX_LATANCY_1T;
+		phy_config &= ~CV18XX_PHY_TX_BPS;
+		/* tap delay is set by cv1800b_execute_tuning() right after this */
+		sdhci_writel(host, PHY_TX_SRC_INVERT, SDHCI_PHY_TX_RX_DLY);
+	} else {
+		mshc_ctrl |= CV18XX_LATANCY_1T;
+		phy_config |= CV18XX_PHY_TX_BPS;
+		sdhci_writel(host, PHY_TX_SRC_INVERT | PHY_RX_SRC_INVERT, SDHCI_PHY_TX_RX_DLY);
+	}
 
-	val = sdhci_readl(host, CV18XX_SDHCI_PHY_CONFIG);
-	val |= CV18XX_PHY_TX_BPS;
-	sdhci_writel(host, val, CV18XX_SDHCI_PHY_CONFIG);
-
-	val = PHY_TX_SRC_INVERT | PHY_RX_SRC_INVERT;
-	sdhci_writel(host, val, SDHCI_PHY_TX_RX_DLY);
+	sdhci_writel(host, mshc_ctrl, CV18XX_SDHCI_MSHC_CTRL);
+	sdhci_writel(host, phy_config, CV18XX_SDHCI_PHY_CONFIG);
 
 	return 0;
 }
 
 const struct sdhci_ops cv1800b_sdhci_sd_ops = {
+	.set_ios_post = cv1800b_sdhci_set_ios_post,
 #if CONFIG_IS_ENABLED(MMC_SUPPORTS_TUNING)
 	.platform_execute_tuning = cv1800b_execute_tuning,
 #endif
@@ -129,11 +134,7 @@ static int cv1800b_sdhci_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
-	ret = sdhci_probe(dev);
-	if (ret)
-		return ret;
-
-	return cv1800b_configure_phy(host);
+	return sdhci_probe(dev);
 }
 
 static const struct udevice_id cv1800b_sdhci_match[] = {
```

Regards,
-- 
Hiago

Re: [PATCH v2] mmc: cv1800b_sdhci: configure SDHCI PHY

[Andrei Lalaev]

Hi Hiago,

On 09.09.26 22:18, Hiago De Franco wrote:
> Hi Andrei,
> 
> On Wed, Sep 09, 2026 at 05:21:12PM +0200, Andrei Lalaev wrote:
>> Some samples of the Milk-V Duo and Duo 256M have issues with SD card
>> communication. As a result, the SD card is not detected, or the correct
>> mode is not selected.
>>
>> Configure SDHCI PHY in the same way as in downstream, to ensure
>> that the PHY is initialized properly.
> 
> Thanks for the v2 patch.
> 
> I tested this locally with my Milk-V Duo 256MB, same tests as before, and
> it works without issues. So:
> 
> Tested-by: Hiago De Franco <hfranco@baylibre.com> # Milk-V Duo 256M

Thank you for the tag :)

> My only minor comment would be related to the 'CV18XX_*' defines, the
> others do not see to have this prefix, so I would drop them and keep it
> like 'SDHCI_MSHC_CTRL'.

To be honest, I would rename the other defines as they are not generic SDHCI,
but CV1800B-specific, so I think it makes sense to prepend CV18XX.
No strong opinion, though.

> However, as usual, I also asked and AI agent (Claude Sonnet 5 - high, in
> this case) to review this patch and looks like it found something
> useful:
> 
> ```
> This always sets PHY_TX_BPS and writes TX_SRC_INVERT | RX_SRC_INVERT,
> and it only runs once, from probe(). Sophgo's own driver treats this
> as mode-dependent, not a fixed value - cvi_general_reset() in their
> tree branches on the live SDHCI_HOST_CONTROL2 UHS bits:
> 
>     if SDR104:
>         PHY_CONFIG    &= ~BIT(0)                 /* TX_BPS off */
>         PHY_TX_RX_DLY  = 0x00000100              /* TX_SRC_INVERT only, tap=0 */
>     else:
>         PHY_CONFIG    |= BIT(0)                  /* TX_BPS on */
>         PHY_TX_RX_DLY  = 0x01000100              /* TX_SRC_INVERT | RX_SRC_INVERT */
> 
> and it does this on every reset, not once. With this patch, a board
> that actually negotiates SDR104 (LicheeRV Nano, or any cv1800b board
> without no-1-8-v in the DT) would go into the tuning sweep with
> TX_BPS stuck on - cv1800b_execute_tuning() never touches offset 0x24c.
> 
> I don't think this affects Duo or Duo 256M today - both set no-1-8-v,
> so they stay in HS and only ever want the else-branch values, which is
> exactly what's tested above. But since cv1800b_sdhci.c is shared with
> LicheeRV Nano, it seemed worth flagging rather than silently baking in
> a value that's only correct for one mode."
> ```
> 
> I can't test this since I do not have other boards apart from Milk-V duo
> with me, maybe you can take a look at this review to check if it is
> valid or not?

I don't have a LicheeRC Nano either, but both DTSes have 'no-1-8-v' property:
  - arch/riscv/dts/sg2002-licheerv-nano-b.dts +38
  - dts/upstream/src/riscv/sophgo/sg2002-licheerv-nano-b.dts +85

So it shouldn't break the board.

That said, since I'd prefer not to introduce changes I can't test
myself (like SDR104 mode), we can do something like this instead:

```
diff --git a/drivers/mmc/cv1800b_sdhci.c b/drivers/mmc/cv1800b_sdhci.c
index b756649f90f3..2512352383b2 100644
--- a/drivers/mmc/cv1800b_sdhci.c
+++ b/drivers/mmc/cv1800b_sdhci.c
@@ -8,11 +8,17 @@
 #include <sdhci.h>
 #include <linux/delay.h>
 
+#define SDHCI_MSHC_CTRL      0x200
+#define SDHCI_PHY_CONFIG     0x24c
 #define SDHCI_PHY_TX_RX_DLY  0x240
 #define MMC_MAX_CLOCK        375000000
 #define TUNE_MAX_PHCODE      128
 
 #define PHY_TX_SRC_INVERT  BIT(8)
+#define PHY_RX_SRC_INVERT  BIT(24)
+
+#define PHY_LATANCY_1T BIT(1)
+#define PHY_TX_BPS BIT(0)
 
 struct cv1800b_sdhci_plat {
 	struct mmc_config cfg;
@@ -64,10 +70,34 @@ static int cv1800b_execute_tuning(struct mmc *mmc, u8 opcode)
 }
 #endif
 
+static int cv1800b_set_ios_post(struct sdhci_host *host)
+{
+	struct mmc *mmc = host->mmc;
+	u32 val;
+
+	if (mmc->selected_mode != MMC_LEGACY && mmc->selected_mode != MMC_HS &&
+	    mmc->selected_mode != SD_HS)
+		return 0;
+
+	val = sdhci_readl(host, SDHCI_MSHC_CTRL);
+	val |= PHY_LATANCY_1T;
+	sdhci_writel(host, val, SDHCI_MSHC_CTRL);
+
+	val = sdhci_readl(host, SDHCI_PHY_CONFIG);
+	val |= PHY_TX_BPS;
+	sdhci_writel(host, val, SDHCI_PHY_CONFIG);
+
+	val = PHY_TX_SRC_INVERT | PHY_RX_SRC_INVERT;
+	sdhci_writel(host, val, SDHCI_PHY_TX_RX_DLY);
+
+	return 0;
+}
+
 const struct sdhci_ops cv1800b_sdhci_sd_ops = {
 #if CONFIG_IS_ENABLED(MMC_SUPPORTS_TUNING)
 	.platform_execute_tuning = cv1800b_execute_tuning,
 #endif
+	.set_ios_post = cv1800b_set_ios_post,
 };
 
 static int cv1800b_sdhci_bind(struct udevice *dev)
```


> According to the agent, a possible fix would be:
> ...
> 
> Regards,


-- 
Best regards,
Andrei Lalaev