[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[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 <kaloz-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
---
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

diff --git a/arch/arm/boot/dts/armada-xp-mamba.dts b/arch/arm/boot/dts/armada-xp-mamba.dts
new file mode 100644
index 0000000..e368d18
--- /dev/null
+++ b/arch/arm/boot/dts/armada-xp-mamba.dts
@@ -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 <kaloz-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
+ *
+ * Based on armada-xp-axpwifiap.dts:
+ *
+ *     Copyright (C) 2013 Marvell
+ *
+ *     Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * 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@1,0 {
+				/* Port 0, Lane 0 */
+				status = "okay";
+			};
+
+			/* First mini-PCIe port */
+			pcie@2,0 {
+				/* Port 0, Lane 1 */
+				status = "okay";
+			};
+
+			/* Second mini-PCIe port */
+			pcie@3,0 {
+				/* Port 0, Lane 3 */
+				status = "okay";
+			};
+		};
+
+		internal-regs {
+			serial@12000 {
+				status = "okay";
+			};
+
+			sata@a0000 {
+				nr-ports = <1>;
+				status = "okay";
+			};
+
+			ethernet@70000 {
+				pinctrl-0 = <&ge0_rgmii_pins>;
+				pinctrl-names = "default";
+				status = "okay";
+				phy-mode = "rgmii-id";
+				fixed-link {
+					speed = <1000>;
+					full-duplex;
+				};
+			};
+
+			ethernet@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@50000 {
+				status = "okay";
+			};
+
+			i2c@11000 {
+				status = "okay";
+				clock-frequency = <100000>;
+
+				tmp421@4c {
+					compatible = "ti,tmp421";
+					reg = <0x4c>;
+				};
+			};
+
+			nand@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@0 {
+					label = "u-boot";
+					reg = <0x0000000 0x100000>;  /* 1MB */
+					read-only;
+				};
+
+				partition@100000 {
+					label = "u_env";
+					reg = <0x100000 0x40000>;    /* 256KB */
+				};
+
+				partition@140000 {
+					label = "s_env";
+					reg = <0x140000 0x40000>;    /* 256KB */
+				};
+
+				partition@900000 {
+					label = "devinfo";
+					reg = <0x900000 0x100000>;    /* 1MB */
+					read-only;
+				};
+
+				partition@a00000 {
+					label = "kernel1";
+					reg = <0xa00000 0x2800000>;    /* 3MB + rootfs1 */
+				};
+
+				partition@d00000 {
+					label = "rootfs1";
+					reg = <0xd00000 0x2500000>;    /* 37MB */
+				};
+
+				partition@3200000 {
+					label = "kernel2";
+					reg = <0x3200000 0x2800000>;    /* 3MB + rootfs2 */
+				};
+
+				partition@3500000 {
+					label = "rootfs2";
+					reg = <0x3500000 0x2500000>;    /* 37MB */
+				};
+
+				/* Last MB is for the BBT, i.e. not writable */
+				partition@5a00000 {
+					label = "syscfg";
+					reg = <0x5a00000 0x2600000>;    /* 38MB */
+				};
+			};
+
+			spi0: spi@10600 {
+				status = "okay";
+
+				spi-flash@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@1 {
+			label = "WPS";
+			linux,code = <KEY_WPS_BUTTON>;
+			gpios = <&gpio1 0 GPIO_ACTIVE_HIGH>;
+		};
+
+		button@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";
+	};
+};
-- 
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[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 <kaloz@openwrt.org>
---
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

diff --git a/arch/arm/boot/dts/armada-xp-mamba.dts b/arch/arm/boot/dts/armada-xp-mamba.dts
new file mode 100644
index 0000000..e368d18
--- /dev/null
+++ b/arch/arm/boot/dts/armada-xp-mamba.dts
@@ -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 <kaloz@openwrt.org>
+ *
+ * Based on armada-xp-axpwifiap.dts:
+ *
+ *     Copyright (C) 2013 Marvell
+ *
+ *     Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+ *
+ * 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";
+	};
+};
-- 
2.1.0

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Andrew Lunn]

