[PATCH] MXS: Add DENX M28 dts file

[PATCH] MXS: Add DENX M28 dts file

[Marek Vasut]

Missing:
AUART
LCDIF

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
CC: Dong Aisheng <b29396@freescale.com>
CC: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
CC: Shawn Guo <shawn.guo@linaro.org>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
---
 arch/arm/boot/dts/imx28-m28.dts |  260 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 260 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx28-m28.dts

diff --git a/arch/arm/boot/dts/imx28-m28.dts b/arch/arm/boot/dts/imx28-m28.dts
new file mode 100644
index 0000000..7892020
--- /dev/null
+++ b/arch/arm/boot/dts/imx28-m28.dts
@@ -0,0 +1,260 @@
+/*
+ * Copyright (C) 2012 Marek Vasut <marex@denx.de>
+ *
+ * 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
+ */
+
+/dts-v1/;
+/include/ "imx28.dtsi"
+
+/ {
+	model = "DENX M28EVK";
+	compatible = "fsl,imx28-evk", "fsl,imx28";
+
+	memory {
+		reg = <0x40000000 0x08000000>;
+	};
+
+	apb at 80000000 {
+		apbh at 80000000 {
+			pinctrl at 80018000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx28-pinctrl", "simple-bus";
+				reg = <0x80018000 2000>;
+
+				gpmi_pins_data_m28: gpmi1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x0000 0x0010 0x0020
+							  0x0030 0x0040 0x0050
+							  0x0060 0x0070 0x0100
+							  0x0140>;
+					fsl,drive-strength = <0>;
+					fsl,voltage = <0>;
+					fsl,pull-up = <0>;
+				};
+
+				gpmi_pins_ctrl_m28: gpmi1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x180 0x0190
+							  0x01a0 0x01b0>;
+					fsl,drive-strength = <3>;
+					fsl,voltage = <0>;
+					fsl,pull-up = <1>;
+				};
+
+				mmc0_pins_data_m28: mmc0-8bit at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x2000 0x2010 0x2020
+							  0x2030 0x2040 0x2050
+							  0x2060 0x2070 0x2080>;
+					fsl,drive-strength = <1>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <1>;
+				};
+
+				mmc0_pins_ctrl_m28: mmc0-8bit at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x2090 0x20a0
+							  0x30a3 0x31c3>;
+					fsl,drive-strength = <2>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <0>;
+				};
+
+				auart0_pins_m28: auart0 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x3000 0x3010>;
+					fsl,drive-strength = <2>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <0>;
+				};
+
+				auart3_pins_m28: auart3 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x30c0 0x30d0
+							  0x30e0 0x30f0>;
+					fsl,drive-strength = <2>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <0>;
+				};
+
+				mac_pin_reset: mac at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x30b3>;
+					fsl,drive-strength = <2>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <0>;
+				};
+
+				flexcan0_pins_m28: flexcan0 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x0161 0x0162>;
+					fsl,drive-strength = <2>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <0>;
+				};
+
+				flexcan1_pins_m28: flexcan1 at 0 {
+					reg = <0>;
+					fsl,pinmux-ids = <0x0121 0x0122>;
+					fsl,drive-strength = <2>;
+					fsl,voltage = <1>;
+					fsl,pull-up = <0>;
+				};
+			};
+
+			ssp0: ssp at 80010000 {
+				compatible = "fsl,imx28-mmc";
+				pinctrl-names = "default";
+				pinctrl-0 = <&mmc0_pins_data_m28
+					     &mmc0_pins_ctrl_m28>;
+				bus-width = <8>;
+				wp-gpios = <&gpio3 10 1>;
+				status = "okay";
+			};
+
+			gpmi at 8000c000 {
+				compatible = "fsl,imx28-gpmi-nand";
+				#address-cells = <1>;
+				#size-cells = <1>;
+				reg = <0x8000c000 2000>, <0x8000a000 2000>;
+				reg-names = "gpmi-nand", "bch";
+				pinctrl-names = "default";
+				pinctrl-0 = <&gpmi_pins_data_m28
+					     &gpmi_pins_ctrl_m28>;
+				interrupts = <88>, <41>;
+				interrupt-names = "gpmi-dma", "bch";
+				fsl,gpmi-dma-channel = <4>;
+				status = "okay";
+
+				partition at 0 {
+					label = "bootloader";
+					reg = <0x00000000 0x00300000>;
+					read-only;
+				};
+
+				partition at 1 {
+					label = "environment";
+					reg = <0x00300000 0x00080000>;
+				};
+
+				partition at 2 {
+					label = "redundant-environment";
+					reg = <0x00380000 0x00080000>;
+				};
+
+				partition at 3 {
+					label = "kernel";
+					reg = <0x00400000 0x00400000>;
+				};
+
+				partition at 4 {
+					label = "filesystem";
+					reg = <0x00800000 0x0f800000>;
+				};
+			};
+
+			can0: can at 80032000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&flexcan0_pins_m28>;
+				status = "okay";
+			};
+
+			can1: can at 80034000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&flexcan1_pins_m28>;
+				status = "okay";
+			};
+		};
+
+		apbx at 80040000 {
+			saif0: saif at 80042000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&saif0_pins_a>;
+				status = "okay";
+			};
+
+			saif1: saif at 80046000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&saif1_pins_a>;
+				fsl,saif-master = <&saif0>;
+				status = "okay";
+			};
+
+			i2c0: i2c at 80058000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&i2c0_pins_a>;
+				status = "okay";
+
+				sgtl5000: codec at 0a {
+					compatible = "fsl,sgtl5000";
+					reg = <0x0a>;
+					VDDA-supply = <&reg_3p3v>;
+					VDDIO-supply = <&reg_3p3v>;
+
+				};
+
+				eeprom: eeprom at 51 {
+					compatible = "atmel,24c128";
+					reg = <0x51>;
+					pagesize = <32>;
+				};
+
+				rtc: rtc at 68 {
+					compatible = "stm,mt41t62";
+					reg = <0x68>;
+				};
+			};
+
+			duart: serial at 80074000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&duart_pins_a>;
+				status = "okay";
+			};
+		};
+	};
+
+	ahb at 80080000 {
+		mac0: ethernet at 800f0000 {
+			phy-mode = "rmii";
+			pinctrl-names = "default";
+			pinctrl-0 = <&mac0_pins_a &mac_pin_reset>;
+			phy-reset-gpios = <&gpio3 11 0>;
+			status = "okay";
+		};
+
+		mac1: ethernet at 800f4000 {
+			phy-mode = "rmii";
+			pinctrl-names = "default";
+			pinctrl-0 = <&mac1_pins_a>;
+			phy-reset-gpios = <&gpio3 11 0>;
+			status = "okay";
+		};
+	};
+
+	regulators {
+		compatible = "simple-bus";
+
+		reg_3p3v: 3p3v {
+			compatible = "regulator-fixed";
+			regulator-name = "3P3V";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			regulator-always-on;
+		};
+	};
+
+	sound {
+		compatible = "fsl,imx28-evk-sgtl5000",
+			     "fsl,mxs-audio-sgtl5000";
+		model = "imx28-evk-sgtl5000";
+		saif-controllers = <&saif0 &saif1>;
+		audio-codec = <&sgtl5000>;
+	};
+};
-- 
1.7.10

