ARM: dts: imx28: add gpio-ranges for internal gpio controller

PATCH New Uwe Kleine-König 3 participants 8 messages May 8, 2017 – May 22, 2017

Subsystems (3)

Device tree bindings Arm i.MX and Layerscape GPIO

lore

Share & tools

lore

Uwe Kleine-König

author

2017-05-08 08:59 UTC #0 lore raw

PATCH 1/1 ARM: dts: imx28: add gpio-ranges for internal gpio controller

Signed-off-by Uwe Kleine-K?nig <[email protected]>

---
Hello,

with this patch applied I get the following lines in dmesg which looks fine:

	[    0.227913] gpio gpiochip0: (80018000.pinctrl:gpio at 0): created GPIO range 0->31 ==> 80018000.pinctrl PIN 0->31
	[    0.236100] gpio gpiochip1: (80018000.pinctrl:gpio at 1): created GPIO range 0->31 ==> 80018000.pinctrl PIN 32->63
	[    0.244463] gpio gpiochip2: (80018000.pinctrl:gpio at 2): created GPIO range 0->31 ==> 80018000.pinctrl PIN 64->95
	[    0.253020] gpio gpiochip3: (80018000.pinctrl:gpio at 3): created GPIO range 0->31 ==> 80018000.pinctrl PIN 96->127
	[    0.261639] gpio gpiochip4: (80018000.pinctrl:gpio at 4): created GPIO range 0->31 ==> 80018000.pinctrl PIN 128->159

But when looking at a used gpio

	# cat /sys/kernel/debug/gpio
	gpiochip0: GPIOs 0-31, parent: platform/80018000.pinctrl:gpio at 0, 80018000.pinctrl:gpio at 0:
	...
	 gpio-20  (LED4                |?                   ) out hi
	...

	# grep "pin 20 " /sys/kernel/debug/pinctrl/80018000.pinctrl/pinmux-pins
	pin 20 (GPMI_RDY0): leds (GPIO UNCLAIMED) function leds group leds.0

I wonder why there is still "GPIO UNCLAIMED". I would have expected that this disappears and somehow references the gpio_request issued by the led-gpio driver after my patch.

What am I missing?

Best regards
Uwe

 arch/arm/boot/dts/imx28.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

arch/arm/boot/dts/imx28.dtsi +5 -0

