| Message ID | 1421687701-5667-1-git-send-email-kaloz@openwrt.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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
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@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
> > > >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
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
> >>>+ > >>>+ 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
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
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
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
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 >
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
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
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@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"; + }; +};
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