[PATCH] MXS: Add DENX M28 dts file

[Fabio Estevam]

Hi Marek,

On Sat, May 26, 2012 at 11:12 PM, Marek Vasut <marex@denx.de> wrote:

> + ? ? ? sound {
> + ? ? ? ? ? ? ? compatible = "fsl,imx28-evk-sgtl5000",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ?"fsl,mxs-audio-sgtl5000";
> + ? ? ? ? ? ? ? model = "imx28-evk-sgtl5000";
> + ? ? ? ? ? ? ? saif-controllers = <&saif0 &saif1>;
> + ? ? ? ? ? ? ? audio-codec = <&sgtl5000>;
> + ? ? ? };
> +};

Shouldn't this be:

sound {
	compatible = "fsl,imx28-m28-sgtl5000",
		     "fsl,mxs-audio-sgtl5000";
	model = "imx28-m28-sgtl5000";
	saif-controllers = <&saif0 &saif1>;
	audio-codec = <&sgtl5000>;
};

[PATCH] MXS: Add DENX M28 dts file

[Marek Vasut]

Dear Fabio Estevam,

> Hi Marek,
> 
> On Sat, May 26, 2012 at 11:12 PM, Marek Vasut <marex@denx.de> wrote:
> > +       sound {
> > +               compatible = "fsl,imx28-evk-sgtl5000",
> > +                            "fsl,mxs-audio-sgtl5000";
> > +               model = "imx28-evk-sgtl5000";
> > +               saif-controllers = <&saif0 &saif1>;
> > +               audio-codec = <&sgtl5000>;
> > +       };
> > +};
> 
> Shouldn't this be:

it should ... you know the drill, s/xxx/yyy/g ;-)

btw the way I set up GPIOs is correct? I'm quite unsure about it, can you please 
verify?

Thanks!

> 
> sound {
> 	compatible = "fsl,imx28-m28-sgtl5000",
> 		     "fsl,mxs-audio-sgtl5000";
> 	model = "imx28-m28-sgtl5000";
> 	saif-controllers = <&saif0 &saif1>;
> 	audio-codec = <&sgtl5000>;
> };

Best regards,
Marek Vasut

[PATCH] MXS: Add DENX M28 dts file

[Marek Vasut]

