Thread (5 messages) 5 messages, 2 authors, 2018-08-12

Re: [PATCH] pinctrl: qcom: Add sdm660 pinctrl driver

From: Christian Lamparter <hidden>
Date: 2018-08-12 12:42:44
Also in: linux-arm-msm, linux-gpio, lkml

On Sunday, August 12, 2018 9:18:19 AM CEST you wrote:
On 11 August 2018 18:27:43 BST, Christian Lamparter [off-list ref] wrote:
quoted
On Saturday, August 11, 2018 6:25:19 PM CEST Craig Tatlor wrote:
quoted
Add initial pinctrl driver to support pin configuration with
pinctrl framework for sdm660.
Based off CAF implementation.

Signed-off-by: Craig Tatlor <redacted>
---

diff --git
a/Documentation/devicetree/bindings/pinctrl/qcom,sdm660-pinctrl.txt
b/Documentation/devicetree/bindings/pinctrl/qcom,sdm660-pinctrl.txt
quoted
new file mode 100644
index 000000000000..85e6c6c17c04
--- /dev/null
+++
b/Documentation/devicetree/bindings/pinctrl/qcom,sdm660-pinctrl.txt
quoted
@@ -0,0 +1,195 @@
+Qualcomm Technologies, Inc. SDM660 TLMM block
+
+This binding describes the Top Level Mode Multiplexer block found in
the
quoted
+SDM660 platform.
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be "qcom,sdm660-pinctrl"
+
+- reg:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: the base address and size of the TLMM register space.
+
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: should specify the TLMM summary IRQ.
+
+- interrupt-controller:
+	Usage: required
+	Value type: <none>
+	Definition: identifies this node as an interrupt controller
+
+- #interrupt-cells:
+	Usage: required
+	Value type: <u32>
+	Definition: must be 2. Specifying the pin number and flags, as
defined
quoted
+		    in <dt-bindings/interrupt-controller/irq.h>
+
+- gpio-controller:
+	Usage: required
+	Value type: <none>
+	Definition: identifies this node as a gpio controller
+
+- #gpio-cells:
+	Usage: required
+	Value type: <u32>
+	Definition: must be 2. Specifying the pin number and flags, as
defined
quoted
+		    in <dt-bindings/gpio/gpio.h>
+
+Please refer to ../gpio/gpio.txt and
../interrupt-controller/interrupts.txt for
quoted
+a general description of GPIO and interrupt bindings.
You want to specify gpio-ranges here as well. The property is explained
in Section "2.1) gpio- and pin-controller interaction" in
../gpio/gpio.txt

Without it, the gpio-hogs construct (part of ../gpio/gpio.txt) will
cause
the driver to fail during boot. (try it, ;-) )
Would gpio-ranges make sense for this, as the gpio and pinctrl are in same block?
Yes, it's part of the ../gpio/gpio.txt which you link.
Here's a copy of the relevant section that explains this
gpio- and pin-controller interaction.


|2.1) gpio- and pin-controller interaction
|-----------------------------------------
|
|Some or all of the GPIOs provided by a GPIO controller may be routed to pins
|on the package via a pin controller. This allows muxing those pins between
|GPIO and other functions.
|It is useful to represent which GPIOs correspond to which pins on which pin
|controllers. The gpio-ranges property described below represents this, and
|contains information structures as follows:
|
|       gpio-range-list ::= <single-gpio-range> [gpio-range-list]
|       single-gpio-range ::= <numeric-gpio-range> | <named-gpio-range>
|       numeric-gpio-range ::=
|                       <pinctrl-phandle> <gpio-base> <pinctrl-base> <count>
|       named-gpio-range ::= <pinctrl-phandle> <gpio-base> '<0 0>'
|       pinctrl-phandle : phandle to pin controller node
|       gpio-base : Base GPIO ID in the GPIO controller
|       pinctrl-base : Base pinctrl pin ID in the pin controller
|       count : The number of GPIOs/pins in this range
|
|The "pin controller node" mentioned above must conform to the bindings
|described in ../pinctrl/pinctrl-bindings.txt.
|...

As for the reason why gpio-ranges is what it is, please look at the ML
discussion from the "pinctrl: msm: fix gpio-hog related boot issues" thread
on <https://patchwork.kernel.org/patch/10339127/> and the posts by 
Linus Walleij: <https://patchwork.kernel.org/patch/10339127/#21903101> and
Stephen Boyd: <https://patchwork.kernel.org/patch/10339127/#21867185>.
(It's quite a bit to take in)
Seems no other qcom pinctrl drivers have it and I'm able to boot without it.
Ok, let's run an experiment. Please remove the gpio-ranges property and try
adding a test gpio-hog to your device's DTS:

something like (I randomly selected GPIO5, but it shouldn't
matter which gpio you select here. If you know a unused/NC
pin/gpio, then you can use it instead):

&tlmm {
	test-hog {
		gpio-hog;
		gpios = <5 0>;
		output-low;
		line-name = "test hog";
	};
};

compile&install it and then watch the kernel on the next boot:

without the gpio-ranges present, it will spew out something along the
lines of:

| requesting hog GPIO test hog (chip 3000000.pinctrl, offset 5) failed, -517
| gpiochip_add_data: GPIOs 0..114 (3000000.pinctrl) failed to register
| sdm660-pinctrl 3000000.pinctrl: Failed register gpiochip

The single gpio-hog causes havoc and takes down the sdm660-pinctrl with it.
And every driver that depends on the pinctrl to setup the pin muxing/config
will fail to load as well. (check out /sys/kernel/debug/pinctrl/, it will be
empty.)

Regards,
Christian
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help