diff mbox series

[V2,1/6] dt-bindings: pinctrl: support specifying pins, groups & functions

Message ID 20211124230439.17531-2-zajec5@gmail.com (mailing list archive)
State New, archived
Headers show
Series pinctrl: support platform (e.g. DT) stored pins, groups & functions | expand

Commit Message

Rafał Miłecki Nov. 24, 2021, 11:04 p.m. UTC 
From: Rafał Miłecki <rafal@milecki.pl>

This binding change is meant to introduce a generic way of describing
pinctrl blocks details. Every pinmux block is expected to have:
1. Named pins
2. Named groups containing one or more pins
3. Named functions referencing one or more groups

It doesn't describe how hw should be programmed. That remains binding
and driver specific.

This commit describes syntax for "pins", "groups" & "functions" nodes in
a standard pinctrl binding. Every above node allows specifying its
entries and it's done using subnodes.

That design it meant to be generic and extendable (hw specific
properties can be added). Using subnodes allows storing hw specific
pin/group/function configuration.

While it would be possible to have:
groups {
    foo-pins = <0 1>;
    bar-pins = <2 3>;
};
that doesn't allow hw specific quirks.

Introduced design allows e.g.:
groups {
    foo {
        pins = <0 1>;
        vendor,magic = <0xbeaf>;
    };
};

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
Rob: this patch depends on another one I sent 2 weeks ago:
[PATCH 2/2] dt-bindings: pinctrl: use pinctrl.yaml
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20211110165720.30242-2-zajec5@gmail.com/

V2: Don't use "reg" property as explained by Tony
---
 .../devicetree/bindings/pinctrl/pinctrl.yaml  | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Tony Lindgren Nov. 25, 2021, 8:49 a.m. UTC | #1 
Hi,

* Rafał Miłecki <zajec5@gmail.com> [211124 23:05]:
> --- a/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
> @@ -42,4 +42,44 @@ properties:
>        This property can be set either globally for the pin controller or in
>        child nodes for individual pin group control.
>  
> +  pins:
> +    type: object
> +
> +    patternProperties:
> +      "^.*$":
> +        type: object
> +
> +        properties:
> +          number:
> +            description: Pin number
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +
> +        additionalProperties: false

Please don't introduce Linux kernel internal numbering here. It's
like bringing back the interrupt numbers again. Just make this into
a proper hardware offset from the controller base, so a reg property.
Sure in some cases the reg property is just an index depending on
the controller, we don't really care from the binding point of view.

We already have #pinctrl-cells, so plase do something like the four
ximaginary examples below:

	#pinctrl-cells = <1>;
	...
	pin@foo {
		reg = <0xf00 MUX_MODE0>;
		label = "foo_pin";
	};


	#pinctrl-cells = <2>;
	...
	pin@foo {
		reg = <0xf00 PIN_INPUT_PULLUP MUX_MODE3>;
	};


	#pinctrl-cells = <2>;
	...
	pin@f00 {
		reg = <0xf00 DELAY_PS(0) DELAY_PS(0)>;
	};


	#pinctrl-cells = <3>;
	...
	pin@f00 {
		reg = <0xf00 MUX_MODE3 PULL_UP_STRENGTH(36) PULL_DOWN_STRENGTH(20)>;
	};


Then let's attempt to use just standard numbers and defines for the
values where possible. Then a group of pins is just a list of the pin
phandles in the devicetree.

Regards,

Tony
Rafał Miłecki Nov. 25, 2021, 12:28 p.m. UTC | #2 
On 25.11.2021 09:49, Tony Lindgren wrote:
> * Rafał Miłecki <zajec5@gmail.com> [211124 23:05]:
>> --- a/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
>> +++ b/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
>> @@ -42,4 +42,44 @@ properties:
>>         This property can be set either globally for the pin controller or in
>>         child nodes for individual pin group control.
>>   
>> +  pins:
>> +    type: object
>> +
>> +    patternProperties:
>> +      "^.*$":
>> +        type: object
>> +
>> +        properties:
>> +          number:
>> +            description: Pin number
>> +            $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +        additionalProperties: false
> 
> Please don't introduce Linux kernel internal numbering here. It's
> like bringing back the interrupt numbers again.

This is a new bit to me and the reason why I got this binding that way.