Dear Fabio Estevam,

> Hi Marek,
> 
> On Sat, May 26, 2012 at 11:12 PM, Marek Vasut <marex@denx.de> wrote:
> > +       sound {
> > +               compatible = "fsl,imx28-evk-sgtl5000",
> > +                            "fsl,mxs-audio-sgtl5000";
> > +               model = "imx28-evk-sgtl5000";
> > +               saif-controllers = <&saif0 &saif1>;
> > +               audio-codec = <&sgtl5000>;
> > +       };
> > +};
> 
> Shouldn't this be:
> 
> sound {
> 	compatible = "fsl,imx28-m28-sgtl5000",
> 		     "fsl,mxs-audio-sgtl5000";

Wait, no ... it should not, why should it? (I'm still very lame with this device 
tree stuff, so any advice is very welcome)

> 	model = "imx28-m28-sgtl5000";
> 	saif-controllers = <&saif0 &saif1>;
> 	audio-codec = <&sgtl5000>;
> };

Best regards,
Marek Vasut

[PATCH] MXS: Add DENX M28 dts file

[Fabio Estevam]

On Mon, May 28, 2012 at 1:30 PM, Marek Vasut <marex@denx.de> wrote:

>> Shouldn't this be:
>>
>> sound {
>> ? ? ? compatible = "fsl,imx28-m28-sgtl5000",
>> ? ? ? ? ? ? ? ? ? ?"fsl,mxs-audio-sgtl5000";
>
> Wait, no ... it should not, why should it? (I'm still very lame with this device
> tree stuff, so any advice is very welcome)

My understanding is that you need to reference your machine name in
the compatible field instead of referencing another one (mx28evk).
Maybe someone else can confirm this.

[PATCH] MXS: Add DENX M28 dts file

[Shawn Guo]

On Mon, May 28, 2012 at 12:27:29PM -0300, Fabio Estevam wrote:
> Hi Marek,
> 
> On Sat, May 26, 2012 at 11:12 PM, Marek Vasut <marex@denx.de> wrote:
> 
> > + ? ? ? sound {
> > + ? ? ? ? ? ? ? compatible = "fsl,imx28-evk-sgtl5000",
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ?"fsl,mxs-audio-sgtl5000";
> > + ? ? ? ? ? ? ? model = "imx28-evk-sgtl5000";
> > + ? ? ? ? ? ? ? saif-controllers = <&saif0 &saif1>;
> > + ? ? ? ? ? ? ? audio-codec = <&sgtl5000>;
> > + ? ? ? };
> > +};
> 
> Shouldn't this be:
> 
> sound {
> 	compatible = "fsl,imx28-m28-sgtl5000",
> 		     "fsl,mxs-audio-sgtl5000";
> 	model = "imx28-m28-sgtl5000";
> 	saif-controllers = <&saif0 &saif1>;
> 	audio-codec = <&sgtl5000>;
> };

Yes, this one looks sane to me.

-- 
Regards,
Shawn

[PATCH] MXS: Add DENX M28 dts file

[Shawn Guo]

On Sun, May 27, 2012 at 04:12:15AM +0200, Marek Vasut wrote:
> Missing:
> AUART
> LCDIF
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> CC: Dong Aisheng <b29396@freescale.com>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> Cc: Linux ARM kernel <linux-arm-kernel@lists.infradead.org>
> CC: Shawn Guo <shawn.guo@linaro.org>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> ---
>  arch/arm/boot/dts/imx28-m28.dts |  260 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 260 insertions(+)
>  create mode 100644 arch/arm/boot/dts/imx28-m28.dts
> 
> diff --git a/arch/arm/boot/dts/imx28-m28.dts b/arch/arm/boot/dts/imx28-m28.dts
> new file mode 100644
> index 0000000..7892020
> --- /dev/null
> +++ b/arch/arm/boot/dts/imx28-m28.dts
> @@ -0,0 +1,260 @@
> +/*
> + * Copyright (C) 2012 Marek Vasut <marex@denx.de>
> + *
> + * 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
> + */
> +
> +/dts-v1/;
> +/include/ "imx28.dtsi"
> +
> +/ {
> +	model = "DENX M28EVK";
> +	compatible = "fsl,imx28-evk", "fsl,imx28";

"fsl,imx28-evk" shouldn't be used here.  If you do not want to patch
arch/arm/mach-mxs/mach-mxs.c to have "imx28-m28" added in
imx28_dt_compat, you can just have compatible = "fsl,imx28"; here
and the machine will still match DT_MACHINE_START(IMX28, ...).

> +
> +	memory {
> +		reg = <0x40000000 0x08000000>;
> +	};
> +
> +	apb at 80000000 {
> +		apbh at 80000000 {
> +			pinctrl at 80018000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "fsl,imx28-pinctrl", "simple-bus";
> +				reg = <0x80018000 2000>;
> +

The point of having "imx28.dtsi" is to define soc specific data in
there and then those data do not have to be duplicated in every single
<board>.dts.

Above pinctrl data have been defined in imx28.dtsi, so you do not need
to define them here again.

> +				gpmi_pins_data_m28: gpmi1 at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x0000 0x0010 0x0020
> +							  0x0030 0x0040 0x0050
> +							  0x0060 0x0070 0x0100
> +							  0x0140>;
> +					fsl,drive-strength = <0>;
> +					fsl,voltage = <0>;
> +					fsl,pull-up = <0>;
> +				};
> +
> +				gpmi_pins_ctrl_m28: gpmi1 at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x180 0x0190
> +							  0x01a0 0x01b0>;
> +					fsl,drive-strength = <3>;
> +					fsl,voltage = <0>;
> +					fsl,pull-up = <1>;
> +				};

I just applied Huang's gpmi changes below.  Can you please rebase the
the patch on top of it?  If you have the same pin setup as imx28-evk,
we can save above data completely.

http://thread.gmane.org/gmane.linux.ports.arm.kernel/169351

> +
> +				mmc0_pins_data_m28: mmc0-8bit at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x2000 0x2010 0x2020
> +							  0x2030 0x2040 0x2050
> +							  0x2060 0x2070 0x2080>;
> +					fsl,drive-strength = <1>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <1>;
> +				};
> +
> +				mmc0_pins_ctrl_m28: mmc0-8bit at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x2090 0x20a0
> +							  0x30a3 0x31c3>;
> +					fsl,drive-strength = <2>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <0>;
> +				};

