Skip to contentSkip to conversation

mvebu: add Linksys WRT1900AC (Mamba) support

PATCH Superseded Imre Kaloz 5 participants 12 messages Jan 19, 2015 – Jan 27, 2015 Device tree bindings

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Imre Kaloz

author
#0 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

The Linksys WRT1900AC (Mamba) is a router that has

- 2 mini-PCIe slots with Marvell 88W8864 radios
- 1 USB 3.0 port
- 1 USB 2.0/eSATAp port
- 2 Ethernet interfaces connected to a 88E6172 switch (1x WAN + 4x LAN)
- 128MB NAND flash
- 256MB RAM

Signed-off-by Imre Kaloz <[email protected]>

---
Changes since v1:
* add dual license
* lower SPI speed to meet the chip's maximum
* pinctrl cleanups based on Andrew Lunn's suggestions

---
 arch/arm/boot/dts/armada-xp-mamba.dts | 273 ++++++++++++++++++++++++++++++++++
 1 file changed, 273 insertions(+)
 create mode 100644 arch/arm/boot/dts/armada-xp-mamba.dts
arch/arm/boot/dts/armada-xp-mamba.dts new file +273 -0
new file mode 100644
index 0000000..e368d18
@@ -0,0 +1,273 @@
+/*
+ * Device Tree file for the Linksys WRT1900AC (Mamba).
+ *
+ * Note: this board is shipped with a new generation boot loader that
+ * remaps internal registers at 0xf1000000. Therefore, if earlyprintk
+ * is used, the CONFIG_DEBUG_MVEBU_UART_ALTERNATE option should be
+ * used.
+ *
+ * Copyright (C) 2014 Imre Kaloz <[email protected]>
+ *
+ * Based on armada-xp-axpwifiap.dts:
+ *
+ *     Copyright (C) 2013 Marvell
+ *
+ *     Thomas Petazzoni <[email protected]>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "armada-xp-mv78230.dtsi"
+
+/ {
+	model = "Linksys WRT1900AC (Mamba)";
+	compatible = "linksys,mamba", "marvell,armadaxp-mv78230",
+		     "marvell,armadaxp", "marvell,armada-370-xp";
+
+	chosen {
+		bootargs = "console=ttyS0,115200";
+		stdout-path = &uart0;
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x00000000 0x00000000 0x10000000>; /* 256MB */
+	};
+
+	soc {
+		ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000
+			  MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000>;
+
+		pcie-controller {
+			status = "okay";
+
+			/* Etron EJ168 USB 3.0 controller */
+			pcie at 1,0 {
+				/* Port 0, Lane 0 */
+				status = "okay";
+			};
+
+			/* First mini-PCIe port */
+			pcie at 2,0 {
+				/* Port 0, Lane 1 */
+				status = "okay";
+			};
+
+			/* Second mini-PCIe port */
+			pcie at 3,0 {
+				/* Port 0, Lane 3 */
+				status = "okay";
+			};
+		};
+
+		internal-regs {
+			serial at 12000 {
+				status = "okay";
+			};
+
+			sata at a0000 {
+				nr-ports = <1>;
+				status = "okay";
+			};
+
+			ethernet at 70000 {
+				pinctrl-0 = <&ge0_rgmii_pins>;
+				pinctrl-names = "default";
+				status = "okay";
+				phy-mode = "rgmii-id";
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
+			};
+
+			ethernet at 74000 {
+				pinctrl-0 = <&ge1_rgmii_pins>;
+				pinctrl-names = "default";
+				status = "okay";
+				phy-mode = "rgmii-id";
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
+			};
+
+			/* USB part of the eSATA/USB 2.0 port */
+			usb at 50000 {
+				status = "okay";
+			};
+
+			i2c at 11000 {
+				status = "okay";
+				clock-frequency = <100000>;
+
+				tmp421 at 4c {
+					compatible = "ti,tmp421";
+					reg = <0x4c>;
+				};
+			};
+
+			nand at d0000 {
+				status = "okay";
+				num-cs = <1>;
+				marvell,nand-keep-config;
+				marvell,nand-enable-arbiter;
+				nand-on-flash-bbt;
+				nand-ecc-strength = <4>;
+				nand-ecc-step-size = <512>;
+
+				partition at 0 {
+					label = "u-boot";
+					reg = <0x0000000 0x100000>;  /* 1MB */
+					read-only;
+				};
+
+				partition at 100000 {
+					label = "u_env";
+					reg = <0x100000 0x40000>;    /* 256KB */
+				};
+
+				partition at 140000 {
+					label = "s_env";
+					reg = <0x140000 0x40000>;    /* 256KB */
+				};
+
+				partition at 900000 {
+					label = "devinfo";
+					reg = <0x900000 0x100000>;    /* 1MB */
+					read-only;
+				};
+
+				partition at a00000 {
+					label = "kernel1";
+					reg = <0xa00000 0x2800000>;    /* 3MB + rootfs1 */
+				};
+
+				partition at d00000 {
+					label = "rootfs1";
+					reg = <0xd00000 0x2500000>;    /* 37MB */
+				};
+
+				partition at 3200000 {
+					label = "kernel2";
+					reg = <0x3200000 0x2800000>;    /* 3MB + rootfs2 */
+				};
+
+				partition at 3500000 {
+					label = "rootfs2";
+					reg = <0x3500000 0x2500000>;    /* 37MB */
+				};
+
+				/* Last MB is for the BBT, i.e. not writable */
+				partition at 5a00000 {
+					label = "syscfg";
+					reg = <0x5a00000 0x2600000>;    /* 38MB */
+				};
+			};
+
+			spi0: spi at 10600 {
+				status = "okay";
+
+				spi-flash at 0 {
+					#address-cells = <1>;
+					#size-cells = <1>;
+					compatible = "everspin,mr25h256";
+					reg = <0>; /* Chip select 0 */
+					spi-max-frequency = <40000000>;
+				};
+			};
+		};
+	};
+
+	gpio_keys {
+		compatible = "gpio-keys";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-0 = <&keys_pin>;
+		pinctrl-names = "default";
+
+		button at 1 {
+			label = "WPS";
+			linux,code = <KEY_WPS_BUTTON>;
+			gpios = <&gpio1 0 GPIO_ACTIVE_HIGH>;
+		};
+
+		button at 2 {
+			label = "Factory Reset Button";
+			linux,code = <KEY_RESTART>;
+			gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;
+		};
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+		pinctrl-0 = <&power_led_pin>;
+		pinctrl-names = "default";
+
+		power {
+			label = "mamba:white:power";
+			gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
+			default-state = "on";
+		};
+	};
+
+	gpio_fan {
+		/* SUNON HA4010V4-0000-C99 */
+		compatible = "gpio-fan";
+		gpios = <&gpio0 24 0>;
+
+		gpio-fan,speed-map = <0    0
+				      4500 1>;
+	};
+};
+
+&pinctrl {
+
+	keys_pin: keys-pin {
+		marvell,pins = "mpp32", "mpp33";
+		marvell,function = "gpio";
+	};
+
+	power_led_pin: power-led-pin {
+		marvell,pins = "mpp40";
+		marvell,function = "gpio";
+	};
+
+	gpio_fan_pin: gpio-fan-pin {
+		marvell,pins = "mpp24";
+		marvell,function = "gpio";
+	};
+};
Signature · 1 line
2.1.0
#1 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

