| Message ID | 20170508085925.18342-1-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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
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
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
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 {
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(+)