You did a couple of wrong things here (as well as gpmi above).  First
of all, there shouldn't be multiple pin group nodes for one device,
supposing you are intending to set up pins for mmc0 in 2 groups nodes
here.  See the following copied from fsl,mxs-pinctrl.txt.

"On mxs, there is no hardware pin group. The pin group in this binding only
means a group of pins put together for particular peripheral to work in
particular function, like SSP0 functioning as mmc0-8bit.  That said, the
group node should include all the pins needed for one function rather than
having these pins defined in several group nodes.  It also means each of
"pinctrl-*" phandle in client device node should only have one group node
pointed in there, while the phandle can have multiple config node referenced
there to adjust configurations for some pins in the group."

The reason I say above "if you are intending to ..." is you may just
end up with having one "mmc0-8bit at 0" node in the dtb where only the
data in the last occurrence get encoded in, because these two nodes
are identical and thus the later one will overwrite the former one.

Secondly, the pin group node should be defined in imx28.dtsi, as the
mux setting for particular device is determined by soc.  That said,
if the existing mmc0_8bit_pins_a works for you, you can just have your
mmc0 device refer to it than define it over again in your board.dts.
Otherwise, you should define mmc0_8bit_pins_b in imx28.dtsi, so that
any other board use that mmc0 mux scheme can refer to it later as too.

Lastly, the gpio should currently stay away from here.  There are some
ongoing work at pinctrl core level to have gpio pin properly set up
when gpio_request() gets called.

> +
> +				auart0_pins_m28: auart0 at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x3000 0x3010>;
> +					fsl,drive-strength = <2>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <0>;
> +				};
> +
> +				auart3_pins_m28: auart3 at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x30c0 0x30d0
> +							  0x30e0 0x30f0>;
> +					fsl,drive-strength = <2>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <0>;
> +				};

Should be defined in imx28.dtsi, so that other boards can refer to
them too.

> +
> +				mac_pin_reset: mac at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x30b3>;
> +					fsl,drive-strength = <2>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <0>;
> +				};

gpio, let's wait for pinctrl core support.

> +
> +				flexcan0_pins_m28: flexcan0 at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x0161 0x0162>;
> +					fsl,drive-strength = <2>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <0>;
> +				};
> +
> +				flexcan1_pins_m28: flexcan1 at 0 {
> +					reg = <0>;
> +					fsl,pinmux-ids = <0x0121 0x0122>;
> +					fsl,drive-strength = <2>;
> +					fsl,voltage = <1>;
> +					fsl,pull-up = <0>;
> +				};

Put them into imx28.dtsi.