Quoted 26 lines · Imre Kaloz

The Linksys WRT1900AC (Mamba) is a router that has

- 2 mini-PCIe slots with Marvell 88W8864 radios
- 1 USB 3.0 port
- 1 USB 2.0/eSATAp port
- 2 Ethernet interfaces connected to a 88E6172 switch (1x WAN + 4x LAN)
- 128MB NAND flash
- 256MB RAM

Signed-off-by Imre Kaloz <[email protected]>

---
Changes since v1:
* add dual license
* lower SPI speed to meet the chip's maximum
* pinctrl cleanups based on Andrew Lunn's suggestions

---
 arch/arm/boot/dts/armada-xp-mamba.dts | 273 ++++++++++++++++++++++++++++++++++
 1 file changed, 273 insertions(+)
 create mode 100644 arch/arm/boot/dts/armada-xp-mamba.dts
arch/arm/boot/dts/armada-xp-mamba.dts new file +0 -0
new file mode 100644
index 0000000..e368d18

arch/arm/boot/dts/armada-xp-mamba.dts line 2

Quoted 2 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +2 -0
@@ -0,0 +1,273 @@
+/*
+ * Device Tree file for the Linksys WRT1900AC (Mamba).

Hi Imre

Thanks for the v2. I have a few comments, and some points we will need to discuss.

arch/arm/boot/dts/armada-xp-mamba.dts line 6

Quoted 4 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +4 -0
+ *
+ * Note: this board is shipped with a new generation boot loader that
+ * remaps internal registers at 0xf1000000. Therefore, if earlyprintk
+ * is used, the CONFIG_DEBUG_MVEBU_UART_ALTERNATE option should be

There is a patch already queued up for this cycle:

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313605.html

Please can you change CONFIG_DEBUG_MVEBU_UART_ALTERNATE to CONFIG_DEBUG_MVEBU_UART0_ALTERNATE

Quoted 42 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +42 -0
+ * used.
+ *
+ * Copyright (C) 2014 Imre Kaloz <[email protected]>
+ *
+ * Based on armada-xp-axpwifiap.dts:
+ *
+ *     Copyright (C) 2013 Marvell
+ *
+ *     Thomas Petazzoni <[email protected]>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */

arch/arm/boot/dts/armada-xp-mamba.dts line 58

