[PATCH for 3.10] Misc small improvements to Kirkwood-based OpenBlocks A6 platform

[PATCH for 3.10] Misc small improvements to Kirkwood-based OpenBlocks A6 platform

[Thomas Petazzoni]

Jason, Andrew, Greg,

Here is a set of 3 patches making small improvements to the support of
the Kirkwood-based PlatHome OpenBlocks A6 platform.

The first two patches clean up the pinctrl definitions in the board
Device Tree.

The third patch adds the support for a GPIO-connected button.

Thanks,

Thomas

[PATCH 1/3] arm: kirkwood: affect pins to their devices on OpenBlocks A6

[Thomas Petazzoni]

The pinctrl configuration done for the Kirkwood-based OpenBlocks A6
board was somewhat strange: all pins for all devices were selected
through "hogs", i.e as pins of the pinctrl device. Even though it
works, it isn't the proper way of doing things: pins should preferably
be associated to whatever device they belong to.

Therefore, this patch moves the selection of pin muxing of the
relevant pins to the UART nodes, the I2C node, the NAND node and the
gpio-leds node.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/kirkwood-openblocks_a6.dts |   19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
index ede7fe0d..087681d 100644
--- a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
+++ b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
@@ -19,17 +19,23 @@
 	ocp at f1000000 {
 		serial at 12000 {
 			clock-frequency = <200000000>;
+			pinctrl-0 = <&pmx_uart0>;
+			pinctrl-names = "default";
 			status = "ok";
 		};
 
 		serial at 12100 {
 			clock-frequency = <200000000>;
+			pinctrl-0 = <&pmx_uart1>;
+			pinctrl-names = "default";
 			status = "ok";
 		};
 
 		nand at 3000000 {
 			chip-delay = <25>;
 			status = "okay";
+			pinctrl-0 = <&pmx_nand>;
+			pinctrl-names = "default";
 
 			partition at 0 {
 				label = "uboot";
@@ -69,6 +75,8 @@
 
 		i2c at 11100 {
 			status = "okay";
+			pinctrl-0 = <&pmx_twsi1>;
+			pinctrl-names = "default";
 
 			s35390a: s35390a at 30 {
 				compatible = "s35390a";
@@ -77,16 +85,12 @@
 		};
 
 		pinctrl: pinctrl at 10000 {
-			pinctrl-0 = < &pmx_nand &pmx_uart0
-				&pmx_uart1 &pmx_twsi1
-				&pmx_dip_sw0 &pmx_dip_sw1
+			pinctrl-0 = <&pmx_dip_sw0 &pmx_dip_sw1
 				&pmx_dip_sw2 &pmx_dip_sw3
 				&pmx_gpio_0 &pmx_gpio_1
 				&pmx_gpio_2 &pmx_gpio_3
 				&pmx_gpio_4 &pmx_gpio_5
-				&pmx_gpio_6 &pmx_gpio_7
-				&pmx_led_red &pmx_led_green
-				&pmx_led_yellow >;
+				&pmx_gpio_6 &pmx_gpio_7>;
 			pinctrl-names = "default";
 
 			pmx_uart0: pmx-uart0 {
@@ -195,6 +199,9 @@
 
 	gpio-leds {
 		compatible = "gpio-leds";
+		pinctrl-0 = <&pmx_led_red &pmx_led_green
+				&pmx_led_yellow>;
+		pinctrl-names = "default";
 
 		led-red {
 			label = "obsa6:red:stat";
-- 
1.7.9.5

[PATCH 1/3] arm: kirkwood: affect pins to their devices on OpenBlocks A6

[Andrew Lunn]

On Wed, Mar 06, 2013 at 05:23:39PM +0100, Thomas Petazzoni wrote:
> The pinctrl configuration done for the Kirkwood-based OpenBlocks A6
> board was somewhat strange: all pins for all devices were selected
> through "hogs", i.e as pins of the pinctrl device. Even though it
> works, it isn't the proper way of doing things: pins should preferably
> be associated to whatever device they belong to.

Hi Thomas

Being able to add pinctrl nodes to the device is new. I think it was
added in 3.8. Before then each device driver had to explicitly request
its pins and most didn't, so hogs was the way to do it. Now the core
driver code gets the pins. So i would not say it is odd, just
outdated. It would be nice to change the comment to reflect this.

> Therefore, this patch moves the selection of pin muxing of the
> relevant pins to the UART nodes, the I2C node, the NAND node and the
> gpio-leds node.

For gpio-leds, see the comment i just made for the next patch.

Otherwise this look O.K.

	  Andrew

> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/boot/dts/kirkwood-openblocks_a6.dts |   19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
> index ede7fe0d..087681d 100644
> --- a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
> +++ b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
> @@ -19,17 +19,23 @@
>  	ocp at f1000000 {
>  		serial at 12000 {
>  			clock-frequency = <200000000>;
> +			pinctrl-0 = <&pmx_uart0>;
> +			pinctrl-names = "default";
>  			status = "ok";
>  		};
>  
>  		serial at 12100 {
>  			clock-frequency = <200000000>;
> +			pinctrl-0 = <&pmx_uart1>;
> +			pinctrl-names = "default";
>  			status = "ok";
>  		};
>  
>  		nand at 3000000 {
>  			chip-delay = <25>;
>  			status = "okay";
> +			pinctrl-0 = <&pmx_nand>;
> +			pinctrl-names = "default";
>  
>  			partition at 0 {
>  				label = "uboot";
> @@ -69,6 +75,8 @@
>  
>  		i2c at 11100 {
>  			status = "okay";
> +			pinctrl-0 = <&pmx_twsi1>;
> +			pinctrl-names = "default";
>  
>  			s35390a: s35390a at 30 {
>  				compatible = "s35390a";
> @@ -77,16 +85,12 @@
>  		};
>  
>  		pinctrl: pinctrl at 10000 {
> -			pinctrl-0 = < &pmx_nand &pmx_uart0
> -				&pmx_uart1 &pmx_twsi1
> -				&pmx_dip_sw0 &pmx_dip_sw1
> +			pinctrl-0 = <&pmx_dip_sw0 &pmx_dip_sw1
>  				&pmx_dip_sw2 &pmx_dip_sw3
>  				&pmx_gpio_0 &pmx_gpio_1
>  				&pmx_gpio_2 &pmx_gpio_3
>  				&pmx_gpio_4 &pmx_gpio_5
> -				&pmx_gpio_6 &pmx_gpio_7
> -				&pmx_led_red &pmx_led_green
> -				&pmx_led_yellow >;
> +				&pmx_gpio_6 &pmx_gpio_7>;
>  			pinctrl-names = "default";
>  
>  			pmx_uart0: pmx-uart0 {
> @@ -195,6 +199,9 @@
>  
>  	gpio-leds {
>  		compatible = "gpio-leds";
> +		pinctrl-0 = <&pmx_led_red &pmx_led_green
> +				&pmx_led_yellow>;
> +		pinctrl-names = "default";
>  
>  		led-red {
>  			label = "obsa6:red:stat";
> -- 
> 1.7.9.5
> 

[PATCH 1/3] arm: kirkwood: affect pins to their devices on OpenBlocks A6

[Thomas Petazzoni]

Dear Andrew Lunn,

On Wed, 6 Mar 2013 19:21:31 +0100, Andrew Lunn wrote:

> Being able to add pinctrl nodes to the device is new. I think it was
> added in 3.8. Before then each device driver had to explicitly request
> its pins and most didn't, so hogs was the way to do it.

A number of drivers were already capable of doing this, like gpio-leds
for example. But as you say, in 3.9 a patch making this globally
available to all drivers without having to modify them has been merged,
and that's what I rely on.

> Now the core
> driver code gets the pins. So i would not say it is odd, just
> outdated. It would be nice to change the comment to reflect this.

Ok, I'll fix that up.

> > Therefore, this patch moves the selection of pin muxing of the
> > relevant pins to the UART nodes, the I2C node, the NAND node and the
> > gpio-leds node.
> 
> For gpio-leds, see the comment i just made for the next patch.
> 
> Otherwise this look O.K.

Thanks.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 2/3] arm: kirkwood: factor pinmux descriptors for OpenBlocks A6

[Thomas Petazzoni]

The OpenBlocks A6 .dts file was using a long list of pinmux
descriptors to select each GPIO of the external GPIO connector and the
internal DIP switch, for no apparent reason. This commit factors those
GPIO pins into two descriptors: one for the external GPIO connector
and one for the internal DIP switch.

As an added bonus, this commit also adds comments that says what those
GPIOs are used for.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/kirkwood-openblocks_a6.dts |   74 +++++---------------------
 1 file changed, 14 insertions(+), 60 deletions(-)

diff --git a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
index 087681d..0488d6a 100644
--- a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
+++ b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
@@ -85,12 +85,7 @@
 		};
 
 		pinctrl: pinctrl at 10000 {
-			pinctrl-0 = <&pmx_dip_sw0 &pmx_dip_sw1
-				&pmx_dip_sw2 &pmx_dip_sw3
-				&pmx_gpio_0 &pmx_gpio_1
-				&pmx_gpio_2 &pmx_gpio_3
-				&pmx_gpio_4 &pmx_gpio_5
-				&pmx_gpio_6 &pmx_gpio_7>;
+			pinctrl-0 = <&pmx_dip_sw &pmx_ext_gpios>;
 			pinctrl-names = "default";
 
 			pmx_uart0: pmx-uart0 {
@@ -110,63 +105,22 @@
 				marvell,function = "sysrst";
 			};
 
-			pmx_dip_sw0: pmx-dip-sw0 {
-				marvell,pins = "mpp20";
+			/*
+			 * Four input GPIOs connected to the 4-way
+			 * DIP-switch inside the device.
+			 */
+			pmx_dip_sw: pmx-dip-sw {
+				marvell,pins = "mpp20", "mpp21", "mpp22", "mpp23";
 				marvell,function = "gpio";
 			};
 
-			pmx_dip_sw1: pmx-dip-sw1 {
-				marvell,pins = "mpp21";
-				marvell,function = "gpio";
-			};
-
-			pmx_dip_sw2: pmx-dip-sw2 {
-				marvell,pins = "mpp22";
-				marvell,function = "gpio";
-			};
-
-			pmx_dip_sw3: pmx-dip-sw3 {
-				marvell,pins = "mpp23";
-				marvell,function = "gpio";
-			};
-
-			pmx_gpio_0: pmx-gpio-0 {
-				marvell,pins = "mpp24";
-				marvell,function = "gpio";
-			};
-
-			pmx_gpio_1: pmx-gpio-1 {
-				marvell,pins = "mpp25";
-				marvell,function = "gpio";
-			};
-
-			pmx_gpio_2: pmx-gpio-2 {
-				marvell,pins = "mpp26";
-				marvell,function = "gpio";
-			};
-
-			pmx_gpio_3: pmx-gpio-3 {
-				marvell,pins = "mpp27";
-				marvell,function = "gpio";
-			};
-
-			pmx_gpio_4: pmx-gpio-4 {
-				marvell,pins = "mpp28";
-				marvell,function = "gpio";
-			};
-
-			pmx_gpio_5: pmx-gpio-5 {
-				marvell,pins = "mpp29";
-				marvell,function = "gpio";
-			};
-
-			pmx_gpio_6: pmx-gpio-6 {
-				marvell,pins = "mpp30";
-				marvell,function = "gpio";
-			};
-
-			pmx_gpio_7: pmx-gpio-7 {
-				marvell,pins = "mpp31";
+			/*
+			 * 8 GPIOs available through the Hirose 16
+			 * pins header at the back of the device.
+			 */
+			pmx_ext_gpios: pmx-ext-gpios {
+				marvell,pins = "mpp24", "mpp25", "mpp26", "mpp27",
+					"mpp28", "mpp29", "mpp30", "mpp31";
 				marvell,function = "gpio";
 			};
 
-- 
1.7.9.5

[PATCH 2/3] arm: kirkwood: factor pinmux descriptors for OpenBlocks A6

[Andrew Lunn]

On Wed, Mar 06, 2013 at 05:23:40PM +0100, Thomas Petazzoni wrote:
> The OpenBlocks A6 .dts file was using a long list of pinmux
> descriptors to select each GPIO of the external GPIO connector and the
> internal DIP switch, for no apparent reason. This commit factors those
> GPIO pins into two descriptors: one for the external GPIO connector
> and one for the internal DIP switch.

Hi Thomas

There is no need to pinmux gpio pins at all. The pinctrl driver does
it when the gpio driver requests the pins.

This stems from an error i made. I also didn't know this and added
hogs for gpio pins as i converted some boards. Others have then just
cut/paste my error.....

	  Andrew

> 
> As an added bonus, this commit also adds comments that says what those
> GPIOs are used for.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/boot/dts/kirkwood-openblocks_a6.dts |   74 +++++---------------------
>  1 file changed, 14 insertions(+), 60 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
> index 087681d..0488d6a 100644
> --- a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
> +++ b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
> @@ -85,12 +85,7 @@
>  		};
>  
>  		pinctrl: pinctrl at 10000 {
> -			pinctrl-0 = <&pmx_dip_sw0 &pmx_dip_sw1
> -				&pmx_dip_sw2 &pmx_dip_sw3
> -				&pmx_gpio_0 &pmx_gpio_1
> -				&pmx_gpio_2 &pmx_gpio_3
> -				&pmx_gpio_4 &pmx_gpio_5
> -				&pmx_gpio_6 &pmx_gpio_7>;
> +			pinctrl-0 = <&pmx_dip_sw &pmx_ext_gpios>;
>  			pinctrl-names = "default";
>  
>  			pmx_uart0: pmx-uart0 {
> @@ -110,63 +105,22 @@
>  				marvell,function = "sysrst";
>  			};
>  
> -			pmx_dip_sw0: pmx-dip-sw0 {
> -				marvell,pins = "mpp20";
> +			/*
> +			 * Four input GPIOs connected to the 4-way
> +			 * DIP-switch inside the device.
> +			 */
> +			pmx_dip_sw: pmx-dip-sw {
> +				marvell,pins = "mpp20", "mpp21", "mpp22", "mpp23";
>  				marvell,function = "gpio";
>  			};
>  
> -			pmx_dip_sw1: pmx-dip-sw1 {
> -				marvell,pins = "mpp21";
> -				marvell,function = "gpio";
> -			};
> -
> -			pmx_dip_sw2: pmx-dip-sw2 {
> -				marvell,pins = "mpp22";
> -				marvell,function = "gpio";
> -			};
> -
> -			pmx_dip_sw3: pmx-dip-sw3 {
> -				marvell,pins = "mpp23";
> -				marvell,function = "gpio";
> -			};
> -
> -			pmx_gpio_0: pmx-gpio-0 {
> -				marvell,pins = "mpp24";
> -				marvell,function = "gpio";
> -			};
> -
> -			pmx_gpio_1: pmx-gpio-1 {
> -				marvell,pins = "mpp25";
> -				marvell,function = "gpio";
> -			};
> -
> -			pmx_gpio_2: pmx-gpio-2 {
> -				marvell,pins = "mpp26";
> -				marvell,function = "gpio";
> -			};
> -
> -			pmx_gpio_3: pmx-gpio-3 {
> -				marvell,pins = "mpp27";
> -				marvell,function = "gpio";
> -			};
> -
> -			pmx_gpio_4: pmx-gpio-4 {
> -				marvell,pins = "mpp28";
> -				marvell,function = "gpio";
> -			};
> -
> -			pmx_gpio_5: pmx-gpio-5 {
> -				marvell,pins = "mpp29";
> -				marvell,function = "gpio";
> -			};
> -
> -			pmx_gpio_6: pmx-gpio-6 {
> -				marvell,pins = "mpp30";
> -				marvell,function = "gpio";
> -			};
> -
> -			pmx_gpio_7: pmx-gpio-7 {
> -				marvell,pins = "mpp31";
> +			/*
> +			 * 8 GPIOs available through the Hirose 16
> +			 * pins header at the back of the device.
> +			 */
> +			pmx_ext_gpios: pmx-ext-gpios {
> +				marvell,pins = "mpp24", "mpp25", "mpp26", "mpp27",
> +					"mpp28", "mpp29", "mpp30", "mpp31";
>  				marvell,function = "gpio";
>  			};
>  
> -- 
> 1.7.9.5
> 

[PATCH 2/3] arm: kirkwood: factor pinmux descriptors for OpenBlocks A6

[Thomas Petazzoni]

Dear Andrew Lunn,

On Wed, 6 Mar 2013 19:16:48 +0100, Andrew Lunn wrote:

> There is no need to pinmux gpio pins at all. The pinctrl driver does
> it when the gpio driver requests the pins.

Right, that's true. I knew about this, but since the pinmux
configuration was made explicit for those GPIOs, I thought the original
author(s) of this .dts did this intentionally. For example, it somewhat
documents the 8 GPIOs that are available in the bottom connector of the
OpenBlocks A6. They are not tied to anyway particular device, so they
wouldn't appear anywhere in the DT. Ditto for the DIP switch GPIOs. Do
you also want those to be removed from the DT?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 2/3] arm: kirkwood: factor pinmux descriptors for OpenBlocks A6

[Andrew Lunn]

On Wed, Mar 06, 2013 at 08:16:46PM +0100, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Wed, 6 Mar 2013 19:16:48 +0100, Andrew Lunn wrote:
> 
> > There is no need to pinmux gpio pins at all. The pinctrl driver does
> > it when the gpio driver requests the pins.
> 
> Right, that's true. I knew about this, but since the pinmux
> configuration was made explicit for those GPIOs, I thought the original
> author(s) of this .dts did this intentionally. For example, it somewhat
> documents the 8 GPIOs that are available in the bottom connector of the
> OpenBlocks A6. They are not tied to anyway particular device, so they
> wouldn't appear anywhere in the DT. Ditto for the DIP switch GPIOs. Do
> you also want those to be removed from the DT?

Hi Thomas

Good point. They are not required, but don't harm. So yes, leave them
there for documentation purposes.

      Andrew

[PATCH 2/3] arm: kirkwood: factor pinmux descriptors for OpenBlocks A6

[Thomas Petazzoni]

Dear Andrew Lunn,

On Thu, 7 Mar 2013 07:22:55 +0100, Andrew Lunn wrote:

> > Right, that's true. I knew about this, but since the pinmux
> > configuration was made explicit for those GPIOs, I thought the original
> > author(s) of this .dts did this intentionally. For example, it somewhat
> > documents the 8 GPIOs that are available in the bottom connector of the
> > OpenBlocks A6. They are not tied to anyway particular device, so they
> > wouldn't appear anywhere in the DT. Ditto for the DIP switch GPIOs. Do
> > you also want those to be removed from the DT?
> 
> Hi Thomas
> 
> Good point. They are not required, but don't harm. So yes, leave them
> there for documentation purposes.

So I keep the GPIO muxing for the external GPIOs and the DIP switch
GPIOs, but the one that are "claimed" by another device (like gpio-leds
or gpio-keys), I should remove the muxing? I'm fine with that, just
want to confirm the choice.

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 2/3] arm: kirkwood: factor pinmux descriptors for OpenBlocks A6

[Andrew Lunn]

> So I keep the GPIO muxing for the external GPIOs and the DIP switch
> GPIOs, but the one that are "claimed" by another device (like gpio-leds
> or gpio-keys), I should remove the muxing? I'm fine with that, just
> want to confirm the choice.

Hi Thomas,

Yes, i'm happy with that.

     Andrew

[PATCH 2/3] arm: kirkwood: factor pinmux descriptors for OpenBlocks A6

[Thomas Petazzoni]

Dear Andrew Lunn,

On Wed, 6 Mar 2013 19:16:48 +0100, Andrew Lunn wrote:
> On Wed, Mar 06, 2013 at 05:23:40PM +0100, Thomas Petazzoni wrote:
> > The OpenBlocks A6 .dts file was using a long list of pinmux
> > descriptors to select each GPIO of the external GPIO connector and the
> > internal DIP switch, for no apparent reason. This commit factors those
> > GPIO pins into two descriptors: one for the external GPIO connector
> > and one for the internal DIP switch.
> 
> Hi Thomas
> 
> There is no need to pinmux gpio pins at all. The pinctrl driver does
> it when the gpio driver requests the pins.
> 
> This stems from an error i made. I also didn't know this and added
> hogs for gpio pins as i converted some boards. Others have then just
> cut/paste my error.....

I was looking at this today to send an updated version of those
patches. However, it turns out that having the pinctrl-0 property to
associate pinmux configurations to a device has an advantage: pinctrl
knows about this association.

If you keep the pinctrl-0 property in gpio-leds and gpio-keys,
then /sys/kernel/debug/pinctrl/f1010000.pinctrl/pinmux-pins looks like
this:

======================================================================
pin 38 (PIN38): gpio_keys.2 mvebu-gpio:38 function gpio group mpp38
pin 39 (PIN39): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 40 (PIN40): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 41 (PIN41): gpio-leds.1 mvebu-gpio:41 function gpio group mpp41
pin 42 (PIN42): gpio-leds.1 mvebu-gpio:42 function gpio group mpp42
pin 43 (PIN43): gpio-leds.1 mvebu-gpio:43 function gpio group mpp43
======================================================================

On the other hand, if you remove the pinctrl-0 property, the same file
looks like this:

======================================================================
pin 38 (PIN38): (MUX UNCLAIMED) mvebu-gpio:38
pin 39 (PIN39): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 40 (PIN40): (MUX UNCLAIMED) (GPIO UNCLAIMED)
pin 41 (PIN41): (MUX UNCLAIMED) mvebu-gpio:41
pin 42 (PIN42): (MUX UNCLAIMED) mvebu-gpio:42
pin 43 (PIN43): (MUX UNCLAIMED) mvebu-gpio:43
======================================================================

So you no longer know by what device the pin is claimed. It is not
horrible of course, but I find it quite nice to be able to see which
pin is used by what device.

What do you think?

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 2/3] arm: kirkwood: factor pinmux descriptors for OpenBlocks A6

[Andrew Lunn]

On Fri, Mar 22, 2013 at 04:44:40PM +0100, Thomas Petazzoni wrote:
> Dear Andrew Lunn,
> 
> On Wed, 6 Mar 2013 19:16:48 +0100, Andrew Lunn wrote:
> > On Wed, Mar 06, 2013 at 05:23:40PM +0100, Thomas Petazzoni wrote:
> > > The OpenBlocks A6 .dts file was using a long list of pinmux
> > > descriptors to select each GPIO of the external GPIO connector and the
> > > internal DIP switch, for no apparent reason. This commit factors those
> > > GPIO pins into two descriptors: one for the external GPIO connector
> > > and one for the internal DIP switch.
> > 
> > Hi Thomas
> > 
> > There is no need to pinmux gpio pins at all. The pinctrl driver does
> > it when the gpio driver requests the pins.
> > 
> > This stems from an error i made. I also didn't know this and added
> > hogs for gpio pins as i converted some boards. Others have then just
> > cut/paste my error.....
> 
> I was looking at this today to send an updated version of those
> patches. However, it turns out that having the pinctrl-0 property to
> associate pinmux configurations to a device has an advantage: pinctrl
> knows about this association.
> 
> If you keep the pinctrl-0 property in gpio-leds and gpio-keys,
> then /sys/kernel/debug/pinctrl/f1010000.pinctrl/pinmux-pins looks like
> this:
> 
> ======================================================================
> pin 38 (PIN38): gpio_keys.2 mvebu-gpio:38 function gpio group mpp38
> pin 39 (PIN39): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 40 (PIN40): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 41 (PIN41): gpio-leds.1 mvebu-gpio:41 function gpio group mpp41
> pin 42 (PIN42): gpio-leds.1 mvebu-gpio:42 function gpio group mpp42
> pin 43 (PIN43): gpio-leds.1 mvebu-gpio:43 function gpio group mpp43
> ======================================================================
> 
> On the other hand, if you remove the pinctrl-0 property, the same file
> looks like this:
> 
> ======================================================================
> pin 38 (PIN38): (MUX UNCLAIMED) mvebu-gpio:38
> pin 39 (PIN39): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 40 (PIN40): (MUX UNCLAIMED) (GPIO UNCLAIMED)
> pin 41 (PIN41): (MUX UNCLAIMED) mvebu-gpio:41
> pin 42 (PIN42): (MUX UNCLAIMED) mvebu-gpio:42
> pin 43 (PIN43): (MUX UNCLAIMED) mvebu-gpio:43
> ======================================================================
> 
> So you no longer know by what device the pin is claimed. It is not
> horrible of course, but I find it quite nice to be able to see which
> pin is used by what device.
> 
> What do you think?

Hi Thomas

I think the first is more informative. Please use that.

  Thanks
	Andrew

[PATCH 3/3] arm: kirkwood: add support for Init button on OpenBlocks A6

[Thomas Petazzoni]

The Kirkwood-based PlatHome OpenBlocks A6 board has an Init button
connected to MPP pin 38. This commit adds support for this button.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/kirkwood-openblocks_a6.dts |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
index 0488d6a..7ac831e 100644
--- a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
+++ b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
@@ -172,4 +172,18 @@
 			gpios = <&gpio1 11 1>;
 		};
         };
+
+	gpio_keys {
+		compatible = "gpio-keys";
+		pinctrl-0 = <&pmx_gpio_init>;
+		pinctrl-names = "default";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		button at 1 {
+			label = "Init Button";
+			linux,code = <116>;
+			gpios = <&gpio1 6 0>;
+		};
+	};
 };
-- 
1.7.9.5

[PATCH 3/3] arm: kirkwood: add support for Init button on OpenBlocks A6

[Andrew Lunn]

On Wed, Mar 06, 2013 at 05:23:41PM +0100, Thomas Petazzoni wrote:
> The Kirkwood-based PlatHome OpenBlocks A6 board has an Init button
> connected to MPP pin 38. This commit adds support for this button.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/boot/dts/kirkwood-openblocks_a6.dts |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
> index 0488d6a..7ac831e 100644
> --- a/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
> +++ b/arch/arm/boot/dts/kirkwood-openblocks_a6.dts
> @@ -172,4 +172,18 @@
>  			gpios = <&gpio1 11 1>;
>  		};
>          };
> +
> +	gpio_keys {
> +		compatible = "gpio-keys";
> +		pinctrl-0 = <&pmx_gpio_init>;
> +		pinctrl-names = "default";

Hi Thomas

I know i'm repeating myself.....

pinctrl* are not needed.

    Andrew

> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		button at 1 {
> +			label = "Init Button";
> +			linux,code = <116>;
> +			gpios = <&gpio1 6 0>;
> +		};
> +	};
>  };
> -- 
> 1.7.9.5
>