> +			};
> +
> +			ssp0: ssp at 80010000 {
> +				compatible = "fsl,imx28-mmc";
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&mmc0_pins_data_m28
> +					     &mmc0_pins_ctrl_m28>;
> +				bus-width = <8>;
> +				wp-gpios = <&gpio3 10 1>;
> +				status = "okay";
> +			};
> +
> +			gpmi at 8000c000 {
> +				compatible = "fsl,imx28-gpmi-nand";
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				reg = <0x8000c000 2000>, <0x8000a000 2000>;
> +				reg-names = "gpmi-nand", "bch";
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&gpmi_pins_data_m28
> +					     &gpmi_pins_ctrl_m28>;
> +				interrupts = <88>, <41>;
> +				interrupt-names = "gpmi-dma", "bch";
> +				fsl,gpmi-dma-channel = <4>;
> +				status = "okay";

Please rebase on Huang's patches which are available on my for-next
branch now.  This gpmi controller node should be firstly defined in
imx28.dtsi and only board specific data should be in <board>.dts.

Regards,
Shawn

> +
> +				partition at 0 {
> +					label = "bootloader";
> +					reg = <0x00000000 0x00300000>;
> +					read-only;
> +				};
> +
> +				partition at 1 {
> +					label = "environment";
> +					reg = <0x00300000 0x00080000>;
> +				};
> +
> +				partition at 2 {
> +					label = "redundant-environment";
> +					reg = <0x00380000 0x00080000>;
> +				};
> +
> +				partition at 3 {
> +					label = "kernel";
> +					reg = <0x00400000 0x00400000>;
> +				};
> +
> +				partition at 4 {
> +					label = "filesystem";
> +					reg = <0x00800000 0x0f800000>;
> +				};
> +			};
> +
> +			can0: can at 80032000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&flexcan0_pins_m28>;
> +				status = "okay";
> +			};
> +
> +			can1: can at 80034000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&flexcan1_pins_m28>;
> +				status = "okay";
> +			};
> +		};
> +
> +		apbx at 80040000 {
> +			saif0: saif at 80042000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&saif0_pins_a>;
> +				status = "okay";
> +			};
> +
> +			saif1: saif at 80046000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&saif1_pins_a>;
> +				fsl,saif-master = <&saif0>;
> +				status = "okay";
> +			};
> +
> +			i2c0: i2c at 80058000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&i2c0_pins_a>;
> +				status = "okay";
> +
> +				sgtl5000: codec at 0a {
> +					compatible = "fsl,sgtl5000";
> +					reg = <0x0a>;
> +					VDDA-supply = <&reg_3p3v>;
> +					VDDIO-supply = <&reg_3p3v>;
> +
> +				};
> +
> +				eeprom: eeprom at 51 {
> +					compatible = "atmel,24c128";
> +					reg = <0x51>;
> +					pagesize = <32>;
> +				};
> +
> +				rtc: rtc at 68 {
> +					compatible = "stm,mt41t62";
> +					reg = <0x68>;
> +				};
> +			};
> +
> +			duart: serial at 80074000 {
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&duart_pins_a>;
> +				status = "okay";
> +			};
> +		};
> +	};
> +
> +	ahb at 80080000 {
> +		mac0: ethernet at 800f0000 {
> +			phy-mode = "rmii";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&mac0_pins_a &mac_pin_reset>;
> +			phy-reset-gpios = <&gpio3 11 0>;
> +			status = "okay";
> +		};
> +
> +		mac1: ethernet at 800f4000 {
> +			phy-mode = "rmii";
> +			pinctrl-names = "default";
> +			pinctrl-0 = <&mac1_pins_a>;
> +			phy-reset-gpios = <&gpio3 11 0>;
> +			status = "okay";
> +		};
> +	};
> +
> +	regulators {
> +		compatible = "simple-bus";
> +
> +		reg_3p3v: 3p3v {
> +			compatible = "regulator-fixed";
> +			regulator-name = "3P3V";
> +			regulator-min-microvolt = <3300000>;
> +			regulator-max-microvolt = <3300000>;
> +			regulator-always-on;
> +		};
> +	};
> +
> +	sound {
> +		compatible = "fsl,imx28-evk-sgtl5000",
> +			     "fsl,mxs-audio-sgtl5000";
> +		model = "imx28-evk-sgtl5000";
> +		saif-controllers = <&saif0 &saif1>;
> +		audio-codec = <&sgtl5000>;
> +	};
> +};
> -- 
> 1.7.10
> 

[PATCH] MXS: Add DENX M28 dts file

[Marek Vasut]

Dear Shawn Guo,

> On Sun, May 27, 2012 at 04:12:15AM +0200, Marek Vasut wrote:
> > Missing:
> > AUART
> > LCDIF
[...]

> I just applied Huang's gpmi changes below.  Can you please rebase the
> the patch on top of it?  If you have the same pin setup as imx28-evk,
> we can save above data completely.
> 
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/169351

Ok, I see ... FSL patches get merged with priority?

