diff mbox

ARM: dts: kirkwood: Add DTS for Linksys EA4200v2/EA4500

Message ID 1459601753-31193-1-git-send-email-bert@biot.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bert Vermeulen April 2, 2016, 12:55 p.m. UTC
This platform is based on a Marvell 88E6282 SoC and 88E6171 switch.

The DSA port labels follow the switchdev convention.

Signed-off-by: Bert Vermeulen <bert@biot.com>
---
 arch/arm/boot/dts/kirkwood-candyhouse.dts | 228 ++++++++++++++++++++++++++++++
 1 file changed, 228 insertions(+)
 create mode 100644 arch/arm/boot/dts/kirkwood-candyhouse.dts

Comments

Andrew Lunn April 2, 2016, 4:16 p.m. UTC | #1
On Sat, Apr 02, 2016 at 02:55:52PM +0200, Bert Vermeulen wrote:
> This platform is based on a Marvell 88E6282 SoC and 88E6171 switch.

Hi Bert

Thanks for contributing this. I have a few comments.
 
> The DSA port labels follow the switchdev convention.

I've generally been using the labels on the case for the port names,
if they have labels. I think this is more useful than some abstract
names which are hard to match to the real hardware. A quick look at
images on Amazon suggests ethernet1, ethernet2, ... internet.

> Signed-off-by: Bert Vermeulen <bert@biot.com>
> ---
>  arch/arm/boot/dts/kirkwood-candyhouse.dts | 228 ++++++++++++++++++++++++++++++
>  1 file changed, 228 insertions(+)
>  create mode 100644 arch/arm/boot/dts/kirkwood-candyhouse.dts
> 
> diff --git a/arch/arm/boot/dts/kirkwood-candyhouse.dts b/arch/arm/boot/dts/kirkwood-candyhouse.dts
> new file mode 100644
> index 0000000..1d16cef
> --- /dev/null
> +++ b/arch/arm/boot/dts/kirkwood-candyhouse.dts
> @@ -0,0 +1,228 @@
> +/*
> + * kirkwood-candyhouse.dts - Device Tree file for Linksys Candyhouse (E4200v2 / EA4500)
> + *
> + * (c) 2013 Jonas Gorski <jogo@openwrt.org>
> + * (c) 2013 Deutsche Telekom Innovation Laboratories
> + * (c) 2014 Luka Perkov <luka@openwrt.org>
> + * (c) 2014 Randy C. Will <randall.will@gmail.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2.  This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */

Could you dual license this under X11 as well? You will need agreement
from the copyright holders above, if they wrote DT fragments.

Not many kirkwood boards are, but we encourage most Marvell SoCs DT
files to be dual license.

> +
> +/dts-v1/;
> +
> +#include "kirkwood.dtsi"
> +#include "kirkwood-6282.dtsi"
> +
> +/ {
> +	model = "Linksys Candyhouse (E4200v2 / EA4500)";
> +	compatible = "linksys,ea4500", "linksys,e4200v2", "marvell,kirkwood-88f6282", "marvell,kirkwood";
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x00000000 0x8000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "console=ttyS0,115200n8 earlyprintk";

Please change this to stdout-path = "serial0:115200n8";

> +	};
> +

> +		/* There is no battery on the boards, so the RTC does not keep
> +		   time when there is no power, making it useless. */
> +		rtc@10300 {
> +			status = "disabled";
> +		};
> +
> +		serial@12000 {
> +			status = "okay";
> +		};

rtc and serial have labels, so you can take these out of the hierarchy.
See kirkwood-dir665.dts for an example how it does rtc, mdio, etc.

