[PATCH] pinctrl: msm: fix gpio-hog related boot issues

[PATCH] pinctrl: msm: fix gpio-hog related boot issues

[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@1000000/serial_pinmux, deferring probe
| [    0.499828] spi_qup 78b5000.spi: could not find pctldev for node /soc/pinctrl@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@1000000/serial_pinmux, deferring probe
| [    1.322415] spi_qup 78b5000.spi: could not find pctldev for node /soc/pinctrl@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.).

The problem is that Pinctrl+GPIO registration is currently
peformed in the following order in pinctrl-msm.c:
	1. pinctrl_register()
	2. gpiochip_add()
	3. gpiochip_add_pin_range()

The actual error code -517 == -EPROBE_DEFER is coming from
pinctrl_get_device_gpio_range(), which is called through:
        gpiochip_add
            of_gpiochip_add
                of_gpiochip_scan_gpios
                    gpiod_hog
                        gpiochip_request_own_desc
                            __gpiod_request
                                chip->request
                                    gpiochip_generic_request
                                       pinctrl_gpio_request
                                          pinctrl_get_device_gpio_range

pinctrl_get_device_gpio_range() is unable to find any valid
pin ranges, since nothing has been added to the pinctrldev_list yet.
so the range can't be found, and the operation fails with -EPROBE_DEFER.

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

Cc: stable@vger.kernel.org
Reported-by: Sven Eckelmann <sven.eckelmann@openmesh.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
The msm8998 would need something like
	gpio-ranges = <&tlmm 0 0 150>;
---
 arch/arm/boot/dts/qcom-apq8064.dtsi   | 1 +
 arch/arm/boot/dts/qcom-apq8084.dtsi   | 1 +
 arch/arm/boot/dts/qcom-ipq4019.dtsi   | 1 +
 arch/arm/boot/dts/qcom-ipq8064.dtsi   | 1 +
 arch/arm/boot/dts/qcom-mdm9615.dtsi   | 1 +
 arch/arm/boot/dts/qcom-msm8660.dtsi   | 1 +
 arch/arm/boot/dts/qcom-msm8960.dtsi   | 1 +
 arch/arm/boot/dts/qcom-msm8974.dtsi   | 1 +
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 3 ++-
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
 arch/arm64/boot/dts/qcom/msm8992.dtsi | 1 +
 arch/arm64/boot/dts/qcom/msm8994.dtsi | 1 +
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 1 +
 drivers/pinctrl/qcom/pinctrl-msm.c    | 7 -------
 14 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 3ca96e361878..17ad9cbd9f8c 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -327,6 +327,7 @@
 			reg = <0x800000 0x4000>;
 
 			gpio-controller;
+			gpio-ranges = <&tlmm_pinmux 0 0 90>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 0e1e98707e3f..d9481d083802 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -396,6 +396,7 @@
 			compatible = "qcom,apq8084-pinctrl";
 			reg = <0xfd510000 0x4000>;
 			gpio-controller;
+			gpio-ranges = <&tlmm 0 0 147>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi
index c1e65aaf3cad..d85b15774c64 100644
--- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
@@ -147,6 +147,7 @@
 			compatible = "qcom,ipq4019-pinctrl";
 			reg = <0x01000000 0x300000>;
 			gpio-controller;
+			gpio-ranges = <&tlmm 0 0 100>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index 1e0a3b446f7a..26eab9a68d90 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -108,6 +108,7 @@
 			reg = <0x800000 0x4000>;
 
 			gpio-controller;