> > +
> > +				mmc0_pins_data_m28: mmc0-8bit at 0 {
> > +					reg = <0>;
> > +					fsl,pinmux-ids = <0x2000 0x2010 0x2020
> > +							  0x2030 0x2040 0x2050
> > +							  0x2060 0x2070 0x2080>;
> > +					fsl,drive-strength = <1>;
> > +					fsl,voltage = <1>;
> > +					fsl,pull-up = <1>;
> > +				};
> > +
> > +				mmc0_pins_ctrl_m28: mmc0-8bit at 0 {
> > +					reg = <0>;
> > +					fsl,pinmux-ids = <0x2090 0x20a0
> > +							  0x30a3 0x31c3>;
> > +					fsl,drive-strength = <2>;
> > +					fsl,voltage = <1>;
> > +					fsl,pull-up = <0>;
> > +				};
> 
> You did a couple of wrong things here (as well as gpmi above).  First
> of all, there shouldn't be multiple pin group nodes for one device,
> supposing you are intending to set up pins for mmc0 in 2 groups nodes
> here.  See the following copied from fsl,mxs-pinctrl.txt.

I see ... so if I need to configure two set of pins, how do I do that? Or it's 
not possible with DT too?

> "On mxs, there is no hardware pin group. The pin group in this binding only
> means a group of pins put together for particular peripheral to work in
> particular function, like SSP0 functioning as mmc0-8bit.  That said, the
> group node should include all the pins needed for one function rather than
> having these pins defined in several group nodes.  It also means each of
> "pinctrl-*" phandle in client device node should only have one group node
> pointed in there, while the phandle can have multiple config node
> referenced there to adjust configurations for some pins in the group."
> 
> The reason I say above "if you are intending to ..." is you may just
> end up with having one "mmc0-8bit at 0" node in the dtb where only the
> data in the last occurrence get encoded in, because these two nodes
> are identical and thus the later one will overwrite the former one.
> 
> Secondly, the pin group node should be defined in imx28.dtsi, as the
> mux setting for particular device is determined by soc.  That said,
> if the existing mmc0_8bit_pins_a works for you, you can just have your
> mmc0 device refer to it than define it over again in your board.dts.
> Otherwise, you should define mmc0_8bit_pins_b in imx28.dtsi, so that
> any other board use that mmc0 mux scheme can refer to it later as too.
> 
> Lastly, the gpio should currently stay away from here.  There are some
> ongoing work at pinctrl core level to have gpio pin properly set up
> when gpio_request() gets called.
> 
> > +
> > +				auart0_pins_m28: auart0 at 0 {
> > +					reg = <0>;
> > +					fsl,pinmux-ids = <0x3000 0x3010>;
> > +					fsl,drive-strength = <2>;
> > +					fsl,voltage = <1>;
> > +					fsl,pull-up = <0>;
> > +				};
> > +
> > +				auart3_pins_m28: auart3 at 0 {
> > +					reg = <0>;
> > +					fsl,pinmux-ids = <0x30c0 0x30d0
> > +							  0x30e0 0x30f0>;
> > +					fsl,drive-strength = <2>;
> > +					fsl,voltage = <1>;
> > +					fsl,pull-up = <0>;
> > +				};
> 
> Should be defined in imx28.dtsi, so that other boards can refer to
> them too.

It's specific to our platform, why would it?

> > +
> > +				mac_pin_reset: mac at 0 {
> > +					reg = <0>;
> > +					fsl,pinmux-ids = <0x30b3>;
> > +					fsl,drive-strength = <2>;
> > +					fsl,voltage = <1>;
> > +					fsl,pull-up = <0>;
> > +				};
> 
> gpio, let's wait for pinctrl core support.

Hm, how long do you intend to block all possible platforms from mainline other 
than FSL EVK ones? I consider having stuff in mainline much better, as it helps 
testing it, but it seems you don't share this opinion.

[...]

[PATCH] MXS: Add DENX M28 dts file

[Shawn Guo]

On Fri, Jun 08, 2012 at 08:32:22PM +0200, Marek Vasut wrote:
> Ok, I see ... FSL patches get merged with priority?
> 
If I'm not mistaken, Huang's patches hit list earlier than yours.
That's the only reason I applied his patches with priority.  Anything
wrong with this approach?

> I see ... so if I need to configure two set of pins, how do I do that? Or it's 
> not possible with DT too?
> 
It's possible.  You already did that with ethernet pins, one set of
mac function pins and another phy reset gpio, didn't you?

> > > +
> > > +				auart0_pins_m28: auart0 at 0 {
> > > +					reg = <0>;
> > > +					fsl,pinmux-ids = <0x3000 0x3010>;
> > > +					fsl,drive-strength = <2>;
> > > +					fsl,voltage = <1>;
> > > +					fsl,pull-up = <0>;
> > > +				};
> > > +
> > > +				auart3_pins_m28: auart3 at 0 {
> > > +					reg = <0>;
> > > +					fsl,pinmux-ids = <0x30c0 0x30d0
> > > +							  0x30e0 0x30f0>;
> > > +					fsl,drive-strength = <2>;
> > > +					fsl,voltage = <1>;
> > > +					fsl,pull-up = <0>;
> > > +				};
> > 
> > Should be defined in imx28.dtsi, so that other boards can refer to
> > them too.
> 
> It's specific to our platform, why would it?
> 
Which pins can be mux-ed on auart function is never specific to your
platform, it's determined by SoC design.  Different platforms may need
different pad configuration though.  That's can be done by defining
config node in <board>.dts.