index 148fcf4d3b98..cfad2295cc46 100644
@@ -182,6 +182,7 @@
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 0 32>;
 				};
 
 				gpio1: gpio at 1 {

@@ -192,6 +193,7 @@
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 32 32>;
 				};
 
 				gpio2: gpio at 2 {

@@ -202,6 +204,7 @@
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 64 32>;
 				};
 
 				gpio3: gpio at 3 {

@@ -212,6 +215,7 @@
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 96 32>;
 				};
 
 				gpio4: gpio at 4 {

@@ -222,6 +226,7 @@
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 128 32>;
 				};
 
 				duart_pins_a: duart at 0 {

Signature · 1 line

2.11.0

Shawn Guo

2017-05-11 07:51 UTC #1 lore raw

PATCH 1/1 ARM: dts: imx28: add gpio-ranges for internal gpio controller

Quoted 21 lines · Uwe Kleine-K?nig

Signed-off-by Uwe Kleine-K?nig <[email protected]>

---
Hello,

with this patch applied I get the following lines in dmesg which looks fine:

	[    0.227913] gpio gpiochip0: (80018000.pinctrl:gpio at 0): created GPIO range 0->31 ==> 80018000.pinctrl PIN 0->31
	[    0.236100] gpio gpiochip1: (80018000.pinctrl:gpio at 1): created GPIO range 0->31 ==> 80018000.pinctrl PIN 32->63
	[    0.244463] gpio gpiochip2: (80018000.pinctrl:gpio at 2): created GPIO range 0->31 ==> 80018000.pinctrl PIN 64->95
	[    0.253020] gpio gpiochip3: (80018000.pinctrl:gpio at 3): created GPIO range 0->31 ==> 80018000.pinctrl PIN 96->127
	[    0.261639] gpio gpiochip4: (80018000.pinctrl:gpio at 4): created GPIO range 0->31 ==> 80018000.pinctrl PIN 128->159

But when looking at a used gpio

	# cat /sys/kernel/debug/gpio
	gpiochip0: GPIOs 0-31, parent: platform/80018000.pinctrl:gpio at 0, 80018000.pinctrl:gpio at 0:
	...
	 gpio-20  (LED4                |?                   ) out hi
	...

Quoted 8 lines

	# grep "pin 20 " /sys/kernel/debug/pinctrl/80018000.pinctrl/pinmux-pins
	pin 20 (GPMI_RDY0): leds (GPIO UNCLAIMED) function leds group leds.0
I wonder why there is still "GPIO UNCLAIMED". I would have expected that this disappears and somehow references the gpio_request issued by the led-gpio driver after my patch.
What am I missing?

It seems that's only the case where @strict of struct pinmux_ops is true. We should set it true for pinctrl-mxs, I guess?

Shawn

Quoted 54 lines

Best regards
Uwe

 arch/arm/boot/dts/imx28.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

arch/arm/boot/dts/imx28.dtsi +5 -0

index 148fcf4d3b98..cfad2295cc46 100644
@@ -182,6 +182,7 @@
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 0 32>;
 				};
 
 				gpio1: gpio at 1 {

@@ -192,6 +193,7 @@
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 32 32>;
 				};
 
 				gpio2: gpio at 2 {

@@ -202,6 +204,7 @@
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 64 32>;
 				};
 
 				gpio3: gpio at 3 {

@@ -212,6 +215,7 @@
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 96 32>;
 				};
 
 				gpio4: gpio at 4 {

@@ -222,6 +226,7 @@
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 128 32>;
 				};
 
 				duart_pins_a: duart at 0 {

Signature · 1 line

2.11.0

Uwe Kleine-König

author

2017-05-11 08:09 UTC #2 lore raw

PATCH 1/1 ARM: dts: imx28: add gpio-ranges for internal gpio controller

Quoted 25 lines · Shawn Guo

Quoted 24 lines · Uwe Kleine-K?nig

Signed-off-by Uwe Kleine-K?nig <[email protected]>

---
Hello,

with this patch applied I get the following lines in dmesg which looks fine:

	[    0.227913] gpio gpiochip0: (80018000.pinctrl:gpio at 0): created GPIO range 0->31 ==> 80018000.pinctrl PIN 0->31
	[    0.236100] gpio gpiochip1: (80018000.pinctrl:gpio at 1): created GPIO range 0->31 ==> 80018000.pinctrl PIN 32->63
	[    0.244463] gpio gpiochip2: (80018000.pinctrl:gpio at 2): created GPIO range 0->31 ==> 80018000.pinctrl PIN 64->95
	[    0.253020] gpio gpiochip3: (80018000.pinctrl:gpio at 3): created GPIO range 0->31 ==> 80018000.pinctrl PIN 96->127
	[    0.261639] gpio gpiochip4: (80018000.pinctrl:gpio at 4): created GPIO range 0->31 ==> 80018000.pinctrl PIN 128->159

But when looking at a used gpio

	# cat /sys/kernel/debug/gpio
	gpiochip0: GPIOs 0-31, parent: platform/80018000.pinctrl:gpio at 0, 80018000.pinctrl:gpio at 0:
	...
	 gpio-20  (LED4                |?                   ) out hi
	...

	# grep "pin 20 " /sys/kernel/debug/pinctrl/80018000.pinctrl/pinmux-pins
	pin 20 (GPMI_RDY0): leds (GPIO UNCLAIMED) function leds group leds.0

Quoted 8 lines

Quoted 5 lines
I wonder why there is still "GPIO UNCLAIMED". I would have expected that this disappears and somehow references the gpio_request issued by the led-gpio driver after my patch.
What am I missing?
It seems that's only the case where @strict of struct pinmux_ops is true. We should set it true for pinctrl-mxs, I guess?

The description is:

* @strict: do not allow simultaneous use of the same pin for GPIO and another
* function. Check both gpio_owner and mux_owner strictly before approving
* the pin request.

so if I understand correctly that means that if a device has configured the pin MX28_PAD_SSP2_SCK as function SSP2_SCK it's impossible to do gpio_request on <&gpio2 16> (which is the matching GPIO)? I don't like that. My use case for exactly this is that I want the MX28_PAD_SSP2_SCK pin to be high-Z when the spi bus is not in use. I do this as follows:

	&ssp2 {
		pinctrl-names = "default", "idle";
		pinctrl-0 = <&spi2_pins_a>;
		pinctrl-1 = <&spi2_pins_a_gpio>;
		...
	};

where spi2_pins_a_gpio includes MX28_PAD_SSP2_SCK__GPIO_2_16, and then

	&gpio2 {
		ssp2_sck {
			gpio-hog;
			gpio = <16 0>;
			input;
		};
		...
	};

. So I think strict is a bad idea, not only for pinctrl-mxs.

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Shawn Guo

2017-05-12 03:05 UTC #3 lore raw

PATCH 1/1 ARM: dts: imx28: add gpio-ranges for internal gpio controller

Quoted 32 lines · Uwe Kleine-K?nig

Quoted 31 lines · Shawn Guo
Quoted 29 lines · Uwe Kleine-K?nig
Signed-off-by Uwe Kleine-K?nig <[email protected]>
---
Hello,
with this patch applied I get the following lines in dmesg which looks fine:
	[    0.227913] gpio gpiochip0: (80018000.pinctrl:gpio at 0): created GPIO range 0->31 ==> 80018000.pinctrl PIN 0->31
	[    0.236100] gpio gpiochip1: (80018000.pinctrl:gpio at 1): created GPIO range 0->31 ==> 80018000.pinctrl PIN 32->63
	[    0.244463] gpio gpiochip2: (80018000.pinctrl:gpio at 2): created GPIO range 0->31 ==> 80018000.pinctrl PIN 64->95
	[    0.253020] gpio gpiochip3: (80018000.pinctrl:gpio at 3): created GPIO range 0->31 ==> 80018000.pinctrl PIN 96->127
	[    0.261639] gpio gpiochip4: (80018000.pinctrl:gpio at 4): created GPIO range 0->31 ==> 80018000.pinctrl PIN 128->159
But when looking at a used gpio
	# cat /sys/kernel/debug/gpio
	gpiochip0: GPIOs 0-31, parent: platform/80018000.pinctrl:gpio at 0, 80018000.pinctrl:gpio at 0:
	...
	 gpio-20  (LED4                |?                   ) out hi
	...
	# grep "pin 20 " /sys/kernel/debug/pinctrl/80018000.pinctrl/pinmux-pins
	pin 20 (GPMI_RDY0): leds (GPIO UNCLAIMED) function leds group leds.0
I wonder why there is still "GPIO UNCLAIMED". I would have expected that this disappears and somehow references the gpio_request issued by the led-gpio driver after my patch.
What am I missing?

Quoted 8 lines

Quoted 2 lines
It seems that's only the case where @strict of struct pinmux_ops is true. We should set it true for pinctrl-mxs, I guess?
The description is:
* @strict: do not allow simultaneous use of the same pin for GPIO and another
* function. Check both gpio_owner and mux_owner strictly before approving
* the pin request.

Sorry, I misread the 'strict' code and my comment about it is completely a noise.

I went through the code around requesting a pin, and found that we need to call pinctrl_request_gpio() from gpio driver to get the result you want. In that case, pin_request() will be called with a valid gpio_range as below.

    pinctrl_request_gpio()
        pinmux_request_gpio()
            pin_request(..., gpio_range)

Right now, pin_request() is being called with a NULL gpio_range from pinmux_enable_setting(). That gets us the mux_owner rather than gpio_owner for the pin.

Shawn

Uwe Kleine-König

author

2017-05-12 08:01 UTC #4 lore raw

PATCH 1/1 ARM: dts: imx28: add gpio-ranges for internal gpio controller

Quoted 50 lines · Shawn Guo

Quoted 32 lines · Uwe Kleine-K?nig
Quoted 31 lines · Shawn Guo
Quoted 29 lines · Uwe Kleine-K?nig
Signed-off-by Uwe Kleine-K?nig <[email protected]>
---
Hello,
with this patch applied I get the following lines in dmesg which looks fine:
	[    0.227913] gpio gpiochip0: (80018000.pinctrl:gpio at 0): created GPIO range 0->31 ==> 80018000.pinctrl PIN 0->31
	[    0.236100] gpio gpiochip1: (80018000.pinctrl:gpio at 1): created GPIO range 0->31 ==> 80018000.pinctrl PIN 32->63
	[    0.244463] gpio gpiochip2: (80018000.pinctrl:gpio at 2): created GPIO range 0->31 ==> 80018000.pinctrl PIN 64->95
	[    0.253020] gpio gpiochip3: (80018000.pinctrl:gpio at 3): created GPIO range 0->31 ==> 80018000.pinctrl PIN 96->127
	[    0.261639] gpio gpiochip4: (80018000.pinctrl:gpio at 4): created GPIO range 0->31 ==> 80018000.pinctrl PIN 128->159
But when looking at a used gpio
	# cat /sys/kernel/debug/gpio
	gpiochip0: GPIOs 0-31, parent: platform/80018000.pinctrl:gpio at 0, 80018000.pinctrl:gpio at 0:
	...
	 gpio-20  (LED4                |?                   ) out hi
	...
	# grep "pin 20 " /sys/kernel/debug/pinctrl/80018000.pinctrl/pinmux-pins
	pin 20 (GPMI_RDY0): leds (GPIO UNCLAIMED) function leds group leds.0
I wonder why there is still "GPIO UNCLAIMED". I would have expected that this disappears and somehow references the gpio_request issued by the led-gpio driver after my patch.
What am I missing?
Quoted 8 lines
Quoted 2 lines
It seems that's only the case where @strict of struct pinmux_ops is true. We should set it true for pinctrl-mxs, I guess?
The description is:
* @strict: do not allow simultaneous use of the same pin for GPIO and another
* function. Check both gpio_owner and mux_owner strictly before approving
* the pin request.
Sorry, I misread the 'strict' code and my comment about it is completely a noise.
I went through the code around requesting a pin, and found that we need to call pinctrl_request_gpio() from gpio driver to get the result you want. In that case, pin_request() will be called with a valid gpio_range as below.

Quoted 7 lines

    pinctrl_request_gpio()
        pinmux_request_gpio()
            pin_request(..., gpio_range)
Right now, pin_request() is being called with a NULL gpio_range from pinmux_enable_setting(). That gets us the mux_owner rather than gpio_owner for the pin.

But then again I cannot mux a pin to a different function when the gpio is requested, right?

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Shawn Guo

2017-05-15 02:21 UTC #5 lore raw

PATCH 1/1 ARM: dts: imx28: add gpio-ranges for internal gpio controller

Quoted 10 lines · Uwe Kleine-K?nig

Quoted 9 lines · Shawn Guo
I went through the code around requesting a pin, and found that we need to call pinctrl_request_gpio() from gpio driver to get the result you want. In that case, pin_request() will be called with a valid gpio_range as below.
    pinctrl_request_gpio()
        pinmux_request_gpio()
            pin_request(..., gpio_range)

Quoted 6 lines

Quoted 3 lines
Right now, pin_request() is being called with a NULL gpio_range from pinmux_enable_setting(). That gets us the mux_owner rather than gpio_owner for the pin.
But then again I cannot mux a pin to a different function when the gpio is requested, right?

You will need to free the GPIO before muxing it to a different function, I think.

Shawn

Uwe Kleine-König

author

2017-05-15 07:16 UTC #6 lore raw

PATCH 1/1 ARM: dts: imx28: add gpio-ranges for internal gpio controller

Hello Shawn,

Quoted 11 lines · Shawn Guo

Quoted 10 lines · Uwe Kleine-K?nig
Quoted 9 lines · Shawn Guo
I went through the code around requesting a pin, and found that we need to call pinctrl_request_gpio() from gpio driver to get the result you want. In that case, pin_request() will be called with a valid gpio_range as below.
    pinctrl_request_gpio()
        pinmux_request_gpio()
            pin_request(..., gpio_range)

Quoted 6 lines

Quoted 6 lines
Quoted 3 lines
Right now, pin_request() is being called with a NULL gpio_range from pinmux_enable_setting(). That gets us the mux_owner rather than gpio_owner for the pin.
But then again I cannot mux a pin to a different function when the gpio is requested, right?

(Actually I intended to postpone this mail, but sent it instead by accident.)

Quoted 2 lines

You will need to free the GPIO before muxing it to a different function, I think.

IMHO this is a bad concept. This makes GPIOs more special than for example PWMs or LEDs. And it breaks some configurations (for example the make-pins-highz-on-idle setup in my previous mails).

Best regards
Uwe

Signature · 2 lines

Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Linus Walleij

2017-05-22 15:54 UTC #7 lore raw

PATCH 1/1 ARM: dts: imx28: add gpio-ranges for internal gpio controller

Quoted 8 lines · Uwe Kleine-K?nig

Quoted 2 lines · Shawn Guo
You will need to free the GPIO before muxing it to a different function, I think.
IMHO this is a bad concept. This makes GPIOs more special than for example PWMs or LEDs. And it breaks some configurations (for example the make-pins-highz-on-idle setup in my previous mails).

So this is the reason why pin controllers can choose to be strict or not: people disagree on the semantics.

But it's good if the driver maintainers agree for a certain driver :D

Yours,
Linus Walleij