On Mon, Jan 19, 2015 at 06:15:01PM +0100, Imre Kaloz wrote:
> 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 <kaloz-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
> ---
> 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
> 
> diff --git a/arch/arm/boot/dts/armada-xp-mamba.dts b/arch/arm/boot/dts/armada-xp-mamba.dts
> new file mode 100644
> index 0000000..e368d18
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-xp-mamba.dts
> @@ -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.

> + *
> + * 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

> + * used.
> + *
> + * Copyright (C) 2014 Imre Kaloz <kaloz-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
> + *
> + * Based on armada-xp-axpwifiap.dts:
> + *
> + *     Copyright (C) 2013 Marvell
> + *
> + *     Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * 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";

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.

> +
> +	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@1,0 {
> +				/* Port 0, Lane 0 */
> +				status = "okay";
> +			};
> +
> +			/* First mini-PCIe port */
> +			pcie@2,0 {
> +				/* Port 0, Lane 1 */
> +				status = "okay";
> +			};
> +
> +			/* Second mini-PCIe port */
> +			pcie@3,0 {
> +				/* Port 0, Lane 3 */
> +				status = "okay";
> +			};
> +		};
> +
> +		internal-regs {

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

> +			serial@12000 {
> +				status = "okay";
> +			};
> +
> +			sata@a0000 {
> +				nr-ports = <1>;
> +				status = "okay";
> +			};
> +
> +			ethernet@70000 {
> +				pinctrl-0 = <&ge0_rgmii_pins>;
> +				pinctrl-names = "default";
> +				status = "okay";
> +				phy-mode = "rgmii-id";
> +				fixed-link {
> +					speed = <1000>;
> +					full-duplex;
> +				};
> +			};
> +
> +			ethernet@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@50000 {
> +				status = "okay";
> +			};
> +
> +			i2c@11000 {
> +				status = "okay";
> +				clock-frequency = <100000>;
> +
> +				tmp421@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.

> +			nand@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@0 {
> +					label = "u-boot";
> +					reg = <0x0000000 0x100000>;  /* 1MB */
> +					read-only;
> +				};
> +
> +				partition@100000 {
> +					label = "u_env";
> +					reg = <0x100000 0x40000>;    /* 256KB */
> +				};
> +
> +				partition@140000 {
> +					label = "s_env";
> +					reg = <0x140000 0x40000>;    /* 256KB */
> +				};

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

> +
> +				partition@900000 {
> +					label = "devinfo";
> +					reg = <0x900000 0x100000>;    /* 1MB */
> +					read-only;
> +				};
> +
> +				partition@a00000 {
> +					label = "kernel1";
> +					reg = <0xa00000 0x2800000>;    /* 3MB + rootfs1 */

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

> +				};

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...

> +
> +				partition@d00000 {
> +					label = "rootfs1";
> +					reg = <0xd00000 0x2500000>;    /* 37MB */
> +				};
> +
> +				partition@3200000 {
> +					label = "kernel2";
> +					reg = <0x3200000 0x2800000>;    /* 3MB + rootfs2 */
> +				};
> +
> +				partition@3500000 {
> +					label = "rootfs2";
> +					reg = <0x3500000 0x2500000>;    /* 37MB */
> +				};
> +
> +				/* Last MB is for the BBT, i.e. not writable */
> +				partition@5a00000 {
> +					label = "syscfg";
> +					reg = <0x5a00000 0x2600000>;    /* 38MB */
> +				};
> +			};
> +
> +			spi0: spi@10600 {
> +				status = "okay";
> +
> +				spi-flash@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@1 {
> +			label = "WPS";
> +			linux,code = <KEY_WPS_BUTTON>;
> +			gpios = <&gpio1 0 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		button@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";

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.

> +			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";
> +	};
> +};
> -- 
> 2.1.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Andrew Lunn]

On Mon, Jan 19, 2015 at 06:15:01PM +0100, Imre Kaloz wrote:
> 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 <kaloz@openwrt.org>
> ---
> 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
> 
> diff --git a/arch/arm/boot/dts/armada-xp-mamba.dts b/arch/arm/boot/dts/armada-xp-mamba.dts
> new file mode 100644
> index 0000000..e368d18
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-xp-mamba.dts
> @@ -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.

> + *
> + * 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