> +	dsa@0 {

No need for the @0 here. I need to go fix that in a few different .dts
files, which i think will soon cause warnings from the compiler.

> +		compatible = "marvell,dsa";
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		dsa,ethernet = <&eth0port>;
> +		dsa,mii-bus = <&mdio>;
> +
> +		switch@0 {

The reg is <0x10,0>, so this @0 is wrong. I don't actually know how
you are supposed to represent a two part address....

> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0x10 0>;	/* MDIO address 16, switch 0 in tree */
> +
> +			port@0 {
> +				reg = <0>;
> +				label = "sw0p0";
> +			};
> +
> +			port@1 {
> +				reg = <1>;
> +				label = "sw0p1";
> +			};
> +
> +			port@2 {
> +				reg = <2>;
> +				label = "sw0p2";
> +			};
> +
> +			port@3 {
> +				reg = <3>;
> +				label = "sw0p3";
> +			};
> +
> +			port@4 {
> +				reg = <4>;
> +				label = "sw0p4";
> +			};
> +
> +			port@5 {
> +				reg = <5>;
> +				label = "cpu";
> +			};

The 6171 is a 7 port device. Is the last port also a 'cpu' port? At
the moment we cannot use it, but it would be nice to have a comment if
it is. Ah, you have a comment at the end. 

> +		};
> +	};
> +};
> +
> +&nand {
> +	status = "okay";
> +	pinctrl-0 = <&pmx_nand>;
> +	pinctrl-names = "default";
> +
> +	partition@0 {
> +		label = "u-boot";
> +		reg = <0x0 0x80000>;
> +	};

Please read Documentation/devicetree/bindings/mtd/partition.txt, in
particular the partitions node. I'm never quite sure when it works, i
know i've asked somebody to use it in a situation when it did not
actually work...

> +
> +	partition@80000 {
> +		label = "u_env";
> +		reg = <0x80000 0x20000>;
> +	};
> +
> +	partition@A0000 {
> +		label = "s_env";
> +		reg = <0xA0000 0x20000>;
> +	};
> +
> +	partition@200000 {
> +		label = "kernel";
> +		reg = <0x200000 0x2A0000>;
> +	};

Do we have a hole between s_env and kernel? We have seen that before
with Linksys devices, e.g. armada-xp-linksys-mamba.dts. Maybe put a
partition at the end to cover this hole.

	  Andrew
Andrew Lunn April 2, 2016, 4:26 p.m. UTC | #2
> +		button@15 {
> +			label = "WPS Button";
> +			linux,code = <KEY_WPS_BUTTON>;
> +			gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		button@16 {
> +			label = "Reset Button";
> +			linux,code = <KEY_RESTART>;
> +			gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
> +		};
> +	};

I'm looking at the warnings the new DT compiler produces....

Buttons should not have a @15, etc, since they don't have a register
property.

If you can, it would be good it you can compile your .dts file using
the dtc compile in linux-next and pass W=1.

	Andrew
Imre Kaloz April 3, 2016, 9:56 a.m. UTC | #3
On Sat, 02 Apr 2016 14:55:52 +0200, Bert Vermeulen <bert@biot.com> wrote:

> This platform is based on a Marvell 88E6282 SoC and 88E6171 switch.
>
> The DSA port labels follow the switchdev convention.
>
> Signed-off-by: Bert Vermeulen <bert@biot.com>

> --- /dev/null
> +++ b/arch/arm/boot/dts/kirkwood-candyhouse.dts

Candyhouse is the name of the platform, not the router. please use

arch/arm/boot/dts/kirkwood-linksys-viper.dts

> +	model = "Linksys Candyhouse (E4200v2 / EA4500)";
> +	compatible = "linksys,ea4500", "linksys,e4200v2",  
> "marvell,kirkwood-88f6282", "marvell,kirkwood";

The EA4200v2 is the same as the EA4500. The name is viper, so

compatible = "linksys,viper"



Best,

Imre
Bert Vermeulen April 4, 2016, 11:51 a.m. UTC | #4
On 04/02/2016 06:16 PM, Andrew Lunn wrote:
> On Sat, Apr 02, 2016 at 02:55:52PM +0200, Bert Vermeulen wrote:
>> + * (c) 2013 Jonas Gorski <jogo@openwrt.org>
>> + * (c) 2013 Deutsche Telekom Innovation Laboratories
>> + * (c) 2014 Luka Perkov <luka@openwrt.org>
>> + * (c) 2014 Randy C. Will <randall.will@gmail.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2.  This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>
> Could you dual license this under X11 as well? You will need agreement
> from the copyright holders above, if they wrote DT fragments.
 >
 > Not many kirkwood boards are, but we encourage most Marvell SoCs DT
 > files to be dual license.

I did not get responses from all authors, so no dice.

>> +	chosen {
>> +		bootargs = "console=ttyS0,115200n8 earlyprintk";
>
> Please change this to stdout-path = "serial0:115200n8";

I've added a serial alias (the actual label is uart0). That alias and one 
for serial1 might perhaps be better in kirkwood.dts, but I'll leave that up 
to you.

> The 6171 is a 7 port device. Is the last port also a 'cpu' port?

I don't think so. I didn't manage to get anything through it, and I can't 
see the CPU traces on the PCB (neither 5 nor 6).
Andrew Lunn April 4, 2016, 12:15 p.m. UTC | #5
> >Could you dual license this under X11 as well? You will need agreement
> >from the copyright holders above, if they wrote DT fragments.
> >
> > Not many kirkwood boards are, but we encourage most Marvell SoCs DT
> > files to be dual license.
> 
> I did not get responses from all authors, so no dice.

O.K. We can change this later if they do reply.
 
> >The 6171 is a 7 port device. Is the last port also a 'cpu' port?
> 
> I don't think so. I didn't manage to get anything through it, and I
> can't see the CPU traces on the PCB (neither 5 nor 6).

You might need to set the RGMII delay. Here is an example i use with
another board:

port@6 {
	reg = <6>;
        label = "cpu";
	phy-mode = "rgmii-txid";
};

	Andrew
diff mbox

Patch

diff --git a/arch/arm/boot/dts/kirkwood-candyhouse.dts b/arch/arm/boot/dts/kirkwood-candyhouse.dts
new file mode 100644
index 0000000..1d16cef
--- /dev/null
+++ b/arch/arm/boot/dts/kirkwood-candyhouse.dts
@@ -0,0 +1,228 @@ 
+/*
+ * kirkwood-candyhouse.dts - Device Tree file for Linksys Candyhouse (E4200v2 / EA4500)
+ *
+ * (c) 2013 Jonas Gorski <jogo@openwrt.org>
+ * (c) 2013 Deutsche Telekom Innovation Laboratories
+ * (c) 2014 Luka Perkov <luka@openwrt.org>
+ * (c) 2014 Randy C. Will <randall.will@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+/dts-v1/;
+
+#include "kirkwood.dtsi"
+#include "kirkwood-6282.dtsi"
+
+/ {
+	model = "Linksys Candyhouse (E4200v2 / EA4500)";
+	compatible = "linksys,ea4500", "linksys,e4200v2", "marvell,kirkwood-88f6282", "marvell,kirkwood";
+
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x8000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttyS0,115200n8 earlyprintk";
+	};
+
+	mbus {
+		pcie-controller {
+			status = "okay";
+
+			pcie@1,0 {
+				status = "okay";
+			};
+
+			pcie@2,0 {
+				status = "okay";
+			};
+		};
+	};
+
+	ocp@f1000000 {
+		pinctrl: pin-controller@10000 {
+			pmx_led_white_health: pmx-led-white-health {
+				marvell,pins = "mpp7";
+				marvell,function = "gpo";
+			};
+			pmx_led_white_pulse: pmx-led-white-pulse {
+				marvell,pins = "mpp14";
+				marvell,function = "gpio";
+			};
+			pmx_btn_wps: pmx-btn-wps {
+				marvell,pins = "mpp47";
+				marvell,function = "gpio";
+			};
+			pmx_btn_reset: pmx-btn-reset {
+				marvell,pins = "mpp48";
+				marvell,function = "gpio";
+			};
+		};
+
+		/* There is no battery on the boards, so the RTC does not keep
+		   time when there is no power, making it useless. */
+		rtc@10300 {
+			status = "disabled";
+		};
+
+		serial@12000 {
+			status = "okay";
+		};
+
+	};
+
+	gpio_keys {
+		compatible = "gpio-keys";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-0 = < &pmx_btn_wps &pmx_btn_reset >;
+		pinctrl-names = "default";
+
+		button@15 {
+			label = "WPS Button";
+			linux,code = <KEY_WPS_BUTTON>;
+			gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
+		};
+
+		button@16 {
+			label = "Reset Button";
+			linux,code = <KEY_RESTART>;
+			gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+		pinctrl-0 = < &pmx_led_white_health &pmx_led_white_pulse >;
+		pinctrl-names = "default";
+
+		white-health {
+			label = "candyhouse:white:health";
+			gpios = <&gpio0 7 GPIO_ACTIVE_HIGH>;
+		};
+
+		white-pulse {
+			label = "candyhouse:white:pulse";
+			gpios = <&gpio0 14 GPIO_ACTIVE_HIGH>;
+		};
+	};
+
+	dsa@0 {
+		compatible = "marvell,dsa";
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		dsa,ethernet = <&eth0port>;
+		dsa,mii-bus = <&mdio>;
+
+		switch@0 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x10 0>;	/* MDIO address 16, switch 0 in tree */
+
+			port@0 {
+				reg = <0>;
+				label = "sw0p0";
+			};
+
+			port@1 {
+				reg = <1>;
+				label = "sw0p1";
+			};
+
+			port@2 {
+				reg = <2>;
+				label = "sw0p2";
+			};
+
+			port@3 {
+				reg = <3>;
+				label = "sw0p3";
+			};
+
+			port@4 {
+				reg = <4>;
+				label = "sw0p4";
+			};
+
+			port@5 {
+				reg = <5>;
+				label = "cpu";
+			};
+		};
+	};
+};
+
+&nand {
+	status = "okay";
+	pinctrl-0 = <&pmx_nand>;
+	pinctrl-names = "default";
+
+	partition@0 {
+		label = "u-boot";
+		reg = <0x0 0x80000>;
+	};
+
+	partition@80000 {
+		label = "u_env";
+		reg = <0x80000 0x20000>;
+	};
+
+	partition@A0000 {
+		label = "s_env";
+		reg = <0xA0000 0x20000>;
+	};
+
+	partition@200000 {
+		label = "kernel";
+		reg = <0x200000 0x2A0000>;
+	};
+
+	partition@4A0000 {
+		label = "rootfs";
+		reg = <0x4A0000 0x1760000>;
+	};
+
+	partition@1C00000 {
+		label = "alt_kernel";
+		reg = <0x1C00000 0x2A0000>;
+	};
+
+	partition@1EA0000 {
+		label = "alt_rootfs";
+		reg = <0x1EA0000 0x1760000>;
+	};
+
+	partition@3600000 {
+		label = "syscfg";
+		reg = <0x3600000 0x4A00000>;
+	};
+};
+
+&mdio {
+	status = "okay";
+};
+
+/* eth0 is connected to a Marvell 88E6171 switch, without a PHY. So set
+ * fixed speed and duplex. */
+&eth0 {
+	status = "okay";
+	ethernet0-port@0 {
+		speed = <1000>;
+		duplex = <1>;
+	};
+};
+
+/* eth1 is connected to the switch as well. However DSA only supports a
+ * single CPU port. So leave this port disabled to avoid confusion. */
+&eth1 {
+	status = "disabled";
+	ethernet1-port@0 {
+		speed = <1000>;
+		duplex = <1>;
+	};
+};