[PATCH] ARM: dts: vf610: make pinctrl nodes board specific

[PATCH] ARM: dts: vf610: make pinctrl nodes board specific

[Shawn Guo]

Currently, all pinctrl setting nodes are defined in <soc>.dtsi, so that
boards that share the same pinctrl setting do not have to define it time
and time again in <board>.dts.  However, along with the devices and use
cases being added continuously, the pinctrl setting nodes under iomuxc
becomes more than expected.  This bloats device tree blob for particular
board unnecessarily since only a small subset of those pinctrl setting
nodes will be used by the board.  It impacts not only the DTB file size
but also the run-time device tree lookup efficiency.

The patch provides a solution to avoid this device tree bloating problem
while still keeping boards share the common pinctrl setting data by
using DTC macro support.  It creates <soc>-pingrp.h and move all those
pinctrl setting data into there as macro definitions.  The <board>.dts
will instead define the pinctrl setting nodes that are necessary for the
board by referring to the macros in <soc>-pingrp.h, so that only the
pinctrl setting data that will be used by the board will get compiled
into the DTB for the board.

With the changes, the pinctrl setting nodes becomes local to particular
board, and it makes no sense to continue numbering the setting for
given peripheral.  Thus, all the pinctrl phandler name gets updated to
have only peripheral name in there.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
---
This should have been done in the patch series [1], but somehow got
missed in the first place.  So add one more patch to take care of VF610.

