RE: [PATCH 5/8] dt-bindings: pinctrl: realtek: add RTD1625 pinctrl binding

["Yu-Chun Lin [林祐君]" <eleanor.lin@realtek.com>]

Hi Linus,

Sorry for the late reply.

> Hi Yu-Chun,
> 
> thanks for your patch!
> 
> [Uwe, can you check this a bit down!]
> 
> On Wed, Jan 28, 2026 at 4:39 AM Yu-Chun Lin <eleanor.lin@realtek.com>
> wrote:
> 
> > From: Tzuyi Chang <tychang@realtek.com>
> >
> > Add device tree bindings for RTD1625.
> >
> > Signed-off-by: Tzuyi Chang <tychang@realtek.com>
> > Co-developed-by: Yu-Chun Lin <eleanor.lin@realtek.com>
> > Signed-off-by: Yu-Chun Lin <eleanor.lin@realtek.com>
> 
> Overall this looks good!
> 
> > +      power-source:
> > +        description: |
> > +          Valid arguments are described as below:
> > +          0: power supply of 1.8V
> > +          1: power supply of 3.3V
> > +        enum: [0, 1]
> 
> OK...
> 
> > +      slew-rate:
> > +        description: |
> > +          Valid arguments are described as below:
> > +            0: ~1ns falling time
> > +            1: ~10ns falling time
> > +            2: ~20ns falling time
> > +            3: ~30ns falling time
> > +        enum: [0, 1, 2, 3]
> 
> Slew rate is usually something like volts/second in SI units, I would expect that
> this differs depending on which power-source is selected?
> I.e. are these values for 1.8V or 3.3V?
> 

This slew-rate configuration is specifically applied to the HDMI I2C pins only.
These pins operate at 1.8V.

> > +      realtek,drive-strength-p:
> > +        description: |
> > +          Some of pins can be driven using the P-MOS and N-MOS
> transistor to
> > +          achieve finer adjustments.
> 
> Finer compared to what? Compared to the overall setting for slew-rate or
> drive-strength, or both?
> 

It provides finer pad driving capability compared to the generic drive-strength
property. This property allows for higher resolution control.

> > The block-diagram representation is as
> > +          follows:
> > +                         VDD
> > +                          |
> > +                      ||--+
> > +               +-----o||     P-MOS-FET
> > +               |      ||--+
> > +          IN --+          +----- out
> > +               |      ||--+
> > +               +------||     N-MOS-FET
> > +                      ||--+
> > +                          |
> > +                         GND
> 
> Nice picture!
> 
> > +          The driving strength of the P-MOS/N-MOS transistors impacts
> the
> > +          waveform's rise/fall times. Greater driving strength results in
> > +          shorter rise/fall times. Each P-MOS and N-MOS transistor
> offers
> > +          8 configurable levels (0 to 7), with higher values indicating
> > +          greater driving strength, contributing to achieving the desired
> > +          speed.
> > +
> > +          The realtek,drive-strength-p is used to control the driving
> strength
> > +          of the P-MOS output.
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        minimum: 0
> > +        maximum: 7
> > +
> > +      realtek,drive-strength-n:
> > +        description: |
> > +          Similar to the realtek,drive-strength-p, the
> realtek,drive-strength-n
> > +          is used to control the driving strength of the N-MOS output.
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        minimum: 0
> > +        maximum: 7
> 
> These two are really interesting. But what do these settings represent?
> 
> I would *guess* it represents the number of transistors used, simply, so 0
> means just one P/N transistor is driving and 7 means 8 transistors are driving.
> 
> Can you provide details here?
> 
> In this case, maybe we want a generalized property such as drive-stages-p =
> <n>; drive-stages-n = <n>;
> 
> in the generic bindings, if this will appear from more vendors?
> 

These values are not a simple count of transistors (so 0 is not 1 transistor).

