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@xxxxxxxxxx wrote:
> From: Tze Yee Ng <tze.yee.ng@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
>   .../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