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@xxxxxxxxxxx> > --- > 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@xxxxxxxxxxx> > + * > + * Based on armada-xp-axpwifiap.dts: > + * > + * Copyright (C) 2013 Marvell > + * > + * Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxxxxxxxxx> > + * > + * 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html