Quoted 10 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +10 -0
+
+/dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "armada-xp-mv78230.dtsi"
+
+/ {
+	model = "Linksys WRT1900AC (Mamba)";
+	compatible = "linksys,mamba", "marvell,armadaxp-mv78230",
+		     "marvell,armadaxp", "marvell,armada-370-xp";

So this is where the discussion starts. I don't like Mamba being so prominent. As far as i understand, Mamba is the board, not the device. In theory, another device could be created using the same board as a basis, but with different PCIe cards, etc. At that point, i would suggest refactoring the common parts out into a armada-xp-linksys-mamba.dtsi which is then included into any device .dts file using the Mamba board.

This file describes the device. So i would prefer it to be called armada-xp-linksys-wrt1900ac.dts. The first compatible should be "linksys,wrt1900ac". Having "linksys,mamba" second is O.K.

Quoted 28 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +28 -0
+
+	chosen {
+		bootargs = "console=ttyS0,115200";
+		stdout-path = &uart0;
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x00000000 0x00000000 0x10000000>; /* 256MB */
+	};
+
+	soc {
+		ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xf1000000 0x100000
+			  MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000>;
+
+		pcie-controller {
+			status = "okay";
+
+			/* Etron EJ168 USB 3.0 controller */
+			pcie at 1,0 {
+				/* Port 0, Lane 0 */
+				status = "okay";
+			};
+
+			/* First mini-PCIe port */
+			pcie at 2,0 {
+				/* Port 0, Lane 1 */
+				status = "okay";

arch/arm/boot/dts/armada-xp-mamba.dts line 96

Quoted 10 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +10 -0
+			};
+
+			/* Second mini-PCIe port */
+			pcie at 3,0 {
+				/* Port 0, Lane 3 */
+				status = "okay";
+			};
+		};
+
+		internal-regs {

It would be nice to document the jumper and pinout here.

Quoted 36 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +36 -0
+			serial at 12000 {
+				status = "okay";
+			};
+
+			sata at a0000 {
+				nr-ports = <1>;
+				status = "okay";
+			};
+
+			ethernet at 70000 {
+				pinctrl-0 = <&ge0_rgmii_pins>;
+				pinctrl-names = "default";
+				status = "okay";
+				phy-mode = "rgmii-id";
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
+			};
+
+			ethernet at 74000 {
+				pinctrl-0 = <&ge1_rgmii_pins>;
+				pinctrl-names = "default";
+				status = "okay";
+				phy-mode = "rgmii-id";
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
+			};
+
+			/* USB part of the eSATA/USB 2.0 port */
+			usb at 50000 {
+				status = "okay";
+			};
+

arch/arm/boot/dts/armada-xp-mamba.dts line 142

Quoted 10 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +10 -0
+			i2c at 11000 {
+				status = "okay";
+				clock-frequency = <100000>;
+
+				tmp421 at 4c {
+					compatible = "ti,tmp421";
+					reg = <0x4c>;
+				};
+			};
+

The discussion about the i2c LED controller is still ongoing, but i think TI are unlikely to get a PWM based driver accepted in place of a LED driver. So you could include the LEDs here, and something unexpected happens, we can remove it.

Quoted 14 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +14 -0
+			nand at d0000 {
+				status = "okay";
+				num-cs = <1>;
+				marvell,nand-keep-config;
+				marvell,nand-enable-arbiter;
+				nand-on-flash-bbt;
+				nand-ecc-strength = <4>;
+				nand-ecc-step-size = <512>;
+
+				partition at 0 {
+					label = "u-boot";
+					reg = <0x0000000 0x100000>;  /* 1MB */
+					read-only;
+				};

arch/arm/boot/dts/armada-xp-mamba.dts line 166

Quoted 10 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +10 -0
+
+				partition at 100000 {
+					label = "u_env";
+					reg = <0x100000 0x40000>;    /* 256KB */
+				};
+
+				partition at 140000 {
+					label = "s_env";
+					reg = <0x140000 0x40000>;    /* 256KB */
+				};

So there is a big gap here? 768KB of unused space before the devinfo section?

arch/arm/boot/dts/armada-xp-mamba.dts line 176

Quoted 10 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +10 -0
+
+				partition at 900000 {
+					label = "devinfo";
+					reg = <0x900000 0x100000>;    /* 1MB */
+					read-only;
+				};
+
+				partition at a00000 {
+					label = "kernel1";
+					reg = <0xa00000 0x2800000>;    /* 3MB + rootfs1 */

There are some long lines here. We try to keep to the 80 character rule.

arch/arm/boot/dts/armada-xp-mamba.dts line 177

Quoted 1 line
arch/arm/boot/dts/armada-xp-mamba.dts new file +1 -0
+				};

This partition overlaps the next one? That is new to me. Seems to be accepted practice for OpenWRT, but i'm not aware of any mainline Marvell DT doing this. Comments from others welcome...

Quoted 54 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +54 -0
+
+				partition at d00000 {
+					label = "rootfs1";
+					reg = <0xd00000 0x2500000>;    /* 37MB */
+				};
+
+				partition at 3200000 {
+					label = "kernel2";
+					reg = <0x3200000 0x2800000>;    /* 3MB + rootfs2 */
+				};
+
+				partition at 3500000 {
+					label = "rootfs2";
+					reg = <0x3500000 0x2500000>;    /* 37MB */
+				};
+
+				/* Last MB is for the BBT, i.e. not writable */
+				partition at 5a00000 {
+					label = "syscfg";
+					reg = <0x5a00000 0x2600000>;    /* 38MB */
+				};
+			};
+
+			spi0: spi at 10600 {
+				status = "okay";
+
+				spi-flash at 0 {
+					#address-cells = <1>;
+					#size-cells = <1>;
+					compatible = "everspin,mr25h256";
+					reg = <0>; /* Chip select 0 */
+					spi-max-frequency = <40000000>;
+				};
+			};
+		};
+	};
+
+	gpio_keys {
+		compatible = "gpio-keys";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-0 = <&keys_pin>;
+		pinctrl-names = "default";
+
+		button at 1 {
+			label = "WPS";
+			linux,code = <KEY_WPS_BUTTON>;
+			gpios = <&gpio1 0 GPIO_ACTIVE_HIGH>;
+		};
+
+		button at 2 {
+			label = "Factory Reset Button";
+			linux,code = <KEY_RESTART>;
+			gpios = <&gpio1 1 GPIO_ACTIVE_HIGH>;

arch/arm/boot/dts/armada-xp-mamba.dts line 241

Quoted 10 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +10 -0
+		};
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+		pinctrl-0 = <&power_led_pin>;
+		pinctrl-names = "default";
+
+		power {
+			label = "mamba:white:power";

Please replace this mamba with wrt1900ac. It is a property of the device, not the board. Another device using the mamba board may use it differently.

Quoted 34 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +32 -0
+			gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
+			default-state = "on";
+		};
+	};
+
+	gpio_fan {
+		/* SUNON HA4010V4-0000-C99 */
+		compatible = "gpio-fan";
+		gpios = <&gpio0 24 0>;
+
+		gpio-fan,speed-map = <0    0
+				      4500 1>;
+	};
+};
+
+&pinctrl {
+
+	keys_pin: keys-pin {
+		marvell,pins = "mpp32", "mpp33";
+		marvell,function = "gpio";
+	};
+
+	power_led_pin: power-led-pin {
+		marvell,pins = "mpp40";
+		marvell,function = "gpio";
+	};
+
+	gpio_fan_pin: gpio-fan-pin {
+		marvell,pins = "mpp24";
+		marvell,function = "gpio";
+	};
+};
Signature · 1 line
2.1.0
author
#2 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

