pinctrl: msm: fix gpio-hog related boot issues

Sven Eckelmann reported an issue with the current IPQ4019 pinctrl. Setting up any gpio-hog in the device-tree for his device would "kill the bootup completely":

| [    0.477838] msm_serial 78af000.serial: could not find pctldev for node /soc/pinctrl at 1000000/serial_pinmux, deferring probe
| [    0.499828] spi_qup 78b5000.spi: could not find pctldev for node /soc/pinctrl at 1000000/spi_0_pinmux, deferring probe
| [    1.298883] requesting hog GPIO enable USB2 power (chip 1000000.pinctrl, offset 58) failed, -517
| [    1.299609] gpiochip_add_data: GPIOs 0..99 (1000000.pinctrl) failed to register
| [    1.308589] ipq4019-pinctrl 1000000.pinctrl: Failed register gpiochip
| [    1.316586] msm_serial 78af000.serial: could not find pctldev for node /soc/pinctrl at 1000000/serial_pinmux, deferring probe
| [    1.322415] spi_qup 78b5000.spi: could not find pctldev for node /soc/pinctrl at 1000000/spi_0_pinmux, deferri

This was also verified on a RT-AC58U (IPQ4018) which would no longer boot, if a gpio-hog was specified. (Tried forcing the USB LED PIN (GPIO0) to high.).

arch/arm/boot/dts/qcom-apq8064.dtsi +0 -0

 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;

drivers/pinctrl/qcom/pinctrl-msm.c +0 -0

index 495432f3341b..f2580bbba741 100644

Quoted 12 lines · Christian Lamparter

Sven Eckelmann reported an issue with the current IPQ4019 pinctrl. Setting up any gpio-hog in the device-tree for his device would "kill the bootup completely":

| [    0.477838] msm_serial 78af000.serial: could not find pctldev for node /soc/pinctrl at 1000000/serial_pinmux, deferring probe
| [    0.499828] spi_qup 78b5000.spi: could not find pctldev for node /soc/pinctrl at 1000000/spi_0_pinmux, deferring probe
| [    1.298883] requesting hog GPIO enable USB2 power (chip 1000000.pinctrl, offset 58) failed, -517
| [    1.299609] gpiochip_add_data: GPIOs 0..99 (1000000.pinctrl) failed to register
| [    1.308589] ipq4019-pinctrl 1000000.pinctrl: Failed register gpiochip
| [    1.316586] msm_serial 78af000.serial: could not find pctldev for node /soc/pinctrl at 1000000/serial_pinmux, deferring probe
| [    1.322415] spi_qup 78b5000.spi: could not find pctldev for node /soc/pinctrl at 1000000/spi_0_pinmux, deferri

Quoted 4 lines

This was also verified on a RT-AC58U (IPQ4018) which would no longer boot, if a gpio-hog was specified. (Tried forcing the USB LED PIN (GPIO0) to high.).

It's quite clear that this is a generic issue with the msm pinctrl driver.
From what I understand it's not only the msm-pinctrl. All pinctrl and gpio

For the cases where I've been in need of static configuration of certain pins I've simply made the pinctrl node itself have a pinctrl-0 that refer to a state that I want to hold. This has worked well and I didn't even know about the gpio-hog property.

[..]

arch/arm/boot/dts/qcom-apq8064.dtsi line 330

Quoted 9 lines

arch/arm/boot/dts/qcom-apq8064.dtsi +1 -0

index 3ca96e361878..17ad9cbd9f8c 100644
@@ -327,6 +327,7 @@
 			reg = <0x800000 0x4000>;
 
 			gpio-controller;
+			gpio-ranges = <&tlmm_pinmux 0 0 90>;

This seems reasonable.

Quoted 3 lines

arch/arm/boot/dts/qcom-apq8064.dtsi +0 -0

 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;

[..]

Quoted 8 lines

drivers/pinctrl/qcom/pinctrl-msm.c +0 -0

index 495432f3341b..f2580bbba741 100644
@@ -831,13 +831,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 		return ret;
 	}
 

drivers/pinctrl/qcom/pinctrl-msm.c static int msm_gpio_init(struct msm_pinctrl *pctrl) · line 840

Quoted 7 lines

drivers/pinctrl/qcom/pinctrl-msm.c +0 -7

-	ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev), 0, 0, chip->ngpio);
-	if (ret) {
-		dev_err(pctrl->dev, "Failed to add pin range\n");
-		gpiochip_remove(&pctrl->chip);
-		return ret;
-	}
-

But, this will break backwards compatibility with existing DTBs and I don't see how this would work with the ACPI based platforms using this driver.

Iirc this driver follows the same pattern as several other pinctrl drivers providing gpios, how are they handling this?

Ok I see, I was aware of ACPI but not that a pinctrl-msm based driver is using it. Well, I thinks is possible to use is_acpi_device_node() or !is_of_node() to detect whenever we are dealing with a OF or not:

would it be ok to do something like this?

| if (!is_of_node(chip->of_node)) {

|					/*
|					 * (lengthy note about gpiochip_add_pin_range and OF with
|					 * reference to Documentation/devicetree/bindings/gpio/gpio.txt
|					 * - TBD)
|					 */
|					ret = gpiochip_add_pin_range(&pctrl->chip,
|					[...]
|		}

This patch fixes the issue by adding the "gpio-ranges" property to the pinctrl device node of all upstream Qcom SoC, so the ranges are added by of_gpiochip_add_pin_range(), which is called by of_gpiochip_add() before the call to of_gpiochip_scan_gpios() happens.gpiochip_add_pin_range() is longer needed and removed (to prevent adding the same entry to the pinctrldev_list twice).

                        gpio-controller;

+ gpio-ranges = <&tlmm_pinmux 0 0 90>;

drivers/pinctrl/qcom/pinctrl-msm.c static int msm_gpio_init(struct msm_pinctrl *pctrl) · line 839

Quoted 6 lines · Christian Lamparter

drivers/pinctrl/qcom/pinctrl-msm.c +0 -6

-       ret = gpiochip_add_pin_range(&pctrl->chip, dev_name(pctrl->dev), 0, 0, chip->ngpio);
-       if (ret) {
-               dev_err(pctrl->dev, "Failed to add pin range\n");
-               gpiochip_remove(&pctrl->chip);
-               return ret;
-       }

If you instead of deleteing this, just wrap it inside something like:

if (!of_property_read_bool(np, "gpio-ranges") {
(...)
}

You will stay compatible with elder device trees, solving Bj?rns issue. You will only be adding hogs to newer device trees with the ranges defined anyway.

Be genereous with comments in the code if you choose this approach so everyone realize what is going on.