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

[NG, TZE YEE]

Hi Krzysztof,

On 12/10/2026 4:48 pm, Krzysztof Kozlowski wrote:
> Why aren't you using b4 to send patches? Why making it difficult for us?

Sorry for the friction, that was my mistake. I'll switch to b4 for v3 so 
the series carries a stable change-id and the cover letter links back to 
the earlier versions, and b4 diff works for you.

> > ---
> >   .../devicetree/bindings/mmc/cdns,sd6hc.yaml   | 30 +++++++++++++++++++
> >   1 file changed, 30 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
> > index d5ea2717904b..911ce58f8dab 100644
> > --- a/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/cdns,sd6hc.yaml
> > @@ -74,6 +74,36 @@ properties:
> >       maximum: 1000
> >       default: 24
> >   
> > +  cdns,ddr-read-dqs-delay-ps:
> > +    description: >
> > +      Read-DQS delay (DLL_SLAVE[7:0]) in picoseconds that centres the read
> > +      eye in the extended-read DDR modes (SD DDR50, eMMC DDR52), which have
> > +      no CMD19 tuning. The PHY runs the DLL bypassed in these modes, so the
> > +      driver converts this to delay elements (divided by
> > +      cdns,delay-element-ps). The 8-bit field holds at most 255 elements;
> > +      larger values are clamped to that limit.
> > +    minimum: 0
>
> Drop, 0 is the default.

Will drop it in v3.

> Where is the maximum value?

The value is in ps, but the register (DLL_SLAVE[7:0]) counts delay
elements, so the driver divides by cdns,delay-element-ps and clamps to
the 8-bit field. There is therefore no fixed ps maximum, the real limit
is 255 * cdns,delay-element-ps, which varies with that property.

Rather than pick an arbitrary ps cap, I'd prefer to express this 
property directly as the delay-element count the field holds:

  cdns,ddr-read-dqs-delay: 0..255 (DLL delay elements, each element
  cdns,delay-element-ps wide)

That maps 1:1 to the hardware field, gives a clean maximum: 255, and the
element size is already described by cdns,delay-element-ps. Would that 
be acceptable, or would you rather keep picoseconds with a fixed cap 
(e.g. ~one DDR50 bit period, 50MHz = 20ns = 20000 ps)?

> > +    default: 0
> > +
> > +  cdns,ddr-no-lpbk-dqs:
> > +    description: >
>
> Drop > everywhere

Will drop > in v3. I'll use plain "description:" on all three new 
properties.

> Best regards,
> Krzysztof

Thanks,
Tze Yee