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
