Hi TY Chang,
thanks for your patch!
On Wed, Jul 26, 2023 at 11:06 AM TY Chang <tychang@xxxxxxxxxxx> wrote:
> Add device tree bindings for RTD1315E.
>
> Signed-off-by: TY Chang <tychang@xxxxxxxxxxx>
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