I had no idea pin numbering is system specific thing. I always thought
pin numbers are present in every chip datasheets and that is just a part
of hardware.

Now I'm reading https://www.kernel.org/doc/Documentation/pinctrl.txt
again it indeed seems to mention that numbering is handled in a way not
related to specs: "I enumerated the pins from 0 in the upper left corner
to 63 in the lower right corner.".

Sorry for that, I hopefully understand your point correctly now.


> Just make this into
> a proper hardware offset from the controller base, so a reg property.
> Sure in some cases the reg property is just an index depending on
> the controller, we don't really care from the binding point of view.
> 
> We already have #pinctrl-cells, so plase do something like the four
> ximaginary examples below:
> 
> 	#pinctrl-cells = <1>;
> 	...
> 	pin@foo {
> 		reg = <0xf00 MUX_MODE0>;
> 		label = "foo_pin";
> 	};
> 
> 
> 	#pinctrl-cells = <2>;
> 	...
> 	pin@foo {
> 		reg = <0xf00 PIN_INPUT_PULLUP MUX_MODE3>;
> 	};
> 
> 
> 	#pinctrl-cells = <2>;
> 	...
> 	pin@f00 {
> 		reg = <0xf00 DELAY_PS(0) DELAY_PS(0)>;
> 	};
> 
> 
> 	#pinctrl-cells = <3>;
> 	...
> 	pin@f00 {
> 		reg = <0xf00 MUX_MODE3 PULL_UP_STRENGTH(36) PULL_DOWN_STRENGTH(20)>;
> 	};
> 
> 
> Then let's attempt to use just standard numbers and defines for the
> values where possible. Then a group of pins is just a list of the pin
> phandles in the devicetree.

I need to ask for help on understanding that reg = <...> syntax.

(Why) do we need to put that extra info in a "reg" property? That seems
like either:
1. Pin specific info
or
2. Phandle arguments

In the first case, instead of:
	pin@f00 {
		reg = <0xf00 MUX_MODE3 PULL_UP_STRENGTH(36) PULL_DOWN_STRENGTH(20)>;
	};
I'd rather use:
	pin@f00 {
		reg = <0xf00>;
		mux_mode3;
		pull_up_strength = <36>;
		pull_down_strength = <20>;
	};

In the second case, shouldn't that be something like:
	pins {
		bar: pin@f00 {
			reg = <0xf00>;
			#pinctrl-cells = <3>;
		};
	};

	groups {
		qux {
			pins = <&bar MUX_MODE3 PULL_UP_STRENGTH(36) PULL_DOWN_STRENGTH(20)>;
		}
	};
Linus Walleij Nov. 26, 2021, 1:03 a.m. UTC | #3 
On Thu, Nov 25, 2021 at 1:28 PM Rafał Miłecki <zajec5@gmail.com> wrote:

> This is a new bit to me and the reason why I got this binding that way.
>
> I had no idea pin numbering is system specific thing. I always thought
> pin numbers are present in every chip datasheets and that is just a part
> of hardware.

It's this way because some chips do not even have numbers on the
pins, just names, like "UARTTX" or so.

The numbering is often not numeral, more like a chessboard
at least for BGAs. So "C14" is a typical pin number.

If the datasheet had numbers we just include it in the string naming
that pin like "UARTTX_C14" or so.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
index d471563119a9..25d8188fbb26 100644
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl.yaml
@@ -42,4 +42,44 @@  properties:
       This property can be set either globally for the pin controller or in
       child nodes for individual pin group control.
 
+  pins:
+    type: object
+
+    patternProperties:
+      "^.*$":
+        type: object
+
+        properties:
+          number:
+            description: Pin number
+            $ref: /schemas/types.yaml#/definitions/uint32
+
+        additionalProperties: false
+
+  groups:
+    type: object
+
+    patternProperties:
+      "^.*$":
+        type: object
+        description: Group identified by node name
+
+        properties:
+          pins:
+            $ref: /schemas/types.yaml#/definitions/uint32-array
+            description: Array of pins belonging to this group
+
+  functions:
+    type: object
+
+    patternProperties:
+      "^.*$":
+        type: object
+        description: Function identified by node name
+
+        properties:
+          groups:
+            $ref: /schemas/types.yaml#/definitions/phandle-array
+            description: Array of pins groups used by this function
+
 additionalProperties: true