+			gpio-ranges = <&qcom_pinmux 0 0 69>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
index c852b69229c9..cfdaca5f259a 100644
--- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
+++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
@@ -128,6 +128,7 @@
 		msmgpio: pinctrl@800000 {
 			compatible = "qcom,mdm9615-pinctrl";
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 88>;
 			#gpio-cells = <2>;
 			interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-controller;
diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi
index 33030f9419fe..47cf9c4ca062 100644
--- a/arch/arm/boot/dts/qcom-msm8660.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8660.dtsi
@@ -110,6 +110,7 @@
 			reg = <0x800000 0x4000>;
 
 			gpio-controller;
+			gpio-ranges = <&tlmm 0 0 173>;
 			#gpio-cells = <2>;
 			interrupts = <0 16 0x4>;
 			interrupt-controller;
diff --git a/arch/arm/boot/dts/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom-msm8960.dtsi
index 1733d8f40ab1..f6d8b1af5a8a 100644
--- a/arch/arm/boot/dts/qcom-msm8960.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8960.dtsi
@@ -102,6 +102,7 @@
 		msmgpio: pinctrl@800000 {
 			compatible = "qcom,msm8960-pinctrl";
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 152>;
 			#gpio-cells = <2>;
 			interrupts = <0 16 0x4>;
 			interrupt-controller;
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index d9019a49b292..1250e071a6e2 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -696,6 +696,7 @@
 			compatible = "qcom,msm8974-pinctrl";
 			reg = <0xfd510000 0x4000>;
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 146>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 2bc5dec5614d..d2c36b467466 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -24,11 +24,12 @@
 		ranges = <0 0 0 0xffffffff>;
 		compatible = "simple-bus";
 
-		pinctrl@1000000 {
+		tlmm: pinctrl@1000000 {
 			compatible = "qcom,ipq8074-pinctrl";
 			reg = <0x1000000 0x300000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
+			gpio-ranges = <&tlmm 0 0 70>;
 			#gpio-cells = <0x2>;
 			interrupt-controller;
 			#interrupt-cells = <0x2>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index e51b04900726..e06cb90c8ec3 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -281,6 +281,7 @@
 			reg = <0x1000000 0x300000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 122>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm64/boot/dts/qcom/msm8992.dtsi b/arch/arm64/boot/dts/qcom/msm8992.dtsi
index 171578747ed0..173b6bc60816 100644
--- a/arch/arm64/boot/dts/qcom/msm8992.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8992.dtsi
@@ -179,6 +179,7 @@
 			reg = <0xfd510000 0x4000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 146>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm64/boot/dts/qcom/msm8994.dtsi b/arch/arm64/boot/dts/qcom/msm8994.dtsi
index f33c41d01c86..68705db4696b 100644
--- a/arch/arm64/boot/dts/qcom/msm8994.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8994.dtsi
@@ -141,6 +141,7 @@
 			reg = <0xfd510000 0x4000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 146>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 0a6f7952bbb1..18511e782cbd 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -530,6 +530,7 @@
 			reg = <0x01010000 0x300000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 150>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 495432f3341b..f2580bbba741 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -831,13 +831,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 		return ret;
 	}
 
-	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;
-	}
-
 	ret = gpiochip_irqchip_add(chip,
 				   &msm_gpio_irq_chip,
 				   0,
-- 
2.16.3

[PATCH] pinctrl: msm: fix gpio-hog related boot issues

[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

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

The problem is that Pinctrl+GPIO registration is currently
peformed in the following order in pinctrl-msm.c:
	1. pinctrl_register()
	2. gpiochip_add()
	3. gpiochip_add_pin_range()

The actual error code -517 == -EPROBE_DEFER is coming from
pinctrl_get_device_gpio_range(), which is called through:
        gpiochip_add
            of_gpiochip_add
                of_gpiochip_scan_gpios
                    gpiod_hog
                        gpiochip_request_own_desc
                            __gpiod_request
                                chip->request
                                    gpiochip_generic_request
                                       pinctrl_gpio_request
                                          pinctrl_get_device_gpio_range

pinctrl_get_device_gpio_range() is unable to find any valid
pin ranges, since nothing has been added to the pinctrldev_list yet.
so the range can't be found, and the operation fails with -EPROBE_DEFER.

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

Cc: stable at vger.kernel.org
Reported-by: Sven Eckelmann <sven.eckelmann@openmesh.com>
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
---
The msm8998 would need something like
	gpio-ranges = <&tlmm 0 0 150>;
---
 arch/arm/boot/dts/qcom-apq8064.dtsi   | 1 +
 arch/arm/boot/dts/qcom-apq8084.dtsi   | 1 +
 arch/arm/boot/dts/qcom-ipq4019.dtsi   | 1 +
 arch/arm/boot/dts/qcom-ipq8064.dtsi   | 1 +
 arch/arm/boot/dts/qcom-mdm9615.dtsi   | 1 +
 arch/arm/boot/dts/qcom-msm8660.dtsi   | 1 +
 arch/arm/boot/dts/qcom-msm8960.dtsi   | 1 +
 arch/arm/boot/dts/qcom-msm8974.dtsi   | 1 +
 arch/arm64/boot/dts/qcom/ipq8074.dtsi | 3 ++-
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
 arch/arm64/boot/dts/qcom/msm8992.dtsi | 1 +
 arch/arm64/boot/dts/qcom/msm8994.dtsi | 1 +
 arch/arm64/boot/dts/qcom/msm8996.dtsi | 1 +
 drivers/pinctrl/qcom/pinctrl-msm.c    | 7 -------
 14 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 3ca96e361878..17ad9cbd9f8c 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -327,6 +327,7 @@
 			reg = <0x800000 0x4000>;
 
 			gpio-controller;
+			gpio-ranges = <&tlmm_pinmux 0 0 90>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm/boot/dts/qcom-apq8084.dtsi b/arch/arm/boot/dts/qcom-apq8084.dtsi
index 0e1e98707e3f..d9481d083802 100644
--- a/arch/arm/boot/dts/qcom-apq8084.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8084.dtsi
@@ -396,6 +396,7 @@
 			compatible = "qcom,apq8084-pinctrl";
 			reg = <0xfd510000 0x4000>;
 			gpio-controller;
+			gpio-ranges = <&tlmm 0 0 147>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm/boot/dts/qcom-ipq4019.dtsi b/arch/arm/boot/dts/qcom-ipq4019.dtsi
index c1e65aaf3cad..d85b15774c64 100644
--- a/arch/arm/boot/dts/qcom-ipq4019.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq4019.dtsi
@@ -147,6 +147,7 @@
 			compatible = "qcom,ipq4019-pinctrl";
 			reg = <0x01000000 0x300000>;
 			gpio-controller;
+			gpio-ranges = <&tlmm 0 0 100>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm/boot/dts/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom-ipq8064.dtsi
index 1e0a3b446f7a..26eab9a68d90 100644
--- a/arch/arm/boot/dts/qcom-ipq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-ipq8064.dtsi
@@ -108,6 +108,7 @@
 			reg = <0x800000 0x4000>;
 
 			gpio-controller;
+			gpio-ranges = <&qcom_pinmux 0 0 69>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm/boot/dts/qcom-mdm9615.dtsi b/arch/arm/boot/dts/qcom-mdm9615.dtsi
index c852b69229c9..cfdaca5f259a 100644
--- a/arch/arm/boot/dts/qcom-mdm9615.dtsi
+++ b/arch/arm/boot/dts/qcom-mdm9615.dtsi
@@ -128,6 +128,7 @@
 		msmgpio: pinctrl at 800000 {
 			compatible = "qcom,mdm9615-pinctrl";
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 88>;
 			#gpio-cells = <2>;
 			interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-controller;
diff --git a/arch/arm/boot/dts/qcom-msm8660.dtsi b/arch/arm/boot/dts/qcom-msm8660.dtsi
index 33030f9419fe..47cf9c4ca062 100644
--- a/arch/arm/boot/dts/qcom-msm8660.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8660.dtsi
@@ -110,6 +110,7 @@
 			reg = <0x800000 0x4000>;
 
 			gpio-controller;
+			gpio-ranges = <&tlmm 0 0 173>;
 			#gpio-cells = <2>;
 			interrupts = <0 16 0x4>;
 			interrupt-controller;
diff --git a/arch/arm/boot/dts/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom-msm8960.dtsi
index 1733d8f40ab1..f6d8b1af5a8a 100644
--- a/arch/arm/boot/dts/qcom-msm8960.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8960.dtsi
@@ -102,6 +102,7 @@
 		msmgpio: pinctrl at 800000 {
 			compatible = "qcom,msm8960-pinctrl";
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 152>;
 			#gpio-cells = <2>;
 			interrupts = <0 16 0x4>;
 			interrupt-controller;
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index d9019a49b292..1250e071a6e2 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -696,6 +696,7 @@
 			compatible = "qcom,msm8974-pinctrl";
 			reg = <0xfd510000 0x4000>;
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 146>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm64/boot/dts/qcom/ipq8074.dtsi b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
index 2bc5dec5614d..d2c36b467466 100644
--- a/arch/arm64/boot/dts/qcom/ipq8074.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq8074.dtsi
@@ -24,11 +24,12 @@
 		ranges = <0 0 0 0xffffffff>;
 		compatible = "simple-bus";
 
-		pinctrl at 1000000 {
+		tlmm: pinctrl at 1000000 {
 			compatible = "qcom,ipq8074-pinctrl";
 			reg = <0x1000000 0x300000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
+			gpio-ranges = <&tlmm 0 0 70>;
 			#gpio-cells = <0x2>;
 			interrupt-controller;
 			#interrupt-cells = <0x2>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index e51b04900726..e06cb90c8ec3 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -281,6 +281,7 @@
 			reg = <0x1000000 0x300000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 122>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm64/boot/dts/qcom/msm8992.dtsi b/arch/arm64/boot/dts/qcom/msm8992.dtsi
index 171578747ed0..173b6bc60816 100644
--- a/arch/arm64/boot/dts/qcom/msm8992.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8992.dtsi
@@ -179,6 +179,7 @@
 			reg = <0xfd510000 0x4000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 146>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm64/boot/dts/qcom/msm8994.dtsi b/arch/arm64/boot/dts/qcom/msm8994.dtsi
index f33c41d01c86..68705db4696b 100644
--- a/arch/arm64/boot/dts/qcom/msm8994.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8994.dtsi
@@ -141,6 +141,7 @@
 			reg = <0xfd510000 0x4000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 146>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 0a6f7952bbb1..18511e782cbd 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -530,6 +530,7 @@
 			reg = <0x01010000 0x300000>;
 			interrupts = <GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>;
 			gpio-controller;
+			gpio-ranges = <&msmgpio 0 0 150>;
 			#gpio-cells = <2>;
 			interrupt-controller;
 			#interrupt-cells = <2>;
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index 495432f3341b..f2580bbba741 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -831,13 +831,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
 		return ret;
 	}
 
-	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;
-	}
-
 	ret = gpiochip_irqchip_add(chip,
 				   &msm_gpio_irq_chip,
 				   0,
-- 
2.16.3

Re: [PATCH] pinctrl: msm: fix gpio-hog related boot issues

[Bjorn Andersson]

On Wed 28 Mar 11:07 PDT 2018, Christian Lamparter wrote:

> 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@1000000/serial_pinmux, deferring probe
> | [    0.499828] spi_qup 78b5000.spi: could not find pctldev for node /soc/pinctrl@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@1000000/serial_pinmux, deferring probe
> | [    1.322415] spi_qup 78b5000.spi: could not find pctldev for node /soc/pinctrl@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.).
> 

It's quite clear that this is a generic issue with the msm pinctrl
driver.

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.

[..]
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index 3ca96e361878..17ad9cbd9f8c 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -327,6 +327,7 @@
>  			reg = <0x800000 0x4000>;
>  
>  			gpio-controller;
> +			gpio-ranges = <&tlmm_pinmux 0 0 90>;

This seems reasonable.

>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
[..]
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index 495432f3341b..f2580bbba741 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -831,13 +831,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
>  		return ret;
>  	}
>  
> -	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?

Regards,
Bjorn

[PATCH] pinctrl: msm: fix gpio-hog related boot issues

[Bjorn Andersson]

On Wed 28 Mar 11:07 PDT 2018, Christian Lamparter wrote:

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

It's quite clear that this is a generic issue with the msm pinctrl
driver.

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.

[..]
> diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> index 3ca96e361878..17ad9cbd9f8c 100644
> --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> @@ -327,6 +327,7 @@
>  			reg = <0x800000 0x4000>;
>  
>  			gpio-controller;
> +			gpio-ranges = <&tlmm_pinmux 0 0 90>;

This seems reasonable.

>  			#gpio-cells = <2>;
>  			interrupt-controller;
>  			#interrupt-cells = <2>;
[..]
> diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> index 495432f3341b..f2580bbba741 100644
> --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> @@ -831,13 +831,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
>  		return ret;
>  	}
>  
> -	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?

Regards,
Bjorn

Re: [PATCH] pinctrl: msm: fix gpio-hog related boot issues

[Christian Lamparter]

On Thu 29 Mar 2018 02:27:23 CEST Bjorn Andersson wrote:
> On Wed 28 Mar 11:07 PDT 2018, Christian Lamparter wrote:
> 
> > 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@1000000/serial_pinmux, deferring probe
> > | [    0.499828] spi_qup 78b5000.spi: could not find pctldev for node /soc/pinctrl@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@1000000/serial_pinmux, deferring probe
> > | [    1.322415] spi_qup 78b5000.spi: could not find pctldev for node /soc/pinctrl@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.).
> > 
> 
> 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
drivers that support a DT binding but use gpiochip_add_pin_range as the 
sole way to add the ranges. I made a (probably incomplete) list in
the thread by Sven: <https://lkml.kernel.org/r/46130418.GjRpGRXmAF@debian64>

> 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.
yes, that will work too.

One advantage of the gpio-hog is that it will also request the gpio properly.
So it cannot be exported (and changed) without getting rid of the gpio-hog
first. It also allows to specify a user-friendly line-name that shows up in
various places.

i.e.:
|root@mbl:/# cat /sys/kernel/debug/gpio 
|gpiochip1: GPIOs 472-479, parent: platform/4e0000000.gpio1, 4e0000000.gpio1:
| gpio-472 (                    |enable EMAC PHY     ) out hi    
| gpio-475 (                    |Power Drive Port 1  ) out hi  
|

> [..]
> > diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> > index 3ca96e361878..17ad9cbd9f8c 100644
> > --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> > +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> > @@ -327,6 +327,7 @@
> >  			reg = <0x800000 0x4000>;
> >  
> >  			gpio-controller;
> > +			gpio-ranges = <&tlmm_pinmux 0 0 90>;
> 
> This seems reasonable.
> 
> >  			#gpio-cells = <2>;
> >  			interrupt-controller;
> >  			#interrupt-cells = <2>;
> [..]
> > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> > index 495432f3341b..f2580bbba741 100644
> > --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> > @@ -831,13 +831,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
> >  		return ret;
> >  	}
> >  
> > -	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.
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,
|					[...]
|		}


> Iirc this driver follows the same pattern as several other pinctrl
> drivers providing gpios, how are they handling this?
Well, my commit message was based on a similar patch done by
Geert Uytterhoeven <geert+renesas@glider.be> for the sh73a0:
<https://marc.info/?l=git-commits-head&m=144114029919118&w=2>

Another one was this patch from Linus:
<https://patches.linaro.org/patch/51382/>

and there are many more (basically git blame on every .dts* in
arch/ that already has a gpio-ranges property.) 

Regards,
Christian

[PATCH] pinctrl: msm: fix gpio-hog related boot issues

[Christian Lamparter]

On Thu 29 Mar 2018 02:27:23 CEST Bjorn Andersson wrote:
> On Wed 28 Mar 11:07 PDT 2018, Christian Lamparter wrote:
> 
> > 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.).
> > 
> 
> 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
drivers that support a DT binding but use gpiochip_add_pin_range as the 
sole way to add the ranges. I made a (probably incomplete) list in
the thread by Sven: <https://lkml.kernel.org/r/46130418.GjRpGRXmAF@debian64>

