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

Message ID 20170508085925.18342-1-u.kleine-koenig@pengutronix.de
State New
Headers show

Commit Message

Uwe Kleine-König May 8, 2017, 8:59 a.m. UTC
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

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

	[    0.227913] gpio gpiochip0: (80018000.pinctrl:gpio@0): created GPIO range 0->31 ==> 80018000.pinctrl PIN 0->31
	[    0.236100] gpio gpiochip1: (80018000.pinctrl:gpio@1): created GPIO range 0->31 ==> 80018000.pinctrl PIN 32->63
	[    0.244463] gpio gpiochip2: (80018000.pinctrl:gpio@2): created GPIO range 0->31 ==> 80018000.pinctrl PIN 64->95
	[    0.253020] gpio gpiochip3: (80018000.pinctrl:gpio@3): created GPIO range 0->31 ==> 80018000.pinctrl PIN 96->127
	[    0.261639] gpio gpiochip4: (80018000.pinctrl:gpio@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@0, 80018000.pinctrl:gpio@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(+)

Comments

Shawn Guo May 11, 2017, 7:51 a.m. UTC | #1
On Mon, May 08, 2017 at 10:59:25AM +0200, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> with this patch applied I get the following lines in dmesg which looks
> fine:
> 
> 	[    0.227913] gpio gpiochip0: (80018000.pinctrl:gpio@0): created GPIO range 0->31 ==> 80018000.pinctrl PIN 0->31
> 	[    0.236100] gpio gpiochip1: (80018000.pinctrl:gpio@1): created GPIO range 0->31 ==> 80018000.pinctrl PIN 32->63
> 	[    0.244463] gpio gpiochip2: (80018000.pinctrl:gpio@2): created GPIO range 0->31 ==> 80018000.pinctrl PIN 64->95
> 	[    0.253020] gpio gpiochip3: (80018000.pinctrl:gpio@3): created GPIO range 0->31 ==> 80018000.pinctrl PIN 96->127
> 	[    0.261639] gpio gpiochip4: (80018000.pinctrl:gpio@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@0, 80018000.pinctrl:gpio@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?

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

> 
> Best regards
> Uwe
> 
>  arch/arm/boot/dts/imx28.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 148fcf4d3b98..cfad2295cc46 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -182,6 +182,7 @@
>  					#gpio-cells = <2>;
>  					interrupt-controller;
>  					#interrupt-cells = <2>;
> +					gpio-ranges = <&pinctrl 0 0 32>;
>  				};
>  
>  				gpio1: gpio@1 {
> @@ -192,6 +193,7 @@
>  					#gpio-cells = <2>;
>  					interrupt-controller;
>  					#interrupt-cells = <2>;
> +					gpio-ranges = <&pinctrl 0 32 32>;
>  				};
>  
>  				gpio2: gpio@2 {
> @@ -202,6 +204,7 @@
>  					#gpio-cells = <2>;
>  					interrupt-controller;
>  					#interrupt-cells = <2>;
> +					gpio-ranges = <&pinctrl 0 64 32>;
>  				};
>  
>  				gpio3: gpio@3 {
> @@ -212,6 +215,7 @@
>  					#gpio-cells = <2>;
>  					interrupt-controller;
>  					#interrupt-cells = <2>;
> +					gpio-ranges = <&pinctrl 0 96 32>;
>  				};
>  
>  				gpio4: gpio@4 {
> @@ -222,6 +226,7 @@
>  					#gpio-cells = <2>;
>  					interrupt-controller;
>  					#interrupt-cells = <2>;
> +					gpio-ranges = <&pinctrl 0 128 32>;
>  				};
>  
>  				duart_pins_a: duart@0 {
> -- 
> 2.11.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Uwe Kleine-König May 11, 2017, 8:09 a.m. UTC | #2
On Thu, May 11, 2017 at 03:51:36PM +0800, Shawn Guo wrote:
> On Mon, May 08, 2017 at 10:59:25AM +0200, Uwe Kleine-König wrote:
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > with this patch applied I get the following lines in dmesg which looks
> > fine:
> > 
> > 	[    0.227913] gpio gpiochip0: (80018000.pinctrl:gpio@0): created GPIO range 0->31 ==> 80018000.pinctrl PIN 0->31
> > 	[    0.236100] gpio gpiochip1: (80018000.pinctrl:gpio@1): created GPIO range 0->31 ==> 80018000.pinctrl PIN 32->63
> > 	[    0.244463] gpio gpiochip2: (80018000.pinctrl:gpio@2): created GPIO range 0->31 ==> 80018000.pinctrl PIN 64->95
> > 	[    0.253020] gpio gpiochip3: (80018000.pinctrl:gpio@3): created GPIO range 0->31 ==> 80018000.pinctrl PIN 96->127
> > 	[    0.261639] gpio gpiochip4: (80018000.pinctrl:gpio@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@0, 80018000.pinctrl:gpio@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?
> 
> 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
Shawn Guo May 12, 2017, 3:05 a.m. UTC | #3
On Thu, May 11, 2017 at 10:09:16AM +0200, Uwe Kleine-König wrote:
> On Thu, May 11, 2017 at 03:51:36PM +0800, Shawn Guo wrote:
> > On Mon, May 08, 2017 at 10:59:25AM +0200, Uwe Kleine-König wrote:
> > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > ---
> > > Hello,
> > > 
> > > with this patch applied I get the following lines in dmesg which looks
> > > fine:
> > > 
> > > 	[    0.227913] gpio gpiochip0: (80018000.pinctrl:gpio@0): created GPIO range 0->31 ==> 80018000.pinctrl PIN 0->31
> > > 	[    0.236100] gpio gpiochip1: (80018000.pinctrl:gpio@1): created GPIO range 0->31 ==> 80018000.pinctrl PIN 32->63
> > > 	[    0.244463] gpio gpiochip2: (80018000.pinctrl:gpio@2): created GPIO range 0->31 ==> 80018000.pinctrl PIN 64->95
> > > 	[    0.253020] gpio gpiochip3: (80018000.pinctrl:gpio@3): created GPIO range 0->31 ==> 80018000.pinctrl PIN 96->127
> > > 	[    0.261639] gpio gpiochip4: (80018000.pinctrl:gpio@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@0, 80018000.pinctrl:gpio@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?
> > 
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Uwe Kleine-König May 12, 2017, 8:01 a.m. UTC | #4
On Fri, May 12, 2017 at 11:05:38AM +0800, Shawn Guo wrote:
> On Thu, May 11, 2017 at 10:09:16AM +0200, Uwe Kleine-König wrote:
> > On Thu, May 11, 2017 at 03:51:36PM +0800, Shawn Guo wrote:
> > > On Mon, May 08, 2017 at 10:59:25AM +0200, Uwe Kleine-König wrote:
> > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > > > ---
> > > > Hello,
> > > > 
> > > > with this patch applied I get the following lines in dmesg which looks
> > > > fine:
> > > > 
> > > > 	[    0.227913] gpio gpiochip0: (80018000.pinctrl:gpio@0): created GPIO range 0->31 ==> 80018000.pinctrl PIN 0->31
> > > > 	[    0.236100] gpio gpiochip1: (80018000.pinctrl:gpio@1): created GPIO range 0->31 ==> 80018000.pinctrl PIN 32->63
> > > > 	[    0.244463] gpio gpiochip2: (80018000.pinctrl:gpio@2): created GPIO range 0->31 ==> 80018000.pinctrl PIN 64->95
> > > > 	[    0.253020] gpio gpiochip3: (80018000.pinctrl:gpio@3): created GPIO range 0->31 ==> 80018000.pinctrl PIN 96->127
> > > > 	[    0.261639] gpio gpiochip4: (80018000.pinctrl:gpio@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@0, 80018000.pinctrl:gpio@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?
> > > 
> > > 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.

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

Best regards
Uwe
Shawn Guo May 15, 2017, 2:21 a.m. UTC | #5
On Fri, May 12, 2017 at 10:01:50AM +0200, Uwe Kleine-König wrote:
> On Fri, May 12, 2017 at 11:05:38AM +0800, Shawn Guo wrote:
> > 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.
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Uwe Kleine-König May 15, 2017, 7:16 a.m. UTC | #6
Hello Shawn,

On Mon, May 15, 2017 at 10:21:30AM +0800, Shawn Guo wrote:
> On Fri, May 12, 2017 at 10:01:50AM +0200, Uwe Kleine-König wrote:
> > On Fri, May 12, 2017 at 11:05:38AM +0800, Shawn Guo wrote:
> > > 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.
> > 
> > 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.)
 
> 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
Linus Walleij May 22, 2017, 3:54 p.m. UTC | #7
On Mon, May 15, 2017 at 9:16 AM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
> On Mon, May 15, 2017 at 10:21:30AM +0800, Shawn Guo wrote:

>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 148fcf4d3b98..cfad2295cc46 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -182,6 +182,7 @@ 
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 0 32>;
 				};
 
 				gpio1: gpio@1 {
@@ -192,6 +193,7 @@ 
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 32 32>;
 				};
 
 				gpio2: gpio@2 {
@@ -202,6 +204,7 @@ 
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 64 32>;
 				};
 
 				gpio3: gpio@3 {
@@ -212,6 +215,7 @@ 
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 96 32>;
 				};
 
 				gpio4: gpio@4 {
@@ -222,6 +226,7 @@ 
 					#gpio-cells = <2>;
 					interrupt-controller;
 					#interrupt-cells = <2>;
+					gpio-ranges = <&pinctrl 0 128 32>;
 				};
 
 				duart_pins_a: duart@0 {