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

[Yao Zi <me@ziyao.cc>]

On Tue, Sep 08, 2026 at 05:27:43PM +0200, Andrei Lalaev wrote:
> On 07.09.26 20:44, Andrei Lalaev wrote:
> > Hi Yao,
> > 
> > On 07.09.26 20:08, Yao Zi wrote:
> >> On Mon, Sep 07, 2026 at 04:54:35PM +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.
> >>>
> >>> 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/
> >>> ---
> >>>  drivers/mmc/cv1800b_sdhci.c | 25 +++++++++++++++++++++++++
> >>>  1 file changed, 25 insertions(+)
> >>>
> >>> diff --git a/drivers/mmc/cv1800b_sdhci.c b/drivers/mmc/cv1800b_sdhci.c
> >>> index b756649f90f3..5c129682daba 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,10 +70,29 @@ static int cv1800b_execute_tuning(struct mmc *mmc, u8 opcode)
> >>>  }
> >>>  #endif
> >>>  
> >>> +static int cv1800b_deferred_probe(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,
> >>>  #endif
> >>> +	.deferred_probe = cv1800b_deferred_probe,
> >>
> >> Is there a reason to create a new function, instead of including the
> >> logic somewhere already existing, for example, cv1800b_sdhci_reset()?
> >> It seems to fit in cv1800b_sdhci_reset(), too, if it's for
> >> initialization of the PHY.
> > 
> > 
> > No, there is no particular reason. I simply didn't think about cv1800b_sdhci_reset (:
> > 
> > Thank you for the hint! I'll check it and will probably move it to reset.
> > 
> > 
> 
> I've rechecked, and unfortunately, moving it to cv1800b_sdhci_reset() is not
> an option because reset is called only from cv1800b_execute_tuning() and
> not accessible to SDHCI core (unlike in Linux MMC subsystem).
> 
> So I think I'll just drop deferred_probe() (wasn't actually required)
> and call the function directly from probe().

Sounds reasonable, too. Thanks.

> -- 
> Best regards,
> Andrei Lalaev

Best regards,
Yao Zi