> + * used.
> + *
> + * Copyright (C) 2014 Imre Kaloz <kaloz@openwrt.org>
> + *
> + * Based on armada-xp-axpwifiap.dts:
> + *
> + *     Copyright (C) 2013 Marvell
> + *
> + *     Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> + *
> + * 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";

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.

> +
> +	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 {

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

> +			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>;
> +				};
> +			};
> +

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.

> +			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 */
> +				};

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

> +
> +				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.

> +				};

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...

> +
> +				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";

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.

> +			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";
> +	};
> +};
> -- 
> 2.1.0

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Imre Kaloz]

Hi Andrew,

On Mon, 19 Jan 2015 19:21:13 +0100, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:

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

Sure thing, thanks.

>> + *
>> + * 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.

>> +	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.

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.

>> +		internal-regs {
>
> It would be nice to document the jumper and pinout here.
>
>> +			serial@12000 {
>> +				status = "okay";
>> +			};

Do you mean serial console pinout?


> 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.

>
>> +			nand@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@0 {
>> +					label = "u-boot";
>> +					reg = <0x0000000 0x100000>;  /* 1MB */
>> +					read-only;
>> +				};
>> +
>> +				partition@100000 {
>> +					label = "u_env";
>> +					reg = <0x100000 0x40000>;    /* 256KB */
>> +				};
>> +
>> +				partition@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.

>> +
>> +				partition@900000 {
>> +					label = "devinfo";
>> +					reg = <0x900000 0x100000>;    /* 1MB */
>> +					read-only;
>> +				};
>> +
>> +				partition@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.

>
>> +				};
>
> 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.

>> +
>> +		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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Imre Kaloz]

Hi Andrew,

On Mon, 19 Jan 2015 19:21:13 +0100, Andrew Lunn <andrew@lunn.ch> wrote:

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

Sure thing, thanks.

>> + *
>> + * 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.

>> +	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.

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.

>> +		internal-regs {
>
> It would be nice to document the jumper and pinout here.
>
>> +			serial at 12000 {
>> +				status = "okay";
>> +			};

Do you mean serial console pinout?


> 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.

>
>> +			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 */
>> +				};
>
> So there is a big gap here? 768KB of unused space before the devinfo  
> section?

See below.

>> +
>> +				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.

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

>
>> +				};
>
> 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.

>> +
>> +		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

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Andrew Lunn]

> >
> >It would be nice to document the jumper and pinout here.
> >
> >>+			serial@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?
 
> >>+			nand@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@0 {
> >>+					label = "u-boot";
> >>+					reg = <0x0000000 0x100000>;  /* 1MB */
> >>+					read-only;
> >>+				};
> >>+
> >>+				partition@100000 {
> >>+					label = "u_env";
> >>+					reg = <0x100000 0x40000>;    /* 256KB */
> >>+				};
> >>+
> >>+				partition@140000 {
> >>+					label = "s_env";
> >>+					reg = <0x140000 0x40000>;    /* 256KB */
> >>+				};
> >
> >So there is a big gap here? 768KB of unused space before the
> >devinfo section?

... 
 
> 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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Andrew Lunn]

> >
> >It would be nice to document the jumper and pinout here.
> >
> >>+			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?
 
> >>+			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 */
> >>+				};
> >
> >So there is a big gap here? 768KB of unused space before the
> >devinfo section?

... 
 
> 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

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Sebastian Hesselbarth]

On 20.01.2015 11:57, Imre Kaloz wrote:
> On Mon, 19 Jan 2015 19:21:13 +0100, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>> Thanks for the v2. I have a few comments, and some points we will need
>> to discuss.
[...]
>>> +    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.
>
> 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.

> - 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.

[...]

>>> +
>>> +        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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Sebastian Hesselbarth]

On 20.01.2015 11:57, Imre Kaloz wrote:
> On Mon, 19 Jan 2015 19:21:13 +0100, Andrew Lunn <andrew@lunn.ch> wrote:
>> Thanks for the v2. I have a few comments, and some points we will need
>> to discuss.
[...]
>>> +    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.
>
> 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.

> - 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.

[...]

>>> +
>>> +        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

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Andrew Lunn]