Instead, the 3-bit value represents like a weighted configuration. There is a base
driving capability (even at value 0), and each bit adds a different weight to the
total strength (e.g., Bit 0 adds a small weight, Bit 2 adds a larger weight).

The resulting current is non-linear and also varies significantly based on the IO
voltage (1.8V vs 3.3V) and the specific pad group (e.g., eMMC vs SDIO).

Given this complexity, I am not sure if this implementation is suitable for a
generic binding shared with other vendors.

> > +      realtek,duty-cycle:
> > +        description: |
> > +          An integer describing the level to adjust output duty cycle,
> > +          controlling the proportion of positive and negative waveforms
> in
> > +          nanoseconds.
> > +          Valid arguments are described as below:
> > +          0: 0ns
> > +          2: + 0.25ns
> > +          3: + 0.5ns
> > +          4: -0.25ns
> > +          5: -0.5ns
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        enum: [0, 2, 3, 4, 5]
> 
> This is a bit dubious.
> 
> Isn't this one of those cases where you should be using the PWM bindings,
> directly in this node? Just slam in #pwm-cells = <...> etc, I think this is what you
> really want.
> 
> Please consult/reference:
> Documentation/devicetree/bindings/pwm/pwm.yaml
> consumers would not use pinctrl phandles but something like pwms =
> <&pwm ....>;
> 
> It's maybe a bit trixy to use two generic bindings in the node but it should be
> just fine.
> 
> I don't feel confident mergeing this without Uwe Kleine-König's review.
> 

This hardware block is not a PWM generator. It does not generate a signal with a
specific frequency and duty ratio.

Instead, it provides a fixed nanosecond-level adjustment to the rising/falling edges
of an existing signal. It is used for Signal Integrity tuning (adding/subtracting
delay to fine-tune the high/low duration).

To avoid confusion with PWM bindings, would you suggest a name like
realtek,pulse-width-adjust?

> > +      realtek,input-voltage:
> > +        description: |
> > +          Select the input receiver voltage domain for the pin (1.8V or
> 3.3V).
> > +          This defines the reference for VIH/VIL and must match the
> external
> > +          signal level.
> > +
> > +          This does not control the output drive voltage, which is handled
> by
> > +          the standard generic 'power-source' property.
> > +
> > +          Valid arguments are described as below:
> > +          0: 1.8V input logic level
> > +          1: 3.3V input logic level
> > +        $ref: /schemas/types.yaml#/definitions/uint32
> > +        enum: [0, 1]
> 
> This looks very generic. Can you please just add input-voltage to
> pincfg-node.yaml with a custom format and reference that?
> 

I will do it in v2.

> > +      realtek,high-vil:
> > +        type: boolean
> > +        description: |
> > +          Select the input receiver with a higher LOW recognition
> threshold
> > +          (VIL) to improve detection for sources with weak pull-down or
> slow
> > +          falling edges.
> 
> Isn't this supposed to be input-schmitt-microvolt?
> 
> Or is this something else than a schmitt trigger?
> 
> In either case, try to figure out the typical recognition threshold in microvolt
> and use that, please.
> 

This is not a configuration for the Schmitt trigger hysteresis window, but rather
a selection of the input low voltage level (VIL) to address a specific HDMI I2C
compatibility issue.

We have encountered some HDMI sinks (TVs) with weak pull-down capabilities.
These devices fail to pull the I2C bus voltage below the standard VIL threshold
(~0.7V), causing the SoC to fail to recognize the LOW state.

This property enables a specialized input receiver mode that raises the effective
VIL threshold to approximately 1.1V.

The hardware design only supports these two discrete levels (Standard ~0.7V and
High-VIL ~1.1V).

Since the hardware acts as a discrete switch between these two levels, do you prefer
we replace this boolean with a value-based property,
such as realtek,high-vil-microvolt = <1100000>?

Best Regards,
Yu-Chun

> Yours,
> Linus Walleij