[PATCH 0/4] mmc: sdhci-cadence: SD6HC DDR50 read-path tuning and fixes

[PATCH 0/4] mmc: sdhci-cadence: SD6HC DDR50 read-path tuning and fixes

[tze.yee.ng]

From: Tze Yee Ng <tze.yee.ng@altera.com>

This series enables DDR50 on the Cadence SD6HC (SD/eMMC v6) PHY. DDR50
has no CMD19 tuning, so the read path must be centred by static,
board/SoC-characterised PHY settings. Bringing it up on Agilex5 also
exposed PHY issues affecting the existing tuned modes.

  - Patch 1: add the post-tuning settle delay the PHY needs after the
    DLL re-locks, matching sdhci_cdns6_phy_init().
  - Patch 2: program PHONY_DQS_TIMING (left 0 in all modes); in
    extended-read DDR it must be the rebar pulse width in clk_phy cycles
    minus 1, else one DDR beat is duplicated.
  - Patch 3: document three optional SD6HC DDR50 read-path properties.
  - Patch 4: read those properties at probe and apply them in DDR50,
    and fix the DQS_TIMING read-modify-write so USE_LPBK_DQS is cleared
    before being reprogrammed.

The new cdns,ddr50-* properties are unused in this series; they will be
consumed by the socfpga_agilex5_socdk and socfpga_agilex5_socdk_emmc
device trees, submitted as a separate series.

Tested on Agilex5 SoCDK: SD DDR50 and eMMC DDR52.

Tze Yee Ng (4):
  mmc: sdhci-cadence6: add PHY settle delay after tuning DLL re-lock
  mmc: sdhci-cadence6: program PHONY_DQS_TIMING for extended-read DDR
  dt-bindings: mmc: cdns,sdhci: add SD6HC DDR50 read-path tuning
  mmc: sdhci-cadence: read SD6HC DDR50 tuning from device tree

 .../devicetree/bindings/mmc/cdns,sd6hc.yaml   | 27 ++++++
 drivers/mmc/host/sdhci-cadence-phy-v6.c       | 90 ++++++++++++++++++-
 2 files changed, 116 insertions(+), 1 deletion(-)

-- 
2.43.7

[PATCH 1/4] mmc: sdhci-cadence6: add PHY settle delay after tuning DLL re-lock

[tze.yee.ng]

From: Tze Yee Ng <tze.yee.ng@altera.com>

After re-locking the DLL, allow the same 5 to 5.5 ms for the clock and
PHY signals to stabilize as sdhci_cdns6_phy_init() does. Without this
settle time the command issued immediately after tuning (e.g. the R1b
CMD6 that switches eMMC from HS200 down to HS during HS400 selection)
can time out.

Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 drivers/mmc/host/sdhci-cadence-phy-v6.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
index 22d56bb46d75..35f35ef9c710 100644
--- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
+++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
@@ -811,6 +811,9 @@ int sdhci_cdns6_set_tune_val(struct sdhci_host *host, unsigned int val)
 	if (ret)
 		dev_warn(mmc_dev(host->mmc), "%s: DLL reset release failed: %d\n", __func__, ret);
 
+	/* Allow 5 to 5.5 ms for clock and PHY signals to stabilize after configuration */
+	usleep_range(5000, 5500);
+
 	return ret;
 }
 
-- 
2.43.7

[PATCH 2/4] mmc: sdhci-cadence6: program PHONY_DQS_TIMING for extended-read DDR

[tze.yee.ng]

From: Tze Yee Ng <tze.yee.ng@altera.com>

The SD6HC PHY left PHONY_DQS_TIMING (phy_ctrl_reg[9:4]) at 0 in all
modes. Per the Cadence DLL PHY documentation it must be the rebar (RE#)
pulse width in clk_phy cycles minus 1 in extended read mode, and 0
otherwise. Leaving it 0 in extended-read DDR duplicates one DDR edge
(the silent odd/even edge-capture defect).

This controller's rebar pulse is a fixed 2 clk_phy cycles, so extended-
read DDR needs 1; confirmed on DDR50 hardware (1 captures both beats, 2
corrupts reads). Derive it from the extended-read-mode state and apply
it only in DDR modes, since SDR extended-read samples a single edge and
is unaffected. This is generic to any SD6HC-PHY SoC, so it is kept
separate from the per-SoC read-path tuning.

Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 drivers/mmc/host/sdhci-cadence-phy-v6.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
index 35f35ef9c710..84592ae42762 100644
--- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
+++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
@@ -90,6 +90,9 @@
 #define SDHCI_CDNS6_PHY_CTRL_REG			0x2080
 #define   SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING			GENMASK(9, 4)
 
+/* Width of this controller's rebar (RE#) pulse in clk_phy cycles. */
+#define SDHCI_CDNS6_PHY_REBAR_PULSE_CYCLES	2
+
 /* Default PHY settings */
 #define SDHCI_CDNS6_PHY_DEFAULT_IOCELL_DELAY		2500
 #define SDHCI_CDNS6_PHY_DEFAULT_DELAY_ELEMENT		24
@@ -143,6 +146,9 @@ struct sdhci_cdns6_phy {
 	bool cp_use_phony_dqs;		/* bit [20] */
 	bool cp_use_phony_dqs_cmd;	/* bit [19] */
 
+	/* PHY_CTRL register fields */
+	u32 cp_phony_dqs_timing;
+
 	/* HRS07 register - IO delay Information */
 	u8 sdhc_rw_compensate;		/* bits [20:16] */
 	u8 sdhc_idelay_val;		/* bits [4:0] */
@@ -517,6 +523,13 @@ 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->sdhc_extended_rd_mode &&
+	    (phy->mode == MMC_TIMING_UHS_DDR50 ||
+	     phy->mode == MMC_TIMING_MMC_DDR52))
+		phy->cp_phony_dqs_timing = SDHCI_CDNS6_PHY_REBAR_PULSE_CYCLES - 1;
+	else
+		phy->cp_phony_dqs_timing = 0;
+
 	if (strobe_dat) {
 		/* dqs loopback input via IO cell */
 		hcsdclkadj += phy->iocell_input_delay;
@@ -715,6 +728,8 @@ int sdhci_cdns6_phy_init(struct sdhci_cdns_priv *priv)
 
 	reg = sdhci_cdns6_read_phy_reg(priv, SDHCI_CDNS6_PHY_CTRL_REG);
 	reg &= ~SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING;
+	reg |= FIELD_PREP(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING,
+			  phy->cp_phony_dqs_timing);
 	sdhci_cdns6_write_phy_reg(priv, SDHCI_CDNS6_PHY_CTRL_REG, reg);
 
 	/*
-- 
2.43.7

[PATCH 3/4] dt-bindings: mmc: cdns,sdhci: add SD6HC DDR50 read-path tuning

[tze.yee.ng]

From: Tze Yee Ng <tze.yee.ng@altera.com>

DDR50 has no CMD19 tuning, so the SD6HC read path must be centred by
static, board/SoC-characterised PHY settings. Add three optional SD6HC
properties:

  - cdns,ddr50-read-dqs-delay:   DLL_SLAVE[7:0] read-DQS delay that centres
    the read eye (0-255).
  - cdns,ddr50-use-lpbk-dqs:     DQS_TIMING[21] read-DQS source
    (0 = phony, 1 = loopback).
  - cdns,ddr50-phony-dqs-timing: PHY_CTRL[9:4] phony DQS assertion timing
    (0-63) that positions the fabricated strobe relative to the returning
    DDR data; not produced by the Cadence timing calculation.

All three are disallowed for the SD4HC variant.

Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 .../devicetree/bindings/mmc/cdns,sd6hc.yaml   | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
index d5ea2717904b..df86872603d0 100644
--- a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
+++ b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
@@ -74,6 +74,33 @@ properties:
     maximum: 1000
     default: 24
 
+  cdns,ddr50-read-dqs-delay:
+    description: |
+      SD6HC DDR50 read-DQS delay (DLL_SLAVE[7:0]) used to centre the read
+      eye. DDR50 has no CMD19 tuning, so this is a board/SoC-characterised
+      value. If absent, the driver default is used.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 0xff
+
+  cdns,ddr50-use-lpbk-dqs:
+    description: |
+      SD6HC DDR50 read-DQS source (DQS_TIMING[21]): 0 = phony DQS,
+      1 = loopback DQS. If absent, the driver default is used.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+
+  cdns,ddr50-phony-dqs-timing:
+    description: |
+      SD6HC DDR50 phony DQS assertion timing (PHY_CTRL[9:4]). Positions the
+      fabricated read strobe relative to the returning DDR data; the correct
+      value depends on the board's SD flight time and is not produced by the
+      Cadence timing calculation. If absent, the driver default
+      (REBAR_PULSE_CYCLES-1) is used.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 0x3f
+
 required:
   - compatible
   - reg
-- 
2.43.7

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

[tze.yee.ng]

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,ddr50-read-dqs-delay, cdns,ddr50-use-lpbk-dqs and
cdns,ddr50-phony-dqs-timing DT properties at PHY probe, range-check them,
and apply them only in DDR50. When a property is absent the existing
driver default is kept - for the phony DQS timing, the derived
REBAR_PULSE_CYCLES-1.

Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 drivers/mmc/host/sdhci-cadence-phy-v6.c | 72 ++++++++++++++++++++++++-
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
index 84592ae42762..0f47fa62d894 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) */
+	s32 ddr_read_dqs_delay;
+	s32 ddr_use_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,12 @@ 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->ddr_read_dqs_delay >= 0)
+		phy->cp_read_dqs_delay = phy->ddr_read_dqs_delay &
+			SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY;
+
+	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 +542,18 @@ static void sdhci_cdns6_phy_calc_dat_in(struct sdhci_cdns6_phy *phy)
 	else
 		phy->cp_phony_dqs_timing = 0;
 
+	if (phy->mode == MMC_TIMING_UHS_DDR50 && phy->ddr_use_lpbk_dqs >= 0)
+		phy->cp_use_lpbk_dqs = phy->ddr_use_lpbk_dqs &
+			FIELD_MAX(SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS);
+
+	/*
+	 * The phony DQS timing derived above depends on the board's SD flight
+	 * time, so allow a DT override to re-position the fabricated strobe.
+	 */
+	if (phy->mode == MMC_TIMING_UHS_DDR50 && phy->ddr_phony_dqs_timing >= 0)
+		phy->cp_phony_dqs_timing = phy->ddr_phony_dqs_timing &
+			FIELD_MAX(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING);
+
 	if (strobe_dat) {
 		/* dqs loopback input via IO cell */
 		hcsdclkadj += phy->iocell_input_delay;
@@ -693,10 +717,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);
@@ -881,6 +906,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);
@@ -919,6 +945,50 @@ int sdhci_cdns6_phy_probe(struct platform_device *pdev, struct sdhci_cdns_priv *
 
 	phy->delay_element_org = phy->delay_element;
 
+	/*
+	 * Optional DDR50 read-path tuning. These are board/card-characterised
+	 * values with no CMD19 tuning in DDR50; absence keeps the driver
+	 * default (-1 => not overridden).
+	 */
+	phy->ddr_read_dqs_delay = -1;
+	if (!of_property_read_u32(dev->of_node, "cdns,ddr50-read-dqs-delay",
+				  &prop)) {
+		if (prop > SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY) {
+			dev_warn(dev,
+				 "cdns,ddr50-read-dqs-delay %u out of range, clamping to %lu\n",
+				 prop,
+				 (unsigned long)SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY);
+			prop = SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY;
+		}
+		phy->ddr_read_dqs_delay = prop;
+	}
+
+	phy->ddr_use_lpbk_dqs = -1;
+	if (!of_property_read_u32(dev->of_node, "cdns,ddr50-use-lpbk-dqs",
+				  &prop)) {
+		if (prop > FIELD_MAX(SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS)) {
+			dev_warn(dev,
+				 "cdns,ddr50-use-lpbk-dqs %u out of range, clamping to %lu\n",
+				 prop,
+				 (unsigned long)FIELD_MAX(SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS));
+			prop = FIELD_MAX(SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS);
+		}
+		phy->ddr_use_lpbk_dqs = prop;
+	}
+
+	phy->ddr_phony_dqs_timing = -1;
+	if (!of_property_read_u32(dev->of_node, "cdns,ddr50-phony-dqs-timing",
+				  &prop)) {
+		if (prop > FIELD_MAX(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING)) {
+			dev_warn(dev,
+				 "cdns,ddr50-phony-dqs-timing %u out of range, clamping to %lu\n",
+				 prop,
+				 (unsigned long)FIELD_MAX(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING));
+			prop = FIELD_MAX(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING);
+		}
+		phy->ddr_phony_dqs_timing = prop;
+	}
+
 	priv->phy = phy;
 
 	return 0;
-- 
2.43.7

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

[Adrian Hunter]

On 22/09/2026 14:12, 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,ddr50-read-dqs-delay, cdns,ddr50-use-lpbk-dqs and
> cdns,ddr50-phony-dqs-timing DT properties at PHY probe, range-check them,

Firmware values are expected to be correct, and so are not validated.

> and apply them only in DDR50. When a property is absent the existing
> driver default is kept - for the phony DQS timing, the derived
> REBAR_PULSE_CYCLES-1.
> 
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
>  drivers/mmc/host/sdhci-cadence-phy-v6.c | 72 ++++++++++++++++++++++++-
>  1 file changed, 71 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> index 84592ae42762..0f47fa62d894 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) */
> +	s32 ddr_read_dqs_delay;
> +	s32 ddr_use_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,12 @@ 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->ddr_read_dqs_delay >= 0)
> +		phy->cp_read_dqs_delay = phy->ddr_read_dqs_delay &
> +			SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY;

