Messages in this thread | | | From | Linus Walleij <> | Date | Mon, 7 Aug 2023 14:52:36 +0200 | Subject | Re: [PATCH 5/7] dt-bindings: pinctrl: realtek: add RTD1315E pinctrl binding |
| |
Hi TY Chang,
thanks for your patch!
On Wed, Jul 26, 2023 at 11:06 AM TY Chang <tychang@realtek.com> wrote:
> Add device tree bindings for RTD1315E. > > Signed-off-by: TY Chang <tychang@realtek.com>
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
| |