> >>>+
> >>>+        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".

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Andrew Lunn]

> >>>+
> >>>+        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".

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

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Imre Kaloz]

Hi Sebastian,

On Sun, 25 Jan 2015 17:54:02 +0100, Sebastian Hesselbarth  
<sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

>> - 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.

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Imre Kaloz]

Hi Sebastian,

On Sun, 25 Jan 2015 17:54:02 +0100, Sebastian Hesselbarth  
<sebastian.hesselbarth@gmail.com> wrote:

>> - 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.

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

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Jason Cooper]

On Sun, Jan 25, 2015 at 06:09:25PM +0100, Andrew Lunn wrote:
> > >>>+
> > >>>+        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".
> 
> 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.

> 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

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Jason Cooper]

On Sun, Jan 25, 2015 at 06:09:25PM +0100, Andrew Lunn wrote:
> > >>>+
> > >>>+        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".
> 
> 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.

> 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

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Gregory CLEMENT]

Hi Jason, Andrew,

On 25/01/2015 20:18, Jason Cooper wrote:
> On Sun, Jan 25, 2015 at 06:09:25PM +0100, Andrew Lunn wrote:
>>>>>> +
>>>>>> +        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".
>>
>> 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.
> 
>> 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";
> 

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


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Gregory CLEMENT]

Hi Jason, Andrew,

On 25/01/2015 20:18, Jason Cooper wrote:
> On Sun, Jan 25, 2015 at 06:09:25PM +0100, Andrew Lunn wrote:
>>>>>> +
>>>>>> +        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".
>>
>> 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.
> 
>> 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";
> 

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


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Gregory CLEMENT]

On 25/01/2015 20:16, Imre Kaloz wrote:
> Hi Sebastian,
> 
> On Sun, 25 Jan 2015 17:54:02 +0100, Sebastian Hesselbarth  
> <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 
>>> - 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.
> 
> 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.

> 
> 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

> 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
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Gregory CLEMENT]

On 25/01/2015 20:16, Imre Kaloz wrote:
> Hi Sebastian,
> 
> On Sun, 25 Jan 2015 17:54:02 +0100, Sebastian Hesselbarth  
> <sebastian.hesselbarth@gmail.com> wrote:
> 
>>> - 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.
> 
> 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.

> 
> 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

> 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
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Andrew Lunn]

On Mon, Jan 19, 2015 at 06:15:01PM +0100, Imre Kaloz wrote:
> 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 <kaloz-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
> ---
> 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
> 
> diff --git a/arch/arm/boot/dts/armada-xp-mamba.dts b/arch/arm/boot/dts/armada-xp-mamba.dts
> new file mode 100644
> index 0000000..e368d18
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-xp-mamba.dts
> @@ -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 <kaloz-p3rKhJxN3npAfugRpC6u6w@public.gmane.org>
> + *
> + * Based on armada-xp-axpwifiap.dts:
> + *
> + *     Copyright (C) 2013 Marvell
> + *
> + *     Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * 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";

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Andrew Lunn]

On Mon, Jan 19, 2015 at 06:15:01PM +0100, Imre Kaloz wrote:
> 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 <kaloz@openwrt.org>
> ---
> 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
> 
> diff --git a/arch/arm/boot/dts/armada-xp-mamba.dts b/arch/arm/boot/dts/armada-xp-mamba.dts
> new file mode 100644
> index 0000000..e368d18
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-xp-mamba.dts
> @@ -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 <kaloz@openwrt.org>
> + *
> + * Based on armada-xp-axpwifiap.dts:
> + *
> + *     Copyright (C) 2013 Marvell
> + *
> + *     Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> + *
> + * 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";

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

Re: [PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Imre Kaloz]

Hi Andrew,

On Mon, 26 Jan 2015 19:44:36 +0100, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:

>> +	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.

Okie.

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

Will do, thanks everyone for the help.


Imre
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] mvebu: add Linksys WRT1900AC (Mamba) support

[Imre Kaloz]

Hi Andrew,

On Mon, 26 Jan 2015 19:44:36 +0100, Andrew Lunn <andrew@lunn.ch> wrote:

>> +	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.

Okie.

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

Will do, thanks everyone for the help.


Imre