So here is unnecessarily assuming DT is providing a bad value.  It could just be:

	if (phy->mode == MMC_TIMING_UHS_DDR50 && phy->ddr_read_dqs_delay >= 0)
		phy->cp_read_dqs_delay = phy->ddr_read_dqs_delay;

> +
> +	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 +542,18 @@ static void sdhci_cdns6_phy_calc_dat_in(struct sdhci_cdns6_phy *phy)
>  	else
>  		phy->cp_phony_dqs_timing = 0;
>  
> +	if (phy->mode == MMC_TIMING_UHS_DDR50 && phy->ddr_use_lpbk_dqs >= 0)
> +		phy->cp_use_lpbk_dqs = phy->ddr_use_lpbk_dqs &
> +			FIELD_MAX(SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS);

Ditto

> +
> +	/*
> +	 * The phony DQS timing derived above depends on the board's SD flight
> +	 * time, so allow a DT override to re-position the fabricated strobe.
> +	 */
> +	if (phy->mode == MMC_TIMING_UHS_DDR50 && phy->ddr_phony_dqs_timing >= 0)
> +		phy->cp_phony_dqs_timing = phy->ddr_phony_dqs_timing &
> +			FIELD_MAX(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING);

Ditto

> +
>  	if (strobe_dat) {
>  		/* dqs loopback input via IO cell */
>  		hcsdclkadj += phy->iocell_input_delay;
> @@ -693,10 +717,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);
> @@ -881,6 +906,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);
> @@ -919,6 +945,50 @@ int sdhci_cdns6_phy_probe(struct platform_device *pdev, struct sdhci_cdns_priv *
>  
>  	phy->delay_element_org = phy->delay_element;
>  
> +	/*
> +	 * Optional DDR50 read-path tuning. These are board/card-characterised
> +	 * values with no CMD19 tuning in DDR50; absence keeps the driver
> +	 * default (-1 => not overridden).
> +	 */
> +	phy->ddr_read_dqs_delay = -1;
> +	if (!of_property_read_u32(dev->of_node, "cdns,ddr50-read-dqs-delay",
> +				  &prop)) {
> +		if (prop > SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY) {
> +			dev_warn(dev,
> +				 "cdns,ddr50-read-dqs-delay %u out of range, clamping to %lu\n",
> +				 prop,
> +				 (unsigned long)SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY);
> +			prop = SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY;
> +		}
> +		phy->ddr_read_dqs_delay = prop;
> +	}

If the unnecessary validation is dropped:

	phy->ddr_read_dqs_delay = -1;
	of_property_read_u32(dev->of_node, "cdns,ddr50-read-dqs-delay", &phy->ddr_read_dqs_delay);

etc

> +
> +	phy->ddr_use_lpbk_dqs = -1;
> +	if (!of_property_read_u32(dev->of_node, "cdns,ddr50-use-lpbk-dqs",
> +				  &prop)) {
> +		if (prop > FIELD_MAX(SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS)) {
> +			dev_warn(dev,
> +				 "cdns,ddr50-use-lpbk-dqs %u out of range, clamping to %lu\n",
> +				 prop,
> +				 (unsigned long)FIELD_MAX(SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS));
> +			prop = FIELD_MAX(SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS);
> +		}
> +		phy->ddr_use_lpbk_dqs = prop;
> +	}
> +
> +	phy->ddr_phony_dqs_timing = -1;
> +	if (!of_property_read_u32(dev->of_node, "cdns,ddr50-phony-dqs-timing",
> +				  &prop)) {
> +		if (prop > FIELD_MAX(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING)) {
> +			dev_warn(dev,
> +				 "cdns,ddr50-phony-dqs-timing %u out of range, clamping to %lu\n",
> +				 prop,
> +				 (unsigned long)FIELD_MAX(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING));
> +			prop = FIELD_MAX(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING);
> +		}
> +		phy->ddr_phony_dqs_timing = prop;
> +	}
> +
>  	priv->phy = phy;
>  
>  	return 0;

Re: [PATCH 1/4] mmc: sdhci-cadence6: add PHY settle delay after tuning DLL re-lock

[Adrian Hunter]

On 22/09/2026 14:12, tze.yee.ng@altera.com wrote:
> From: Tze Yee Ng <tze.yee.ng@altera.com>
> 
> After re-locking the DLL, allow the same 5 to 5.5 ms for the clock and
> PHY signals to stabilize as sdhci_cdns6_phy_init() does. Without this
> settle time the command issued immediately after tuning (e.g. the R1b
> CMD6 that switches eMMC from HS200 down to HS during HS400 selection)
> can time out.
> 
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>

Presume you realize the delay is inside the tuning loop, so 40x 5ms
is 200ms total.

Nevertheless:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-cadence-phy-v6.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> index 22d56bb46d75..35f35ef9c710 100644
> --- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
> +++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> @@ -811,6 +811,9 @@ int sdhci_cdns6_set_tune_val(struct sdhci_host *host, unsigned int val)
>  	if (ret)
>  		dev_warn(mmc_dev(host->mmc), "%s: DLL reset release failed: %d\n", __func__, ret);
>  
> +	/* Allow 5 to 5.5 ms for clock and PHY signals to stabilize after configuration */
> +	usleep_range(5000, 5500);
> +
>  	return ret;
>  }
>  

