On 12 August 2018 13:42:27 BST, Christian Lamparter [off-list ref] wrote:

On Sunday, August 12, 2018 9:18:19 AM CEST you wrote:

quoted

On 11 August 2018 18:27:43 BST, Christian Lamparter

[off-list ref] wrote:

quoted

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

quoted

quoted

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.

quoted

quoted

quoted

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

quoted

quoted

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)

Thanks for the links, makes sense now, I'll add in v2.

quoted

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

Yup

Regards,
Christian

--