diff mbox

[7/8] ARM: dts: imx: add IMX50 SoC device tree bindings

Message ID 1382076260-6422-8-git-send-email-gerg@uclinux.org (mailing list archive)
State New, archived
Headers show

Commit Message

Greg Ungerer Oct. 18, 2013, 6:04 a.m. UTC
From: Greg Ungerer <gerg@uclinux.org>

Create device tree bindings for the Freescale IMX50 SoC. This was based on
the IMX53 bindings with changes made as necessary.

Signed-off-by: Greg Ungerer <gerg@uclinux.org>
---
 arch/arm/boot/dts/imx50.dtsi | 659 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 659 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx50.dtsi

Comments

Sascha Hauer Oct. 22, 2013, 12:35 p.m. UTC | #1
On Fri, Oct 18, 2013 at 04:04:19PM +1000, gerg@uclinux.org wrote:
> From: Greg Ungerer <gerg@uclinux.org>
> 
> Create device tree bindings for the Freescale IMX50 SoC. This was based on
> the IMX53 bindings with changes made as necessary.
> 
> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
> ---
> +
> +			iomuxc: iomuxc@53fa8000 {
> +				compatible = "fsl,imx50-iomuxc";
> +				reg = <0x53fa8000 0x4000>;
> +
> +				fec {
> +					pinctrl_fec_1: fecgrp-1 {
> +						fsl,pins = <
> +							MX50_PAD_SSI_RXFS__FEC_MDC	0x80
> +							MX50_PAD_SSI_RXC__FEC_MDIO	0x80
> +							MX50_PAD_DISP_D0__FEC_TX_CLK	0x80
> +							MX50_PAD_DISP_D1__FEC_RX_ERR	0x80
> +							MX50_PAD_DISP_D2__FEC_RX_DV	0x80
> +							MX50_PAD_DISP_D3__FEC_RDATA_1	0x80
> +							MX50_PAD_DISP_D4__FEC_RDATA_0	0x80
> +							MX50_PAD_DISP_D5__FEC_TX_EN	0x80
> +							MX50_PAD_DISP_D6__FEC_TDATA_1	0x80
> +							MX50_PAD_DISP_D7__FEC_TDATA_0	0x80
> +						>;
> +					};
> +

Shawn recently removed the pinctrl groups here and referenced to this
node by doing

&iomuxc {
	fec {
		...
	};
};

> +				cspi {
> +					pinctrl_cspi_1: cspigrp-1 {
> +						fsl,pins = <
> +							MX50_PAD_CSPI_SCLK__CSPI_SCLK	0

0 is definitely wrong here. We have 0x80000000 for "Don't touch
padctrl", but otherwise this should contain some real padctrl settings.

Sascha
Matt Sealey Oct. 22, 2013, 8:08 p.m. UTC | #2
On Tue, Oct 22, 2013 at 7:35 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Fri, Oct 18, 2013 at 04:04:19PM +1000, gerg@uclinux.org wrote:
>> From: Greg Ungerer <gerg@uclinux.org>
>>
>> Create device tree bindings for the Freescale IMX50 SoC. This was based on
>> the IMX53 bindings with changes made as necessary.
>>
>> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
>> ---
>> +
>> +                     iomuxc: iomuxc@53fa8000 {
>> +                             compatible = "fsl,imx50-iomuxc";
>> +                             reg = <0x53fa8000 0x4000>;
>> +
>> +                             fec {
>> +                                     pinctrl_fec_1: fecgrp-1 {
>> +                                             fsl,pins = <
>> +                                                     MX50_PAD_SSI_RXFS__FEC_MDC      0x80
>> +                                                     MX50_PAD_SSI_RXC__FEC_MDIO      0x80
>> +                                                     MX50_PAD_DISP_D0__FEC_TX_CLK    0x80
>> +                                                     MX50_PAD_DISP_D1__FEC_RX_ERR    0x80
>> +                                                     MX50_PAD_DISP_D2__FEC_RX_DV     0x80
>> +                                                     MX50_PAD_DISP_D3__FEC_RDATA_1   0x80
>> +                                                     MX50_PAD_DISP_D4__FEC_RDATA_0   0x80
>> +                                                     MX50_PAD_DISP_D5__FEC_TX_EN     0x80
>> +                                                     MX50_PAD_DISP_D6__FEC_TDATA_1   0x80
>> +                                                     MX50_PAD_DISP_D7__FEC_TDATA_0   0x80
>> +                                             >;
>> +                                     };
>> +
>
> Shawn recently removed the pinctrl groups here and referenced to this
> node by doing
>
> &iomuxc {
>         fec {
>                 ...
>         };
> };
>
>> +                             cspi {
>> +                                     pinctrl_cspi_1: cspigrp-1 {
>> +                                             fsl,pins = <
>> +                                                     MX50_PAD_CSPI_SCLK__CSPI_SCLK   0
>
> 0 is definitely wrong here. We have 0x80000000 for "Don't touch
> padctrl", but otherwise this should contain some real padctrl settings.

A more pressing question is in what world did the bootloader not
already set these pins up and if they are already set up, why are they
loitering in the device tree?
Sascha Hauer Oct. 22, 2013, 9:57 p.m. UTC | #3
On Tue, Oct 22, 2013 at 03:08:43PM -0500, Matt Sealey wrote:
> On Tue, Oct 22, 2013 at 7:35 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Fri, Oct 18, 2013 at 04:04:19PM +1000, gerg@uclinux.org wrote:
> >> From: Greg Ungerer <gerg@uclinux.org>
> >>
> >> Create device tree bindings for the Freescale IMX50 SoC. This was based on
> >> the IMX53 bindings with changes made as necessary.
> >>
> >> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
> >> ---
> >> +
> >> +                     iomuxc: iomuxc@53fa8000 {
> >> +                             compatible = "fsl,imx50-iomuxc";
> >> +                             reg = <0x53fa8000 0x4000>;
> >> +
> >> +                             fec {
> >> +                                     pinctrl_fec_1: fecgrp-1 {
> >> +                                             fsl,pins = <
> >> +                                                     MX50_PAD_SSI_RXFS__FEC_MDC      0x80
> >> +                                                     MX50_PAD_SSI_RXC__FEC_MDIO      0x80
> >> +                                                     MX50_PAD_DISP_D0__FEC_TX_CLK    0x80
> >> +                                                     MX50_PAD_DISP_D1__FEC_RX_ERR    0x80
> >> +                                                     MX50_PAD_DISP_D2__FEC_RX_DV     0x80
> >> +                                                     MX50_PAD_DISP_D3__FEC_RDATA_1   0x80
> >> +                                                     MX50_PAD_DISP_D4__FEC_RDATA_0   0x80
> >> +                                                     MX50_PAD_DISP_D5__FEC_TX_EN     0x80
> >> +                                                     MX50_PAD_DISP_D6__FEC_TDATA_1   0x80
> >> +                                                     MX50_PAD_DISP_D7__FEC_TDATA_0   0x80
> >> +                                             >;
> >> +                                     };
> >> +
> >
> > Shawn recently removed the pinctrl groups here and referenced to this
> > node by doing
> >
> > &iomuxc {
> >         fec {
> >                 ...
> >         };
> > };
> >
> >> +                             cspi {
> >> +                                     pinctrl_cspi_1: cspigrp-1 {
> >> +                                             fsl,pins = <
> >> +                                                     MX50_PAD_CSPI_SCLK__CSPI_SCLK   0
> >
> > 0 is definitely wrong here. We have 0x80000000 for "Don't touch
> > padctrl", but otherwise this should contain some real padctrl settings.
> 
> A more pressing question is in what world did the bootloader not
> already set these pins up and if they are already set up, why are they
> loitering in the device tree?

Having NO_PAD_CTRL in the devicetree doesn't make sense, you're right.
Either a pin has to be configured by the bootloader completely or not at
all. Having the mux configured by the kernel and the drive strength by
the bootloader is broken by design. All pins should have a complete
padctrl setup and NO_PAD_CTRL should be dropped.

Sascha
Benoît Thébaudeau Oct. 22, 2013, 10:42 p.m. UTC | #4
Dear Sascha Hauer,

On Tuesday, October 22, 2013 11:57:47 PM, Sascha Hauer wrote:
> On Tue, Oct 22, 2013 at 03:08:43PM -0500, Matt Sealey wrote:
> > On Tue, Oct 22, 2013 at 7:35 AM, Sascha Hauer <s.hauer@pengutronix.de>
> > wrote:
> > > On Fri, Oct 18, 2013 at 04:04:19PM +1000, gerg@uclinux.org wrote:
> > >> From: Greg Ungerer <gerg@uclinux.org>
> > >>
> > >> Create device tree bindings for the Freescale IMX50 SoC. This was based
> > >> on
> > >> the IMX53 bindings with changes made as necessary.
> > >>
> > >> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
> > >> ---
> > >> +
> > >> +                     iomuxc: iomuxc@53fa8000 {
> > >> +                             compatible = "fsl,imx50-iomuxc";
> > >> +                             reg = <0x53fa8000 0x4000>;
> > >> +
> > >> +                             fec {
> > >> +                                     pinctrl_fec_1: fecgrp-1 {
> > >> +                                             fsl,pins = <
> > >> +
> > >> MX50_PAD_SSI_RXFS__FEC_MDC
> > >> 0x80
> > >> +
> > >> MX50_PAD_SSI_RXC__FEC_MDIO
> > >> 0x80
> > >> +
> > >> MX50_PAD_DISP_D0__FEC_TX_CLK
> > >> 0x80
> > >> +
> > >> MX50_PAD_DISP_D1__FEC_RX_ERR
> > >> 0x80
> > >> +
> > >> MX50_PAD_DISP_D2__FEC_RX_DV
> > >> 0x80
> > >> +
> > >> MX50_PAD_DISP_D3__FEC_RDATA_1
> > >> 0x80
> > >> +
> > >> MX50_PAD_DISP_D4__FEC_RDATA_0
> > >> 0x80
> > >> +
> > >> MX50_PAD_DISP_D5__FEC_TX_EN
> > >> 0x80
> > >> +
> > >> MX50_PAD_DISP_D6__FEC_TDATA_1
> > >> 0x80
> > >> +
> > >> MX50_PAD_DISP_D7__FEC_TDATA_0
> > >> 0x80
> > >> +                                             >;
> > >> +                                     };
> > >> +
> > >
> > > Shawn recently removed the pinctrl groups here and referenced to this
> > > node by doing
> > >
> > > &iomuxc {
> > >         fec {
> > >                 ...
> > >         };
> > > };
> > >
> > >> +                             cspi {
> > >> +                                     pinctrl_cspi_1: cspigrp-1 {
> > >> +                                             fsl,pins = <
> > >> +
> > >> MX50_PAD_CSPI_SCLK__CSPI_SCLK
> > >> 0
> > >
> > > 0 is definitely wrong here. We have 0x80000000 for "Don't touch
> > > padctrl", but otherwise this should contain some real padctrl settings.
> > 
> > A more pressing question is in what world did the bootloader not
> > already set these pins up and if they are already set up, why are they
> > loitering in the device tree?
> 
> Having NO_PAD_CTRL in the devicetree doesn't make sense, you're right.
> Either a pin has to be configured by the bootloader completely or not at
> all. Having the mux configured by the kernel and the drive strength by
> the bootloader is broken by design. All pins should have a complete
> padctrl setup and NO_PAD_CTRL should be dropped.

Some pins may be configured completely by the kernel, and not at all by the
bootloader. In that case, the device tree may wish to keep the SoC reset pad
configuration. NO_PAD_CTRL is useful in that case, although one might argue that
the reset values should not be trusted for various reasons.

But there is another case. One SoC pin may be connected to several external
devices, e.g. through an external analog mux. In that case, the bootloader may
configure the SoC mux and pad for one usage, while the kernel may afterwards
change only the mux configuration of this pin for the other usage.

So NO_PAD_CTRL is not strictly required for pinctrl, but it can be handy.

Best regards,
Benoît
Matt Sealey Oct. 22, 2013, 11:06 p.m. UTC | #5
On Tue, Oct 22, 2013 at 5:42 PM, Benoît Thébaudeau
<benoit.thebaudeau@advansee.com> wrote:
> Dear Sascha Hauer,
>
> On Tuesday, October 22, 2013 11:57:47 PM, Sascha Hauer wrote:
>> > >
>> > > 0 is definitely wrong here. We have 0x80000000 for "Don't touch
>> > > padctrl", but otherwise this should contain some real padctrl settings.
>> >
>> > A more pressing question is in what world did the bootloader not
>> > already set these pins up and if they are already set up, why are they
>> > loitering in the device tree?
>>
>> Having NO_PAD_CTRL in the devicetree doesn't make sense, you're right.
>> Either a pin has to be configured by the bootloader completely or not at
>> all. Having the mux configured by the kernel and the drive strength by
>> the bootloader is broken by design. All pins should have a complete
>> padctrl setup and NO_PAD_CTRL should be dropped.
>
> Some pins may be configured completely by the kernel, and not at all by the
> bootloader. In that case, the device tree may wish to keep the SoC reset pad
> configuration. NO_PAD_CTRL is useful in that case, although one might argue that
> the reset values should not be trusted for various reasons.

No, that's totally illogical. The magic bit to "leave my pad controls
alone" makes no sense since you're re-configuring mux mode and input
select (even if they're the same as the bootloader, you're doing it
twice).

There are two reasons this gets done, both of which are very very odd..

1) The definition of the pinmux tuple of 3 register/value pairs is
ordered such that the pad control can be set independently of the
preprocessor definition for the pad (which supplies the first 5 values
in the 6 value pin definition). This is weird since one of the values
doesn't belong to the register the value pair is intended for, and
saying "do not

2) The reason we have to define the three registers even if we "don't
change values" is so users (or userspace) can fiddle with the GPIOs
through some never-used non-existent (since it doesn't expose these
values) sysfs pinmuxing API.

In this case, you don't "set the pad control to 'do not touch'" but
supply the exact values from the manual or empirically derived from
inspection at SoC power on. There is ZERO use for a 'do not touch'
bit. In the case where it turns out the bootloader is wrong, you'll
have to supply a value here anyway. So supply the default.

In reality what would happen is the binding gets a kick in the pants
so it makes more sense, and we define that the bootloader MUST set up
all static pin multiplexing configuration at the earliest opportunity
and save Linux the hassle. pinctrl is for microcontrollers and
robotics boards - Arduino or BeagleBone Black where you have expansion
connectors which could have multiple useful configurations, or be
changed on a hotpluggable attachment.

Even if your bootloader doesn't use the pins, it should at least set
them up for later, for electrical reasons. If the default direction
for a gpio mux is output, who knows what it is for a custom peripheral
mux, and if your peripheral has an output on that pin.. then you're
driving outputs from both sides, which is nonsensical. If the default
direction is input, and your peripheral is an input, you can set up
conditions where essentially the value floats (or at least is
unpredictable) at the peripheral side.

Your hardware guy will usually slap you for leaving it in the wrong
state for the time it takes to load Linux from SD card or SPI NOR, and
decompress, and get to several seconds into the boot process before
configuring pin muxing, or even to past the init daemon, so it can do
a module or firmware load first.

> But there is another case. One SoC pin may be connected to several external
> devices, e.g. through an external analog mux. In that case, the bootloader may
> configure the SoC mux and pad for one usage, while the kernel may afterwards
> change only the mux configuration of this pin for the other usage.

This is the ONLY use case it makes any sense in a static
configuration, but at this point, why not just supply the required
value, even if it's the chip default? If the configuration for "kernel
time" is fixed, the bootloader should quiesce the device and configure
it for the kernel to match the device tree. There are a bunch of
configurations on i.MX51 where the DT is redefining the default pinmux
for the SoC... pointlessly..

> So NO_PAD_CTRL is not strictly required for pinctrl, but it can be handy.

Given that 0x0 (or 0x80000000) takes up exactly the same amount of
space and time to research and put in the tree as the ACTUAL required
pad settings from the manual or empirically supplied, supplying the
pad settings makes more sense (and is far, far more descriptive).
There is a case here for cross-checking ALL DT hardware configuration
details against the actual configuration of the hardware, and
supplying a weird pinctrl definition (for instance, setting a RAZ bit
as 1, or a RAO bit to 0) could be caught this way (there are *lots* of
pin definitions out there in the world for i.MX at least which are
doing this, some of them in the Boot ROM DCD table...)
Benoît Thébaudeau Oct. 23, 2013, 12:17 a.m. UTC | #6
Hi Matt,

On Wednesday, October 23, 2013 1:06:44 AM, Matt Sealey wrote:
> On Tue, Oct 22, 2013 at 5:42 PM, Benoît Thébaudeau
> <benoit.thebaudeau@advansee.com> wrote:
> > Dear Sascha Hauer,
> >
> > On Tuesday, October 22, 2013 11:57:47 PM, Sascha Hauer wrote:
> >> > >
> >> > > 0 is definitely wrong here. We have 0x80000000 for "Don't touch
> >> > > padctrl", but otherwise this should contain some real padctrl
> >> > > settings.
> >> >
> >> > A more pressing question is in what world did the bootloader not
> >> > already set these pins up and if they are already set up, why are they
> >> > loitering in the device tree?
> >>
> >> Having NO_PAD_CTRL in the devicetree doesn't make sense, you're right.
> >> Either a pin has to be configured by the bootloader completely or not at
> >> all. Having the mux configured by the kernel and the drive strength by
> >> the bootloader is broken by design. All pins should have a complete
> >> padctrl setup and NO_PAD_CTRL should be dropped.
> >
> > Some pins may be configured completely by the kernel, and not at all by the
> > bootloader. In that case, the device tree may wish to keep the SoC reset
> > pad
> > configuration. NO_PAD_CTRL is useful in that case, although one might argue
> > that
> > the reset values should not be trusted for various reasons.
> 
> No, that's totally illogical. The magic bit to "leave my pad controls
> alone" makes no sense since you're re-configuring mux mode and input
> select (even if they're the same as the bootloader, you're doing it
> twice).
> 
> There are two reasons this gets done, both of which are very very odd..
> 
> 1) The definition of the pinmux tuple of 3 register/value pairs is
> ordered such that the pad control can be set independently of the
> preprocessor definition for the pad (which supplies the first 5 values
> in the 6 value pin definition). This is weird since one of the values
> doesn't belong to the register the value pair is intended for, and
> saying "do not
> 
> 2) The reason we have to define the three registers even if we "don't
> change values" is so users (or userspace) can fiddle with the GPIOs
> through some never-used non-existent (since it doesn't expose these
> values) sysfs pinmuxing API.
> 
> In this case, you don't "set the pad control to 'do not touch'" but
> supply the exact values from the manual or empirically derived from
> inspection at SoC power on. There is ZERO use for a 'do not touch'
> bit. In the case where it turns out the bootloader is wrong, you'll
> have to supply a value here anyway. So supply the default.

That makes sense, but it has the disadvantage of being more error prone than
keeping reset values if they're fine, because errors in pad settings may lead to
bugs less obvious than bugs in mux settings. On the other hand, forcing the
developers to check all pad settings has the advantage of not using reset values
that may be wrong for some applications. All in all, removing NO_PAD_CTRL seems
better here.

> In reality what would happen is the binding gets a kick in the pants
> so it makes more sense, and we define that the bootloader MUST set up
> all static pin multiplexing configuration at the earliest opportunity
> and save Linux the hassle. pinctrl is for microcontrollers and
> robotics boards - Arduino or BeagleBone Black where you have expansion
> connectors which could have multiple useful configurations, or be
> changed on a hotpluggable attachment.
> 
> Even if your bootloader doesn't use the pins, it should at least set
> them up for later, for electrical reasons. If the default direction
> for a gpio mux is output, who knows what it is for a custom peripheral
> mux, and if your peripheral has an output on that pin.. then you're
> driving outputs from both sides, which is nonsensical. If the default
> direction is input, and your peripheral is an input, you can set up
> conditions where essentially the value floats (or at least is
> unpredictable) at the peripheral side.
> 
> Your hardware guy will usually slap you for leaving it in the wrong
> state for the time it takes to load Linux from SD card or SPI NOR, and
> decompress, and get to several seconds into the boot process before
> configuring pin muxing, or even to past the init daemon, so it can do
> a module or firmware load first.

I agree, except that a properly designed board should not connect pins set as
outputs upon reset to the outputs of external devices, since this leaves
potential electrical level conflicts from reset at least until the software
cleans the mess. However, you're right that such things happen and should be
handled as you say.

But if some SoC pins are used as inputs, set up as inputs by the SoC reset but
with the wrong mux value, and unused by the bootloader, there is no reason to
set up these pins in the bootloader rather than in the DT. Same for output pins
connected to a bus disabled by default and unused in the bootloader. Actually,
the bootloader really has to set up pins because it uses them, or for electrical
reasons, or to disable some unused feature, but the two latter are most of the
time an exception among all connected pins.

> > But there is another case. One SoC pin may be connected to several external
> > devices, e.g. through an external analog mux. In that case, the bootloader
> > may
> > configure the SoC mux and pad for one usage, while the kernel may
> > afterwards
> > change only the mux configuration of this pin for the other usage.
> 
> This is the ONLY use case it makes any sense in a static
> configuration, but at this point, why not just supply the required
> value, even if it's the chip default? If the configuration for "kernel
> time" is fixed, the bootloader should quiesce the device and configure
> it for the kernel to match the device tree. There are a bunch of
> configurations on i.MX51 where the DT is redefining the default pinmux
> for the SoC... pointlessly..

This is a possibility.

> > So NO_PAD_CTRL is not strictly required for pinctrl, but it can be handy.
> 
> Given that 0x0 (or 0x80000000) takes up exactly the same amount of
> space and time to research and put in the tree as the ACTUAL required
> pad settings from the manual or empirically supplied, supplying the
> pad settings makes more sense (and is far, far more descriptive).
> There is a case here for cross-checking ALL DT hardware configuration
> details against the actual configuration of the hardware, and
> supplying a weird pinctrl definition (for instance, setting a RAZ bit
> as 1, or a RAO bit to 0) could be caught this way (there are *lots* of
> pin definitions out there in the world for i.MX at least which are
> doing this, some of them in the Boot ROM DCD table...)

Agreed. It may be better to drop NO_PAD_CTRL from DT, but I still find debatable
whether to compel to move all pin configurations to the bootloader for static
configurations, since the less the kernel has to rely on the bootloader, the
more reliable and self-sufficient it is (with DT).

Note that in the patch above, 0 may be intended to be the actual pad setting,
and not to mean by mistake NO_PAD_CTRL.

Best regards,
Benoît
Greg Ungerer Oct. 23, 2013, 5:40 a.m. UTC | #7
Hi Sascha,

n 22/10/13 22:35, Sascha Hauer wrote:
> On Fri, Oct 18, 2013 at 04:04:19PM +1000, gerg@uclinux.org wrote:
>> From: Greg Ungerer <gerg@uclinux.org>
>>
>> Create device tree bindings for the Freescale IMX50 SoC. This was based on
>> the IMX53 bindings with changes made as necessary.
>>
>> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
>> ---
>> +
>> +			iomuxc: iomuxc@53fa8000 {
>> +				compatible = "fsl,imx50-iomuxc";
>> +				reg = <0x53fa8000 0x4000>;
>> +
>> +				fec {
>> +					pinctrl_fec_1: fecgrp-1 {
>> +						fsl,pins = <
>> +							MX50_PAD_SSI_RXFS__FEC_MDC	0x80
>> +							MX50_PAD_SSI_RXC__FEC_MDIO	0x80
>> +							MX50_PAD_DISP_D0__FEC_TX_CLK	0x80
>> +							MX50_PAD_DISP_D1__FEC_RX_ERR	0x80
>> +							MX50_PAD_DISP_D2__FEC_RX_DV	0x80
>> +							MX50_PAD_DISP_D3__FEC_RDATA_1	0x80
>> +							MX50_PAD_DISP_D4__FEC_RDATA_0	0x80
>> +							MX50_PAD_DISP_D5__FEC_TX_EN	0x80
>> +							MX50_PAD_DISP_D6__FEC_TDATA_1	0x80
>> +							MX50_PAD_DISP_D7__FEC_TDATA_0	0x80
>> +						>;
>> +					};
>> +
> 
> Shawn recently removed the pinctrl groups here and referenced to this
> node by doing
> 
> &iomuxc {
> 	fec {
> 		...
> 	};
> };

Ok, I'll move them in the same way then.


>> +				cspi {
>> +					pinctrl_cspi_1: cspigrp-1 {
>> +						fsl,pins = <
>> +							MX50_PAD_CSPI_SCLK__CSPI_SCLK	0
> 
> 0 is definitely wrong here. We have 0x80000000 for "Don't touch
> padctrl", but otherwise this should contain some real padctrl settings.

0 was the intended value here. Although I am happy to hear arguments that 
it is not the correct value.

0 in this case translates to:

   . Low driver strength
   . Open drain disabled
   . Pull/Keep disabled

If the norm is to leave what the boot loader set then I will change them
to 0x80000000. Is the net out of the discussion that followed to do this?

Thanks for looking over the patches.

Regards
Greg
Greg Ungerer Oct. 23, 2013, 5:55 a.m. UTC | #8
Hi Benoit, Matt, Sascha,

On 23/10/13 10:17, Benoît Thébaudeau wrote:
> Hi Matt,
> 
> On Wednesday, October 23, 2013 1:06:44 AM, Matt Sealey wrote:
>> On Tue, Oct 22, 2013 at 5:42 PM, Benoît Thébaudeau
>> <benoit.thebaudeau@advansee.com> wrote:
>>> Dear Sascha Hauer,
>>>
>>> On Tuesday, October 22, 2013 11:57:47 PM, Sascha Hauer wrote:
>>>>>>
>>>>>> 0 is definitely wrong here. We have 0x80000000 for "Don't touch
>>>>>> padctrl", but otherwise this should contain some real padctrl
>>>>>> settings.
>>>>>
>>>>> A more pressing question is in what world did the bootloader not
>>>>> already set these pins up and if they are already set up, why are they
>>>>> loitering in the device tree?
>>>>
>>>> Having NO_PAD_CTRL in the devicetree doesn't make sense, you're right.
>>>> Either a pin has to be configured by the bootloader completely or not at
>>>> all. Having the mux configured by the kernel and the drive strength by
>>>> the bootloader is broken by design. All pins should have a complete
>>>> padctrl setup and NO_PAD_CTRL should be dropped.
>>>
>>> Some pins may be configured completely by the kernel, and not at all by the
>>> bootloader. In that case, the device tree may wish to keep the SoC reset
>>> pad
>>> configuration. NO_PAD_CTRL is useful in that case, although one might argue
>>> that
>>> the reset values should not be trusted for various reasons.
>>
>> No, that's totally illogical. The magic bit to "leave my pad controls
>> alone" makes no sense since you're re-configuring mux mode and input
>> select (even if they're the same as the bootloader, you're doing it
>> twice).
>>
>> There are two reasons this gets done, both of which are very very odd..
>>
>> 1) The definition of the pinmux tuple of 3 register/value pairs is
>> ordered such that the pad control can be set independently of the
>> preprocessor definition for the pad (which supplies the first 5 values
>> in the 6 value pin definition). This is weird since one of the values
>> doesn't belong to the register the value pair is intended for, and
>> saying "do not
>>
>> 2) The reason we have to define the three registers even if we "don't
>> change values" is so users (or userspace) can fiddle with the GPIOs
>> through some never-used non-existent (since it doesn't expose these
>> values) sysfs pinmuxing API.
>>
>> In this case, you don't "set the pad control to 'do not touch'" but
>> supply the exact values from the manual or empirically derived from
>> inspection at SoC power on. There is ZERO use for a 'do not touch'
>> bit. In the case where it turns out the bootloader is wrong, you'll
>> have to supply a value here anyway. So supply the default.
> 
> That makes sense, but it has the disadvantage of being more error prone than
> keeping reset values if they're fine, because errors in pad settings may lead to
> bugs less obvious than bugs in mux settings. On the other hand, forcing the
> developers to check all pad settings has the advantage of not using reset values
> that may be wrong for some applications. All in all, removing NO_PAD_CTRL seems
> better here.
> 
>> In reality what would happen is the binding gets a kick in the pants
>> so it makes more sense, and we define that the bootloader MUST set up
>> all static pin multiplexing configuration at the earliest opportunity
>> and save Linux the hassle. pinctrl is for microcontrollers and
>> robotics boards - Arduino or BeagleBone Black where you have expansion
>> connectors which could have multiple useful configurations, or be
>> changed on a hotpluggable attachment.
>>
>> Even if your bootloader doesn't use the pins, it should at least set
>> them up for later, for electrical reasons. If the default direction
>> for a gpio mux is output, who knows what it is for a custom peripheral
>> mux, and if your peripheral has an output on that pin.. then you're
>> driving outputs from both sides, which is nonsensical. If the default
>> direction is input, and your peripheral is an input, you can set up
>> conditions where essentially the value floats (or at least is
>> unpredictable) at the peripheral side.
>>
>> Your hardware guy will usually slap you for leaving it in the wrong
>> state for the time it takes to load Linux from SD card or SPI NOR, and
>> decompress, and get to several seconds into the boot process before
>> configuring pin muxing, or even to past the init daemon, so it can do
>> a module or firmware load first.
> 
> I agree, except that a properly designed board should not connect pins set as
> outputs upon reset to the outputs of external devices, since this leaves
> potential electrical level conflicts from reset at least until the software
> cleans the mess. However, you're right that such things happen and should be
> handled as you say.
> 
> But if some SoC pins are used as inputs, set up as inputs by the SoC reset but
> with the wrong mux value, and unused by the bootloader, there is no reason to
> set up these pins in the bootloader rather than in the DT. Same for output pins
> connected to a bus disabled by default and unused in the bootloader. Actually,
> the bootloader really has to set up pins because it uses them, or for electrical
> reasons, or to disable some unused feature, but the two latter are most of the
> time an exception among all connected pins.
> 
>>> But there is another case. One SoC pin may be connected to several external
>>> devices, e.g. through an external analog mux. In that case, the bootloader
>>> may
>>> configure the SoC mux and pad for one usage, while the kernel may
>>> afterwards
>>> change only the mux configuration of this pin for the other usage.
>>
>> This is the ONLY use case it makes any sense in a static
>> configuration, but at this point, why not just supply the required
>> value, even if it's the chip default? If the configuration for "kernel
>> time" is fixed, the bootloader should quiesce the device and configure
>> it for the kernel to match the device tree. There are a bunch of
>> configurations on i.MX51 where the DT is redefining the default pinmux
>> for the SoC... pointlessly..
> 
> This is a possibility.
> 
>>> So NO_PAD_CTRL is not strictly required for pinctrl, but it can be handy.
>>
>> Given that 0x0 (or 0x80000000) takes up exactly the same amount of
>> space and time to research and put in the tree as the ACTUAL required
>> pad settings from the manual or empirically supplied, supplying the
>> pad settings makes more sense (and is far, far more descriptive).
>> There is a case here for cross-checking ALL DT hardware configuration
>> details against the actual configuration of the hardware, and
>> supplying a weird pinctrl definition (for instance, setting a RAZ bit
>> as 1, or a RAO bit to 0) could be caught this way (there are *lots* of
>> pin definitions out there in the world for i.MX at least which are
>> doing this, some of them in the Boot ROM DCD table...)
> 
> Agreed. It may be better to drop NO_PAD_CTRL from DT, but I still find debatable
> whether to compel to move all pin configurations to the bootloader for static
> configurations, since the less the kernel has to rely on the bootloader, the
> more reliable and self-sufficient it is (with DT).
> 
> Note that in the patch above, 0 may be intended to be the actual pad setting,
> and not to mean by mistake NO_PAD_CTRL.

And 0 was the intended pad control setting here. I wasn't even aware of
NO_PAD_CTRL.

Regards
Greg
Shawn Guo Oct. 24, 2013, 3:26 p.m. UTC | #9
On Fri, Oct 18, 2013 at 04:04:19PM +1000, gerg@uclinux.org wrote:
> From: Greg Ungerer <gerg@uclinux.org>
> 
> Create device tree bindings for the Freescale IMX50 SoC. This was based on

I think it's a device tree source rather than bindings.

> the IMX53 bindings with changes made as necessary.
> 
> Signed-off-by: Greg Ungerer <gerg@uclinux.org>
> ---
>  arch/arm/boot/dts/imx50.dtsi | 659 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 659 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx50.dtsi
> 
> diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
> new file mode 100644
> index 0000000..c4de7ac
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx50.dtsi
> @@ -0,0 +1,659 @@
> +/*
> + * Copyright 2013 Greg Ungerer <gerg@uclinux.org>
> + * Copyright 2011 Freescale Semiconductor, Inc.
> + * Copyright 2011 Linaro Ltd.
> + *
> + * The code contained herein is licensed under the GNU General Public
> + * License. You may obtain a copy of the GNU General Public License
> + * Version 2 or later at the following locations:
> + *
> + * http://www.opensource.org/licenses/gpl-license.html
> + * http://www.gnu.org/copyleft/gpl.html
> + */
> +
> +#include "skeleton.dtsi"
> +#include "imx50-pinfunc.h"
> +
> +/ {
> +	aliases {
> +		serial0 = &uart1;
> +		serial1 = &uart2;
> +		serial2 = &uart3;
> +		serial3 = &uart4;
> +		serial4 = &uart5;
> +		gpio0 = &gpio1;
> +		gpio1 = &gpio2;
> +		gpio2 = &gpio3;
> +		gpio3 = &gpio4;
> +		gpio4 = &gpio5;
> +		gpio5 = &gpio6;

Please sort these aliases alphabetically.

Shawn

> +	};
Matt Sealey Oct. 24, 2013, 9:13 p.m. UTC | #10
On Tue, Oct 22, 2013 at 7:17 PM, Benoît Thébaudeau
<benoit.thebaudeau@advansee.com> wrote:
> Hi Matt,

Hello Benoît,

> On Wednesday, October 23, 2013 1:06:44 AM, Matt Sealey wrote:
>
>> No, that's totally illogical. The magic bit to "leave my pad controls
>> alone" makes no sense since you're re-configuring mux mode and input
>> select (even if they're the same as the bootloader, you're doing it
>> twice).
>>
>> There are two reasons this gets done, both of which are very very odd..
>>
>> 1) The definition of the pinmux tuple of 3 register/value pairs is
>> ordered such that the pad control can be set independently of the
>> preprocessor definition for the pad (which supplies the first 5 values
>> in the 6 value pin definition). This is weird since one of the values
>> doesn't belong to the register the value pair is intended for, and
>> saying "do not
>>
>> 2) The reason we have to define the three registers even if we "don't
>> change values" is so users (or userspace) can fiddle with the GPIOs
>> through some never-used non-existent (since it doesn't expose these
>> values) sysfs pinmuxing API.
>>
>> In this case, you don't "set the pad control to 'do not touch'" but
>> supply the exact values from the manual or empirically derived from
>> inspection at SoC power on. There is ZERO use for a 'do not touch'
>> bit. In the case where it turns out the bootloader is wrong, you'll
>> have to supply a value here anyway. So supply the default.
>
> That makes sense, but it has the disadvantage of being more error prone than
> keeping reset values if they're fine, because errors in pad settings may lead to
> bugs less obvious than bugs in mux settings.

That is true, but programmer error is programmer error.. you can
introduce it on either side.

The major use right now for "I want my pads not to be touched" is to
get past the restriction that pinctrl MUST be specified (and people
don't like adding empty, dummy entries) and to make trees more
readable (but kilobytes bigger), and the use cases where it seems
expected to redefine pad settings that are either chip defaults on POR
(hence the special pad control bit) or that the bootloader can
absolutely be guaranteed to have done, for some reason.

If you MUST specify a pinmux, specify the whole damn thing - mux,
input and pad. Not a half-way mux, input, "pad register but please
ignore me."

Having fewer ways of defining it means having fewer things to expect
when debugging.

> developers to check all pad settings has the advantage of not using reset values
> that may be wrong for some applications. All in all, removing NO_PAD_CTRL seems
> better here.

Thanks.

>> Your hardware guy will usually slap you for leaving it in the wrong
>> state for the time it takes to load Linux from SD card or SPI NOR, and
>> decompress, and get to several seconds into the boot process before
>> configuring pin muxing, or even to past the init daemon, so it can do
>> a module or firmware load first.
>
> I agree, except that a properly designed board should not connect pins set as
> outputs upon reset to the outputs of external devices, since this leaves
> potential electrical level conflicts from reset at least until the software
> cleans the mess.

True, but you cannot just say "well all my IOs on my chip default to
GPIO inputs so I cannot connect any peripheral inputs to any of them
until I have configured them". Sometimes you are at the mercy of your
SoC designer and peripheral designer, and the pad layout of your chip.

> However, you're right that such things happen and should be
> handled as you say.
>
> But if some SoC pins are used as inputs, set up as inputs by the SoC reset but
> with the wrong mux value, and unused by the bootloader, there is no reason to
> set up these pins in the bootloader rather than in the DT.

There's all the reason in the world; this SHOULD be the preferred,
default practice.

> connected to a bus disabled by default and unused in the bootloader. Actually,
> the bootloader really has to set up pins because it uses them, or for electrical
> reasons, or to disable some unused feature, but the two latter are most of the
> time an exception among all connected pins.

To give the Efika MX as an example, the bootloader needs to set up
maybe 60-70 out of a possible 80-90 pins (DDR in the DCD or BootROM
plugin, PATA, SD card (twice on some configs), LEDs, board id inputs,
pmic interrupt, spi + chipselects, uart, usb, i2c, clock outs, reset
toggles for all of the above peripherals to effectively make sure
everything is quiesced for the OS).

The things not really set up - video and audio. But a bootloader could
support video, so you could expect this will need to be set up
eventually. It may even play a sound at a future point.

When you are left with basically adding 10 or 15 more pins, and you
already implemented 70.. this is not really a difficult task. On i.MX
at least, you have the opportunity to do a lot of this inside the boot
ROM stage, and considering you want to give user interactivity,
multiple boot sources, and be sure the system is running and powered
okay before you get to the OS, you're stuck doing it.

You could do the bare minimum internally but externally - when it goes
upstream in U-Boot or something - it should all be there. Any pin
definitions in DT would be firmware bug workarounds or dynamically
configured items which can ONLY be determined at runtime (DDR mode or
voltage level selects on SDHC for example) and that are safe to
change. It is not true on i.MX, but on some SoC in the world, changing
pad mux is NOT going to be glitchless and encouraging huge amounts of
reconfiguration at OS boot time is possibly the worst thing ever.

> Agreed. It may be better to drop NO_PAD_CTRL from DT, but I still find debatable
> whether to compel to move all pin configurations to the bootloader for static
> configurations, since the less the kernel has to rely on the bootloader, the
> more reliable and self-sufficient it is (with DT).

There's a status quo of not doing it, and putting the definitions in
Linux, but I am just saying it should be encouraged to do this in the
bootloader and save the device tree space (and hassle) and reserve the
tree definitions for when you REALLY have no other choice in doing so.

I would prefer bootloaders were written with as much care as the DT should be ;)

I can't enforce it, but it can be made to be something developers
might get a 'gold star' and a smile, if they do so.

> Note that in the patch above, 0 may be intended to be the actual pad setting,
> and not to mean by mistake NO_PAD_CTRL.

It could be, but 0 for a pad setting doesn't make a lot of sense for
the vast majority of pins.. in this case, it kind of does.

In this case, I'd prefer 0x00000000 rather than just a 0. The DTC is
going to expand it to a 32-bit cell anyway.. but it'd go a long way to
knowing the intent.. (this is kind of why I don't like the idea of
specifying the pad settings and preprocessor macros, it hides
implementation details which give information to reviewers about what
they're looking at..)

--
Matt Sealey <neko@bakuhatsu.net>
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
new file mode 100644
index 0000000..c4de7ac
--- /dev/null
+++ b/arch/arm/boot/dts/imx50.dtsi
@@ -0,0 +1,659 @@ 
+/*
+ * Copyright 2013 Greg Ungerer <gerg@uclinux.org>
+ * Copyright 2011 Freescale Semiconductor, Inc.
+ * Copyright 2011 Linaro Ltd.
+ *
+ * The code contained herein is licensed under the GNU General Public
+ * License. You may obtain a copy of the GNU General Public License
+ * Version 2 or later at the following locations:
+ *
+ * http://www.opensource.org/licenses/gpl-license.html
+ * http://www.gnu.org/copyleft/gpl.html
+ */
+
+#include "skeleton.dtsi"
+#include "imx50-pinfunc.h"
+
+/ {
+	aliases {
+		serial0 = &uart1;
+		serial1 = &uart2;
+		serial2 = &uart3;
+		serial3 = &uart4;
+		serial4 = &uart5;
+		gpio0 = &gpio1;
+		gpio1 = &gpio2;
+		gpio2 = &gpio3;
+		gpio3 = &gpio4;
+		gpio4 = &gpio5;
+		gpio5 = &gpio6;
+	};
+
+	tzic: tz-interrupt-controller@0fffc000 {
+		compatible = "fsl,imx50-tzic", "fsl,tzic";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		reg = <0x0fffc000 0x4000>;
+	};
+
+	clocks {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		ckil {
+			compatible = "fsl,imx-ckil", "fixed-clock";
+			clock-frequency = <32768>;
+		};
+
+		ckih1 {
+			compatible = "fsl,imx-ckih1", "fixed-clock";
+			clock-frequency = <22579200>;
+		};
+
+		ckih2 {
+			compatible = "fsl,imx-ckih2", "fixed-clock";
+			clock-frequency = <0>;
+		};
+
+		osc {
+			compatible = "fsl,imx-osc", "fixed-clock";
+			clock-frequency = <24000000>;
+		};
+	};
+
+	soc {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "simple-bus";
+		interrupt-parent = <&tzic>;
+		ranges;
+
+		aips@50000000 { /* AIPS1 */
+			compatible = "fsl,aips-bus", "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0x50000000 0x10000000>;
+			ranges;
+
+			spba@50000000 {
+				compatible = "fsl,spba-bus", "simple-bus";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				reg = <0x50000000 0x40000>;
+				ranges;
+
+				esdhc1: esdhc@50004000 {
+					compatible = "fsl,imx50-esdhc";
+					reg = <0x50004000 0x4000>;
+					interrupts = <1>;
+					clocks = <&clks 44>, <&clks 0>, <&clks 71>;
+					clock-names = "ipg", "ahb", "per";
+					bus-width = <4>;
+					status = "disabled";
+				};
+
+				esdhc2: esdhc@50008000 {
+					compatible = "fsl,imx50-esdhc";
+					reg = <0x50008000 0x4000>;
+					interrupts = <2>;
+					clocks = <&clks 45>, <&clks 0>, <&clks 72>;
+					clock-names = "ipg", "ahb", "per";
+					bus-width = <4>;
+					status = "disabled";
+				};
+
+				uart3: serial@5000c000 {
+					compatible = "fsl,imx50-uart", "fsl,imx21-uart";
+					reg = <0x5000c000 0x4000>;
+					interrupts = <33>;
+					clocks = <&clks 32>, <&clks 33>;
+					clock-names = "ipg", "per";
+					status = "disabled";
+				};
+
+				ecspi1: ecspi@50010000 {
+					#address-cells = <1>;
+					#size-cells = <0>;
+					compatible = "fsl,imx50-ecspi", "fsl,imx51-ecspi";
+					reg = <0x50010000 0x4000>;
+					interrupts = <36>;
+					clocks = <&clks 51>, <&clks 52>;
+					clock-names = "ipg", "per";
+					status = "disabled";
+				};
+
+				ssi2: ssi@50014000 {
+					compatible = "fsl,imx50-ssi", "fsl,imx21-ssi";
+					reg = <0x50014000 0x4000>;
+					interrupts = <30>;
+					clocks = <&clks 49>;
+					fsl,fifo-depth = <15>;
+					fsl,ssi-dma-events = <25 24 23 22>; /* TX0 RX0 TX1 RX1 */
+					status = "disabled";
+				};
+
+				esdhc3: esdhc@50020000 {
+					compatible = "fsl,imx50-esdhc";
+					reg = <0x50020000 0x4000>;
+					interrupts = <3>;
+					clocks = <&clks 46>, <&clks 0>, <&clks 73>;
+					clock-names = "ipg", "ahb", "per";
+					bus-width = <4>;
+					status = "disabled";
+				};
+
+				esdhc4: esdhc@50024000 {
+					compatible = "fsl,imx50-esdhc";
+					reg = <0x50024000 0x4000>;
+					interrupts = <4>;
+					clocks = <&clks 47>, <&clks 0>, <&clks 74>;
+					clock-names = "ipg", "ahb", "per";
+					bus-width = <4>;
+					status = "disabled";
+				};
+			};
+
+			usbotg: usb@53f80000 {
+				compatible = "fsl,imx50-usb", "fsl,imx27-usb";
+				reg = <0x53f80000 0x0200>;
+				interrupts = <18>;
+				clocks = <&clks 124>;
+				status = "disabled";
+			};
+
+			usbh1: usb@53f80200 {
+				compatible = "fsl,imx50-usb", "fsl,imx27-usb";
+				reg = <0x53f80200 0x0200>;
+				interrupts = <14>;
+				clocks = <&clks 125>;
+				status = "disabled";
+			};
+
+			usbh2: usb@53f80400 {
+				compatible = "fsl,imx50-usb", "fsl,imx27-usb";
+				reg = <0x53f80400 0x0200>;
+				interrupts = <16>;
+				clocks = <&clks 108>;
+				status = "disabled";
+			};
+
+			usbh3: usb@53f80600 {
+				compatible = "fsl,imx50-usb", "fsl,imx27-usb";
+				reg = <0x53f80600 0x0200>;
+				interrupts = <17>;
+				clocks = <&clks 108>;
+				status = "disabled";
+			};
+
+			gpio1: gpio@53f84000 {
+				compatible = "fsl,imx50-gpio", "fsl,imx35-gpio";
+				reg = <0x53f84000 0x4000>;
+				interrupts = <50 51>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
+
+			gpio2: gpio@53f88000 {
+				compatible = "fsl,imx50-gpio", "fsl,imx35-gpio";
+				reg = <0x53f88000 0x4000>;
+				interrupts = <52 53>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
+
+			gpio3: gpio@53f8c000 {
+				compatible = "fsl,imx50-gpio", "fsl,imx35-gpio";
+				reg = <0x53f8c000 0x4000>;
+				interrupts = <54 55>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
+
+			gpio4: gpio@53f90000 {
+				compatible = "fsl,imx50-gpio", "fsl,imx35-gpio";
+				reg = <0x53f90000 0x4000>;
+				interrupts = <56 57>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
+
+			wdog1: wdog@53f98000 {
+				compatible = "fsl,imx50-wdt", "fsl,imx21-wdt";
+				reg = <0x53f98000 0x4000>;
+				interrupts = <58>;
+				clocks = <&clks 0>;
+			};
+
+			gpt: timer@53fa0000 {
+				compatible = "fsl,imx50-gpt", "fsl,imx31-gpt";
+				reg = <0x53fa0000 0x4000>;
+				interrupts = <39>;
+				clocks = <&clks 36>, <&clks 41>;
+				clock-names = "ipg", "per";
+			};
+
+			iomuxc: iomuxc@53fa8000 {
+				compatible = "fsl,imx50-iomuxc";
+				reg = <0x53fa8000 0x4000>;
+
+				fec {
+					pinctrl_fec_1: fecgrp-1 {
+						fsl,pins = <
+							MX50_PAD_SSI_RXFS__FEC_MDC	0x80
+							MX50_PAD_SSI_RXC__FEC_MDIO	0x80
+							MX50_PAD_DISP_D0__FEC_TX_CLK	0x80
+							MX50_PAD_DISP_D1__FEC_RX_ERR	0x80
+							MX50_PAD_DISP_D2__FEC_RX_DV	0x80
+							MX50_PAD_DISP_D3__FEC_RDATA_1	0x80
+							MX50_PAD_DISP_D4__FEC_RDATA_0	0x80
+							MX50_PAD_DISP_D5__FEC_TX_EN	0x80
+							MX50_PAD_DISP_D6__FEC_TDATA_1	0x80
+							MX50_PAD_DISP_D7__FEC_TDATA_0	0x80
+						>;
+					};
+
+					pinctrl_fec_2: fecgrp-2 {
+						fsl,pins = <
+							MX50_PAD_I2C3_SCL__FEC_MDC	0x80
+							MX50_PAD_I2C3_SDA__FEC_MDIO	0x80
+							MX50_PAD_DISP_D0__FEC_TX_CLK	0x80
+							MX50_PAD_DISP_D10__FEC_RX_DV	0x80
+							MX50_PAD_DISP_D11__FEC_RDATA_1	0x80
+							MX50_PAD_DISP_D12__FEC_RDATA_0	0x80
+							MX50_PAD_DISP_D13__FEC_TX_EN	0x80
+							MX50_PAD_DISP_D14__FEC_TDATA_1	0x80
+							MX50_PAD_DISP_D15__FEC_TDATA_0	0x80
+						>;
+					};
+
+				};
+
+				cspi {
+					pinctrl_cspi_1: cspigrp-1 {
+						fsl,pins = <
+							MX50_PAD_CSPI_SCLK__CSPI_SCLK	0
+							MX50_PAD_CSPI_MISO__CSPI_MISO	0
+							MX50_PAD_CSPI_MOSI__CSPI_MOSI	0
+							MX50_PAD_CSPI_SS0__GPIO4_11	0xc4
+							MX50_PAD_ECSPI1_MOSI__CSPI_SS1	0xf4
+						>;
+					};
+				};
+
+				ecspi1 {
+					pinctrl_ecspi1_1: ecspi1grp-1 {
+						fsl,pins = <
+							MX50_PAD_ECSPI1_SCLK__ECSPI1_SCLK 0
+							MX50_PAD_ECSPI1_SS0__ECSPI1_SS0   0
+							MX50_PAD_ECSPI1_MISO__ECSPI1_MISO 0
+							MX50_PAD_ECSPI1_MOSI__ECSPI1_MOSI 0
+						>;
+					};
+				};
+
+				esdhc1 {
+					pinctrl_esdhc1_1: esdhc1grp-1 {
+						fsl,pins = <
+							MX50_PAD_SD1_D0__ESDHC1_DAT0    0
+							MX50_PAD_SD1_D1__ESDHC1_DAT1    0
+							MX50_PAD_SD1_D2__ESDHC1_DAT2    0
+							MX50_PAD_SD1_D3__ESDHC1_DAT3    0
+							MX50_PAD_SD1_CMD__ESDHC1_CMD	0
+							MX50_PAD_SD1_CLK__ESDHC1_CLK	0
+						>;
+					};
+
+					pinctrl_esdhc1_2: esdhc1grp-2 {
+						fsl,pins = <
+							MX50_PAD_SD1_D0__ESDHC1_DAT0	0
+							MX50_PAD_SD1_D1__ESDHC1_DAT1	0
+							MX50_PAD_SD1_D2__ESDHC1_DAT2	0
+							MX50_PAD_SD1_D3__ESDHC1_DAT3	0
+							MX50_PAD_UART3_TXD__ESDHC1_DAT4	0
+							MX50_PAD_UART3_RXD__ESDHC1_DAT5	0
+							MX50_PAD_UART4_TXD__ESDHC1_DAT6	0
+							MX50_PAD_UART4_RXD__ESDHC1_DAT7	0
+							MX50_PAD_SD1_CMD__ESDHC1_CMD	0
+							MX50_PAD_SD1_CLK__ESDHC1_CLK	0
+						>;
+					};
+				};
+
+				esdhc2 {
+					pinctrl_esdhc2_1: esdhc2grp-1 {
+						fsl,pins = <
+							MX50_PAD_SD2_CMD__ESDHC2_CMD	0
+							MX50_PAD_SD2_CLK__ESDHC2_CLK	0
+							MX50_PAD_SD2_D0__ESDHC2_DAT0	0
+							MX50_PAD_SD2_D1__ESDHC2_DAT1	0
+							MX50_PAD_SD2_D2__ESDHC2_DAT2	0
+							MX50_PAD_SD2_D3__ESDHC2_DAT3	0
+							MX50_PAD_SD2_D4__ESDHC2_DAT4	0
+							MX50_PAD_SD2_D5__ESDHC2_DAT5	0
+							MX50_PAD_SD2_D6__ESDHC2_DAT6	0
+							MX50_PAD_SD2_D7__ESDHC2_DAT7	0
+						>;
+					};
+				};
+
+				esdhc3 {
+					pinctrl_esdhc3_1: esdhc3grp-1 {
+						fsl,pins = <
+							MX50_PAD_SD3_D0__ESDHC3_DAT0	0
+							MX50_PAD_SD3_D1__ESDHC3_DAT1	0
+							MX50_PAD_SD3_D2__ESDHC3_DAT2	0
+							MX50_PAD_SD3_D3__ESDHC3_DAT3	0
+							MX50_PAD_SD3_D4__ESDHC3_DAT4	0
+							MX50_PAD_SD3_D5__ESDHC3_DAT5	0
+							MX50_PAD_SD3_D6__ESDHC3_DAT6	0
+							MX50_PAD_SD3_D7__ESDHC3_DAT7	0
+							MX50_PAD_SD3_CMD__ESDHC3_CMD	0
+							MX50_PAD_SD3_CLK__ESDHC3_CLK	0
+						>;
+					};
+				};
+
+				i2c1 {
+					pinctrl_i2c1_1: i2c1grp-1 {
+						fsl,pins = <
+							MX50_PAD_I2C1_SDA__I2C1_SDA	0
+							MX50_PAD_I2C1_SCL__I2C1_SCL	0
+						>;
+					};
+				};
+
+				i2c2 {
+					pinctrl_i2c2_1: i2c2grp-1 {
+						fsl,pins = <
+							MX50_PAD_I2C2_SDA__I2C2_SDA	0
+							MX50_PAD_I2C2_SCL__I2C2_SCL	0
+						>;
+					};
+				};
+
+				i2c3 {
+					pinctrl_i2c3_1: i2c3grp-1 {
+						fsl,pins = <
+							MX50_PAD_I2C3_SDA__I2C3_SDA	0
+							MX50_PAD_I2C3_SCL__I2C3_SCL	0
+						>;
+					};
+				};
+
+				owire {
+					pinctrl_owire_1: owiregrp-1 {
+						fsl,pins = <
+							MX50_PAD_OWIRE__OWIRE_LINE	0
+						>;
+					};
+				};
+
+				uart1 {
+					pinctrl_uart1_1: uart1grp-1 {
+						fsl,pins = <
+							MX50_PAD_UART1_TXD__UART1_TXD_MUX 0
+							MX50_PAD_UART1_RXD__UART1_RXD_MUX 0
+							MX50_PAD_UART1_RTS__UART1_RTS     0
+							MX50_PAD_UART1_CTS__UART1_CTS     0
+						>;
+					};
+				};
+
+				uart2 {
+					pinctrl_uart2_1: uart2grp-1 {
+						fsl,pins = <
+							MX50_PAD_UART2_TXD__UART2_TXD_MUX 0
+							MX50_PAD_UART2_RXD__UART2_RXD_MUX 0
+							MX50_PAD_UART2_RTS__UART2_RTS     0
+							MX50_PAD_UART2_CTS__UART2_CTS     0
+						>;
+					};
+
+					pinctrl_uart2_2: uart2grp-2 {
+						fsl,pins = <
+							MX50_PAD_I2C1_SCL__UART2_TXD_MUX  0
+							MX50_PAD_I2C1_SDA__UART2_RXD_MUX  0
+							MX50_PAD_I2C2_SDA__UART2_RTS      0
+							MX50_PAD_I2C2_SCL__UART2_CTS      0
+						>;
+					};
+				};
+
+				uart3 {
+					pinctrl_uart3_1: uart3grp-1 {
+						fsl,pins = <
+							MX50_PAD_UART3_TXD__UART3_TXD_MUX 0
+							MX50_PAD_UART3_RXD__UART3_RXD_MUX 0
+							MX50_PAD_ECSPI1_SCLK__UART3_RTS	  0
+							MX50_PAD_ECSPI1_MOSI__UART3_CTS	  0
+						>;
+					};
+				};
+
+				uart4 {
+					pinctrl_uart4_1: uart4grp-1 {
+						fsl,pins = <
+							MX50_PAD_UART4_TXD__UART4_TXD_MUX 0
+							MX50_PAD_UART4_RXD__UART4_RXD_MUX 0
+							MX50_PAD_ECSPI1_MISO__UART4_RTS   0
+							MX50_PAD_ECSPI1_SS0__UART4_CTS    0
+						>;
+					};
+				};
+
+				uart5 {
+					pinctrl_uart5_1: uart5grp-1 {
+						fsl,pins = <
+							MX50_PAD_ECSPI2_MISO__UART5_TXD_MUX 0
+							MX50_PAD_ECSPI2_SS0__UART5_RXD_MUX  0
+							MX50_PAD_ECSPI2_SCLK__UART5_RTS     0
+							MX50_PAD_ECSPI2_MOSI__UART5_CTS     0
+						>;
+					};
+				};
+
+			};
+
+			gpr: iomuxc-gpr@53fa8000 {
+				compatible = "fsl,imx50-iomuxc-gpr", "syscon";
+				reg = <0x53fa8000 0xc>;
+			};
+
+			pwm1: pwm@53fb4000 {
+				#pwm-cells = <2>;
+				compatible = "fsl,imx50-pwm", "fsl,imx27-pwm";
+				reg = <0x53fb4000 0x4000>;
+				clocks = <&clks 37>, <&clks 38>;
+				clock-names = "ipg", "per";
+				interrupts = <61>;
+			};
+
+			pwm2: pwm@53fb8000 {
+				#pwm-cells = <2>;
+				compatible = "fsl,imx50-pwm", "fsl,imx27-pwm";
+				reg = <0x53fb8000 0x4000>;
+				clocks = <&clks 39>, <&clks 40>;
+				clock-names = "ipg", "per";
+				interrupts = <94>;
+			};
+
+			uart1: serial@53fbc000 {
+				compatible = "fsl,imx50-uart", "fsl,imx21-uart";
+				reg = <0x53fbc000 0x4000>;
+				interrupts = <31>;
+				clocks = <&clks 28>, <&clks 29>;
+				clock-names = "ipg", "per";
+				status = "disabled";
+			};
+
+			uart2: serial@53fc0000 {
+				compatible = "fsl,imx50-uart", "fsl,imx21-uart";
+				reg = <0x53fc0000 0x4000>;
+				interrupts = <32>;
+				clocks = <&clks 30>, <&clks 31>;
+				clock-names = "ipg", "per";
+				status = "disabled";
+			};
+
+			src: src@53fd0000 {
+				compatible = "fsl,imx50-src", "fsl,imx51-src";
+				reg = <0x53fd0000 0x4000>;
+				#reset-cells = <1>;
+			};
+
+			clks: ccm@53fd4000{
+				compatible = "fsl,imx50-ccm";
+				reg = <0x53fd4000 0x4000>;
+				interrupts = <0 71 0x04 0 72 0x04>;
+				#clock-cells = <1>;
+			};
+
+			gpio5: gpio@53fdc000 {
+				compatible = "fsl,imx50-gpio", "fsl,imx35-gpio";
+				reg = <0x53fdc000 0x4000>;
+				interrupts = <103 104>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
+
+			gpio6: gpio@53fe0000 {
+				compatible = "fsl,imx50-gpio", "fsl,imx35-gpio";
+				reg = <0x53fe0000 0x4000>;
+				interrupts = <105 106>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				interrupt-controller;
+				#interrupt-cells = <2>;
+			};
+
+			i2c3: i2c@53fec000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx50-i2c", "fsl,imx21-i2c";
+				reg = <0x53fec000 0x4000>;
+				interrupts = <64>;
+				clocks = <&clks 88>;
+				status = "disabled";
+			};
+
+			uart4: serial@53ff0000 {
+				compatible = "fsl,imx50-uart", "fsl,imx21-uart";
+				reg = <0x53ff0000 0x4000>;
+				interrupts = <13>;
+				clocks = <&clks 65>, <&clks 66>;
+				clock-names = "ipg", "per";
+				status = "disabled";
+			};
+		};
+
+		aips@60000000 {	/* AIPS2 */
+			compatible = "fsl,aips-bus", "simple-bus";
+			#address-cells = <1>;
+			#size-cells = <1>;
+			reg = <0x60000000 0x10000000>;
+			ranges;
+
+			uart5: serial@63f90000 {
+				compatible = "fsl,imx50-uart", "fsl,imx21-uart";
+				reg = <0x63f90000 0x4000>;
+				interrupts = <86>;
+				clocks = <&clks 67>, <&clks 68>;
+				clock-names = "ipg", "per";
+				status = "disabled";
+			};
+
+			owire: owire@63fa4000 {
+				compatible = "fsl,imx50-owire", "fsl,imx21-owire";
+				reg = <0x63fa4000 0x4000>;
+				clocks = <&clks 159>;
+				status = "disabled";
+			};
+
+			ecspi2: ecspi@63fac000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx50-ecspi", "fsl,imx51-ecspi";
+				reg = <0x63fac000 0x4000>;
+				interrupts = <37>;
+				clocks = <&clks 53>, <&clks 54>;
+				clock-names = "ipg", "per";
+				status = "disabled";
+			};
+
+			sdma: sdma@63fb0000 {
+				compatible = "fsl,imx50-sdma", "fsl,imx35-sdma";
+				reg = <0x63fb0000 0x4000>;
+				interrupts = <6>;
+				clocks = <&clks 56>, <&clks 56>;
+				clock-names = "ipg", "ahb";
+				fsl,sdma-ram-script-name = "imx/sdma/sdma-imx50.bin";
+			};
+
+			cspi: cspi@63fc0000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx50-cspi", "fsl,imx35-cspi";
+				reg = <0x63fc0000 0x4000>;
+				interrupts = <38>;
+				clocks = <&clks 55>, <&clks 55>;
+				clock-names = "ipg", "per";
+				status = "disabled";
+			};
+
+			i2c2: i2c@63fc4000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx50-i2c", "fsl,imx21-i2c";
+				reg = <0x63fc4000 0x4000>;
+				interrupts = <63>;
+				clocks = <&clks 35>;
+				status = "disabled";
+			};
+
+			i2c1: i2c@63fc8000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx50-i2c", "fsl,imx21-i2c";
+				reg = <0x63fc8000 0x4000>;
+				interrupts = <62>;
+				clocks = <&clks 34>;
+				status = "disabled";
+			};
+
+			ssi1: ssi@63fcc000 {
+				compatible = "fsl,imx50-ssi", "fsl,imx21-ssi";
+				reg = <0x63fcc000 0x4000>;
+				interrupts = <29>;
+				clocks = <&clks 48>;
+				fsl,fifo-depth = <15>;
+				fsl,ssi-dma-events = <29 28 27 26>; /* TX0 RX0 TX1 RX1 */
+				status = "disabled";
+			};
+
+			audmux: audmux@63fd0000 {
+				compatible = "fsl,imx50-audmux", "fsl,imx31-audmux";
+				reg = <0x63fd0000 0x4000>;
+				status = "disabled";
+			};
+
+			fec: ethernet@63fec000 {
+				compatible = "fsl,imx53-fec", "fsl,imx25-fec";
+				reg = <0x63fec000 0x4000>;
+				interrupts = <87>;
+				clocks = <&clks 42>, <&clks 42>, <&clks 42>;
+				clock-names = "ipg", "ahb", "ptp";
+				status = "disabled";
+			};
+		};
+	};
+};