[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/277649

 arch/arm/boot/dts/vf610-cosmic.dts |   16 +++-
 arch/arm/boot/dts/vf610-twr.dts    |   34 +++++--
 arch/arm/boot/dts/vf610.dtsi       |  172 +-----------------------------------
 3 files changed, 44 insertions(+), 178 deletions(-)

diff --git a/arch/arm/boot/dts/vf610-cosmic.dts b/arch/arm/boot/dts/vf610-cosmic.dts
index c42e4f9..432960a 100644
--- a/arch/arm/boot/dts/vf610-cosmic.dts
+++ b/arch/arm/boot/dts/vf610-cosmic.dts
@@ -36,12 +36,24 @@
 &fec1 {
 	phy-mode = "rmii";
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_fec1_1>;
+	pinctrl-0 = <&pinctrl_fec1>;
 	status = "okay";
 };
 
+&iomuxc {
+	vf610-cosmic {
+		pinctrl_fec1: fec1grp {
+			fsl,pins = <VF610_FEC1_PINGRP1>;
+		};
+
+		pinctrl_uart1: uart1grp {
+			fsl,pins = <VF610_UART1_PINGRP1>;
+		};
+	};
+};
+
 &uart1 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_uart1_1>;
+	pinctrl-0 = <&pinctrl_uart1>;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index c8047ca..80db14e 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -39,7 +39,7 @@
 &dspi0 {
 	bus-num = <0>;
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_dspi0_1>;
+	pinctrl-0 = <&pinctrl_dspi0>;
 	status = "okay";
 
 	sflash: at26df081a at 0 {
@@ -56,26 +56,50 @@
 &fec0 {
 	phy-mode = "rmii";
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_fec0_1>;
+	pinctrl-0 = <&pinctrl_fec0>;
 	status = "okay";
 };
 
 &fec1 {
 	phy-mode = "rmii";
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_fec1_1>;
+	pinctrl-0 = <&pinctrl_fec1>;
 	status = "okay";
 };
 
 &i2c0 {
 	clock-frequency = <100000>;
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_i2c0_1>;
+	pinctrl-0 = <&pinctrl_i2c0>;
 	status = "okay";
 };
 
+&iomuxc {
+	vf610-twr {
+		pinctrl_dspi0: dspi0grp {
+			fsl,pins = <VF610_DSPI0_PINGRP1>;
+		};
+
+		pinctrl_fec0: fec0grp {
+			fsl,pins = <VF610_FEC0_PINGRP1>;
+		};
+
+		pinctrl_fec1: fec1grp {
+			fsl,pins = <VF610_FEC1_PINGRP1>;
+		};
+
+		pinctrl_i2c0: i2c0grp {
+			fsl,pins = <VF610_I2C0_PINGRP1>;
+		};
+
+		pinctrl_uart1: uart1grp {
+			fsl,pins = <VF610_UART1_PINGRP1>;
+		};
+	};
+};
+
 &uart1 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_uart1_1>;
+	pinctrl-0 = <&pinctrl_uart1>;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index d31ce1b..ef8a0ee 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -8,7 +8,7 @@
  */
 
 #include "skeleton.dtsi"
-#include "vf610-pinfunc.h"
+#include "vf610-pingrp.h"
 #include <dt-bindings/clock/vf610-clock.h>
 
 / {
@@ -175,176 +175,6 @@
 				compatible = "fsl,vf610-iomuxc";
 				reg = <0x40048000 0x1000>;
 				#gpio-range-cells = <3>;
-
-				/* functions and groups pins */
-
-				dcu0 {
-					pinctrl_dcu0_1: dcu0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTB8__GPIO_30		0x42
-						VF610_PAD_PTE0__DCU0_HSYNC	0x42
-						VF610_PAD_PTE1__DCU0_VSYNC	0x42
-						VF610_PAD_PTE2__DCU0_PCLK	0x42
-						VF610_PAD_PTE4__DCU0_DE		0x42
-						VF610_PAD_PTE5__DCU0_R0		0x42
-						VF610_PAD_PTE6__DCU0_R1		0x42
-						VF610_PAD_PTE7__DCU0_R2		0x42
-						VF610_PAD_PTE8__DCU0_R3		0x42
-						VF610_PAD_PTE9__DCU0_R4		0x42
-						VF610_PAD_PTE10__DCU0_R5	0x42
-						VF610_PAD_PTE11__DCU0_R6	0x42
-						VF610_PAD_PTE12__DCU0_R7	0x42
-						VF610_PAD_PTE13__DCU0_G0	0x42
-						VF610_PAD_PTE14__DCU0_G1	0x42
-						VF610_PAD_PTE15__DCU0_G2	0x42
-						VF610_PAD_PTE16__DCU0_G3	0x42
-						VF610_PAD_PTE17__DCU0_G4	0x42
-						VF610_PAD_PTE18__DCU0_G5	0x42
-						VF610_PAD_PTE19__DCU0_G6	0x42
-						VF610_PAD_PTE20__DCU0_G7	0x42
-						VF610_PAD_PTE21__DCU0_B0	0x42
-						VF610_PAD_PTE22__DCU0_B1	0x42
-						VF610_PAD_PTE23__DCU0_B2	0x42
-						VF610_PAD_PTE24__DCU0_B3	0x42
-						VF610_PAD_PTE25__DCU0_B4	0x42
-						VF610_PAD_PTE26__DCU0_B5	0x42
-						VF610_PAD_PTE27__DCU0_B6	0x42
-						VF610_PAD_PTE28__DCU0_B7	0x42
-						>;
-					};
-				};
-
-				dspi0 {
-					pinctrl_dspi0_1: dspi0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTB19__DSPI0_CS0	0x1182
-						VF610_PAD_PTB20__DSPI0_SIN	0x1181
-						VF610_PAD_PTB21__DSPI0_SOUT	0x1182
-						VF610_PAD_PTB22__DSPI0_SCK	0x1182
-						>;
-					};
-				};
-
-				esdhc1 {
-					pinctrl_esdhc1_1: esdhc1grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTA24__ESDHC1_CLK	0x31ef
-						VF610_PAD_PTA25__ESDHC1_CMD	0x31ef
-						VF610_PAD_PTA26__ESDHC1_DAT0	0x31ef
-						VF610_PAD_PTA27__ESDHC1_DAT1	0x31ef
-						VF610_PAD_PTA28__ESDHC1_DATA2	0x31ef
-						VF610_PAD_PTA29__ESDHC1_DAT3	0x31ef
-						VF610_PAD_PTA7__GPIO_134	0x219d
-						>;
-					};
-				};
-
-				fec0 {
-					pinctrl_fec0_1: fec0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTA6__RMII_CLKIN	0x30d1
-						VF610_PAD_PTC0__ENET_RMII0_MDC	0x30d3
-						VF610_PAD_PTC1__ENET_RMII0_MDIO	0x30d1
-						VF610_PAD_PTC2__ENET_RMII0_CRS	0x30d1
-						VF610_PAD_PTC3__ENET_RMII0_RXD1	0x30d1
-						VF610_PAD_PTC4__ENET_RMII0_RXD0	0x30d1
-						VF610_PAD_PTC5__ENET_RMII0_RXER	0x30d1
-						VF610_PAD_PTC6__ENET_RMII0_TXD1	0x30d2
-						VF610_PAD_PTC7__ENET_RMII0_TXD0	0x30d2
-						VF610_PAD_PTC8__ENET_RMII0_TXEN	0x30d2
-						>;
-					};
-				};
-
-				fec1 {
-					pinctrl_fec1_1: fec1grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTC9__ENET_RMII1_MDC		0x30d2
-						VF610_PAD_PTC10__ENET_RMII1_MDIO	0x30d3
-						VF610_PAD_PTC11__ENET_RMII1_CRS		0x30d1
-						VF610_PAD_PTC12__ENET_RMII_RXD1		0x30d1
-						VF610_PAD_PTC13__ENET_RMII1_RXD0	0x30d1
-						VF610_PAD_PTC14__ENET_RMII1_RXER	0x30d1
-						VF610_PAD_PTC15__ENET_RMII1_TXD1	0x30d2
-						VF610_PAD_PTC16__ENET_RMII1_TXD0	0x30d2
-						VF610_PAD_PTC17__ENET_RMII1_TXEN	0x30d2
-						>;
-					};
-				};
-
-				i2c0 {
-					pinctrl_i2c0_1: i2c0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTB14__I2C0_SCL	0x30d3
-						VF610_PAD_PTB15__I2C0_SDA	0x30d3
-						>;
-					};
-				};
-
-				pwm0 {
-					pinctrl_pwm0_1: pwm0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTB0__FTM0_CH0	0x1582
-						VF610_PAD_PTB1__FTM0_CH1	0x1582
-						VF610_PAD_PTB2__FTM0_CH2	0x1582
-						VF610_PAD_PTB3__FTM0_CH3	0x1582
-						VF610_PAD_PTB6__FTM0_CH6	0x1582
-						VF610_PAD_PTB7__FTM0_CH7	0x1582
-						>;
-					};
-				};
-
-				qspi0 {
-					pinctrl_qspi0_1: qspi0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTD0__QSPI0_A_QSCK	0x307b
-						VF610_PAD_PTD1__QSPI0_A_CS0	0x307f
-						VF610_PAD_PTD2__QSPI0_A_DATA3	0x3073
-						VF610_PAD_PTD3__QSPI0_A_DATA2	0x3073
-						VF610_PAD_PTD4__QSPI0_A_DATA1	0x3073
-						VF610_PAD_PTD5__QSPI0_A_DATA0	0x307b
-						VF610_PAD_PTD7__QSPI0_B_QSCK	0x307b
-						VF610_PAD_PTD8__QSPI0_B_CS0	0x307f
-						VF610_PAD_PTD9__QSPI0_B_DATA3	0x3073
-						VF610_PAD_PTD10__QSPI0_B_DATA2	0x3073
-						VF610_PAD_PTD11__QSPI0_B_DATA1	0x3073
-						VF610_PAD_PTD12__QSPI0_B_DATA0	0x307b
-						>;
-					};
-				};
-
-				sai2 {
-					pinctrl_sai2_1: sai2grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTA16__SAI2_TX_BCLK	0x02ed
-						VF610_PAD_PTA18__SAI2_TX_DATA	0x02ee
-						VF610_PAD_PTA19__SAI2_TX_SYNC	0x02ed
-						VF610_PAD_PTA21__SAI2_RX_BCLK	0x02ed
-						VF610_PAD_PTA22__SAI2_RX_DATA	0x02ed
-						VF610_PAD_PTA23__SAI2_RX_SYNC	0x02ed
-						VF610_PAD_PTB18__EXT_AUDIO_MCLK	0x02ed
-						>;
-					};
-				};
-
-				uart1 {
-					pinctrl_uart1_1: uart1grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTB4__UART1_TX	0x21a2
-						VF610_PAD_PTB5__UART1_RX	0x21a1
-						>;
-					};
-				};
-
-				usbvbus {
-					pinctrl_usbvbus_1: usbvbusgrp_1 {
-						fsl,pins = <
-						VF610_PAD_PTA24__USB1_VBUS_EN	0x219c
-						VF610_PAD_PTA16__USB0_VBUS_EN	0x219c
-						>;
-					};
-				};
-
 			};
 
 			gpio1: gpio at 40049000 {
-- 
1.7.9.5

[PATCH] ARM: dts: vf610: make pinctrl nodes board specific

[fugang.duan at freescale.com]

From: Shawn Guo <shawn.guo@linaro.org>
Data: Monday, December 09, 2013 2:51 PM + 800

>To: linux-arm-kernel at lists.infradead.org
>Cc: Lu Jingchang-B35083; Duan Fugang-B38611; Shawn Guo
>Subject: [PATCH] ARM: dts: vf610: make pinctrl nodes board specific
>
>Currently, all pinctrl setting nodes are defined in <soc>.dtsi, so that boards
>that share the same pinctrl setting do not have to define it time and time
>again in <board>.dts.  However, along with the devices and use cases being
>added continuously, the pinctrl setting nodes under iomuxc becomes more than
>expected.  This bloats device tree blob for particular board unnecessarily
>since only a small subset of those pinctrl setting nodes will be used by the
>board.  It impacts not only the DTB file size but also the run-time device tree
>lookup efficiency.
>
>The patch provides a solution to avoid this device tree bloating problem while
>still keeping boards share the common pinctrl setting data by using DTC macro
>support.  It creates <soc>-pingrp.h and move all those pinctrl setting data
>into there as macro definitions.  The <board>.dts will instead define the
>pinctrl setting nodes that are necessary for the board by referring to the
>macros in <soc>-pingrp.h, so that only the pinctrl setting data that will be
>used by the board will get compiled into the DTB for the board.
>
>With the changes, the pinctrl setting nodes becomes local to particular board,
>and it makes no sense to continue numbering the setting for given peripheral.
>Thus, all the pinctrl phandler name gets updated to have only peripheral name
>in there.
>
>Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>---
>This should have been done in the patch series [1], but somehow got missed in
>the first place.  So add one more patch to take care of VF610.
>
>[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/277649
>
> arch/arm/boot/dts/vf610-cosmic.dts |   16 +++-
> arch/arm/boot/dts/vf610-twr.dts    |   34 +++++--
> arch/arm/boot/dts/vf610.dtsi       |  172 +-----------------------------------
> 3 files changed, 44 insertions(+), 178 deletions(-)
>
>diff --git a/arch/arm/boot/dts/vf610-cosmic.dts b/arch/arm/boot/dts/vf610-
>cosmic.dts
>index c42e4f9..432960a 100644
>--- a/arch/arm/boot/dts/vf610-cosmic.dts
>+++ b/arch/arm/boot/dts/vf610-cosmic.dts
>@@ -36,12 +36,24 @@
> &fec1 {
> 	phy-mode = "rmii";
> 	pinctrl-names = "default";
>-	pinctrl-0 = <&pinctrl_fec1_1>;
>+	pinctrl-0 = <&pinctrl_fec1>;
> 	status = "okay";
> };
>
>+&iomuxc {
>+	vf610-cosmic {
>+		pinctrl_fec1: fec1grp {
>+			fsl,pins = <VF610_FEC1_PINGRP1>;
>+		};
>+
>+		pinctrl_uart1: uart1grp {
>+			fsl,pins = <VF610_UART1_PINGRP1>;
>+		};
>+	};
>+};
>+
Keep things in alphabetical order...

> &uart1 {
> 	pinctrl-names = "default";
>-	pinctrl-0 = <&pinctrl_uart1_1>;
>+	pinctrl-0 = <&pinctrl_uart1>;
> 	status = "okay";
> };

Other is fine.

[PATCH] ARM: dts: vf610: make pinctrl nodes board specific

[Shawn Guo]

On Mon, Dec 09, 2013 at 07:10:34AM +0000, fugang.duan at freescale.com wrote:
> >@@ -36,12 +36,24 @@
> > &fec1 {
> > 	phy-mode = "rmii";
> > 	pinctrl-names = "default";
> >-	pinctrl-0 = <&pinctrl_fec1_1>;
> >+	pinctrl-0 = <&pinctrl_fec1>;
> > 	status = "okay";
> > };
> >
> >+&iomuxc {
> >+	vf610-cosmic {
> >+		pinctrl_fec1: fec1grp {
> >+			fsl,pins = <VF610_FEC1_PINGRP1>;
> >+		};
> >+
> >+		pinctrl_uart1: uart1grp {
> >+			fsl,pins = <VF610_UART1_PINGRP1>;
> >+		};
> >+	};
> >+};
> >+
> Keep things in alphabetical order...

Hmm, what is not in alphabetic order?

Shawn

> 
> > &uart1 {
> > 	pinctrl-names = "default";
> >-	pinctrl-0 = <&pinctrl_uart1_1>;
> >+	pinctrl-0 = <&pinctrl_uart1>;
> > 	status = "okay";
> > };

[PATCH] ARM: dts: vf610: make pinctrl nodes board specific

[fugang.duan at freescale.com]

From: Shawn Guo [mailto:shawn.guo at linaro.org]
Data: Monday, December 09, 2013 4:02 PM

>To: Duan Fugang-B38611
>Cc: linux-arm-kernel at lists.infradead.org; Lu Jingchang-B35083
>Subject: Re: [PATCH] ARM: dts: vf610: make pinctrl nodes board specific
>
>On Mon, Dec 09, 2013 at 07:10:34AM +0000, fugang.duan at freescale.com wrote:
>> >@@ -36,12 +36,24 @@
>> > &fec1 {
>> > 	phy-mode = "rmii";
>> > 	pinctrl-names = "default";
>> >-	pinctrl-0 = <&pinctrl_fec1_1>;
>> >+	pinctrl-0 = <&pinctrl_fec1>;
>> > 	status = "okay";
>> > };
>> >
>> >+&iomuxc {
>> >+	vf610-cosmic {
>> >+		pinctrl_fec1: fec1grp {
>> >+			fsl,pins = <VF610_FEC1_PINGRP1>;
>> >+		};
>> >+
>> >+		pinctrl_uart1: uart1grp {
>> >+			fsl,pins = <VF610_UART1_PINGRP1>;
>> >+		};
>> >+	};
>> >+};
>> >+
>> Keep things in alphabetical order...
>
>Hmm, what is not in alphabetic order?
>
>Shawn

Hehe, my eyes deceive me.
>
>>
>> > &uart1 {
>> > 	pinctrl-names = "default";
>> >-	pinctrl-0 = <&pinctrl_uart1_1>;
>> >+	pinctrl-0 = <&pinctrl_uart1>;
>> > 	status = "okay";
>> > };

Acked-by: Fugang Duan <B38611@freescale.com>

[PATCH v2] ARM: dts: vf610: make pinctrl nodes board specific

[Shawn Guo]

Currently, all pinctrl setting nodes are defined in <soc>.dtsi, so that
boards that share the same pinctrl setting do not have to define it time
and time again in <board>.dts.  However, along with the devices and use
cases being added continuously, the pinctrl setting nodes under iomuxc
becomes more than expected.  This bloats device tree blob for particular
board unnecessarily since only a small subset of those pinctrl setting
nodes will be used by the board.  It impacts not only the DTB file size
but also the run-time device tree lookup efficiency.

The patch provides a solution to avoid this device tree bloating problem
while still keeping boards share the common pinctrl setting data by
using DTC macro support.  It creates <soc>-pingrp.h and move all those
pinctrl setting data into there as macro definitions.  The <board>.dts
will instead define the pinctrl setting nodes that are necessary for the
board by referring to the macros in <soc>-pingrp.h, so that only the
pinctrl setting data that will be used by the board will get compiled
into the DTB for the board.

With the changes, the pinctrl setting nodes becomes local to particular
board, and it makes no sense to continue numbering the setting for
given peripheral.  Thus, all the pinctrl phandler name gets updated to
have only peripheral name in there.

Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
Acked-by: Fugang Duan <B38611@freescale.com>
---
Changes since v1:
 The vf610-pingrp.h was missed from the v1 patch, add it.

 arch/arm/boot/dts/vf610-cosmic.dts |   16 +++-
 arch/arm/boot/dts/vf610-pingrp.h   |  127 ++++++++++++++++++++++++++
 arch/arm/boot/dts/vf610-twr.dts    |   34 +++++--
 arch/arm/boot/dts/vf610.dtsi       |  172 +-----------------------------------
 4 files changed, 171 insertions(+), 178 deletions(-)
 create mode 100644 arch/arm/boot/dts/vf610-pingrp.h

diff --git a/arch/arm/boot/dts/vf610-cosmic.dts b/arch/arm/boot/dts/vf610-cosmic.dts
index c42e4f9..432960a 100644
--- a/arch/arm/boot/dts/vf610-cosmic.dts
+++ b/arch/arm/boot/dts/vf610-cosmic.dts
@@ -36,12 +36,24 @@
 &fec1 {
 	phy-mode = "rmii";
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_fec1_1>;
+	pinctrl-0 = <&pinctrl_fec1>;
 	status = "okay";
 };
 
+&iomuxc {
+	vf610-cosmic {
+		pinctrl_fec1: fec1grp {
+			fsl,pins = <VF610_FEC1_PINGRP1>;
+		};
+
+		pinctrl_uart1: uart1grp {
+			fsl,pins = <VF610_UART1_PINGRP1>;
+		};
+	};
+};
+
 &uart1 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_uart1_1>;
+	pinctrl-0 = <&pinctrl_uart1>;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/vf610-pingrp.h b/arch/arm/boot/dts/vf610-pingrp.h
new file mode 100644
index 0000000..0858f4f
--- /dev/null
+++ b/arch/arm/boot/dts/vf610-pingrp.h
@@ -0,0 +1,127 @@
+/*
+ * Copyright (C) 2013 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __DTS_VF610_PINGRP_H
+#define __DTS_VF610_PINGRP_H
+
+#include "vf610-pinfunc.h"
+
+#define VF610_DCU0_PINGRP1 \
+	VF610_PAD_PTB8__GPIO_30			0x42 \
+	VF610_PAD_PTE0__DCU0_HSYNC		0x42 \
+	VF610_PAD_PTE1__DCU0_VSYNC		0x42 \
+	VF610_PAD_PTE2__DCU0_PCLK		0x42 \
+	VF610_PAD_PTE4__DCU0_DE			0x42 \
+	VF610_PAD_PTE5__DCU0_R0			0x42 \
+	VF610_PAD_PTE6__DCU0_R1			0x42 \
+	VF610_PAD_PTE7__DCU0_R2			0x42 \
+	VF610_PAD_PTE8__DCU0_R3			0x42 \
+	VF610_PAD_PTE9__DCU0_R4			0x42 \
+	VF610_PAD_PTE10__DCU0_R5		0x42 \
+	VF610_PAD_PTE11__DCU0_R6		0x42 \
+	VF610_PAD_PTE12__DCU0_R7		0x42 \
+	VF610_PAD_PTE13__DCU0_G0		0x42 \
+	VF610_PAD_PTE14__DCU0_G1		0x42 \
+	VF610_PAD_PTE15__DCU0_G2		0x42 \
+	VF610_PAD_PTE16__DCU0_G3		0x42 \
+	VF610_PAD_PTE17__DCU0_G4		0x42 \
+	VF610_PAD_PTE18__DCU0_G5		0x42 \
+	VF610_PAD_PTE19__DCU0_G6		0x42 \
+	VF610_PAD_PTE20__DCU0_G7		0x42 \
+	VF610_PAD_PTE21__DCU0_B0		0x42 \
+	VF610_PAD_PTE22__DCU0_B1		0x42 \
+	VF610_PAD_PTE23__DCU0_B2		0x42 \
+	VF610_PAD_PTE24__DCU0_B3		0x42 \
+	VF610_PAD_PTE25__DCU0_B4		0x42 \
+	VF610_PAD_PTE26__DCU0_B5		0x42 \
+	VF610_PAD_PTE27__DCU0_B6		0x42 \
+	VF610_PAD_PTE28__DCU0_B7		0x42
+
+#define VF610_DSPI0_PINGRP1 \
+	VF610_PAD_PTB19__DSPI0_CS0		0x1182 \
+	VF610_PAD_PTB20__DSPI0_SIN		0x1181 \
+	VF610_PAD_PTB21__DSPI0_SOUT		0x1182 \
+	VF610_PAD_PTB22__DSPI0_SCK		0x1182
+
+#define VF610_ESDHC1_PINGRP1 \
+	VF610_PAD_PTA24__ESDHC1_CLK		0x31ef \
+	VF610_PAD_PTA25__ESDHC1_CMD		0x31ef \
+	VF610_PAD_PTA26__ESDHC1_DAT0		0x31ef \
+	VF610_PAD_PTA27__ESDHC1_DAT1		0x31ef \
+	VF610_PAD_PTA28__ESDHC1_DATA2		0x31ef \
+	VF610_PAD_PTA29__ESDHC1_DAT3		0x31ef \
+	VF610_PAD_PTA7__GPIO_134		0x219d
+
+#define VF610_FEC0_PINGRP1 \
+	VF610_PAD_PTA6__RMII_CLKIN		0x30d1 \
+	VF610_PAD_PTC0__ENET_RMII0_MDC		0x30d3 \
+	VF610_PAD_PTC1__ENET_RMII0_MDIO		0x30d1 \
+	VF610_PAD_PTC2__ENET_RMII0_CRS		0x30d1 \
+	VF610_PAD_PTC3__ENET_RMII0_RXD1		0x30d1 \
+	VF610_PAD_PTC4__ENET_RMII0_RXD0		0x30d1 \
+	VF610_PAD_PTC5__ENET_RMII0_RXER		0x30d1 \
+	VF610_PAD_PTC6__ENET_RMII0_TXD1		0x30d2 \
+	VF610_PAD_PTC7__ENET_RMII0_TXD0		0x30d2 \
+	VF610_PAD_PTC8__ENET_RMII0_TXEN		0x30d2
+
+#define VF610_FEC1_PINGRP1 \
+	VF610_PAD_PTC9__ENET_RMII1_MDC		0x30d2 \
+	VF610_PAD_PTC10__ENET_RMII1_MDIO	0x30d3 \
+	VF610_PAD_PTC11__ENET_RMII1_CRS		0x30d1 \
+	VF610_PAD_PTC12__ENET_RMII_RXD1		0x30d1 \
+	VF610_PAD_PTC13__ENET_RMII1_RXD0	0x30d1 \
+	VF610_PAD_PTC14__ENET_RMII1_RXER	0x30d1 \
+	VF610_PAD_PTC15__ENET_RMII1_TXD1	0x30d2 \
+	VF610_PAD_PTC16__ENET_RMII1_TXD0	0x30d2 \
+	VF610_PAD_PTC17__ENET_RMII1_TXEN	0x30d2 \
+
+#define VF610_I2C0_PINGRP1 \
+	VF610_PAD_PTB14__I2C0_SCL		0x30d3 \
+	VF610_PAD_PTB15__I2C0_SDA		0x30d3 \
+
+#define VF610_PWM0_PINGRP1 \
+	VF610_PAD_PTB0__FTM0_CH0		0x1582 \
+	VF610_PAD_PTB1__FTM0_CH1		0x1582 \
+	VF610_PAD_PTB2__FTM0_CH2		0x1582 \
+	VF610_PAD_PTB3__FTM0_CH3		0x1582 \
+	VF610_PAD_PTB6__FTM0_CH6		0x1582 \
+	VF610_PAD_PTB7__FTM0_CH7		0x1582
+
+#define VF610_QSPI0_PINGRP1 \
+	VF610_PAD_PTD0__QSPI0_A_QSCK		0x307b \
+	VF610_PAD_PTD1__QSPI0_A_CS0		0x307f \
+	VF610_PAD_PTD2__QSPI0_A_DATA3		0x3073 \
+	VF610_PAD_PTD3__QSPI0_A_DATA2		0x3073 \
+	VF610_PAD_PTD4__QSPI0_A_DATA1		0x3073 \
+	VF610_PAD_PTD5__QSPI0_A_DATA0		0x307b \
+	VF610_PAD_PTD7__QSPI0_B_QSCK		0x307b \
+	VF610_PAD_PTD8__QSPI0_B_CS0		0x307f \
+	VF610_PAD_PTD9__QSPI0_B_DATA3		0x3073 \
+	VF610_PAD_PTD10__QSPI0_B_DATA2		0x3073 \
+	VF610_PAD_PTD11__QSPI0_B_DATA1		0x3073 \
+	VF610_PAD_PTD12__QSPI0_B_DATA0		0x307b
+
+#define VF610_SAI2_PINGRP1 \
+	VF610_PAD_PTA16__SAI2_TX_BCLK		0x02ed \
+	VF610_PAD_PTA18__SAI2_TX_DATA		0x02ee \
+	VF610_PAD_PTA19__SAI2_TX_SYNC		0x02ed \
+	VF610_PAD_PTA21__SAI2_RX_BCLK		0x02ed \
+	VF610_PAD_PTA22__SAI2_RX_DATA		0x02ed \
+	VF610_PAD_PTA23__SAI2_RX_SYNC		0x02ed \
+	VF610_PAD_PTB18__EXT_AUDIO_MCLK		0x02ed
+
+#define VF610_UART1_PINGRP1 \
+	VF610_PAD_PTB4__UART1_TX		0x21a2 \
+	VF610_PAD_PTB5__UART1_RX		0x21a1
+
+#define VF610_USBVBUS_PINGRP1 \
+	VF610_PAD_PTA24__USB1_VBUS_EN		0x219c \
+	VF610_PAD_PTA16__USB0_VBUS_EN		0x219c
+
+#endif /* __DTS_VF610_PINGRP_H */
diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index c8047ca..80db14e 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -39,7 +39,7 @@
 &dspi0 {
 	bus-num = <0>;
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_dspi0_1>;
+	pinctrl-0 = <&pinctrl_dspi0>;
 	status = "okay";
 
 	sflash: at26df081a at 0 {
@@ -56,26 +56,50 @@
 &fec0 {
 	phy-mode = "rmii";
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_fec0_1>;
+	pinctrl-0 = <&pinctrl_fec0>;
 	status = "okay";
 };
 
 &fec1 {
 	phy-mode = "rmii";
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_fec1_1>;
+	pinctrl-0 = <&pinctrl_fec1>;
 	status = "okay";
 };
 
 &i2c0 {
 	clock-frequency = <100000>;
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_i2c0_1>;
+	pinctrl-0 = <&pinctrl_i2c0>;
 	status = "okay";
 };
 
+&iomuxc {
+	vf610-twr {
+		pinctrl_dspi0: dspi0grp {
+			fsl,pins = <VF610_DSPI0_PINGRP1>;
+		};
+
+		pinctrl_fec0: fec0grp {
+			fsl,pins = <VF610_FEC0_PINGRP1>;
+		};
+
+		pinctrl_fec1: fec1grp {
+			fsl,pins = <VF610_FEC1_PINGRP1>;
+		};
+
+		pinctrl_i2c0: i2c0grp {
+			fsl,pins = <VF610_I2C0_PINGRP1>;
+		};
+
+		pinctrl_uart1: uart1grp {
+			fsl,pins = <VF610_UART1_PINGRP1>;
+		};
+	};
+};
+
 &uart1 {
 	pinctrl-names = "default";
-	pinctrl-0 = <&pinctrl_uart1_1>;
+	pinctrl-0 = <&pinctrl_uart1>;
 	status = "okay";
 };
diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index d31ce1b..ef8a0ee 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -8,7 +8,7 @@
  */
 
 #include "skeleton.dtsi"
-#include "vf610-pinfunc.h"
+#include "vf610-pingrp.h"
 #include <dt-bindings/clock/vf610-clock.h>
 
 / {
@@ -175,176 +175,6 @@
 				compatible = "fsl,vf610-iomuxc";
 				reg = <0x40048000 0x1000>;
 				#gpio-range-cells = <3>;
-
-				/* functions and groups pins */
-
-				dcu0 {
-					pinctrl_dcu0_1: dcu0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTB8__GPIO_30		0x42
-						VF610_PAD_PTE0__DCU0_HSYNC	0x42
-						VF610_PAD_PTE1__DCU0_VSYNC	0x42
-						VF610_PAD_PTE2__DCU0_PCLK	0x42
-						VF610_PAD_PTE4__DCU0_DE		0x42
-						VF610_PAD_PTE5__DCU0_R0		0x42
-						VF610_PAD_PTE6__DCU0_R1		0x42
-						VF610_PAD_PTE7__DCU0_R2		0x42
-						VF610_PAD_PTE8__DCU0_R3		0x42
-						VF610_PAD_PTE9__DCU0_R4		0x42
-						VF610_PAD_PTE10__DCU0_R5	0x42
-						VF610_PAD_PTE11__DCU0_R6	0x42
-						VF610_PAD_PTE12__DCU0_R7	0x42
-						VF610_PAD_PTE13__DCU0_G0	0x42
-						VF610_PAD_PTE14__DCU0_G1	0x42
-						VF610_PAD_PTE15__DCU0_G2	0x42
-						VF610_PAD_PTE16__DCU0_G3	0x42
-						VF610_PAD_PTE17__DCU0_G4	0x42
-						VF610_PAD_PTE18__DCU0_G5	0x42
-						VF610_PAD_PTE19__DCU0_G6	0x42
-						VF610_PAD_PTE20__DCU0_G7	0x42
-						VF610_PAD_PTE21__DCU0_B0	0x42
-						VF610_PAD_PTE22__DCU0_B1	0x42
-						VF610_PAD_PTE23__DCU0_B2	0x42
-						VF610_PAD_PTE24__DCU0_B3	0x42
-						VF610_PAD_PTE25__DCU0_B4	0x42
-						VF610_PAD_PTE26__DCU0_B5	0x42
-						VF610_PAD_PTE27__DCU0_B6	0x42
-						VF610_PAD_PTE28__DCU0_B7	0x42
-						>;
-					};
-				};
-
-				dspi0 {
-					pinctrl_dspi0_1: dspi0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTB19__DSPI0_CS0	0x1182
-						VF610_PAD_PTB20__DSPI0_SIN	0x1181
-						VF610_PAD_PTB21__DSPI0_SOUT	0x1182
-						VF610_PAD_PTB22__DSPI0_SCK	0x1182
-						>;
-					};
-				};
-
-				esdhc1 {
-					pinctrl_esdhc1_1: esdhc1grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTA24__ESDHC1_CLK	0x31ef
-						VF610_PAD_PTA25__ESDHC1_CMD	0x31ef
-						VF610_PAD_PTA26__ESDHC1_DAT0	0x31ef
-						VF610_PAD_PTA27__ESDHC1_DAT1	0x31ef
-						VF610_PAD_PTA28__ESDHC1_DATA2	0x31ef
-						VF610_PAD_PTA29__ESDHC1_DAT3	0x31ef
-						VF610_PAD_PTA7__GPIO_134	0x219d
-						>;
-					};
-				};
-
-				fec0 {
-					pinctrl_fec0_1: fec0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTA6__RMII_CLKIN	0x30d1
-						VF610_PAD_PTC0__ENET_RMII0_MDC	0x30d3
-						VF610_PAD_PTC1__ENET_RMII0_MDIO	0x30d1
-						VF610_PAD_PTC2__ENET_RMII0_CRS	0x30d1
-						VF610_PAD_PTC3__ENET_RMII0_RXD1	0x30d1
-						VF610_PAD_PTC4__ENET_RMII0_RXD0	0x30d1
-						VF610_PAD_PTC5__ENET_RMII0_RXER	0x30d1
-						VF610_PAD_PTC6__ENET_RMII0_TXD1	0x30d2
-						VF610_PAD_PTC7__ENET_RMII0_TXD0	0x30d2
-						VF610_PAD_PTC8__ENET_RMII0_TXEN	0x30d2
-						>;
-					};
-				};
-
-				fec1 {
-					pinctrl_fec1_1: fec1grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTC9__ENET_RMII1_MDC		0x30d2
-						VF610_PAD_PTC10__ENET_RMII1_MDIO	0x30d3
-						VF610_PAD_PTC11__ENET_RMII1_CRS		0x30d1
-						VF610_PAD_PTC12__ENET_RMII_RXD1		0x30d1
-						VF610_PAD_PTC13__ENET_RMII1_RXD0	0x30d1
-						VF610_PAD_PTC14__ENET_RMII1_RXER	0x30d1
-						VF610_PAD_PTC15__ENET_RMII1_TXD1	0x30d2
-						VF610_PAD_PTC16__ENET_RMII1_TXD0	0x30d2
-						VF610_PAD_PTC17__ENET_RMII1_TXEN	0x30d2
-						>;
-					};
-				};
-
-				i2c0 {
-					pinctrl_i2c0_1: i2c0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTB14__I2C0_SCL	0x30d3
-						VF610_PAD_PTB15__I2C0_SDA	0x30d3
-						>;
-					};
-				};
-
-				pwm0 {
-					pinctrl_pwm0_1: pwm0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTB0__FTM0_CH0	0x1582
-						VF610_PAD_PTB1__FTM0_CH1	0x1582
-						VF610_PAD_PTB2__FTM0_CH2	0x1582
-						VF610_PAD_PTB3__FTM0_CH3	0x1582
-						VF610_PAD_PTB6__FTM0_CH6	0x1582
-						VF610_PAD_PTB7__FTM0_CH7	0x1582
-						>;
-					};
-				};
-
-				qspi0 {
-					pinctrl_qspi0_1: qspi0grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTD0__QSPI0_A_QSCK	0x307b
-						VF610_PAD_PTD1__QSPI0_A_CS0	0x307f
-						VF610_PAD_PTD2__QSPI0_A_DATA3	0x3073
-						VF610_PAD_PTD3__QSPI0_A_DATA2	0x3073
-						VF610_PAD_PTD4__QSPI0_A_DATA1	0x3073
-						VF610_PAD_PTD5__QSPI0_A_DATA0	0x307b
-						VF610_PAD_PTD7__QSPI0_B_QSCK	0x307b
-						VF610_PAD_PTD8__QSPI0_B_CS0	0x307f
-						VF610_PAD_PTD9__QSPI0_B_DATA3	0x3073
-						VF610_PAD_PTD10__QSPI0_B_DATA2	0x3073
-						VF610_PAD_PTD11__QSPI0_B_DATA1	0x3073
-						VF610_PAD_PTD12__QSPI0_B_DATA0	0x307b
-						>;
-					};
-				};
-
-				sai2 {
-					pinctrl_sai2_1: sai2grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTA16__SAI2_TX_BCLK	0x02ed
-						VF610_PAD_PTA18__SAI2_TX_DATA	0x02ee
-						VF610_PAD_PTA19__SAI2_TX_SYNC	0x02ed
-						VF610_PAD_PTA21__SAI2_RX_BCLK	0x02ed
-						VF610_PAD_PTA22__SAI2_RX_DATA	0x02ed
-						VF610_PAD_PTA23__SAI2_RX_SYNC	0x02ed
-						VF610_PAD_PTB18__EXT_AUDIO_MCLK	0x02ed
-						>;
-					};
-				};
-
-				uart1 {
-					pinctrl_uart1_1: uart1grp_1 {
-						fsl,pins = <
-						VF610_PAD_PTB4__UART1_TX	0x21a2
-						VF610_PAD_PTB5__UART1_RX	0x21a1
-						>;
-					};
-				};
-
-				usbvbus {
-					pinctrl_usbvbus_1: usbvbusgrp_1 {
-						fsl,pins = <
-						VF610_PAD_PTA24__USB1_VBUS_EN	0x219c
-						VF610_PAD_PTA16__USB0_VBUS_EN	0x219c
-						>;
-					};
-				};
-
 			};
 
 			gpio1: gpio at 40049000 {
-- 
1.7.9.5

[PATCH v2] ARM: dts: vf610: make pinctrl nodes board specific

[Bill Pringlemeir]

On 10 Dec 2013, shawn.guo at linaro.org wrote:

> Currently, all pinctrl setting nodes are defined in <soc>.dtsi, so that
> boards that share the same pinctrl setting do not have to define it time
> and time again in <board>.dts.  However, along with the devices and use
> cases being added continuously, the pinctrl setting nodes under iomuxc
> becomes more than expected.  This bloats device tree blob for particular
> board unnecessarily since only a small subset of those pinctrl setting
> nodes will be used by the board.  It impacts not only the DTB file size
> but also the run-time device tree lookup efficiency.

> arch/arm/boot/dts/vf610-pingrp.h   |  127 ++++++++++++++++++++++++++

Thanks for including this file.  I was looking a NAND flash and the
common configuration are 8/16 bit.  This solution is better as,

   #define VF610_NFC_PINGRP_8BIT \
     ...

   #define VF610_NFC_PINGRP_16BIT \
     VF610_NFC_PINGRP_8BIT        \
     VF610_PAD_PTD31__NF_IO15	0x28df \
     VF610_PAD_PTD30__NF_IO14	0x28df \
     VF610_PAD_PTD29__NF_IO13	0x28df \
     ...

It makes the board file look more straight forward,

   &nfc {  /* 16 bit with group in group */
           /* ... */
           pinctrl-names = "default";
           pinctrl-0 = <&pinctrl_nfc_16bit>;
   };

Versus,

   &nfc {  /* old way _OR_ separate groups */
           /* ... */
           pinctrl-names = "default";
           pinctrl-0 = <&pinctrl_nfc_8bit>; /* 8-bit */
           pinctrl-1 = <&pinctrl_nfc_16_bit>; /* 16-bit extras */
   };

Although for a 'multi-machine' configuration, the sharing maybe
appropriate?  But I guess that is your point; it is not helpful.  

Is a group within a group appropriate in vf610-pingrp.h?  Maybe a
comment?

Regards,
Bill Pringlemeir.

[PATCH v2] ARM: dts: vf610: make pinctrl nodes board specific

[Bill Pringlemeir]

On 12 Dec 2013, bpringlemeir at nbsps.com wrote:

> On 10 Dec 2013, shawn.guo at linaro.org wrote:
>
>> Currently, all pinctrl setting nodes are defined in <soc>.dtsi, so that
>> boards that share the same pinctrl setting do not have to define it time
>> and time again in <board>.dts.  However, along with the devices and use
>> cases being added continuously, the pinctrl setting nodes under iomuxc
>> becomes more than expected.  This bloats device tree blob for particular
>> board unnecessarily since only a small subset of those pinctrl setting
>> nodes will be used by the board.  It impacts not only the DTB file size
>> but also the run-time device tree lookup efficiency.

>> arch/arm/boot/dts/vf610-pingrp.h   |  127 ++++++++++++++++++++++++++

> Thanks for including this file.  I was looking a NAND flash and the
> common configuration are 8/16 bit.  This solution is better as,

Actually, this is a generic file but we have,

+#define VF610_SAI2_PINGRP1 \
+	VF610_PAD_PTA16__SAI2_TX_BCLK		0x02ed \
+	VF610_PAD_PTA18__SAI2_TX_DATA		0x02ee \
+	VF610_PAD_PTA19__SAI2_TX_SYNC		0x02ed \
+	VF610_PAD_PTA21__SAI2_RX_BCLK		0x02ed \
+	VF610_PAD_PTA22__SAI2_RX_DATA		0x02ed \
+	VF610_PAD_PTA23__SAI2_RX_SYNC		0x02ed \
+	VF610_PAD_PTB18__EXT_AUDIO_MCLK		0x02ed

The SAI might not use 'VF610_PAD_PTB18__EXT_AUDIO_MCLK' nor either
'VF610_PAD_PTA16__SAI2_TX_BCLK' or 'VF610_PAD_PTA21__SAI2_RX_BCLK'.
There are some pins that are absolutely needed for a module to function
and then there are some optional pins.

That said, this is nothing that you have changed.  It was like this to
begin with in the generic 'vf610.dtsi' file.

For instance some designs may have NAND flash CS0 and CS1 and other may
only include one (or they have no NAND at all).  Anything that uses the
NAND flash driver needs at least,

	VF610_PAD_PTD23__NF_IO7		0x28df
	VF610_PAD_PTD22__NF_IO6		0x28df
	VF610_PAD_PTD21__NF_IO5		0x28df
	VF610_PAD_PTD20__NF_IO4		0x28df
	VF610_PAD_PTD19__NF_IO3		0x28df
	VF610_PAD_PTD18__NF_IO2		0x28df
	VF610_PAD_PTD17__NF_IO1		0x28df
	VF610_PAD_PTD16__NF_IO0		0x28df
	VF610_PAD_PTB24__NF_WE_B	0x28c2
	VF610_PAD_PTB27__NF_RE_B	0x28c2
	VF610_PAD_PTC26__NF_RB_B	0x280d
	VF610_PAD_PTC27__NF_ALE		0x28c2
	VF610_PAD_PTC28__NF_CLE		0x28c2

and one of,

	VF610_PAD_PTB25__NF_CE0_B	0x28c2
        VF610_PAD_PTB26__NF_CE1_B	0x28c2

For the SAI, I am pretty sure that there are multiple master clocks
possible and that for synchronous modes, only one TX or RX clk is needed
and possibly the SYNC as well (I am thinking I2S).  If these are generic
definitions for any board, then maybe they should be separated more or
the group should be 'VF610_TWR_SAI2_PINGRP1'; Ie, particular to the
machine.

Anyways, this is a pre-existing issue.

Fwiw,
Bill Pringlemeir.

[PATCH v2] ARM: dts: vf610: make pinctrl nodes board specific

[Shawn Guo]

On Thu, Dec 12, 2013 at 03:12:50PM -0500, Bill Pringlemeir wrote:
> On 12 Dec 2013, bpringlemeir at nbsps.com wrote:
> 
> > On 10 Dec 2013, shawn.guo at linaro.org wrote:
> >
> >> Currently, all pinctrl setting nodes are defined in <soc>.dtsi, so that
> >> boards that share the same pinctrl setting do not have to define it time
> >> and time again in <board>.dts.  However, along with the devices and use
> >> cases being added continuously, the pinctrl setting nodes under iomuxc
> >> becomes more than expected.  This bloats device tree blob for particular
> >> board unnecessarily since only a small subset of those pinctrl setting
> >> nodes will be used by the board.  It impacts not only the DTB file size
> >> but also the run-time device tree lookup efficiency.
> 
> >> arch/arm/boot/dts/vf610-pingrp.h   |  127 ++++++++++++++++++++++++++
> 
> > Thanks for including this file.  I was looking a NAND flash and the
> > common configuration are 8/16 bit.  This solution is better as,
> 
> Actually, this is a generic file but we have,
> 
> +#define VF610_SAI2_PINGRP1 \
> +	VF610_PAD_PTA16__SAI2_TX_BCLK		0x02ed \
> +	VF610_PAD_PTA18__SAI2_TX_DATA		0x02ee \
> +	VF610_PAD_PTA19__SAI2_TX_SYNC		0x02ed \
> +	VF610_PAD_PTA21__SAI2_RX_BCLK		0x02ed \
> +	VF610_PAD_PTA22__SAI2_RX_DATA		0x02ed \
> +	VF610_PAD_PTA23__SAI2_RX_SYNC		0x02ed \
> +	VF610_PAD_PTB18__EXT_AUDIO_MCLK		0x02ed
> 
> The SAI might not use 'VF610_PAD_PTB18__EXT_AUDIO_MCLK' nor either
> 'VF610_PAD_PTA16__SAI2_TX_BCLK' or 'VF610_PAD_PTA21__SAI2_RX_BCLK'.
> There are some pins that are absolutely needed for a module to function
> and then there are some optional pins.
> 
> That said, this is nothing that you have changed.  It was like this to
> begin with in the generic 'vf610.dtsi' file.

Yea, the PINGRP defined in vf610-pingrp.h should be generic rather than
board specific.  It seems that VF610_SAI2_PINGRP1 is used nowhere for
now, so we can change it easily.  Patch is welcomed.

Shawn

> 
> For instance some designs may have NAND flash CS0 and CS1 and other may
> only include one (or they have no NAND at all).  Anything that uses the
> NAND flash driver needs at least,
> 
> 	VF610_PAD_PTD23__NF_IO7		0x28df
> 	VF610_PAD_PTD22__NF_IO6		0x28df
> 	VF610_PAD_PTD21__NF_IO5		0x28df
> 	VF610_PAD_PTD20__NF_IO4		0x28df
> 	VF610_PAD_PTD19__NF_IO3		0x28df
> 	VF610_PAD_PTD18__NF_IO2		0x28df
> 	VF610_PAD_PTD17__NF_IO1		0x28df
> 	VF610_PAD_PTD16__NF_IO0		0x28df
> 	VF610_PAD_PTB24__NF_WE_B	0x28c2
> 	VF610_PAD_PTB27__NF_RE_B	0x28c2
> 	VF610_PAD_PTC26__NF_RB_B	0x280d
> 	VF610_PAD_PTC27__NF_ALE		0x28c2
> 	VF610_PAD_PTC28__NF_CLE		0x28c2
> 
> and one of,
> 
> 	VF610_PAD_PTB25__NF_CE0_B	0x28c2
>         VF610_PAD_PTB26__NF_CE1_B	0x28c2
> 
> For the SAI, I am pretty sure that there are multiple master clocks
> possible and that for synchronous modes, only one TX or RX clk is needed
> and possibly the SYNC as well (I am thinking I2S).  If these are generic
> definitions for any board, then maybe they should be separated more or
> the group should be 'VF610_TWR_SAI2_PINGRP1'; Ie, particular to the
> machine.
> 
> Anyways, this is a pre-existing issue.
> 
> Fwiw,
> Bill Pringlemeir.

[PATCH v2] ARM: dts: vf610: make pinctrl nodes board specific

[Bill Pringlemeir]

> On Thu, Dec 12, 2013 at 03:12:50PM -0500, Bill Pringlemeir wrote:

>> The SAI might not use 'VF610_PAD_PTB18__EXT_AUDIO_MCLK' nor either
>> 'VF610_PAD_PTA16__SAI2_TX_BCLK' or 'VF610_PAD_PTA21__SAI2_RX_BCLK'.
>> There are some pins that are absolutely needed for a module to function
>> and then there are some optional pins.
>>
>> That said, this is nothing that you have changed.  It was like this to
>> begin with in the generic 'vf610.dtsi' file.

On 14 Dec 2013, shawn.guo at linaro.org wrote:

> Yea, the PINGRP defined in vf610-pingrp.h should be generic rather than
> board specific.  It seems that VF610_SAI2_PINGRP1 is used nowhere for
> now, so we can change it easily.  Patch is welcomed.

Alison Wang and Xiubo Li are working on this driver now,

http://mailman.alsa-project.org/pipermail/alsa-devel/2013-November/068060.html

I would wait until after that is accepted.  I just wondered if my
understanding was correct.  Sorry, I missed all your IMX patches which
probably explained this.

Thanks,
Bill Pringlemeir.

[PATCH v2] ARM: dts: vf610: make pinctrl nodes board specific

[Shawn Guo]

On Thu, Dec 12, 2013 at 02:32:28PM -0500, Bill Pringlemeir wrote:
> On 10 Dec 2013, shawn.guo at linaro.org wrote:
> 
> > Currently, all pinctrl setting nodes are defined in <soc>.dtsi, so that
> > boards that share the same pinctrl setting do not have to define it time
> > and time again in <board>.dts.  However, along with the devices and use
> > cases being added continuously, the pinctrl setting nodes under iomuxc
> > becomes more than expected.  This bloats device tree blob for particular
> > board unnecessarily since only a small subset of those pinctrl setting
> > nodes will be used by the board.  It impacts not only the DTB file size
> > but also the run-time device tree lookup efficiency.
> 
> > arch/arm/boot/dts/vf610-pingrp.h   |  127 ++++++++++++++++++++++++++
> 
> Thanks for including this file.  I was looking a NAND flash and the
> common configuration are 8/16 bit.  This solution is better as,
> 
>    #define VF610_NFC_PINGRP_8BIT \
>      ...
> 
>    #define VF610_NFC_PINGRP_16BIT \
>      VF610_NFC_PINGRP_8BIT        \
>      VF610_PAD_PTD31__NF_IO15	0x28df \
>      VF610_PAD_PTD30__NF_IO14	0x28df \
>      VF610_PAD_PTD29__NF_IO13	0x28df \
>      ...
> 
> It makes the board file look more straight forward,

Yes, I agree.

> 
>    &nfc {  /* 16 bit with group in group */
>            /* ... */
>            pinctrl-names = "default";
>            pinctrl-0 = <&pinctrl_nfc_16bit>;
>    };
> 
> Versus,
> 
>    &nfc {  /* old way _OR_ separate groups */
>            /* ... */
>            pinctrl-names = "default";
>            pinctrl-0 = <&pinctrl_nfc_8bit>; /* 8-bit */
>            pinctrl-1 = <&pinctrl_nfc_16_bit>; /* 16-bit extras */
>    };
> 
> Although for a 'multi-machine' configuration, the sharing maybe
> appropriate?  But I guess that is your point; it is not helpful.  
> 
> Is a group within a group appropriate in vf610-pingrp.h?  Maybe a
> comment?

Yes, it is.  We already have something like that for USDHC in
arch/arm/boot/dts/imx6qdl-pingrp.h if you're looking at my for-next
branch.

Shawn