RE: [PATCH 1/2] dt-bindings: pinctrl: renesas: Add DT bindings for RZ/V2M pinctrl

[Phil Edworthy <phil.edworthy@xxxxxxxxxxx>]

Hi Geert,

Thanks for your review!

On 14 June 2022 13:10 Geert Uytterhoeven wrote:
> On Fri, May 20, 2022 at 5:41 PM Phil Edworthy wrote:
> > Add device tree binding documentation and header file for Renesas
> > RZ/V2M pinctrl.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzv2m-
> pinctrl.yaml
> > @@ -0,0 +1,174 @@
> 
> > +additionalProperties:
> > +  anyOf:
> > +    - type: object
> > +      allOf:
> > +        - $ref: pincfg-node.yaml#
> > +        - $ref: pinmux-node.yaml#
> > +
> > +      description:
> > +        Pin controller client devices use pin configuration subnodes
> (children
> > +        and grandchildren) for desired pin configuration.
> > +        Client device subnodes use below standard properties.
> > +
> > +      properties:
> > +        phandle: true
> > +        pinmux:
> > +          description:
> > +            Values are constructed from GPIO port number, pin number,
> and
> > +            alternate function configuration number using the
> RZV2M_PORT_PINMUX()
> > +            helper macro in <dt-bindings/pinctrl/rzv2m-pinctrl.h>.
> > +        pins: true
> > +        bias-disable: true
> > +        bias-pull-down: true
> > +        bias-pull-up: true
> > +        drive-strength-microamp:
> > +          # Superset of supported values
> > +          enum: [ 1600, 1800, 2000, 3200, 3800, 4000, 6400, 7800, 8000,
> > +                  9000, 9600, 11000, 12000, 13000, 18000 ]
> > +
> > +        power-source:
> > +          description: I/O voltage in millivolt.
> > +          enum: [ 1800, 3300 ]
> 
> Is power-source actually supported?
> While the documentation shows there are some 1.8/3.3V pin groups,
> I didn't find how to switch voltage?
You are right, there is no way to change the voltage, the I/O voltage
is based on Vdd pin input voltages. See PAMODVDD, PBMODVDD, PCMODVDD,
SD0MODVDD, SD1MODVDD, etc pins.

> > +        slew-rate: true
> 
> What are valid values?
> Looking at the code, 0 = slow, 1 = fast?
Good point, I'll add enum: [0, 1] and a description.

> > +        gpio-hog: true
> > +        gpios: true
> > +        input-enable: true
> 
> Missing output-enable?
If it's an output, wouldn't you only want to set it high or low?

> > +        output-high: true
> > +        output-low: true
> > +        line-name: true

Thanks
Phil