Hi Andrew,

Quoted 2 lines · Andrew Lunn

Thanks for the v2. I have a few comments, and some points we will need to discuss.

Sure thing, thanks.

Quoted 11 lines
Quoted 4 lines

+ *
+ * Note: this board is shipped with a new generation boot loader that
+ * remaps internal registers at 0xf1000000. Therefore, if earlyprintk
+ * is used, the CONFIG_DEBUG_MVEBU_UART_ALTERNATE option should be

There is a patch already queued up for this cycle:

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313605.html

Please can you change CONFIG_DEBUG_MVEBU_UART_ALTERNATE to CONFIG_DEBUG_MVEBU_UART0_ALTERNATE

Ok.

Quoted 12 lines

arch/arm/boot/dts/armada-xp-mamba.dts line 58

Quoted 3 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +3 -0
+	model = "Linksys WRT1900AC (Mamba)";
+	compatible = "linksys,mamba", "marvell,armadaxp-mv78230",
+		     "marvell,armadaxp", "marvell,armada-370-xp";

So this is where the discussion starts. I don't like Mamba being so prominent. As far as i understand, Mamba is the board, not the device. In theory, another device could be created using the same board as a basis, but with different PCIe cards, etc. At that point, i would suggest refactoring the common parts out into a armada-xp-linksys-mamba.dtsi which is then included into any device .dts file using the Mamba board.

Quoted 3 lines

This file describes the device. So i would prefer it to be called armada-xp-linksys-wrt1900ac.dts. The first compatible should be "linksys,wrt1900ac". Having "linksys,mamba" second is O.K.

I would like to ask for others' opinion for multiple reasons, and would decide in v3 based on that.

- The device is called the "mamba", the marketing name is the WRT1900AC. As history showed, it's perfectly possible that exactly the same device go on the market under a different name. The E4200v2 is the same device as the EA4500, with a different factory firmware. There the name of the device is "viper".

- OpenWrt is the only firmware/stack other than the official one and people already know this device as "mamba".

- Let's say the same device gets released under the same name or just the radios change - so no redesign takes place at all. In my opinion that hardly justifies adding multiple .dts files just to change the name of the LEDs to reflect that. I think people who want to run mainline on their device wouldn't be concerned about seeing a codename, but on the other hand we could receive patches to "correct" the marketing name in the LEDs.

Quoted 7 lines

arch/arm/boot/dts/armada-xp-mamba.dts line 96