> 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.
yes, that will work too.

One advantage of the gpio-hog is that it will also request the gpio properly.
So it cannot be exported (and changed) without getting rid of the gpio-hog
first. It also allows to specify a user-friendly line-name that shows up in
various places.

i.e.:
|root at mbl:/# cat /sys/kernel/debug/gpio 
|gpiochip1: GPIOs 472-479, parent: platform/4e0000000.gpio1, 4e0000000.gpio1:
| gpio-472 (                    |enable EMAC PHY     ) out hi    
| gpio-475 (                    |Power Drive Port 1  ) out hi  
|

> [..]
> > diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom-apq8064.dtsi
> > index 3ca96e361878..17ad9cbd9f8c 100644
> > --- a/arch/arm/boot/dts/qcom-apq8064.dtsi
> > +++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
> > @@ -327,6 +327,7 @@
> >  			reg = <0x800000 0x4000>;
> >  
> >  			gpio-controller;
> > +			gpio-ranges = <&tlmm_pinmux 0 0 90>;
> 
> This seems reasonable.
> 
> >  			#gpio-cells = <2>;
> >  			interrupt-controller;
> >  			#interrupt-cells = <2>;
> [..]
> > diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
> > index 495432f3341b..f2580bbba741 100644
> > --- a/drivers/pinctrl/qcom/pinctrl-msm.c
> > +++ b/drivers/pinctrl/qcom/pinctrl-msm.c
> > @@ -831,13 +831,6 @@ static int msm_gpio_init(struct msm_pinctrl *pctrl)
> >  		return ret;
> >  	}
> >  
> > -	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.
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,
|					[...]
|		}


