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 <[email protected]> # 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
