Re: [PATCH 2/4] mmc: sdhci-cadence6: program PHONY_DQS_TIMING for extended-read DDR
From: NG, TZE YEE
Date: Wed Sep 30 2026 - 06:25:43 EST
On 26/9/2026 6:45 pm, Kathpalia, Tanmay wrote:
On 22-09-2026 16:42, tze.yee.ng@xxxxxxxxxx wrote:
From: Tze Yee Ng<tze.yee.ng@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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;