On 12 August 2018 13:42:27 BST, Christian Lamparter <chunkeey@xxxxxxxxx> wrote: >On Sunday, August 12, 2018 9:18:19 AM CEST you wrote: >> On 11 August 2018 18:27:43 BST, Christian Lamparter ><chunkeey@xxxxxxxxx> wrote: >> >On Saturday, August 11, 2018 6:25:19 PM CEST Craig Tatlor wrote: >> >> Add initial pinctrl driver to support pin configuration with >> >> pinctrl framework for sdm660. >> >> Based off CAF implementation. >> >> >> >> Signed-off-by: Craig Tatlor <ctatlor97@xxxxxxxxx> >> >> --- >> >> >> >> diff --git >> >a/Documentation/devicetree/bindings/pinctrl/qcom,sdm660-pinctrl.txt >> >b/Documentation/devicetree/bindings/pinctrl/qcom,sdm660-pinctrl.txt >> >> new file mode 100644 >> >> index 000000000000..85e6c6c17c04 >> >> --- /dev/null >> >> +++ >> >b/Documentation/devicetree/bindings/pinctrl/qcom,sdm660-pinctrl.txt >> >> @@ -0,0 +1,195 @@ >> >> +Qualcomm Technologies, Inc. SDM660 TLMM block >> >> + >> >> +This binding describes the Top Level Mode Multiplexer block found >in >> >the >> >> +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 >> >> + 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 >> >> + in <dt-bindings/gpio/gpio.h> >> >> + >> >> +Please refer to ../gpio/gpio.txt and >> >../interrupt-controller/interrupts.txt for >> >> +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) Thanks for the links, makes sense now, I'll add in v2. > >> 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 --
