Re: [PATCH 8/8] mmc: sdhci-cadence6: Add DLL master control and improve tuning reliability

[Peng Fan <peng.fan@oss.nxp.com>]

On Mon, Nov 10, 2025 at 09:37:37AM -0800, Tanmay Kathpalia wrote:
>- Add support for configuring the PHY DLL master control register for all
>  SD/eMMC timing modes (DS, HS, SDR, DDR, HS200, HS400) by extending the
>  PHY configuration arrays and writing the value during PHY adjustment.
>- Fix tuning reliability by toggling the DLL reset before and after
>  updating the PHY_DLL_SLAVE_CTRL_REG_ADDR register.
>
>Signed-off-by: Tanmay Kathpalia <tanmay.kathpalia@altera.com>
>Reviewed-by: Balsundar Ponnusamy <balsundar.ponnusamy@altera.com>
>---
> drivers/mmc/sdhci-cadence6.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/mmc/sdhci-cadence6.c b/drivers/mmc/sdhci-cadence6.c
>index d9467293807..91a245aa490 100644
>--- a/drivers/mmc/sdhci-cadence6.c
>+++ b/drivers/mmc/sdhci-cadence6.c
>@@ -58,7 +58,7 @@
> #define PHY_DLL_SLAVE_CTRL_REG_READ_DQS_CMD_DELAY	GENMASK(31, 24)
> #define PHY_DLL_SLAVE_CTRL_REG_READ_DQS_DELAY		GENMASK(7, 0)
> 
>-#define SDHCI_CDNS6_PHY_CFG_NUM		4
>+#define SDHCI_CDNS6_PHY_CFG_NUM		5
> #define SDHCI_CDNS6_CTRL_CFG_NUM	4
> 
> struct sdhci_cdns6_phy_cfg {
>@@ -76,6 +76,7 @@ static struct sdhci_cdns6_phy_cfg sd_ds_phy_cfgs[] = {
> 	{ "cdns,phy-gate-lpbk-ctrl-delay-sd-ds", 0x01A00040, },
> 	{ "cdns,phy-dll-slave-ctrl-sd-ds", 0x00000000, },
> 	{ "cdns,phy-dq-timing-delay-sd-ds", 0x00000001, },
>+	{ "cdns,phy-dll-master-ctrl-sd-ds", 0x00800004, },
> };
> 
> static struct sdhci_cdns6_phy_cfg sd_hs_phy_cfgs[] = {
>@@ -83,6 +84,7 @@ static struct sdhci_cdns6_phy_cfg sd_hs_phy_cfgs[] = {
> 	{ "cdns,phy-gate-lpbk-ctrl-delay-sd-hs", 0x01A00040, },
> 	{ "cdns,phy-dll-slave-ctrl-sd-hs", 0x00000000, },
> 	{ "cdns,phy-dq-timing-delay-sd-hs", 0x00000001, },
>+	{ "cdns,phy-dll-master-ctrl-sd-hs", 0x00800004, },
> };
> 
> static struct sdhci_cdns6_phy_cfg emmc_sdr_phy_cfgs[] = {
>@@ -90,6 +92,7 @@ static struct sdhci_cdns6_phy_cfg emmc_sdr_phy_cfgs[] = {
> 	{ "cdns,phy-gate-lpbk-ctrl-delay-emmc-sdr", 0x01A00040, },
> 	{ "cdns,phy-dll-slave-ctrl-emmc-sdr", 0x00000000, },
> 	{ "cdns,phy-dq-timing-delay-emmc-sdr", 0x00000001, },
>+	{ "cdns,phy-dll-master-ctrl-emmc-sdr", 0x00800004, },
> };
> 
> static struct sdhci_cdns6_phy_cfg emmc_ddr_phy_cfgs[] = {
>@@ -97,6 +100,7 @@ static struct sdhci_cdns6_phy_cfg emmc_ddr_phy_cfgs[] = {
> 	{ "cdns,phy-gate-lpbk-ctrl-delay-emmc-ddr", 0x01A00040, },
> 	{ "cdns,phy-dll-slave-ctrl-emmc-ddr", 0x00000000, },
> 	{ "cdns,phy-dq-timing-delay-emmc-ddr", 0x10000001, },
>+	{ "cdns,phy-dll-master-ctrl-emmc-ddr", 0x00800004, },
> };
> 
> static struct sdhci_cdns6_phy_cfg emmc_hs200_phy_cfgs[] = {
>@@ -104,6 +108,7 @@ static struct sdhci_cdns6_phy_cfg emmc_hs200_phy_cfgs[] = {
> 	{ "cdns,phy-gate-lpbk-ctrl-delay-emmc-hs200", 0x01A00040, },
> 	{ "cdns,phy-dll-slave-ctrl-emmc-hs200", 0x00DADA00, },
> 	{ "cdns,phy-dq-timing-delay-emmc-hs200", 0x00000001, },
>+	{ "cdns,phy-dll-master-ctrl-emmc-hs200", 0x00000004, },
> };
> 
> static struct sdhci_cdns6_phy_cfg emmc_hs400_phy_cfgs[] = {
>@@ -111,6 +116,7 @@ static struct sdhci_cdns6_phy_cfg emmc_hs400_phy_cfgs[] = {
> 	{ "cdns,phy-gate-lpbk-ctrl-delay-emmc-hs400", 0x01A00040, },
> 	{ "cdns,phy-dll-slave-ctrl-emmc-hs400", 0x00DAD800, },
> 	{ "cdns,phy-dq-timing-delay-emmc-hs400", 0x00000001, },
>+	{ "cdns,phy-dll-master-ctrl-emmc-hs400", 0x00000004, },
> };
> 
> static struct sdhci_cdns6_ctrl_cfg sd_ds_ctrl_cfgs[] = {
>@@ -252,6 +258,7 @@ int sdhci_cdns6_phy_adj(struct udevice *dev, struct sdhci_cdns_plat *plat, u32 m
> 
> 	sdhci_cdns6_write_phy_reg(plat, PHY_DQS_TIMING_REG_ADDR, sdhci_cdns6_phy_cfgs[0].val);
> 	sdhci_cdns6_write_phy_reg(plat, PHY_GATE_LPBK_CTRL_REG_ADDR, sdhci_cdns6_phy_cfgs[1].val);
>+	sdhci_cdns6_write_phy_reg(plat, PHY_DLL_MASTER_CTRL_REG_ADDR, sdhci_cdns6_phy_cfgs[4].val);
> 	sdhci_cdns6_write_phy_reg(plat, PHY_DLL_SLAVE_CTRL_REG_ADDR, sdhci_cdns6_phy_cfgs[2].val);
> 
> 	/* Switch Off the DLL Reset */
>@@ -296,6 +303,7 @@ int sdhci_cdns6_phy_init(struct udevice *dev, struct sdhci_cdns_plat *plat)
> int sdhci_cdns6_set_tune_val(struct sdhci_cdns_plat *plat, unsigned int val)
> {
> 	u32 tmp, tuneval;
>+	int ret;
> 
> 	tuneval = (val * 256) / SDHCI_CDNS_MAX_TUNING_LOOP;
> 
>@@ -304,7 +312,18 @@ int sdhci_cdns6_set_tune_val(struct sdhci_cdns_plat *plat, unsigned int val)
> 		 PHY_DLL_SLAVE_CTRL_REG_READ_DQS_DELAY);
> 	tmp |= FIELD_PREP(PHY_DLL_SLAVE_CTRL_REG_READ_DQS_CMD_DELAY, tuneval) |
> 		FIELD_PREP(PHY_DLL_SLAVE_CTRL_REG_READ_DQS_DELAY, tuneval);
>+
>+	/* Switch On the DLL Reset */
>+	sdhci_cdns6_reset_phy_dll(plat, true);

There is no err return check, but

>+
> 	sdhci_cdns6_write_phy_reg(plat, PHY_DLL_SLAVE_CTRL_REG_ADDR, tmp);
> 
>+	/* Switch Off the DLL Reset */
>+	ret = sdhci_cdns6_reset_phy_dll(plat, false);

there is err check here. Should these be aligned or the usage is intentional?

Regards
Peng

>+	if (ret) {
>+		printf("sdhci_cdns6_reset_phy is not completed\n");
>+		return ret;
>+	}
>+
> 	return 0;
> }
>-- 
>2.43.7
>
>