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

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:

quoted

Add device tree binding documentation and header file for Renesas
RZ/V2M pinctrl.

Signed-off-by: Phil Edworthy <redacted>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Thanks for your patch!

quoted

--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzv2m-

pinctrl.yaml

quoted

@@ -0,0 +1,174 @@

quoted

+additionalProperties:
+  anyOf:
+    - type: object
+      allOf:
+        - $ref: pincfg-node.yaml#
+        - $ref: pinmux-node.yaml#
+
+      description:
+        Pin controller client devices use pin configuration subnodes

(children

quoted

+        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

quoted

+            alternate function configuration number using the

RZV2M_PORT_PINMUX()

quoted

+            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.

quoted

+        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.

quoted

+        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?

quoted

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

Thanks
Phil