> > > +
> > > +				mac_pin_reset: mac at 0 {
> > > +					reg = <0>;
> > > +					fsl,pinmux-ids = <0x30b3>;
> > > +					fsl,drive-strength = <2>;
> > > +					fsl,voltage = <1>;
> > > +					fsl,pull-up = <0>;
> > > +				};
> > 
> > gpio, let's wait for pinctrl core support.
> 
> Hm, how long do you intend to block all possible platforms from mainline other 
> than FSL EVK ones?

Would you mind giving an example that I block your platform support
for some reason while give it pass on FSL EVK?

> I consider having stuff in mainline much better, as it helps 
> testing it, but it seems you don't share this opinion.
> 
I do share this opinion.  I was thinking that the gpio range support
in pinctrl is close to get it.  Now, it seems not the case and more
importantly I just found one problem with that, the pad configuration
for gpio is not covered by gpio range support.  So we may not be able
to use that support anyway, so go ahead with your approach.

-- 
Regards,
Shawn

[PATCH] MXS: Add DENX M28 dts file

[Marek Vasut]

Dear Shawn Guo,

> On Sun, May 27, 2012 at 04:12:15AM +0200, Marek Vasut wrote:
> > Missing:
> > AUART
> > LCDIF

[...]

> > +				gpmi_pins_ctrl_m28: gpmi1 at 0 {
> > +					reg = <0>;
> > +					fsl,pinmux-ids = <0x180 0x0190
> > +							  0x01a0 0x01b0>;
> > +					fsl,drive-strength = <3>;
> > +					fsl,voltage = <0>;
> > +					fsl,pull-up = <1>;
> > +				};
> 
> I just applied Huang's gpmi changes below.  Can you please rebase the
> the patch on top of it?  If you have the same pin setup as imx28-evk,
> we can save above data completely.
> 
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/169351

Well, GPMI NAND doens't work for me (AGAIN). Screw this, I'm not debuging it for 
the third damn time only to be told to rebase it again and that all I did was 
for naught. Sorry

Best regards,
Marek Vasut

[PATCH] MXS: Add DENX M28 dts file

[Shawn Guo]

On Fri, Jun 08, 2012 at 09:03:09PM +0200, Marek Vasut wrote:
> > I just applied Huang's gpmi changes below.  Can you please rebase the
> > the patch on top of it?  If you have the same pin setup as imx28-evk,
> > we can save above data completely.
> > 
> > http://thread.gmane.org/gmane.linux.ports.arm.kernel/169351
> 
> Well, GPMI NAND doens't work for me (AGAIN). Screw this, I'm not debuging it for 
> the third damn time only to be told to rebase it again and that all I did was 
> for naught. Sorry
> 
Hmm, I was told by Huang that GPMI NAND works for him.  As I do not
have NAND device to play, can you guys aligned here?

-- 
Regards,
Shawn

[PATCH] MXS: Add DENX M28 dts file

[Huang Shijie]

