Re: [PATCH v2 4/4] mmc: sdhci-cadence6: read SD6HC DDR50 tuning from device tree

[Adrian Hunter <adrian.hunter@intel.com>]

On 09/10/2026 05:14, tze.yee.ng@altera.com wrote:
> From: Tze Yee Ng <tze.yee.ng@altera.com>
> 
> DDR50 has no CMD19 tuning, so the SD6HC read path relies on static PHY
> settings that need board/SoC characterisation. Read the read-DQS delay,
> read-DQS source and phony DQS assertion timing from the new
> cdns,ddr-read-dqs-delay-ps, cdns,ddr-no-lpbk-dqs and
> cdns,ddr-phony-dqs-timing DT properties at PHY probe and apply them in
> the extended-read DDR modes (DDR50 and DDR52). The read-DQS delay is
> given in picoseconds and converted to delay elements with
> cdns,delay-element-ps; since that quotient depends on the element size,
> it is clamped to the 8-bit PHY field maximum (255) to avoid wrap-around.
> 
> Loopback DQS is the recommended read-capture path and stays the default;
> cdns,ddr-no-lpbk-dqs lets a board fall back to the fabricated phony DQS.
> When a property is absent the driver-computed value is kept - for the
> phony DQS timing, the value derived from the RE# pulse width
> (REBAR_PULSE_CYCLES-1).
> 
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
> Changes in v2:
> - Updated patch title to "mmc: sdhci-cadence6:" since it touches only the
>   v6 PHY.
> - Renamed the properties to cdns,ddr-*.
> - Read cdns,ddr-read-dqs-delay-ps in ps and convert it to delay elements 
>   (dividing by cdns,delay-element-ps), clamping that quotient to the 8-bit
>   PHY field. (Krzysztof Kozlowski)
> - Read cdns,ddr-no-lpbk-dqs via of_property_read_bool(). (Krzysztof Kozlowski)
> - Dropped the v1 per-property range checks and redundant FIELD_MAX() write
>   masks. (Adrian Hunter)
> - Apply the overrides in both DDR50 and DDR52.
> ---
>  drivers/mmc/host/sdhci-cadence-phy-v6.c | 59 ++++++++++++++++++++++++-
>  1 file changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> index 8d6a9c26c139..2b1b7256731a 100644
> --- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
> +++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> @@ -129,6 +129,11 @@ struct sdhci_cdns6_phy {
>  	/* Active delay element (ps); doubled when one SDMCLK requires > 256 steps */
>  	u32 delay_element;
>  
> +	/* DDR read-path overrides (SoC-specific) */
> +	u32 ddr_read_dqs_delay_ps;
> +	bool ddr_no_lpbk_dqs;
> +	s32 ddr_phony_dqs_timing;
> +
>  	/* PHY_DLL_SLAVE_CTRL register fields */
>  	u8 cp_read_dqs_cmd_delay;	/* bits [31:24] */
>  	u8 cp_clk_wrdqs_delay;		/* bits [23:16] */
> @@ -145,6 +150,7 @@ struct sdhci_cdns6_phy {
>  	/* PHY_DQS_TIMING register fields */
>  	bool cp_use_phony_dqs;		/* bit [20] */
>  	bool cp_use_phony_dqs_cmd;	/* bit [19] */
> +	bool cp_use_lpbk_dqs;		/* bit [21] */
>  
>  	/* PHY_CTRL register fields */
>  	u32 cp_phony_dqs_timing;
> @@ -523,6 +529,16 @@ static void sdhci_cdns6_phy_calc_dat_in(struct sdhci_cdns6_phy *phy)
>  	if (phy->mode == MMC_TIMING_MMC_HS200)
>  		phy->cp_read_dqs_delay = phy->hs200_tune_val;
>  
> +	if ((phy->mode == MMC_TIMING_UHS_DDR50 ||
> +	     phy->mode == MMC_TIMING_MMC_DDR52) && phy->ddr_read_dqs_delay_ps) {

The same (phy->mode == MMC_TIMING_UHS_DDR50 || phy->mode == MMC_TIMING_MMC_DDR52)
appears 4 times.  Might be neater to assign it to a local variable.

> +		u32 steps = phy->ddr_read_dqs_delay_ps / phy->delay_element_org;
> +		u32 max_steps = FIELD_MAX(SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY);
> +
> +		phy->cp_read_dqs_delay = min_t(u32, steps, max_steps);
> +	}
> +
> +	phy->cp_use_lpbk_dqs = 1;
> +
>  	if (phy->sdhc_extended_rd_mode &&
>  	    (phy->mode == MMC_TIMING_UHS_DDR50 ||
>  	     phy->mode == MMC_TIMING_MMC_DDR52))
> @@ -530,6 +546,23 @@ static void sdhci_cdns6_phy_calc_dat_in(struct sdhci_cdns6_phy *phy)
>  	else
>  		phy->cp_phony_dqs_timing = 0;
>  
> +	/*
> +	 * Loopback DQS is the recommended read-capture path; allow boards that
> +	 * need the fabricated phony DQS instead to opt out in the DDR modes.
> +	 */
> +	if ((phy->mode == MMC_TIMING_UHS_DDR50 ||
> +	     phy->mode == MMC_TIMING_MMC_DDR52) && phy->ddr_no_lpbk_dqs)
> +		phy->cp_use_lpbk_dqs = 0;
> +
> +	/*
> +	 * The computed phony DQS timing above is derived from the RE# pulse
> +	 * width. Allow a DT override for boards whose flight time shifts the
> +	 * returning DDR data relative to the fabricated strobe.
> +	 */
> +	if ((phy->mode == MMC_TIMING_UHS_DDR50 ||
> +	     phy->mode == MMC_TIMING_MMC_DDR52) && phy->ddr_phony_dqs_timing >= 0)
> +		phy->cp_phony_dqs_timing = phy->ddr_phony_dqs_timing;
> +
>  	if (strobe_dat) {
>  		/* dqs loopback input via IO cell */
>  		hcsdclkadj += phy->iocell_input_delay;
> @@ -693,10 +726,11 @@ int sdhci_cdns6_phy_init(struct sdhci_cdns_priv *priv)
>  	sdhci_cdns6_dll_reset(priv, true);
>  
>  	reg = sdhci_cdns6_read_phy_reg(priv, SDHCI_CDNS6_PHY_DQS_TIMING_REG);
> +	reg &= ~SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS;
>  	reg &= ~SDHCI_CDNS6_PHY_DQS_TIMING_USE_PHONY_DQS;
>  	reg &= ~SDHCI_CDNS6_PHY_DQS_TIMING_USE_PHONY_DQS_CMD;
>  	reg |= SDHCI_CDNS6_PHY_DQS_TIMING_USE_EXT_LPBK_DQS;
> -	reg |= SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS;
> +	reg |= FIELD_PREP(SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS, phy->cp_use_lpbk_dqs);
>  	reg |= FIELD_PREP(SDHCI_CDNS6_PHY_DQS_TIMING_USE_PHONY_DQS, phy->cp_use_phony_dqs);
>  	reg |= FIELD_PREP(SDHCI_CDNS6_PHY_DQS_TIMING_USE_PHONY_DQS_CMD, phy->cp_use_phony_dqs_cmd);
>  	sdhci_cdns6_write_phy_reg(priv, SDHCI_CDNS6_PHY_DQS_TIMING_REG, reg);
> @@ -878,6 +912,7 @@ int sdhci_cdns6_phy_probe(struct platform_device *pdev, struct sdhci_cdns_priv *
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct sdhci_cdns6_phy *phy;
>  	unsigned long val;
> +	u32 prop;
>  	int ret;
>  
>  	phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
> @@ -916,6 +951,28 @@ int sdhci_cdns6_phy_probe(struct platform_device *pdev, struct sdhci_cdns_priv *
>  
>  	phy->delay_element_org = phy->delay_element;
>  
> +	/*
> +	 * Optional read-path tuning for the extended-read DDR modes (SD DDR50,
> +	 * eMMC DDR52).
> +	 */
> +	phy->ddr_read_dqs_delay_ps = 0;
> +	of_property_read_u32(dev->of_node, "cdns,ddr-read-dqs-delay-ps",
> +			     &phy->ddr_read_dqs_delay_ps);
> +	if (phy->ddr_read_dqs_delay_ps / phy->delay_element_org >
> +	    FIELD_MAX(SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY))

This is still effectively validating the firmware values

> +		dev_warn(dev,
> +			 "cdns,ddr-read-dqs-delay-ps %u too large, clamping to %lu elements\n",
> +			 phy->ddr_read_dqs_delay_ps,
> +			 (unsigned long)FIELD_MAX(SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY));
> +
> +	phy->ddr_no_lpbk_dqs = of_property_read_bool(dev->of_node,
> +						     "cdns,ddr-no-lpbk-dqs");

Unneeded line wrapping - can use 100 cols

> +
> +	phy->ddr_phony_dqs_timing = -1;
> +	if (!of_property_read_u32(dev->of_node, "cdns,ddr-phony-dqs-timing",
> +				  &prop))

Unneeded line wrapping - can use 100 cols

> +		phy->ddr_phony_dqs_timing = prop;
> +
>  	priv->phy = phy;
>  
>  	return 0;