Re: [PATCH 2/4] mmc: sdhci-cadence6: program PHONY_DQS_TIMING for extended-read DDR

[Adrian Hunter]

On 22/09/2026 14:12, tze.yee.ng@altera.com wrote:
> From: Tze Yee Ng <tze.yee.ng@altera.com>
> 
> The SD6HC PHY left PHONY_DQS_TIMING (phy_ctrl_reg[9:4]) at 0 in all
> modes. Per the Cadence DLL PHY documentation it must be the rebar (RE#)
> pulse width in clk_phy cycles minus 1 in extended read mode, and 0
> otherwise. Leaving it 0 in extended-read DDR duplicates one DDR edge
> (the silent odd/even edge-capture defect).
> 
> This controller's rebar pulse is a fixed 2 clk_phy cycles, so extended-
> read DDR needs 1; confirmed on DDR50 hardware (1 captures both beats, 2
> corrupts reads). Derive it from the extended-read-mode state and apply
> it only in DDR modes, since SDR extended-read samples a single edge and
> is unaffected. This is generic to any SD6HC-PHY SoC, so it is kept
> separate from the per-SoC read-path tuning.
> 
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-cadence-phy-v6.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> index 35f35ef9c710..84592ae42762 100644
> --- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
> +++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> @@ -90,6 +90,9 @@
>  #define SDHCI_CDNS6_PHY_CTRL_REG			0x2080
>  #define   SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING			GENMASK(9, 4)
>  
> +/* Width of this controller's rebar (RE#) pulse in clk_phy cycles. */
> +#define SDHCI_CDNS6_PHY_REBAR_PULSE_CYCLES	2
> +
>  /* Default PHY settings */
>  #define SDHCI_CDNS6_PHY_DEFAULT_IOCELL_DELAY		2500
>  #define SDHCI_CDNS6_PHY_DEFAULT_DELAY_ELEMENT		24
> @@ -143,6 +146,9 @@ struct sdhci_cdns6_phy {
>  	bool cp_use_phony_dqs;		/* bit [20] */
>  	bool cp_use_phony_dqs_cmd;	/* bit [19] */
>  
> +	/* PHY_CTRL register fields */
> +	u32 cp_phony_dqs_timing;
> +
>  	/* HRS07 register - IO delay Information */
>  	u8 sdhc_rw_compensate;		/* bits [20:16] */
>  	u8 sdhc_idelay_val;		/* bits [4:0] */
> @@ -517,6 +523,13 @@ 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->sdhc_extended_rd_mode &&
> +	    (phy->mode == MMC_TIMING_UHS_DDR50 ||
> +	     phy->mode == MMC_TIMING_MMC_DDR52))
> +		phy->cp_phony_dqs_timing = SDHCI_CDNS6_PHY_REBAR_PULSE_CYCLES - 1;
> +	else
> +		phy->cp_phony_dqs_timing = 0;
> +
>  	if (strobe_dat) {
>  		/* dqs loopback input via IO cell */
>  		hcsdclkadj += phy->iocell_input_delay;
> @@ -715,6 +728,8 @@ int sdhci_cdns6_phy_init(struct sdhci_cdns_priv *priv)
>  
>  	reg = sdhci_cdns6_read_phy_reg(priv, SDHCI_CDNS6_PHY_CTRL_REG);
>  	reg &= ~SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING;
> +	reg |= FIELD_PREP(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING,
> +			  phy->cp_phony_dqs_timing);
>  	sdhci_cdns6_write_phy_reg(priv, SDHCI_CDNS6_PHY_CTRL_REG, reg);
>  
>  	/*

Re: [PATCH 1/4] mmc: sdhci-cadence6: add PHY settle delay after tuning DLL re-lock

[Kathpalia, Tanmay]

Hi Tze,

On 9/22/2026 4:42 PM, tze.yee.ng@altera.com wrote:
> From: Tze Yee Ng <tze.yee.ng@altera.com>
>
> After re-locking the DLL, allow the same 5 to 5.5 ms for the clock and
> PHY signals to stabilize as sdhci_cdns6_phy_init() does. Without this

I do not think the comparison with sdhci_cdns6_phy_init() holds. That
5 ms comes after a full PHY and host reprogram, including HRS writes
that happen after PHY_INIT_COMPLETE, and that path is only used after
SDCLK or the speed mode changes. set_tune_val() only updates two
phy_dll_slave_ctrl_reg fields and re-locks the DLL; it does not touch
HRS, clock, or mode.

> settle time the command issued immediately after tuning (e.g. the R1b
> CMD6 that switches eMMC from HS200 down to HS during HS400 selection)
> can time out.

Do you have a log for this? Which mode, which card, and how often it
reproduces. I ran a long regression on eMMC and on SD cards from
several vendors and sizes, and never hit a post-tuning CMD6 timeout
without this delay. I would prefer to see the failure before we add an
unconditional 5 ms.

> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
>   drivers/mmc/host/sdhci-cadence-phy-v6.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> index 22d56bb46d75..35f35ef9c710 100644
> --- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
> +++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> @@ -811,6 +811,9 @@ int sdhci_cdns6_set_tune_val(struct sdhci_host *host, unsigned int val)
>   	if (ret)
>   		dev_warn(mmc_dev(host->mmc), "%s: DLL reset release failed: %d\n", __func__, ret);
>   
> +	/* Allow 5 to 5.5 ms for clock and PHY signals to stabilize after configuration */

The comment is copied from phy_init() and says "after configuration",
but here we only reprogrammed the slave delay taps and re-locked the
DLL. Please reword it for this call site.

> +	usleep_range(5000, 5500);
> +

I think the placement also contradicts the rationale. After
sdhci_cdns6_dll_reset(priv, false), PHY_INIT_COMPLETE is already polled,
and per the Cadence DLL PHY user guide section 1.2 that means the master
DLLs have locked and the PHY is ready to accept commands. If a command
still cannot be issued for 5 ms after that, then all 40 scan commands
were sent on an unsettled PHY before this patch, and the real bug is a
wrongly chosen tap rather than a slow CMD6. If the scan was reliable, the
delay is only needed once, after the final tap is programmed.

Also, the sleep is placed after the "DLL reset release failed" warning,
so we also wait 5 ms when the DLL did not re-lock and we are about to
return an error. Skip it when ret is non-zero.

>   	return ret;
>   }
>   
Thanks Adrian - I'd appreciate your view on the comments I posted, given
your experience with this subsystem. I'd like to hold the patch until the
mechanism and the cost are clarified, and happy to go with whatever you
think is right once those points are answered.

Re: [PATCH 1/4] mmc: sdhci-cadence6: add PHY settle delay after tuning DLL re-lock

[Kathpalia, Tanmay]

On 22-09-2026 16:42, tze.yee.ng@altera.com wrote:
> From: Tze Yee Ng<tze.yee.ng@altera.com>
>
> After re-locking the DLL, allow the same 5 to 5.5 ms for the clock and
> PHY signals to stabilize as sdhci_cdns6_phy_init() does. Without this
> settle time the command issued immediately after tuning (e.g. the R1b
> CMD6 that switches eMMC from HS200 down to HS during HS400 selection)
> can time out.

One additional concern is that the testing listed in the cover letter 
does not
exercise the paths described here. The series reports testing SD DDR50 
and eMMC
DDR52, but neither mode performs tuning through sdhci_cdns6_set_tune_val().

Please test this change with eMMC HS200 and HS400. The same function is also
used during SD SDR104 tuning, so SDR104 should be covered as well.

Re: [PATCH 2/4] mmc: sdhci-cadence6: program PHONY_DQS_TIMING for extended-read DDR

[Kathpalia, Tanmay]

On 22-09-2026 16:42, tze.yee.ng@altera.com wrote:
> From: Tze Yee Ng<tze.yee.ng@altera.com>
>
> The SD6HC PHY left PHONY_DQS_TIMING (phy_ctrl_reg[9:4]) at 0 in all
> modes. Per the Cadence DLL PHY documentation it must be the rebar (RE#)
> pulse width in clk_phy cycles minus 1 in extended read mode, and 0
> otherwise. Leaving it 0 in extended-read DDR duplicates one DDR edge
> (the silent odd/even edge-capture defect).
>
> This controller's rebar pulse is a fixed 2 clk_phy cycles, so extended-
> read DDR needs 1; confirmed on DDR50 hardware (1 captures both beats, 2

The code also applies this value to eMMC DDR52. Was the same behavior 
verified in
DDR52? If so, please mention both modes; otherwise, please test DDR52 as 
well.

Also, please provide a reference showing that the SD6HC REBAR pulse is 
fixed at
two clk_phy cycles. The PHY guide defines the formula, but not the two-cycle
pulse width.


> corrupts reads). Derive it from the extended-read-mode state and apply
> it only in DDR modes, since SDR extended-read samples a single edge and
> is unaffected.

The driver sets sdhc_extended_rd_mode whenever t_sdclk != t_sdmclk, so 
extended
read is also on for divided SDR modes. Sampling only one edge may 
explain why the
corruption was observed in DDR, but it does not establish that zero is the
correct value for extended-read SDR.


> This is generic to any SD6HC-PHY SoC, so it is kept
> separate from the per-SoC read-path tuning.

Also, "this controller" and "generic to any SD6HC-PHY SoC" are not the same
claim. The PHY guide's example is a 4-cycle RE# pulse, programmed as 3. 
I do not
see a statement that this pulse is fixed at 2 clk_phy cycles. Please 
cite where 2
comes from, and whether that width is SD6HC IP behavior or specific to this
integration.

>
> Signed-off-by: Tze Yee Ng<tze.yee.ng@altera.com>
> ---
>   drivers/mmc/host/sdhci-cadence-phy-v6.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> index 35f35ef9c710..84592ae42762 100644
> --- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
> +++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
> @@ -90,6 +90,9 @@
>   #define SDHCI_CDNS6_PHY_CTRL_REG			0x2080
>   #define   SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING			GENMASK(9, 4)
>   
> +/* Width of this controller's rebar (RE#) pulse in clk_phy cycles. */
> +#define SDHCI_CDNS6_PHY_REBAR_PULSE_CYCLES	2
> +

Same question as above. Please cite the source of 2, and make the 
comment match
whether this is SD6HC-generic or SoC-specific.

>   /* Default PHY settings */
>   #define SDHCI_CDNS6_PHY_DEFAULT_IOCELL_DELAY		2500
>   #define SDHCI_CDNS6_PHY_DEFAULT_DELAY_ELEMENT		24
> @@ -143,6 +146,9 @@ struct sdhci_cdns6_phy {
>   	bool cp_use_phony_dqs;		/* bit [20] */
>   	bool cp_use_phony_dqs_cmd;	/* bit [19] */
>   
> +	/* PHY_CTRL register fields */
> +	u32 cp_phony_dqs_timing;
> +
>   	/* HRS07 register - IO delay Information */
>   	u8 sdhc_rw_compensate;		/* bits [20:16] */
>   	u8 sdhc_idelay_val;		/* bits [4:0] */
> @@ -517,6 +523,13 @@ 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->sdhc_extended_rd_mode &&
> +	    (phy->mode == MMC_TIMING_UHS_DDR50 ||
> +	     phy->mode == MMC_TIMING_MMC_DDR52))
> +		phy->cp_phony_dqs_timing = SDHCI_CDNS6_PHY_REBAR_PULSE_CYCLES - 1;
> +	else
> +		phy->cp_phony_dqs_timing = 0;
> +

Based on the register description, I would expect:
if (phy->sdhc_extended_rd_mode)
     phy->cp_phony_dqs_timing = SDHCI_CDNS6_PHY_REBAR_PULSE_CYCLES - 1;
else
     phy->cp_phony_dqs_timing = 0;

Re: [PATCH 3/4] dt-bindings: mmc: cdns,sdhci: add SD6HC DDR50 read-path tuning

[Kathpalia, Tanmay]

The subject should use cdns,sd6hc because that is the binding changed by 
this
patch. cdns,sdhci refers to the SD4HC binding.

On 22-09-2026 16:42, 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 must be centred by
> static, board/SoC-characterised PHY settings. Add three optional SD6HC
> properties:

The properties are specific to SD UHS DDR50, but the cover letter says 
they will
also be used by socfpga_agilex5_socdk_emmc. Since eMMC uses modes such 
as DDR52
rather than UHS DDR50, please clarify this and correct either the cover 
letter or
the property names and driver handling as appropriate.

>    - cdns,ddr50-read-dqs-delay:   DLL_SLAVE[7:0] read-DQS delay that centres
>      the read eye (0-255).
>    - cdns,ddr50-use-lpbk-dqs:     DQS_TIMING[21] read-DQS source
>      (0 = phony, 1 = loopback).
>    - cdns,ddr50-phony-dqs-timing: PHY_CTRL[9:4] phony DQS assertion timing
>      (0-63) that positions the fabricated strobe relative to the returning
>      DDR data; not produced by the Cadence timing calculation.

Please drop cdns,ddr50-phony-dqs-timing. The PHY guide defines this 
field from
extended_read_mode and the RE# pulse width. It is not a board flight-time
setting, and patch 2 already calculates it.

> All three are disallowed for the SD4HC variant.

This patch does not add an explicit SD4HC restriction. SD4HC and SD6HC use
separate schemas, and the SD4HC schema already rejects unknown properties. I
suggest dropping this sentence.

>
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
>   .../devicetree/bindings/mmc/cdns,sd6hc.yaml   | 27 +++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
> index d5ea2717904b..df86872603d0 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
> @@ -74,6 +74,33 @@ properties:
>       maximum: 1000
>       default: 24
>   
> +  cdns,ddr50-read-dqs-delay:
> +    description: |
> +      SD6HC DDR50 read-DQS delay (DLL_SLAVE[7:0]) used to centre the read
> +      eye. DDR50 has no CMD19 tuning, so this is a board/SoC-characterised
> +      value. If absent, the driver default is used.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 0xff

default value?

> +
> +  cdns,ddr50-use-lpbk-dqs:
> +    description: |
> +      SD6HC DDR50 read-DQS source (DQS_TIMING[21]): 0 = phony DQS,
> +      1 = loopback DQS. If absent, the driver default is used.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +

default value?

> +  cdns,ddr50-phony-dqs-timing:
> +    description: |
> +      SD6HC DDR50 phony DQS assertion timing (PHY_CTRL[9:4]). Positions the
> +      fabricated read strobe relative to the returning DDR data; the correct
> +      value depends on the board's SD flight time and is not produced by the
> +      Cadence timing calculation. If absent, the driver default
> +      (REBAR_PULSE_CYCLES-1) is used.

The description conflicts with the PHY guide, which derives this value 
from the
RE# pulse width rather than PCB flight time.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 0x3f
> +
>   required:
>     - compatible
>     - reg

Re: [PATCH 3/4] dt-bindings: mmc: cdns,sdhci: add SD6HC DDR50 read-path tuning

[Krzysztof Kozlowski]

On Tue, Sep 22, 2026 at 07:12:37PM +0800, 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 must be centred by
> static, board/SoC-characterised PHY settings. Add three optional SD6HC
> properties:
> 
>   - cdns,ddr50-read-dqs-delay:   DLL_SLAVE[7:0] read-DQS delay that centres
>     the read eye (0-255).
>   - cdns,ddr50-use-lpbk-dqs:     DQS_TIMING[21] read-DQS source
>     (0 = phony, 1 = loopback).
>   - cdns,ddr50-phony-dqs-timing: PHY_CTRL[9:4] phony DQS assertion timing
>     (0-63) that positions the fabricated strobe relative to the returning
>     DDR data; not produced by the Cadence timing calculation.
> 
> All three are disallowed for the SD4HC variant.
> 
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> ---
>  .../devicetree/bindings/mmc/cdns,sd6hc.yaml   | 27 +++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
> index d5ea2717904b..df86872603d0 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
> @@ -74,6 +74,33 @@ properties:
>      maximum: 1000
>      default: 24
>  
> +  cdns,ddr50-read-dqs-delay:
> +    description: |
> +      SD6HC DDR50 read-DQS delay (DLL_SLAVE[7:0]) used to centre the read
> +      eye. DDR50 has no CMD19 tuning, so this is a board/SoC-characterised
> +      value. If absent, the driver default is used.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 0xff

default: ...

Also, look at the existing bindings. How are the delays represented? ps.
Why is this different?


> +
> +  cdns,ddr50-use-lpbk-dqs:
> +    description: |
> +      SD6HC DDR50 read-DQS source (DQS_TIMING[21]): 0 = phony DQS,
> +      1 = loopback DQS. If absent, the driver default is used.

Then this is just type: boolean

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +
> +  cdns,ddr50-phony-dqs-timing:
> +    description: |
> +      SD6HC DDR50 phony DQS assertion timing (PHY_CTRL[9:4]). Positions the
> +      fabricated read strobe relative to the returning DDR data; the correct
> +      value depends on the board's SD flight time and is not produced by the
> +      Cadence timing calculation. If absent, the driver default
> +      (REBAR_PULSE_CYCLES-1) is used.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 0x3f

Best regards,
Krzysztof

Re: [PATCH 2/4] mmc: sdhci-cadence6: program PHONY_DQS_TIMING for extended-read DDR

[NG, TZE YEE]

On 26/9/2026 6:45 pm, Kathpalia, Tanmay wrote:
> 
> On 22-09-2026 16:42, tze.yee.ng@altera.com wrote:
>> From: Tze Yee Ng<tze.yee.ng@altera.com>
>>
>> The SD6HC PHY left PHONY_DQS_TIMING (phy_ctrl_reg[9:4]) at 0 in all
>> modes. Per the Cadence DLL PHY documentation it must be the rebar (RE#)
>> pulse width in clk_phy cycles minus 1 in extended read mode, and 0
>> otherwise. Leaving it 0 in extended-read DDR duplicates one DDR edge
>> (the silent odd/even edge-capture defect).
>>
>> This controller's rebar pulse is a fixed 2 clk_phy cycles, so extended-
>> read DDR needs 1; confirmed on DDR50 hardware (1 captures both beats, 2
> 
> The code also applies this value to eMMC DDR52. Was the same behavior 
> verified in
> DDR52? If so, please mention both modes; otherwise, please test DDR52 as 
> well.
> 

Yes. The final setting (phony_dqs_timing=1, use_lpbk_dqs=0, 
read_dqs_delay=96) was validated on Agilex5 eMMC DDR52 as well as 
Agilex5 SD UHS-I DDR50, in both U-Boot SPL and Linux, read + write with 
byte-exact read-back. I will reword the commit message to name both modes.
> Also, please provide a reference showing that the SD6HC REBAR pulse is 
> fixed at
> two clk_phy cycles. The PHY guide defines the formula, but not the two- 
> cycle
> pulse width.
> 
> 

You are right. The formula (value = rebar pulse width - 1 while extended 
read is active) is from the Cadence DLL PHY guide. The "2 clk_phy 
cycles" is this controller integration's pulse. I don't have an IP-level 
document stating it generically. It is bracketed empirically:

- phony_dqs_timing=0 produces a silent odd/even read duplication (every 
odd byte replaced by a copy of the preceding even byte, zero controller CRC)

- phony_dqs_timing=1 captures both DDR beats cleanly on every card/board

- phony_dqs_timing=2 shifts the strobe a full clk_phy cycle and 
re-corrupts.

So the pulse is 2 and the correct value is 1. I will drop "generic to 
any SD6HC-PHY SoC" and describe it as this integration's value - it is 
not even SoC-generic, as the same SoC's modular SoM variant needs 0 due 
to its longer SD flight time (handled by a follow-up device-tree override).

>> corrupts reads). Derive it from the extended-read-mode state and apply
>> it only in DDR modes, since SDR extended-read samples a single edge and
>> is unaffected.
> 
> The driver sets sdhc_extended_rd_mode whenever t_sdclk != t_sdmclk, so 
> extended
> read is also on for divided SDR modes. Sampling only one edge may 
> explain why the
> corruption was observed in DDR, but it does not establish that zero is the
> correct value for extended-read SDR.
> 
> 

Agreed the single-edge argument alone is not proof. Empirically, the
divided-SDR extended-read modes pass with the field left at 0: on this
hardware only DDR50 ever exhibited the defect (SDR12/25/50/104 and HS 
all pass). So 0 is the verified-good value for the extended-read SDR 
modes, and I scope the non-zero value to DDR to avoid moving modes that 
already pass onto an untested value. I can switch to the simpler
`if (extended_rd_mode) ... else 0` form if you prefer, but that would 
put the SDR modes on an unvalidated setting. I would rather keep the DDR 
gate unless you feel strongly.

>> This is generic to any SD6HC-PHY SoC, so it is kept
>> separate from the per-SoC read-path tuning.
> 
> Also, "this controller" and "generic to any SD6HC-PHY SoC" are not the same
> claim. The PHY guide's example is a 4-cycle RE# pulse, programmed as 3. 
> I do not
> see a statement that this pulse is fixed at 2 clk_phy cycles. Please 
> cite where 2
> comes from, and whether that width is SD6HC IP behavior or specific to this
> integration.
> 
>>
>> Signed-off-by: Tze Yee Ng<tze.yee.ng@altera.com>
>> ---
>>   drivers/mmc/host/sdhci-cadence-phy-v6.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/ 
>> host/sdhci-cadence-phy-v6.c
>> index 35f35ef9c710..84592ae42762 100644
>> --- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
>> +++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
>> @@ -90,6 +90,9 @@
>>   #define SDHCI_CDNS6_PHY_CTRL_REG            0x2080
>>   #define   SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING            
>> GENMASK(9, 4)
>> +/* Width of this controller's rebar (RE#) pulse in clk_phy cycles. */
>> +#define SDHCI_CDNS6_PHY_REBAR_PULSE_CYCLES    2
>> +
> 
> Same question as above. Please cite the source of 2, and make the 
> comment match
> whether this is SD6HC-generic or SoC-specific.
> 
>>   /* Default PHY settings */
>>   #define SDHCI_CDNS6_PHY_DEFAULT_IOCELL_DELAY        2500
>>   #define SDHCI_CDNS6_PHY_DEFAULT_DELAY_ELEMENT        24
>> @@ -143,6 +146,9 @@ struct sdhci_cdns6_phy {
>>       bool cp_use_phony_dqs;        /* bit [20] */
>>       bool cp_use_phony_dqs_cmd;    /* bit [19] */
>> +    /* PHY_CTRL register fields */
>> +    u32 cp_phony_dqs_timing;
>> +
>>       /* HRS07 register - IO delay Information */
>>       u8 sdhc_rw_compensate;        /* bits [20:16] */
>>       u8 sdhc_idelay_val;        /* bits [4:0] */
>> @@ -517,6 +523,13 @@ 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->sdhc_extended_rd_mode &&
>> +        (phy->mode == MMC_TIMING_UHS_DDR50 ||
>> +         phy->mode == MMC_TIMING_MMC_DDR52))
>> +        phy->cp_phony_dqs_timing = SDHCI_CDNS6_PHY_REBAR_PULSE_CYCLES 
>> - 1;
>> +    else
>> +        phy->cp_phony_dqs_timing = 0;
>> +
> 
> Based on the register description, I would expect:
> if (phy->sdhc_extended_rd_mode)
>      phy->cp_phony_dqs_timing = SDHCI_CDNS6_PHY_REBAR_PULSE_CYCLES - 1;
> else
>      phy->cp_phony_dqs_timing = 0;

Re: [PATCH 1/4] mmc: sdhci-cadence6: add PHY settle delay after tuning DLL re-lock

[NG, TZE YEE]

On 24/9/2026 2:41 pm, Adrian Hunter wrote:
> On 22/09/2026 14:12, tze.yee.ng@altera.com wrote:
>> From: Tze Yee Ng <tze.yee.ng@altera.com>
>>
>> After re-locking the DLL, allow the same 5 to 5.5 ms for the clock and
>> PHY signals to stabilize as sdhci_cdns6_phy_init() does. Without this
>> settle time the command issued immediately after tuning (e.g. the R1b
>> CMD6 that switches eMMC from HS200 down to HS during HS400 selection)
>> can time out.
>>
>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> 
> Presume you realize the delay is inside the tuning loop, so 40x 5ms
> is 200ms total.
> 
> Nevertheless:
> 
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> 

Thanks for the Ack, and you are right about the cost. The v1 placement 
put the 5 ms inside the tuning scan, so ~40x ≈ 200 ms. In v2, I will 
move it to a single settle after the winning tap in 
sdhci_cdns_execute_tuning() (5 ms once, success path only) and reworks 
the rationale per Tanmay's comments.

Since v2 changes both the placement and the file it touches, I won't
carry your Acked-by forward automatically - please let me know if it
still stands on the reworked patch.

Thanks,
Tze Yee

>> ---
>>   drivers/mmc/host/sdhci-cadence-phy-v6.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
>> index 22d56bb46d75..35f35ef9c710 100644
>> --- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
>> +++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
>> @@ -811,6 +811,9 @@ int sdhci_cdns6_set_tune_val(struct sdhci_host *host, unsigned int val)
>>   	if (ret)
>>   		dev_warn(mmc_dev(host->mmc), "%s: DLL reset release failed: %d\n", __func__, ret);
>>   
>> +	/* Allow 5 to 5.5 ms for clock and PHY signals to stabilize after configuration */
>> +	usleep_range(5000, 5500);
>> +
>>   	return ret;
>>   }
>>   
> 

Re: [PATCH 1/4] mmc: sdhci-cadence6: add PHY settle delay after tuning DLL re-lock

[NG, TZE YEE]

On 24/9/2026 4:49 pm, Kathpalia, Tanmay wrote:
> Hi Tze,
> 
> On 9/22/2026 4:42 PM, tze.yee.ng@altera.com wrote:
>> From: Tze Yee Ng <tze.yee.ng@altera.com>
>>
>> After re-locking the DLL, allow the same 5 to 5.5 ms for the clock and
>> PHY signals to stabilize as sdhci_cdns6_phy_init() does. Without this
> 
> I do not think the comparison with sdhci_cdns6_phy_init() holds. That
> 5 ms comes after a full PHY and host reprogram, including HRS writes
> that happen after PHY_INIT_COMPLETE, and that path is only used after
> SDCLK or the speed mode changes. set_tune_val() only updates two
> phy_dll_slave_ctrl_reg fields and re-locks the DLL; it does not touch
> HRS, clock, or mode.
> 

Hi Tanmay,

Agreed. The two paths don't do the same work, so justifying the delay
by analogy to phy_init() was wrong. I'll drop that claim from the commit
message.

>> settle time the command issued immediately after tuning (e.g. the R1b
>> CMD6 that switches eMMC from HS200 down to HS during HS400 selection)
>> can time out.
> 
> Do you have a log for this? Which mode, which card, and how often it
> reproduces. I ran a long regression on eMMC and on SD cards from
> several vendors and sizes, and never hit a post-tuning CMD6 timeout
> without this delay. I would prefer to see the failure before we add an
> unconditional 5 ms.
> 

Yes, Agilex5 eMMC in HS400 selection path. The failing command is the 
HS200->HS CM6 (R1b) the core issues in mmc_select_hs400():

   mmc0: switch to high-speed from hs200 failed, err:-110
   mmc0: error -110 whilst initialising MMC card
   mmc0: Failed to initialize a non-removable card

It reproduces on every init on this board without the settle delay. SD 
SDR104 on the same board is unaffected.


>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
>> ---
>>   drivers/mmc/host/sdhci-cadence-phy-v6.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/ 
>> host/sdhci-cadence-phy-v6.c
>> index 22d56bb46d75..35f35ef9c710 100644
>> --- a/drivers/mmc/host/sdhci-cadence-phy-v6.c
>> +++ b/drivers/mmc/host/sdhci-cadence-phy-v6.c
>> @@ -811,6 +811,9 @@ int sdhci_cdns6_set_tune_val(struct sdhci_host 
>> *host, unsigned int val)
>>       if (ret)
>>           dev_warn(mmc_dev(host->mmc), "%s: DLL reset release failed: 
>> %d\n", __func__, ret);
>> +    /* Allow 5 to 5.5 ms for clock and PHY signals to stabilize after 
>> configuration */
> 
> The comment is copied from phy_init() and says "after configuration",
> but here we only reprogrammed the slave delay taps and re-locked the
> DLL. Please reword it for this call site.
> 

I will fix the comment in v2 to remove the phy_init() or "configuration 
references.

>> +    usleep_range(5000, 5500);
>> +
> 
> I think the placement also contradicts the rationale. After
> sdhci_cdns6_dll_reset(priv, false), PHY_INIT_COMPLETE is already polled,
> and per the Cadence DLL PHY user guide section 1.2 that means the master
> DLLs have locked and the PHY is ready to accept commands. If a command
> still cannot be issued for 5 ms after that, then all 40 scan commands
> were sent on an unsettled PHY before this patch, and the real bug is a
> wrongly chosen tap rather than a slow CMD6. If the scan was reliable, the
> delay is only needed once, after the final tap is programmed.
> > Also, the sleep is placed after the "DLL reset release failed" warning,
> so we also wait 5 ms when the DLL did not re-lock and we are about to
> return an error. Skip it when ret is non-zero.
> 

Agreed. The scan issues only data read commands, which sample reliably 
once PHY_INIT_COMPLETE is set.  The command that fails is the first one 
the caller issues after tuning, which in the HS400 path is the R1b busy 
CMD6 rather than a data read. So the settle should be done once, after 
the final tap.

In v2, the settle delay will sits after the set_tune_val() error return, 
so it only runs when tuning succeeds; a tuning failure returns before it.

This also answers Adrian's cost observation: it's a single 5 ms after
the winning tap now, not ~40x during the scan.

Thanks,
Tze Yee

>>       return ret;
>>   }
> Thanks Adrian - I'd appreciate your view on the comments I posted, given
> your experience with this subsystem. I'd like to hold the patch until the
> mechanism and the cost are clarified, and happy to go with whatever you
> think is right once those points are answered.

Re: [PATCH 1/4] mmc: sdhci-cadence6: add PHY settle delay after tuning DLL re-lock

[NG, TZE YEE]

On 26/9/2026 6:44 pm, Kathpalia, Tanmay wrote:
> 
> On 22-09-2026 16:42, tze.yee.ng@altera.com wrote:
>> From: Tze Yee Ng<tze.yee.ng@altera.com>
>>
>> After re-locking the DLL, allow the same 5 to 5.5 ms for the clock and
>> PHY signals to stabilize as sdhci_cdns6_phy_init() does. Without this
>> settle time the command issued immediately after tuning (e.g. the R1b
>> CMD6 that switches eMMC from HS200 down to HS during HS400 selection)
>> can time out.
> 
> One additional concern is that the testing listed in the cover letter 
> does not
> exercise the paths described here. The series reports testing SD DDR50 
> and eMMC
> DDR52, but neither mode performs tuning through sdhci_cdns6_set_tune_val().
> 

Correct. DDR50/DDR52 don't run execute_tuning(). This path is only hit 
by eMMC HS200/HS400 and SD SDR104, so I'll rescope the cover letter.

> Please test this change with eMMC HS200 and HS400. The same function is 
> also
> used during SD SDR104 tuning, so SDR104 should be covered as well.

eMMC HS200/HS400 is covered: HS400 selection runs HS200 tuning first, 
and the retest goes through that path - "mmc0: new HS400 MMC card", with 
the HS200->HS CMD6 that used to return -110 now passing.

Thanks,
Tze Yee

Re: [PATCH 3/4] dt-bindings: mmc: cdns,sdhci: add SD6HC DDR50 read-path tuning

[NG, TZE YEE]

On 26/9/2026 6:48 pm, Kathpalia, Tanmay wrote:
> The subject should use cdns,sd6hc because that is the binding changed by 
> this
> patch. cdns,sdhci refers to the SD4HC binding.
> 

Agreed. I will change to "dt-bindings: mmc: cdns,sd6hc:" in v2.

> On 22-09-2026 16:42, 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 must be centred by
>> static, board/SoC-characterised PHY settings. Add three optional SD6HC
>> properties:
> 
> The properties are specific to SD UHS DDR50, but the cover letter says 
> they will
> also be used by socfpga_agilex5_socdk_emmc. Since eMMC uses modes such 
> as DDR52
> rather than UHS DDR50, please clarify this and correct either the cover 
> letter or
> the property names and driver handling as appropriate.
> 

They're meant for both SD DDR50 and eMMC DDR52 - both are extended-read 
DDR modes with no CMD19 tuning. You're right the naming and handling 
didn't match that. In v2, I'll rename them from cdns,ddr50-* -> 
cdns,ddr-* and apply them in DDR52 as well, and fix the cover
letter to say both modes.

>>    - cdns,ddr50-read-dqs-delay:   DLL_SLAVE[7:0] read-DQS delay that 
>> centres
>>      the read eye (0-255).
>>    - cdns,ddr50-use-lpbk-dqs:     DQS_TIMING[21] read-DQS source
>>      (0 = phony, 1 = loopback).
>>    - cdns,ddr50-phony-dqs-timing: PHY_CTRL[9:4] phony DQS assertion 
>> timing
>>      (0-63) that positions the fabricated strobe relative to the 
>> returning
>>      DDR data; not produced by the Cadence timing calculation.
> 
> Please drop cdns,ddr50-phony-dqs-timing. The PHY guide defines this 
> field from
> extended_read_mode and the RE# pulse width. It is not a board flight-time
> setting, and patch 2 already calculates it.
> 

I'd prefer to keep it with reworded. Patch 2 computes the nominal value 
from the RE# pulse width per the guide, which is correct for 
direct-attach boards. But the fabricated strobe still has to line up 
with when the DDR data actually returns, and that depends on board 
flight time: on Agilex5 modular devkit the computed value mis-samples 
and phony=0 is required - characterised on hardware. So this is a board 
override on top of patch 2's computed default, in the same class as 
read-dqs-delay. I'll reword the description so it no longer contradicts 
the guide:

default = value computed from the RE# pulse width; the property 
overrides it   for boards whose flight time shifts the DDR data return.

>> All three are disallowed for the SD4HC variant.
> 
> This patch does not add an explicit SD4HC restriction. SD4HC and SD6HC use
> separate schemas, and the SD4HC schema already rejects unknown 
> properties. I
> suggest dropping this sentence.
> 

Agreed, will drop.

>>
>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
>> ---
>>   .../devicetree/bindings/mmc/cdns,sd6hc.yaml   | 27 +++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml b/ 
>> Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
>> index d5ea2717904b..df86872603d0 100644
>> --- a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
>> @@ -74,6 +74,33 @@ properties:
>>       maximum: 1000
>>       default: 24
>> +  cdns,ddr50-read-dqs-delay:
>> +    description: |
>> +      SD6HC DDR50 read-DQS delay (DLL_SLAVE[7:0]) used to centre the 
>> read
>> +      eye. DDR50 has no CMD19 tuning, so this is a board/SoC- 
>> characterised
>> +      value. If absent, the driver default is used.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 0xff
> 
> default value?
> 

No fixed constant. When the property is absent, the driver keeps the 
value it computes for the mode. I'll reword the descriptions to say that.

>> +
>> +  cdns,ddr50-use-lpbk-dqs:
>> +    description: |
>> +      SD6HC DDR50 read-DQS source (DQS_TIMING[21]): 0 = phony DQS,
>> +      1 = loopback DQS. If absent, the driver default is used.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1]
>> +
> 
> default value?
> 

No fixed constant. When the property is absent, the driver keeps the 
value it computes for the mode. I'll reword the descriptions to say that.

>> +  cdns,ddr50-phony-dqs-timing:
>> +    description: |
>> +      SD6HC DDR50 phony DQS assertion timing (PHY_CTRL[9:4]). 
>> Positions the
>> +      fabricated read strobe relative to the returning DDR data; the 
>> correct
>> +      value depends on the board's SD flight time and is not produced 
>> by the
>> +      Cadence timing calculation. If absent, the driver default
>> +      (REBAR_PULSE_CYCLES-1) is used.
> 
> The description conflicts with the PHY guide, which derives this value 
> from the
> RE# pulse width rather than PCB flight time.

I will reword the description in v2 to say the driver derives the value 
from the RE# pulse width per the guide, and the property only overrides 
that computed value for boards whose DDR data return is shifted (e.g. 
modular/SoM flight time). No static default is documented since the 
fallback is the computed value.

Thanks,
Tze Yee

Re: [PATCH 3/4] dt-bindings: mmc: cdns,sdhci: add SD6HC DDR50 read-path tuning

[NG, TZE YEE]

On 28/9/2026 4:05 pm, Krzysztof Kozlowski wrote:
> On Tue, Sep 22, 2026 at 07:12:37PM +0800, 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 must be centred by
>> static, board/SoC-characterised PHY settings. Add three optional SD6HC
>> properties:
>>
>>    - cdns,ddr50-read-dqs-delay:   DLL_SLAVE[7:0] read-DQS delay that centres
>>      the read eye (0-255).
>>    - cdns,ddr50-use-lpbk-dqs:     DQS_TIMING[21] read-DQS source
>>      (0 = phony, 1 = loopback).
>>    - cdns,ddr50-phony-dqs-timing: PHY_CTRL[9:4] phony DQS assertion timing
>>      (0-63) that positions the fabricated strobe relative to the returning
>>      DDR data; not produced by the Cadence timing calculation.
>>
>> All three are disallowed for the SD4HC variant.
>>
>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
>> ---
>>   .../devicetree/bindings/mmc/cdns,sd6hc.yaml   | 27 +++++++++++++++++++
>>   1 file changed, 27 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
>> index d5ea2717904b..df86872603d0 100644
>> --- a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
>> @@ -74,6 +74,33 @@ properties:
>>       maximum: 1000
>>       default: 24
>>   
>> +  cdns,ddr50-read-dqs-delay:
>> +    description: |
>> +      SD6HC DDR50 read-DQS delay (DLL_SLAVE[7:0]) used to centre the read
>> +      eye. DDR50 has no CMD19 tuning, so this is a board/SoC-characterised
>> +      value. If absent, the driver default is used.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 0xff
> 
> default: ...
> 

Hi Krysztof,

Thanks for your review.

No fixed constant. When the property is absent, the driver keeps the
value it computes for the mode. I'll reword the descriptions to say that.

> Also, look at the existing bindings. How are the delays represented? ps.
> Why is this different?
> 
> 

In the modes this targets (DDR50/DDR52) the PHY runs the DLL
bypassed, where this field is a count of delay elements - so it maps to 
time via cdns,delay-element-ps just like the existing delay. I'll 
express it in ps:

cdns,ddr-read-dqs-delay-ps, convert in the driver (count = ps / 
delay-element-ps), default 0.

>> +
>> +  cdns,ddr50-use-lpbk-dqs:
>> +    description: |
>> +      SD6HC DDR50 read-DQS source (DQS_TIMING[21]): 0 = phony DQS,
>> +      1 = loopback DQS. If absent, the driver default is used.
> 
> Then this is just type: boolean
> 

Agreed. I'll make it a boolean.

Note: v2 also renames these cdns,ddr50-* -> cdns,ddr-* and applies them 
in both SD DDR50 and eMMC DDR52 (per Tanmay's review).

>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1]
>> +
>> +  cdns,ddr50-phony-dqs-timing:
>> +    description: |
>> +      SD6HC DDR50 phony DQS assertion timing (PHY_CTRL[9:4]). Positions the
>> +      fabricated read strobe relative to the returning DDR data; the correct
>> +      value depends on the board's SD flight time and is not produced by the
>> +      Cadence timing calculation. If absent, the driver default
>> +      (REBAR_PULSE_CYCLES-1) is used.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 0x3f
> 
> Best regards,
> Krzysztof

Re: [PATCH 3/4] dt-bindings: mmc: cdns,sdhci: add SD6HC DDR50 read-path tuning

[NG, TZE YEE]

On 30/9/2026 8:31 pm, NG, TZE YEE wrote:
> 
> 
> On 26/9/2026 6:48 pm, Kathpalia, Tanmay wrote:
>> The subject should use cdns,sd6hc because that is the binding changed 
>> by this
>> patch. cdns,sdhci refers to the SD4HC binding.
>>
> 
> Agreed. I will change to "dt-bindings: mmc: cdns,sd6hc:" in v2.
> 
>> On 22-09-2026 16:42, 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 must be centred by
>>> static, board/SoC-characterised PHY settings. Add three optional SD6HC
>>> properties:
>>
>> The properties are specific to SD UHS DDR50, but the cover letter says 
>> they will
>> also be used by socfpga_agilex5_socdk_emmc. Since eMMC uses modes such 
>> as DDR52
>> rather than UHS DDR50, please clarify this and correct either the 
>> cover letter or
>> the property names and driver handling as appropriate.
>>
> 
> They're meant for both SD DDR50 and eMMC DDR52 - both are extended-read 
> DDR modes with no CMD19 tuning. You're right the naming and handling 
> didn't match that. In v2, I'll rename them from cdns,ddr50-* -> 
> cdns,ddr-* and apply them in DDR52 as well, and fix the cover
> letter to say both modes.
> 
>>>    - cdns,ddr50-read-dqs-delay:   DLL_SLAVE[7:0] read-DQS delay that 
>>> centres
>>>      the read eye (0-255).
>>>    - cdns,ddr50-use-lpbk-dqs:     DQS_TIMING[21] read-DQS source
>>>      (0 = phony, 1 = loopback).
>>>    - cdns,ddr50-phony-dqs-timing: PHY_CTRL[9:4] phony DQS assertion 
>>> timing
>>>      (0-63) that positions the fabricated strobe relative to the 
>>> returning
>>>      DDR data; not produced by the Cadence timing calculation.
>>
>> Please drop cdns,ddr50-phony-dqs-timing. The PHY guide defines this 
>> field from
>> extended_read_mode and the RE# pulse width. It is not a board flight-time
>> setting, and patch 2 already calculates it.
>>
> 
> I'd prefer to keep it with reworded. Patch 2 computes the nominal value 
> from the RE# pulse width per the guide, which is correct for direct- 
> attach boards. But the fabricated strobe still has to line up with when 
> the DDR data actually returns, and that depends on board flight time: on 
> Agilex5 modular devkit the computed value mis-samples and phony=0 is 
> required - characterised on hardware. So this is a board override on top 
> of patch 2's computed default, in the same class as read-dqs-delay. I'll 
> reword the description so it no longer contradicts the guide:
> 
> default = value computed from the RE# pulse width; the property 
> overrides it   for boards whose flight time shifts the DDR data return.
> 
>>> All three are disallowed for the SD4HC variant.
>>
>> This patch does not add an explicit SD4HC restriction. SD4HC and SD6HC 
>> use
>> separate schemas, and the SD4HC schema already rejects unknown 
>> properties. I
>> suggest dropping this sentence.
>>
> 
> Agreed, will drop.
> 
>>>
>>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
>>> ---
>>>   .../devicetree/bindings/mmc/cdns,sd6hc.yaml   | 27 +++++++++++++++++++
>>>   1 file changed, 27 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml b/ 
>>> Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
>>> index d5ea2717904b..df86872603d0 100644
>>> --- a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
>>> @@ -74,6 +74,33 @@ properties:
>>>       maximum: 1000
>>>       default: 24
>>> +  cdns,ddr50-read-dqs-delay:
>>> +    description: |
>>> +      SD6HC DDR50 read-DQS delay (DLL_SLAVE[7:0]) used to centre the 
>>> read
>>> +      eye. DDR50 has no CMD19 tuning, so this is a board/SoC- 
>>> characterised
>>> +      value. If absent, the driver default is used.
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    minimum: 0
>>> +    maximum: 0xff
>>
>> default value?
>>
> 
> No fixed constant. When the property is absent, the driver keeps the 
> value it computes for the mode. I'll reword the descriptions to say that.
> 
>>> +
>>> +  cdns,ddr50-use-lpbk-dqs:
>>> +    description: |
>>> +      SD6HC DDR50 read-DQS source (DQS_TIMING[21]): 0 = phony DQS,
>>> +      1 = loopback DQS. If absent, the driver default is used.
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [0, 1]
>>> +
>>
>> default value?
>>
> 
> No fixed constant. When the property is absent, the driver keeps the 
> value it computes for the mode. I'll reword the descriptions to say that.
> 

Following up on the default question. I need to correct what I said earlier.

After Krzysztof's review I'm changing how two of these are represented, 
which also settles the defaults:

- read-dqs-delay: I said "no fixed constant", but that's wrong. When 
absent the driver uses 0 in these modes. It's now expressed in ps 
(cdns,ddr-read-dqs-delay-ps) with default: 0. (In the DDR modes the DLL 
is bypassed, where this field is a delay-element count, so ps is 
well-defined.)
- use-lpbk-dqs: now a boolean (cdns,ddr-use-lpbk-dqs); absent = phony, 
so there's no default to document.
- phony-dqs-timing: this is the only one with no fixed default. When 
absent the driver uses the value computed from the RE# pulse width 
(patch 2); that's described in the text rather than a schema default.

Thanks,
Tze Yee


>>> +  cdns,ddr50-phony-dqs-timing:
>>> +    description: |
>>> +      SD6HC DDR50 phony DQS assertion timing (PHY_CTRL[9:4]). 
>>> Positions the
>>> +      fabricated read strobe relative to the returning DDR data; the 
>>> correct
>>> +      value depends on the board's SD flight time and is not 
>>> produced by the
>>> +      Cadence timing calculation. If absent, the driver default
>>> +      (REBAR_PULSE_CYCLES-1) is used.
>>
>> The description conflicts with the PHY guide, which derives this value 
>> from the
>> RE# pulse width rather than PCB flight time.
> 
> I will reword the description in v2 to say the driver derives the value 
> from the RE# pulse width per the guide, and the property only overrides 
> that computed value for boards whose DDR data return is shifted (e.g. 
> modular/SoM flight time). No static default is documented since the 
> fallback is the computed value.
> 
> Thanks,
> Tze Yee

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

[NG, TZE YEE]

On 24/9/2026 2:40 pm, Adrian Hunter wrote:
> On 22/09/2026 14:12,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,ddr50-read-dqs-delay, cdns,ddr50-use-lpbk-dqs and
>> cdns,ddr50-phony-dqs-timing DT properties at PHY probe, range-check them,
> Firmware values are expected to be correct, and so are not validated.
> >> and apply them only in DDR50. When a property is absent the existing
>> driver default is kept - for the phony DQS timing, the derived
>> REBAR_PULSE_CYCLES-1.
>>
>> Signed-off-by: Tze Yee Ng<tze.yee.ng@altera.com>
>> ---
>>   drivers/mmc/host/sdhci-cadence-phy-v6.c | 72 ++++++++++++++++++++++++-
>>   1 file changed, 71 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-cadence-phy-v6.c b/drivers/mmc/host/sdhci-cadence-phy-v6.c
>> index 84592ae42762..0f47fa62d894 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) */
>> +	s32 ddr_read_dqs_delay;
>> +	s32 ddr_use_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,12 @@ 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->ddr_read_dqs_delay >= 0)
>> +		phy->cp_read_dqs_delay = phy->ddr_read_dqs_delay &
>> +			SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY;
> So here is unnecessarily assuming DT is providing a bad value.  It could just be:
> 
> 	if (phy->mode == MMC_TIMING_UHS_DDR50 && phy->ddr_read_dqs_delay >= 0)
> 		phy->cp_read_dqs_delay = phy->ddr_read_dqs_delay;
> 
>> +
>> +	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 +542,18 @@ static void sdhci_cdns6_phy_calc_dat_in(struct sdhci_cdns6_phy *phy)
>>   	else
>>   		phy->cp_phony_dqs_timing = 0;
>>   
>> +	if (phy->mode == MMC_TIMING_UHS_DDR50 && phy->ddr_use_lpbk_dqs >= 0)
>> +		phy->cp_use_lpbk_dqs = phy->ddr_use_lpbk_dqs &
>> +			FIELD_MAX(SDHCI_CDNS6_PHY_DQS_TIMING_USE_LPBK_DQS);
> Ditto
> 
>> +
>> +	/*
>> +	 * The phony DQS timing derived above depends on the board's SD flight
>> +	 * time, so allow a DT override to re-position the fabricated strobe.
>> +	 */
>> +	if (phy->mode == MMC_TIMING_UHS_DDR50 && phy->ddr_phony_dqs_timing >= 0)
>> +		phy->cp_phony_dqs_timing = phy->ddr_phony_dqs_timing &
>> +			FIELD_MAX(SDHCI_CDNS6_PHY_CTRL_PHONY_DQS_TIMING);
> Ditto
> 
>> +
>>   	if (strobe_dat) {
>>   		/* dqs loopback input via IO cell */
>>   		hcsdclkadj += phy->iocell_input_delay;
>> @@ -693,10 +717,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);
>> @@ -881,6 +906,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);
>> @@ -919,6 +945,50 @@ int sdhci_cdns6_phy_probe(struct platform_device *pdev, struct sdhci_cdns_priv *
>>   
>>   	phy->delay_element_org = phy->delay_element;
>>   
>> +	/*
>> +	 * Optional DDR50 read-path tuning. These are board/card-characterised
>> +	 * values with no CMD19 tuning in DDR50; absence keeps the driver
>> +	 * default (-1 => not overridden).
>> +	 */
>> +	phy->ddr_read_dqs_delay = -1;
>> +	if (!of_property_read_u32(dev->of_node, "cdns,ddr50-read-dqs-delay",
>> +				  &prop)) {
>> +		if (prop > SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY) {
>> +			dev_warn(dev,
>> +				 "cdns,ddr50-read-dqs-delay %u out of range, clamping to %lu\n",
>> +				 prop,
>> +				 (unsigned long)SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY);
>> +			prop = SDHCI_CDNS6_PHY_DLL_SLAVE_CTRL_READ_DQS_DELAY;
>> +		}
>> +		phy->ddr_read_dqs_delay = prop;
>> +	}
> If the unnecessary validation is dropped:
> 
> 	phy->ddr_read_dqs_delay = -1;
> 	of_property_read_u32(dev->of_node, "cdns,ddr50-read-dqs-delay", &phy->ddr_read_dqs_delay);
> 
> etc

Agreed on all of them. I'll drop the range checks, the clamps and the
& FIELD_MAX() masks. FIELD_PREP() already masks on write, so those masks 
were redundant anyway.

Two of these also change form in v2 after Krzysztof's review, which 
removes the validation naturally:

- read-dqs-delay becomes ps (cdns,ddr-read-dqs-delay-ps): probe is just
of_property_read_u32() into a u32 defaulting to 0; calc converts ps -> 
delay elements. No clamp.

- use-lpbk-dqs becomes a boolean (cdns,ddr-use-lpbk-dqs): 
of_property_read_bool(), so there's nothing to validate.

- phony-dqs-timing stays a u32, and I'll drop its clamp/mask too.

I'll also reword the commit message to drop "range-check them".

Thanks,
Tze Yee