? 2012?06?11? 13:45, Shawn Guo ??:
> On Fri, Jun 08, 2012 at 09:03:09PM +0200, Marek Vasut wrote:
>>> I just applied Huang's gpmi changes below.  Can you please rebase the
>>> the patch on top of it?  If you have the same pin setup as imx28-evk,
>>> we can save above data completely.
>>>
>>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/169351
>> Well, GPMI NAND doens't work for me (AGAIN). Screw this, I'm not debuging it for
>> the third damn time only to be told to rebase it again and that all I did was
>> for naught. Sorry
>>
> Hmm, I was told by Huang that GPMI NAND works for him.  As I do not
sorry. It can not works now. :(

There is a bug in the GPMI-NAND driver( i guess).

The pinmux, clocks, IO resources are all correct.
And the ECC read page/ ECC write page are ok too.

But the UBIFS can not works.

I am debugging it now.

Huang Shijie





> have NAND device to play, can you guys aligned here?
>

[PATCH] MXS: Add DENX M28 dts file

[Marek Vasut]

Dear Huang Shijie,

> ? 2012?06?11? 13:45, Shawn Guo ??:
> > On Fri, Jun 08, 2012 at 09:03:09PM +0200, Marek Vasut wrote:
> >>> I just applied Huang's gpmi changes below.  Can you please rebase the
> >>> the patch on top of it?  If you have the same pin setup as imx28-evk,
> >>> we can save above data completely.
> >>> 
> >>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/169351
> >> 
> >> Well, GPMI NAND doens't work for me (AGAIN). Screw this, I'm not
> >> debuging it for the third damn time only to be told to rebase it again
> >> and that all I did was for naught. Sorry
> > 
> > Hmm, I was told by Huang that GPMI NAND works for him.  As I do not
> 
> sorry. It can not works now. :(
> 
> There is a bug in the GPMI-NAND driver( i guess).
> 
> The pinmux, clocks, IO resources are all correct.
> And the ECC read page/ ECC write page are ok too.
> 
> But the UBIFS can not works.

I sent you that link ... it might be the problem you're facing. And if it isn't, 
then you should fix that too, since you're depending on undefined behavior 
there.

> 
> I am debugging it now.
> 
> Huang Shijie
> 
> > have NAND device to play, can you guys aligned here?

Best regards,
Marek Vasut

[PATCH] MXS: Add DENX M28 dts file

[Huang Shijie]

? 2012?06?11? 18:59, Marek Vasut ??:
> Dear Huang Shijie,
>
>> ? 2012?06?11? 13:45, Shawn Guo ??:
>>> On Fri, Jun 08, 2012 at 09:03:09PM +0200, Marek Vasut wrote:
>>>>> I just applied Huang's gpmi changes below.  Can you please rebase the
>>>>> the patch on top of it?  If you have the same pin setup as imx28-evk,
>>>>> we can save above data completely.
>>>>>
>>>>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/169351
>>>> Well, GPMI NAND doens't work for me (AGAIN). Screw this, I'm not
>>>> debuging it for the third damn time only to be told to rebase it again
>>>> and that all I did was for naught. Sorry
>>> Hmm, I was told by Huang that GPMI NAND works for him.  As I do not
>> sorry. It can not works now. :(
>>
>> There is a bug in the GPMI-NAND driver( i guess).
>>
>> The pinmux, clocks, IO resources are all correct.
>> And the ECC read page/ ECC write page are ok too.
>>
>> But the UBIFS can not works.
> I sent you that link ... it might be the problem you're facing. And if it isn't,
> then you should fix that too, since you're depending on undefined behavior
> there.
>
thanks for your link. But it's not the root cause.

The ECC read-page/write-page are _NOT_ stable. They can fail in some 
unknown cases.

Best Regards
Huang Shijie

>> I am debugging it now.
>>
>> Huang Shijie
>>
>>> have NAND device to play, can you guys aligned here?
> Best regards,
> Marek Vasut
>

[PATCH] MXS: Add DENX M28 dts file

[Marek Vasut]

Dear Huang Shijie,

> ? 2012?06?11? 18:59, Marek Vasut ??:
> > Dear Huang Shijie,
> > 
> >> ? 2012?06?11? 13:45, Shawn Guo ??:
> >>> On Fri, Jun 08, 2012 at 09:03:09PM +0200, Marek Vasut wrote:
> >>>>> I just applied Huang's gpmi changes below.  Can you please rebase the
> >>>>> the patch on top of it?  If you have the same pin setup as imx28-evk,
> >>>>> we can save above data completely.
> >>>>> 
> >>>>> http://thread.gmane.org/gmane.linux.ports.arm.kernel/169351
> >>>> 
> >>>> Well, GPMI NAND doens't work for me (AGAIN). Screw this, I'm not
> >>>> debuging it for the third damn time only to be told to rebase it again
> >>>> and that all I did was for naught. Sorry
> >>> 
> >>> Hmm, I was told by Huang that GPMI NAND works for him.  As I do not
> >> 
> >> sorry. It can not works now. :(
> >> 
> >> There is a bug in the GPMI-NAND driver( i guess).
> >> 
> >> The pinmux, clocks, IO resources are all correct.
> >> And the ECC read page/ ECC write page are ok too.
> >> 
> >> But the UBIFS can not works.
> > 
> > I sent you that link ... it might be the problem you're facing. And if it
> > isn't, then you should fix that too, since you're depending on undefined
> > behavior there.
> 
> thanks for your link. But it's not the root cause.

But that doesn't quite change the fact that it is also a bug which should be 
fixed, right? Will you please investigate and fix it?

> The ECC read-page/write-page are _NOT_ stable. They can fail in some
> unknown cases.

Rant: I think I've said it multiple times already, but Linux is a community 
effort. That's why the quality is so high and bug count so low. It was complete 
flub to have GPMI NAND driver in mainline linux for almost 6 months without 
merging the actual code that'd let anyone else test it and wait for the DT 
bindings. Maybe if you did merge it, this and many other bugs would already be 
fixed.

> Best Regards
> Huang Shijie
> 
> >> I am debugging it now.
> >> 
> >> Huang Shijie
> >> 
> >>> have NAND device to play, can you guys aligned here?
> > 
> > Best regards,
> > Marek Vasut

Best regards,
Marek Vasut