Re: [PATCH 2/9] ARM: dts: imx6sl: remove the use of pingrp macros

Hi Shawn,

On Sunday, 26. January 2014 00:43:04 Shawn Guo wrote:

[... and so on for the other groups ... ]

I'm confused now :-) . Current linux-next [0] shows the pin-settings as part 
of imx6sl.dtsi - a way a lot of other architectures organize their pingroups 
too, with the board file only referencing the relevant pingroups from the 
predefined ones of the soc.

So I guess your move to the pingrp-header moved them out of the imx6sl.dtsi to 
the .h and is not part of linux-next; but this patch (and the others in this 
series) now moves the definitions into the individual board files. Can't you 
just move them back to the soc-dtsi files to prevent each board duplicating 
them?

Or I've simply missed previous discussions about this ;-) .


Thanks
Heiko

[0] https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/arch/arm/boot/dts/imx6sl.dtsi#n640

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Jan 28, 2014 at 11:17:22AM +0100, Heiko Stübner wrote:

Hi Shawn,

On Sunday, 26. January 2014 00:43:04 Shawn Guo wrote:

quoted

We created the pingrp macros in imx6sl-pingrp.h for purpose of less LOC
when same pin group is used by multiple boards.  However, DT maintainers
take it as an abuse of DTC macro support.  So let's get rid of it to
make the pins used by given device more intuitive.

Signed-off-by: Shawn Guo <redacted>
---
 arch/arm/boot/dts/imx6sl-evk.dts  |  120 ++++++++++++++++++++++++++----
 arch/arm/boot/dts/imx6sl-pingrp.h |  148
------------------------------------- arch/arm/boot/dts/imx6sl.dtsi     |  
 1 -
 3 files changed, 107 insertions(+), 162 deletions(-)
 delete mode 100644 arch/arm/boot/dts/imx6sl-pingrp.h

diff --git a/arch/arm/boot/dts/imx6sl-evk.dts

b/arch/arm/boot/dts/imx6sl-evk.dts index f5e4513..8594d13 100644

--- a/arch/arm/boot/dts/imx6sl-evk.dts
+++ b/arch/arm/boot/dts/imx6sl-evk.dts

@@ -86,55 +86,149 @@
 		};

 		pinctrl_ecspi1: ecspi1grp {
-			fsl,pins = <MX6SL_ECSPI1_PINGRP1>;
+			fsl,pins = <
+				MX6SL_PAD_ECSPI1_MISO__ECSPI1_MISO	0x100b1
+				MX6SL_PAD_ECSPI1_MOSI__ECSPI1_MOSI	0x100b1
+				MX6SL_PAD_ECSPI1_SCLK__ECSPI1_SCLK	0x100b1
+			>;
 		};

 		pinctrl_fec: fecgrp {
-			fsl,pins = <MX6SL_FEC_PINGRP1>;
+			fsl,pins = <
+				MX6SL_PAD_FEC_MDC__FEC_MDC		0x1b0b0
+				MX6SL_PAD_FEC_MDIO__FEC_MDIO		0x1b0b0
+				MX6SL_PAD_FEC_CRS_DV__FEC_RX_DV		0x1b0b0
+				MX6SL_PAD_FEC_RXD0__FEC_RX_DATA0	0x1b0b0
+				MX6SL_PAD_FEC_RXD1__FEC_RX_DATA1	0x1b0b0
+				MX6SL_PAD_FEC_TX_EN__FEC_TX_EN		0x1b0b0
+				MX6SL_PAD_FEC_TXD0__FEC_TX_DATA0	0x1b0b0
+				MX6SL_PAD_FEC_TXD1__FEC_TX_DATA1	0x1b0b0
+				MX6SL_PAD_FEC_REF_CLK__FEC_REF_OUT	0x4001b0a8
+			>;
 		};

[... and so on for the other groups ... ]

I'm confused now :-) . Current linux-next [0] shows the pin-settings as part 
of imx6sl.dtsi - a way a lot of other architectures organize their pingroups 
too, with the board file only referencing the relevant pingroups from the 
predefined ones of the soc.

Current mainline has all groups under the iomux node which has the
effect that all possible groups are compiled into every dtb resulting in
very bloated dtbs. So Shawn changed it to what's currently in next, but
this hasn't been accepted by the dt maintainers. Now this series tries
to address the concerns of the dt maintainers by not using macros that
expand to other macros.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Jan 28, 2014 at 11:17:22AM +0100, Heiko Stübner wrote:

[... and so on for the other groups ... ]

I'm confused now :-) . Current linux-next [0] shows the pin-settings as part 
of imx6sl.dtsi - a way a lot of other architectures organize their pingroups 
too, with the board file only referencing the relevant pingroups from the 
predefined ones of the soc.

So I guess your move to the pingrp-header moved them out of the imx6sl.dtsi to 
the .h and is not part of linux-next;

Yes, my for-next branch was excluded from linux-next temporarily for
some reason.  I will ask Stephen to add it back once v3.14-rc1 is out.
That said, you can see nothing we developed in this cycle on linux-next
for now.

but this patch (and the others in this 
series) now moves the definitions into the individual board files. Can't you 
just move them back to the soc-dtsi files to prevent each board duplicating 
them?

No.  That will bring back the problem we try to solve from the
beginning [1].

Shawn

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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html