Quoted 1 line
arch/arm/boot/dts/armada-xp-mamba.dts new file +1 -0
+		internal-regs {

It would be nice to document the jumper and pinout here.

arch/arm/boot/dts/armada-xp-mamba.dts line 99

Quoted 3 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +3 -0
+			serial at 12000 {
+				status = "okay";
+			};

Do you mean serial console pinout?

Quoted 4 lines

The discussion about the i2c LED controller is still ongoing, but i think TI are unlikely to get a PWM based driver accepted in place of a LED driver. So you could include the LEDs here, and something unexpected happens, we can remove it.

Ok, I have that locally already, too.

Quoted 20 lines
Quoted 20 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +19 -0
+			nand at d0000 {
+				status = "okay";
+				num-cs = <1>;
+				marvell,nand-keep-config;
+				marvell,nand-enable-arbiter;
+				nand-on-flash-bbt;
+				nand-ecc-strength = <4>;
+				nand-ecc-step-size = <512>;
+
+				partition at 0 {
+					label = "u-boot";
+					reg = <0x0000000 0x100000>;  /* 1MB */
+					read-only;
+				};
+
+				partition at 100000 {
+					label = "u_env";
+					reg = <0x100000 0x40000>;    /* 256KB */
+				};
Quoted 8 lines

arch/arm/boot/dts/armada-xp-mamba.dts line 166

Quoted 5 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +5 -0
+
+				partition at 140000 {
+					label = "s_env";
+					reg = <0x140000 0x40000>;    /* 256KB */
+				};

So there is a big gap here? 768KB of unused space before the devinfo section?

See below.

Quoted 5 lines
Quoted 5 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +5 -0
+
+				partition at 900000 {
+					label = "devinfo";
+					reg = <0x900000 0x100000>;    /* 1MB */
+					read-only;
Quoted 8 lines

arch/arm/boot/dts/armada-xp-mamba.dts line 176

Quoted 5 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +5 -0
+				};
+
+				partition at a00000 {
+					label = "kernel1";
+					reg = <0xa00000 0x2800000>;    /* 3MB + rootfs1 */

There are some long lines here. We try to keep to the 80 character rule.

I've thought that would look kinda awkward, hence I left it this way.

Quoted 6 lines

Review

Quoted 2 lines
(quoted diff) +1 -0
+				};

This partition overlaps the next one? That is new to me. Seems to be accepted practice for OpenWRT, but i'm not aware of any mainline Marvell DT doing this. Comments from others welcome...

The gap and the overlapping partitions (pretty much the whole layout) are straight from the official firmware. The overlap is used to flash a single file which contains the rootfs, too.

Quoted 8 lines

arch/arm/boot/dts/armada-xp-mamba.dts line 241

Quoted 3 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +3 -0
+
+		power {
+			label = "mamba:white:power";

Please replace this mamba with wrt1900ac. It is a property of the device, not the board. Another device using the mamba board may use it differently.

See above.

Imre

#3 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

Quoted 8 lines
Quoted 6 lines

It would be nice to document the jumper and pinout here.

arch/arm/boot/dts/armada-xp-mamba.dts line 99

Quoted 3 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +3 -0
+			serial at 12000 {
+				status = "okay";
+			};

Do you mean serial console pinout?

Yes. Looking at

https://github.com/Chadster766/McWRT/wiki/Linksys-WRT1900AC-Serial-Port

it seems to be jumper JP10.

1 - GND
2 - RX
4 - TX

What i don't know is, is Rx and Rx from the perspective of the board or the host?

Quoted 27 lines
Quoted 27 lines
Quoted 14 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +14 -0
+			nand at d0000 {
+				status = "okay";
+				num-cs = <1>;
+				marvell,nand-keep-config;
+				marvell,nand-enable-arbiter;
+				nand-on-flash-bbt;
+				nand-ecc-strength = <4>;
+				nand-ecc-step-size = <512>;
+
+				partition at 0 {
+					label = "u-boot";
+					reg = <0x0000000 0x100000>;  /* 1MB */
+					read-only;
+				};

arch/arm/boot/dts/armada-xp-mamba.dts line 166

Quoted 10 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +10 -0
+
+				partition at 100000 {
+					label = "u_env";
+					reg = <0x100000 0x40000>;    /* 256KB */
+				};
+
+				partition at 140000 {
+					label = "s_env";
+					reg = <0x140000 0x40000>;    /* 256KB */
+				};

So there is a big gap here? 768KB of unused space before the devinfo section?

...

Quoted 3 lines

The gap and the overlapping partitions (pretty much the whole layout) are straight from the official firmware. The overlap is used to flash a single file which contains the rootfs, too.

We could put a partition in the gap, to make it usable. You could for example place the DT blob there. I've not actually checked the version of uboot used actually supports this though.

Andrew

#4 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

In reply to Imre Kaloz
Quoted 3 lines · Imre Kaloz
Quoted 2 lines · Andrew Lunn

Thanks for the v2. I have a few comments, and some points we will need to discuss.

[...]

Quoted 16 lines
Quoted 12 lines

arch/arm/boot/dts/armada-xp-mamba.dts line 58

Quoted 3 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +3 -0
+    model = "Linksys WRT1900AC (Mamba)";
+    compatible = "linksys,mamba", "marvell,armadaxp-mv78230",
+             "marvell,armadaxp", "marvell,armada-370-xp";

So this is where the discussion starts. I don't like Mamba being so prominent. As far as i understand, Mamba is the board, not the device. In theory, another device could be created using the same board as a basis, but with different PCIe cards, etc. At that point, i would suggest refactoring the common parts out into a armada-xp-linksys-mamba.dtsi which is then included into any device .dts file using the Mamba board.

Quoted 3 lines

This file describes the device. So i would prefer it to be called armada-xp-linksys-wrt1900ac.dts. The first compatible should be "linksys,wrt1900ac". Having "linksys,mamba" second is O.K.

Quoted 8 lines

I would like to ask for others' opinion for multiple reasons, and would decide in v3 based on that.

- The device is called the "mamba", the marketing name is the WRT1900AC. As history showed, it's perfectly possible that exactly the same device go on the market under a different name. The E4200v2 is the same device as the EA4500, with a different factory firmware. There the name of the device is "viper".

If there is a good reason to have "mamba" in the chain of compatibles and "mamba" is the name of the platform/reference design, the compatible for this very board should look like this:

compatible = "linksys,wrt1900ac", "linksys,mamba",

    "marvell,armadaxp-mv78230", "marvell,armadaxp",
    "marvell,armada-370-xp";

That way you can have wrt1900ac specific stuff captured before the more generic mamba platform.

Quoted 10 lines

- OpenWrt is the only firmware/stack other than the official one and people already know this device as "mamba".

- Let's say the same device gets released under the same name or just the radios change - so no redesign takes place at all. In my opinion that hardly justifies adding multiple .dts files just to change the name of the LEDs to reflect that. I think people who want to run mainline on their device wouldn't be concerned about seeing a codename, but on the other hand we could receive patches to "correct" the marketing name in the LEDs.

As soon as you'll discover another "mamba"-based device, you can split-off the common stuff into a linksys-mamba.dtsi and include it into each of the two device dts files.

Right now, I'd suggest to have just a single linksys-wrt1900ac.dts.

[...]

Quoted 10 lines
Quoted 8 lines

arch/arm/boot/dts/armada-xp-mamba.dts line 241

Quoted 3 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +3 -0
+
+        power {
+            label = "mamba:white:power";

Please replace this mamba with wrt1900ac. It is a property of the device, not the board. Another device using the mamba board may use it differently.

See above.

The LED should be named by the device, not the platform. If OpenWRT userspace already expects "mamba" in here, I guess we are stuck with it. If not, call it "wrt1900ac:white:power".

Sebastian

#5 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines

arch/arm/boot/dts/armada-xp-mamba.dts line 241

Quoted 3 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +3 -0
+
+        power {
+            label = "mamba:white:power";

Please replace this mamba with wrt1900ac. It is a property of the device, not the board. Another device using the mamba board may use it differently.

Quoted 5 lines
Quoted 1 line

See above.

The LED should be named by the device, not the platform. If OpenWRT userspace already expects "mamba" in here, I guess we are stuck with it. If not, call it "wrt1900ac:white:power".

Hi Sebastian

We have some flexibility here. There is the mantra, don't break userspace, but that only applies once code has reached mainline. We don't have to be compatible with OpenWRT, if we feel it would be wrong to do so. OpenWRT should be well aware of the risk of not "Upstream first". However, if it does not cost us anything, we can be compatible.

I think Android is another good example. Some code has been accepted into mainline as is, other parts have been re-written with different APIs.

If it is decided that mamba is used everywhere, then it should be used here. If wrt1900ac is used, then i would like to see this LED named wrt1900ac. What you don't see in this dts file is the other 8 or so LEDs, which are connected to an I2c LED driver. A first version of the kernel driver has now been merged, the DT binding is stable, so we should add these LEDs to this dts file. But we should be consistent with the naming.

Andrew

#6 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

Quoted 11 lines · Andrew Lunn
Quoted 11 lines
Quoted 10 lines
Quoted 8 lines

arch/arm/boot/dts/armada-xp-mamba.dts line 241

Quoted 3 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +3 -0
+
+        power {
+            label = "mamba:white:power";

Please replace this mamba with wrt1900ac. It is a property of the device, not the board. Another device using the mamba board may use it differently.

See above.

Quoted 8 lines
Quoted 3 lines

The LED should be named by the device, not the platform. If OpenWRT userspace already expects "mamba" in here, I guess we are stuck with it. If not, call it "wrt1900ac:white:power".

Hi Sebastian

We have some flexibility here. There is the mantra, don't break userspace, but that only applies once code has reached mainline.

Full agree.

Quoted 3 lines

If it is decided that mamba is used everywhere, then it should be used here. If wrt1900ac is used, then i would like to see this LED named wrt1900ac.

I think the main problem here is that we are trying to predict the future decisions of an opaque entity (Linksys). We can't. All we can do is be specific about what we have before us today. Then we can create a new compatible string in the future that reflects the differences post-opaque-entity-decisions.

To that end, engineering is considerably more reliable than marketing. So I would be inclined to use 'mamba' for this board. If Linksys creates a variant of this board and keeps the board name, then we can call it 'mamba-v2'. Or, 'mamba-revB' if that's on the silk layer.

At any rate, unless the DT maintainers disagree, compatible strings should reflect engineering attributes, and 'model' should reflect marketing names users would be familiar with.

So...

	model = "Linksys WRT1900AC";
	compatible = "linksys,mamba", "...";

...

	power {
		label = "mamba:white:power";

The most deceptive part of this is that 'wrt1900ac' sounds *so* specific that we are inclined to trust it and use it. But beware, marketing folks have never heard of version control, compatibility, or consistency. They are more likely to pull a Crazy Ivan [1] on us than Ivan himself. :-)

thx,

Jason.

[1] https://en.wikipedia.org/wiki/Crazy_Ivan

#7 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

Hi Jason, Andrew,

Quoted 44 lines · Jason Cooper
Quoted 11 lines · Andrew Lunn
Quoted 11 lines
Quoted 10 lines
Quoted 8 lines

arch/arm/boot/dts/armada-xp-mamba.dts line 241

Quoted 3 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +3 -0
+
+        power {
+            label = "mamba:white:power";

Please replace this mamba with wrt1900ac. It is a property of the device, not the board. Another device using the mamba board may use it differently.

See above.

Quoted 8 lines
Quoted 3 lines

The LED should be named by the device, not the platform. If OpenWRT userspace already expects "mamba" in here, I guess we are stuck with it. If not, call it "wrt1900ac:white:power".

Hi Sebastian

We have some flexibility here. There is the mantra, don't break userspace, but that only applies once code has reached mainline.

Full agree.

Quoted 3 lines

If it is decided that mamba is used everywhere, then it should be used here. If wrt1900ac is used, then i would like to see this LED named wrt1900ac.

I think the main problem here is that we are trying to predict the future decisions of an opaque entity (Linksys). We can't. All we can do is be specific about what we have before us today. Then we can create a new compatible string in the future that reflects the differences post-opaque-entity-decisions.

To that end, engineering is considerably more reliable than marketing. So I would be inclined to use 'mamba' for this board. If Linksys creates a variant of this board and keeps the board name, then we can call it 'mamba-v2'. Or, 'mamba-revB' if that's on the silk layer.

At any rate, unless the DT maintainers disagree, compatible strings should reflect engineering attributes, and 'model' should reflect marketing names users would be familiar with.

So...

Quoted 8 lines
	model = "Linksys WRT1900AC";
	compatible = "linksys,mamba", "...";

...

	power {
		label = "mamba:white:power";

Until now for the Armada 37x/38x/XP/ evaluation board we used the marketing (or something close to it) as compatible name and the model was used for the extensive marketing name. It would have made more sens to use the "engineering" name as compatible string as Jason suggested. For this reason I agree with this binding but I don't have a strong opinion on it.

Thanks,

Gregory

Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
author
#8 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

In reply to Sebastian Hesselbarth

Hi Sebastian,

Quoted 11 lines · Sebastian Hesselbarth
Quoted 10 lines

- OpenWrt is the only firmware/stack other than the official one and people already know this device as "mamba".

- Let's say the same device gets released under the same name or just the radios change - so no redesign takes place at all. In my opinion that hardly justifies adding multiple .dts files just to change the name of the LEDs to reflect that. I think people who want to run mainline on their device wouldn't be concerned about seeing a codename, but on the other hand we could receive patches to "correct" the marketing name in the LEDs.

Quoted 5 lines

As soon as you'll discover another "mamba"-based device, you can split-off the common stuff into a linksys-mamba.dtsi and include it into each of the two device dts files.

Right now, I'd suggest to have just a single linksys-wrt1900ac.dts.

Right now the only device using the board is the "mamba" with "WRT1900AC" as the marketing name.

Let me try to change the question: if we'll have a device made by "Company" called "foobar" marketed as "ABC100", mainline really prefers it to be called "company,abc100" (and hence the leds "abc100:color:label") instead of "company,foobar"?

As I've said, the "viper" has been sold as the EA4200v2 and the EA4500. Other than the sticker, it's the same device. So for me calling a single device as "linksys,viper" makes way more sense than creating a linksys-viper.dtsi and have a linksys-ea4200v2.dts and a linksys-ea4500.dts - (so multiple kernels) just to make the model / led name follow the sticker.

Also, please mind we are talking about consumer stuff here, so it's likely there will be multiple versions (maybe with different socs/whatever). Do we really want to have devices named after random marketing name + version number combos? ABC100 can be the same as FOO1000v3 and BAR9999v5 or WTH555v2.

Imre

#9 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

Quoted 22 lines · Imre Kaloz

Hi Sebastian,

Quoted 11 lines · Sebastian Hesselbarth
Quoted 10 lines

- OpenWrt is the only firmware/stack other than the official one and people already know this device as "mamba".

- Let's say the same device gets released under the same name or just the radios change - so no redesign takes place at all. In my opinion that hardly justifies adding multiple .dts files just to change the name of the LEDs to reflect that. I think people who want to run mainline on their device wouldn't be concerned about seeing a codename, but on the other hand we could receive patches to "correct" the marketing name in the LEDs.

Quoted 5 lines

As soon as you'll discover another "mamba"-based device, you can split-off the common stuff into a linksys-mamba.dtsi and include it into each of the two device dts files.

Right now, I'd suggest to have just a single linksys-wrt1900ac.dts.

Quoted 7 lines

Right now the only device using the board is the "mamba" with "WRT1900AC" as the marketing name.

Let me try to change the question: if we'll have a device made by "Company" called "foobar" marketed as "ABC100", mainline really prefers it to be called "company,abc100" (and hence the leds "abc100:color:label") instead of "company,foobar"?

I don't know if there is a guideline about it but as I wrote in the other email. Using the marketing name as model and board name as compatible string would make sens.

Quoted 10 lines

As I've said, the "viper" has been sold as the EA4200v2 and the EA4500. Other than the sticker, it's the same device. So for me calling a single device as "linksys,viper" makes way more sense than creating a linksys-viper.dtsi and have a linksys-ea4200v2.dts and a linksys-ea4500.dts - (so multiple kernels) just to make the model / led name follow the sticker.

From my point of view the compatible name and dts[i|] files are two separate things.

This kind of information can (would?) be updated by the bootlader either natively or by using impedance-matcher. It would even be possible to use dt overlays. We don't have to provide one dts for any variation of the same board.

Gregory

Quoted 9 lines

Also, please mind we are talking about consumer stuff here, so it's likely there will be multiple versions (maybe with different socs/whatever). Do we really want to have devices named after random marketing name + version number combos? ABC100 can be the same as FOO1000v3 and BAR9999v5 or WTH555v2.

Imre

Signature · 4 lines
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#10 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

In reply to Imre Kaloz
Quoted 74 lines · Imre Kaloz

The Linksys WRT1900AC (Mamba) is a router that has

- 2 mini-PCIe slots with Marvell 88W8864 radios
- 1 USB 3.0 port
- 1 USB 2.0/eSATAp port
- 2 Ethernet interfaces connected to a 88E6172 switch (1x WAN + 4x LAN)
- 128MB NAND flash
- 256MB RAM

Signed-off-by Imre Kaloz <[email protected]>

---
Changes since v1:
* add dual license
* lower SPI speed to meet the chip's maximum
* pinctrl cleanups based on Andrew Lunn's suggestions

---
 arch/arm/boot/dts/armada-xp-mamba.dts | 273 ++++++++++++++++++++++++++++++++++
 1 file changed, 273 insertions(+)
 create mode 100644 arch/arm/boot/dts/armada-xp-mamba.dts
arch/arm/boot/dts/armada-xp-mamba.dts new file +48 -0
new file mode 100644
index 0000000..e368d18
@@ -0,0 +1,273 @@
+/*
+ * Device Tree file for the Linksys WRT1900AC (Mamba).
+ *
+ * Note: this board is shipped with a new generation boot loader that
+ * remaps internal registers at 0xf1000000. Therefore, if earlyprintk
+ * is used, the CONFIG_DEBUG_MVEBU_UART_ALTERNATE option should be
+ * used.
+ *
+ * Copyright (C) 2014 Imre Kaloz <[email protected]>
+ *
+ * Based on armada-xp-axpwifiap.dts:
+ *
+ *     Copyright (C) 2013 Marvell
+ *
+ *     Thomas Petazzoni <[email protected]>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */

arch/arm/boot/dts/armada-xp-mamba.dts line 58

Quoted 10 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +10 -0
+
+/dts-v1/;
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "armada-xp-mv78230.dtsi"
+
+/ {
+	model = "Linksys WRT1900AC (Mamba)";
+	compatible = "linksys,mamba", "marvell,armadaxp-mv78230",
+		     "marvell,armadaxp", "marvell,armada-370-xp";

Hi Imre

So the consensus seems to be

model = "Linksys WRT1900AC";
compatible = "linksys,mamba", "...";

with leds using mamba.

Please name the file armada-xp-linksys-mamba.dts.

Andrew

author
#11 lore raw

PATCH v2 1/1 mvebu: add Linksys WRT1900AC (Mamba) support

Hi Andrew,

Quoted 6 lines · Andrew Lunn

arch/arm/boot/dts/armada-xp-mamba.dts line 58

Quoted 3 lines
arch/arm/boot/dts/armada-xp-mamba.dts new file +3 -0
+	model = "Linksys WRT1900AC (Mamba)";
+	compatible = "linksys,mamba", "marvell,armadaxp-mv78230",
+		     "marvell,armadaxp", "marvell,armada-370-xp";

Hi Imre

Quoted 6 lines

So the consensus seems to be

model = "Linksys WRT1900AC";
compatible = "linksys,mamba", "...";

with leds using mamba.

Okie.

Quoted 1 line

Please name the file armada-xp-linksys-mamba.dts.

Will do, thanks everyone for the help.

Imre