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

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

> On Wed, Jan 28, 2026 at 01:33:00PM +0100, Linus Walleij wrote:
> > 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!
>
> Where can I find the binding patch? What this is in-reply-to does not
> exist:
> https://lore.kernel.org/all/CAD++jL=445wx467ZKE3-qm_BaVzKYXE-7zmReTFZA0KUAaSNyw@mail.gmail.com/
> Nor can I find anything else from Eleanor that would appear to be what this is:
> https://lore.kernel.org/all/?q=f%3Aeleanor.lin%40realtek.com
>

Hi Conor,

I apologize for the confusion. I realized that when I sent out the v1 patch, I
missed the mailing list, which is why it's not showing up in the archives.

Since I had already received some feedback on v1, I missed the timing to resend the v1.
However, you were included in the CC list of the initial v1 patch email.
You might be able to find the full patch in your inbox.

> >
> > > +      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?
> > 
> > > +      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?
> > 
> > > 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?
> > 
> > > +      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 does sound like a pwm to me too, but I can't see the rest of the series to comment.
>

Please kindly refer to my response to Linus [1].
[1] https://lore.kernel.org/all/d5be357c14b84155adfa8a9f00a64d83@realtek.com/

>
> > > +      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?
>
> Why a custom format, rather than input-voltage-microvolt or w/e?
> 

I will add input-voltage-microvolt to pincfg-node.yaml and use the microvolt format
as suggested. Thanks

Yu-Chun

> Cheers,
> Conor.
>
> >
> > > +      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.
> > 
> > Yours,
> > Linus Walleij