Hi TY Chang,

thanks for your patch!

On Wed, Jul 26, 2023 at 11:06 AM TY Chang [off-list ref] wrote:

Add device tree bindings for RTD1315E.

Signed-off-by: TY Chang <redacted>

Maybe you could write a short paragraph about the RTD1315E so we
know what this is? I guess it is some SoC with some intended use case?

(...)

+description: |
+  Binding for Realtek DHC RTD1315E SoC pin control.

Same text should go here in that case.

+        realtek,pdrive:
+          description: |
+            An integer describing the level to adjust PMOS output driving capability.
+          $ref: /schemas/types.yaml#/definitions/uint32
+          minimum: 0
+          maximum: 7
+
+        realtek,ndrive:
+          description: |
+            An integer describing the level to adjust NMOS output driving capability.
+          $ref: /schemas/types.yaml#/definitions/uint32
+          minimum: 0
+          maximum: 7

I would rename these realtek,drive-strength-p and realtek,drive-strength-n.

You need to explain what is meant with PMOS and NMOS here. If it is what
I think it is, I think some ASCII art would be handy!

You can reuse my ASCII art from Documentation/driver-api/gpio/driver.rst:

                     VDD
                      |
            OD    ||--+
         +--/ ---o||     P-MOS-FET
         |        ||--+
    IN --+            +----- out
         |        ||--+
         +--/ ----||     N-MOS-FET
            OS    ||--+
                      |
                     GND

Maybe you wanna delete the OD switch if these drivers don't support that.

What does the values 0..7 actually correspond to? Is it the number of
transistors/driver stages simply? Then write that.

We need to think whether this is so generically useful that it should simply
be drive-strength-pmos and drive-strength-nmos, simply put, as other
SoCs may implement the same. What do people think?

+        realtek,dcycle:
+          description: |
+            An integer describing the level to adjust output duty cycle.
+            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 does not explain the duty cycle of *what*?

It looks really useful so please explain thoroughly what it does.

I guess this is not PWM because then you could use
PIN_CONFIG_MODE_PWM.

Yours,
Linus Walleij