| Message ID | 1382950843-26066-5-git-send-email-mpa@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 28, 2013 at 4:00 AM, Markus Pargmann <mpa@pengutronix.de> wrote: > Pinctrl driver node and pin group definitions for other driver nodes. > > Signed-off-by: Markus Pargmann <mpa@pengutronix.de> > Acked-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > arch/arm/boot/dts/imx27.dtsi | 102 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 102 insertions(+) > > diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi > index b7a1c6d..641a63d 100644 > --- a/arch/arm/boot/dts/imx27.dtsi > +++ b/arch/arm/boot/dts/imx27.dtsi > @@ -10,6 +10,7 @@ > */ > > #include "skeleton.dtsi" > +#include "imx27-pinfunc.h" > > / { > aliases { > @@ -235,6 +236,12 @@ > status = "disabled"; > }; > > + iomuxc: iomuxc@10015000 { > + compatible = "fsl,imx27-iomuxc"; > + reg = <0x10015000 0x600>; > + > + }; > + > gpio1: gpio@10015000 { > compatible = "fsl,imx27-gpio", "fsl,imx21-gpio"; > reg = <0x10015000 0x100>; Linus, Case in point, you can't have two nodes which overlap registers. Probably the most reasonable way to effect this is to keep the GPIO bindings, put pinctrl definitions in there (there's no reason pinctrl and gpio drivers can't probe the same compatible property) and use regmap internally on that single node. On the good news side, this makes the pinctrl binding easier as every single one would be is parented by a gpio instance (gpio1 as above, for example), so it doesn't need to specify a port number anymore or do any (port*32)+pin math - just a pin index within that port. Schema guys: is the DT validator code prepped for checking for overlapping register spaces, because this is totally undesirable behavior in device trees.. Matt Sealey <neko@bakuhatsu.net>
On Wed, Nov 6, 2013 at 11:43 PM, Matt Sealey <neko@bakuhatsu.net> wrote: > On Mon, Oct 28, 2013 at 4:00 AM, Markus Pargmann <mpa@pengutronix.de> wrote: >> >> + iomuxc: iomuxc@10015000 { >> + compatible = "fsl,imx27-iomuxc"; >> + reg = <0x10015000 0x600>; >> + >> + }; >> + >> gpio1: gpio@10015000 { >> compatible = "fsl,imx27-gpio", "fsl,imx21-gpio"; >> reg = <0x10015000 0x100>; > > Linus, > > Case in point, you can't have two nodes which overlap registers. > > Probably the most reasonable way to effect this is to keep the GPIO > bindings, put pinctrl definitions in there (there's no reason pinctrl > and gpio drivers can't probe the same compatible property) and use > regmap internally on that single node. If the I/O region is tightly coupled across two subsystems like that we usually merge the driver into one combined pinctrl+gpio driver and put it into drivers/pinctrl/*. This often solves more problems and make the code simpler too. An alternative if address 0x10015000- 0x100150ff is really only used for GPIO is to map the pinctrl like that: reg = <0x10015100 0x500> and rewrite all the base offset handling in the driver. Yours, Linus Walleij
On Fri, Nov 08, 2013 at 10:45:01AM +0100, Linus Walleij wrote: > On Wed, Nov 6, 2013 at 11:43 PM, Matt Sealey <neko@bakuhatsu.net> wrote: > > On Mon, Oct 28, 2013 at 4:00 AM, Markus Pargmann <mpa@pengutronix.de> wrote: > >> > >> + iomuxc: iomuxc@10015000 { > >> + compatible = "fsl,imx27-iomuxc"; > >> + reg = <0x10015000 0x600>; > >> + > >> + }; > >> + > >> gpio1: gpio@10015000 { > >> compatible = "fsl,imx27-gpio", "fsl,imx21-gpio"; > >> reg = <0x10015000 0x100>; > > > > Linus, > > > > Case in point, you can't have two nodes which overlap registers. > > > > Probably the most reasonable way to effect this is to keep the GPIO > > bindings, put pinctrl definitions in there (there's no reason pinctrl > > and gpio drivers can't probe the same compatible property) and use > > regmap internally on that single node. > > If the I/O region is tightly coupled across two subsystems like > that we usually merge the driver into one combined > pinctrl+gpio driver and put it into drivers/pinctrl/*. The gpio module does not necessarily include a pinctrl module. Most of the imx SoCs have the same gpio function registers but without pinctrl. They all use the same gpio driver. > > This often solves more problems and make the code > simpler too. > > An alternative if address 0x10015000- 0x100150ff is > really only used for GPIO is to map the pinctrl like > that: > > reg = <0x10015100 0x500> > > and rewrite all the base offset handling in the driver. There are 6 gpio modules in the memory region from 0x10015000 to 0x100155ff. pinctrl is in the same memory region. I could change the bindings back to that from v1 [1]. One pinmux controller over the full memory range (0x10015000 0x600) and the gpio modules as subnodes using 0x100 each. This would preserve the currently used references to the gpio nodes. Regards, Markus Pargmann [1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/257180/focus=257184
On Fri, Nov 8, 2013 at 2:56 PM, Markus Pargmann <mpa@pengutronix.de> wrote: > On Fri, Nov 08, 2013 at 10:45:01AM +0100, Linus Walleij wrote: >> If the I/O region is tightly coupled across two subsystems like >> that we usually merge the driver into one combined >> pinctrl+gpio driver and put it into drivers/pinctrl/*. > > The gpio module does not necessarily include a pinctrl module. Most of > the imx SoCs have the same gpio function registers but without pinctrl. > They all use the same gpio driver. Yeah but I'm only saying you could make a combined driver, I didn't say you had to use both halves of it all the time. pinctrl/pinctrl-imx1.c can very well handle both and only supply GPIO functionality for some compatible-strings, that's OK. > There are 6 gpio modules in the memory region from 0x10015000 to > 0x100155ff. pinctrl is in the same memory region. OK I get it (I think). > I could change the bindings back to that from v1 [1]. One pinmux > controller over the full memory range (0x10015000 0x600) and the gpio > modules as subnodes using 0x100 each. This would preserve the currently > used references to the gpio nodes. Hm I don't really have opinions on this, better let the DT-folks say what they prefer. But that looks good to me. Can you do this with an incremental patch? Yours, Linus Walleij
diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi index b7a1c6d..641a63d 100644 --- a/arch/arm/boot/dts/imx27.dtsi +++ b/arch/arm/boot/dts/imx27.dtsi @@ -10,6 +10,7 @@ */ #include "skeleton.dtsi" +#include "imx27-pinfunc.h" / { aliases { @@ -235,6 +236,12 @@ status = "disabled"; }; + iomuxc: iomuxc@10015000 { + compatible = "fsl,imx27-iomuxc"; + reg = <0x10015000 0x600>; + + }; + gpio1: gpio@10015000 { compatible = "fsl,imx27-gpio", "fsl,imx21-gpio"; reg = <0x10015000 0x100>; @@ -467,3 +474,98 @@ }; }; }; + +&iomuxc { + fec { + pinctrl_fec1_1: fec1-1 { + fsl,pins = < + MX27_PAD_SD3_CMD__FEC_TXD0 0x0 + MX27_PAD_SD3_CLK__FEC_TXD1 0x0 + MX27_PAD_ATA_DATA0__FEC_TXD2 0x0 + MX27_PAD_ATA_DATA1__FEC_TXD3 0x0 + MX27_PAD_ATA_DATA2__FEC_RX_ER 0x0 + MX27_PAD_ATA_DATA3__FEC_RXD1 0x0 + MX27_PAD_ATA_DATA4__FEC_RXD2 0x0 + MX27_PAD_ATA_DATA5__FEC_RXD3 0x0 + MX27_PAD_ATA_DATA6__FEC_MDIO 0x0 + MX27_PAD_ATA_DATA7__FEC_MDC 0x0 + MX27_PAD_ATA_DATA8__FEC_CRS 0x0 + MX27_PAD_ATA_DATA9__FEC_TX_CLK 0x0 + MX27_PAD_ATA_DATA10__FEC_RXD0 0x0 + MX27_PAD_ATA_DATA11__FEC_RX_DV 0x0 + MX27_PAD_ATA_DATA12__FEC_RX_CLK 0x0 + MX27_PAD_ATA_DATA13__FEC_COL 0x0 + MX27_PAD_ATA_DATA14__FEC_TX_ER 0x0 + MX27_PAD_ATA_DATA15__FEC_TX_EN 0x0 + >; + }; + }; + + i2c { + pinctrl_i2c1_1: i2c1-1 { + fsl,pins = < + MX27_PAD_I2C_DATA__I2C_DATA 0x0 + MX27_PAD_I2C_CLK__I2C_CLK 0x0 + >; + }; + + pinctrl_i2c2_1: i2c2-1 { + fsl,pins = < + MX27_PAD_I2C2_SDA__I2C2_SDA 0x0 + MX27_PAD_I2C2_SCL__I2C2_SCL 0x0 + >; + }; + }; + + owire { + pinctrl_owire1_1: owire1-1 { + fsl,pins = < + MX27_PAD_RTCK__OWIRE 0x0 + >; + }; + }; + + uart { + pinctrl_uart1_1: uart1-1 { + fsl,pins = < + MX27_PAD_UART1_TXD__UART1_TXD 0x0 + MX27_PAD_UART1_RXD__UART1_RXD 0x0 + >; + }; + + pinctrl_uart1_rtscts_1: uart1-rtscts-1 { + fsl,pins = < + MX27_PAD_UART1_CTS__UART1_CTS 0x0 + MX27_PAD_UART1_RTS__UART1_RTS 0x0 + >; + }; + + pinctrl_uart2_1: uart2-1 { + fsl,pins = < + MX27_PAD_UART2_TXD__UART2_TXD 0x0 + MX27_PAD_UART2_RXD__UART2_RXD 0x0 + >; + }; + + pinctrl_uart2_rtscts_1: uart2-rtscts-1 { + fsl,pins = < + MX27_PAD_UART2_CTS__UART2_CTS 0x0 + MX27_PAD_UART2_RTS__UART2_RTS 0x0 + >; + }; + + pinctrl_uart3_1: uart3-1 { + fsl,pins = < + MX27_PAD_UART3_TXD__UART3_TXD 0x0 + MX27_PAD_UART3_RXD__UART3_RXD 0x0 + >; + }; + + pinctrl_uart3_rtscts_1: uart3-rtscts-1 { + fsl,pins = < + MX27_PAD_UART3_CTS__UART3_CTS 0x0 + MX27_PAD_UART3_RTS__UART3_RTS 0x0 + >; + }; + }; +};