> Iirc this driver follows the same pattern as several other pinctrl
> drivers providing gpios, how are they handling this?
Well, my commit message was based on a similar patch done by
Geert Uytterhoeven <geert+renesas@glider.be> for the sh73a0:
<https://marc.info/?l=git-commits-head&m=144114029919118&w=2>

Another one was this patch from Linus:
<https://patches.linaro.org/patch/51382/>

and there are many more (basically git blame on every .dts* in
arch/ that already has a gpio-ranges property.) 

Regards,
Christian

Re: [PATCH] pinctrl: msm: fix gpio-hog related boot issues

[Christian Lamparter]

On Thu 29 Mar 2018 14:23:35 CEST Christian Lamparter wrote:
> 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)) {
oops, this should be:
			if (!is_of_node(pctrl->dev->fwnode)) {
> |					/*
> |					 * (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,
> |					[...]
> |		}

[PATCH] pinctrl: msm: fix gpio-hog related boot issues

[Christian Lamparter]

On Thu 29 Mar 2018 14:23:35 CEST Christian Lamparter wrote:
> 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)) {
oops, this should be:
			if (!is_of_node(pctrl->dev->fwnode)) {
> |					/*
> |					 * (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,
> |					[...]
> |		}

Re: [PATCH] pinctrl: msm: fix gpio-hog related boot issues

[Linus Walleij]

On Wed, Mar 28, 2018 at 8:07 PM, Christian Lamparter <chunkeey@gmail.com> wrote:

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

That's pretty neat!

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

nice!

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

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH] pinctrl: msm: fix gpio-hog related boot issues

[Linus Walleij]

On Wed, Mar 28, 2018 at 8:07 PM, Christian Lamparter <chunkeey@gmail.com> wrote:

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

That's pretty neat!

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

nice!

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

Yours,
Linus Walleij

Re: [PATCH] pinctrl: msm: fix gpio-hog related boot issues

[Christian Lamparter]

On Donnerstag, 12. April 2018 14:48:56 CEST Linus Walleij wrote:
> On Wed, Mar 28, 2018 at 8:07 PM, Christian Lamparter <chunkeey@gmail.com> wrote:
> 
> > -       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.
Thank you for your insightful advice. I just sent out v4 which
goes the of_property_read_bool route. Let's wait and see what
kbuilt-bot has to say.

Best Regards,
Christian

[PATCH] pinctrl: msm: fix gpio-hog related boot issues

[Christian Lamparter]

On Donnerstag, 12. April 2018 14:48:56 CEST Linus Walleij wrote:
> On Wed, Mar 28, 2018 at 8:07 PM, Christian Lamparter <chunkeey@gmail.com> wrote:
> 
> > -       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.
Thank you for your insightful advice. I just sent out v4 which
goes the of_property_read_bool route. Let's wait and see what
kbuilt-bot has to say.

Best Regards,
Christian