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@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.

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@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;