Benoit, Thanks for the patch! A few minor things: Please Cc: the mvebu maintainers patches regarding Armada XP SoCs, I almost missed this. ;-) I know, we probably need to update MAINTAINERS for the dts{i} files... Also, Please adjust your Subject line to start with "ARM: mvebu: ...", you can use 'git log --oneline -- arch/arm/boot/dts/armada*' to get an idea of what we prefer to see. On Wed, Jul 23, 2014 at 05:07:11AM -0700, benoitm974 wrote: > Signed-off-by: benoitm974 <yahoo@xxxxxxxxxxxx> Please use your real name here, as well as in the From: of the email. > --- > arch/arm/boot/dts/Makefile | 1 + > arch/arm/boot/dts/armada-xp-lenovo-ix4300d.dts | 267 +++++++++++++++++++++++++ > 2 files changed, 268 insertions(+) > create mode 100644 arch/arm/boot/dts/armada-xp-lenovo-ix4300d.dts > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index adb5ed9..f759dd2 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -438,6 +438,7 @@ dtb-$(CONFIG_MACH_ARMADA_XP) += \ > armada-xp-db.dtb \ > armada-xp-gp.dtb \ > armada-xp-netgear-rn2120.dtb \ > + armada-xp-lenovo-ix4300d.dtb \ > armada-xp-matrix.dtb \ > armada-xp-openblocks-ax3-4.dtb Please place in alphabetical order. Yes, I know it wasn't to begin with. :( Feel free to fix it up while you are adding your line. > dtb-$(CONFIG_MACH_DOVE) += dove-cm-a510.dtb \ > diff --git a/arch/arm/boot/dts/armada-xp-lenovo-ix4300d.dts b/arch/arm/boot/dts/armada-xp-lenovo-ix4300d.dts > new file mode 100644 > index 0000000..e04e7a6 > --- /dev/null > +++ b/arch/arm/boot/dts/armada-xp-lenovo-ix4300d.dts > @@ -0,0 +1,267 @@ > +/* > + * Device Tree file for LENOVO IX4-300d > + * > + * Copyright (C) 2014, Benoit Masson <yahoo@xxxxxxxxxxxx> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version > + * 2 of the License, or (at your option) any later version. > + */ > + > +/dts-v1/; > + > +#include <dt-bindings/input/input.h> > +#include <dt-bindings/gpio/gpio.h> > +#include "armada-xp-mv78230.dtsi" > + > +/ { > + model = "LENOVO IX4-300d"; > + compatible = "lenovo,ix4-300d", "marvell,armadaxp-mv78230", "marvell,armadaxp", "marvell,armada-370-xp"; > + > + chosen { > + bootargs = "console=ttyS0,115200 earlyprintk"; > + }; > + > + memory { > + device_type = "memory"; > + reg = <0 0x00000000 0 0x20000000>; /* 512MB */ > + }; > + > + soc { > + ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xd0000000 0x100000 > + MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000>; > + > + pcie-controller { > + status = "okay"; > + > + pcie@1,0 { > + /* Port 0, Lane 0 */ > + status = "okay"; > + }; > + pcie@5,0 { > + /* Port 1, Lane 0 */ > + status = "okay"; > + }; > + spurious extra line, and it looks like you have some whitespace issues. Please make sure you use leading tabs. > + }; > + > + internal-regs { > + pinctrl { > + poweroff: poweroff { > + marvell,pins = "mpp24"; > + marvell,function = "gpio"; > + }; > + > + power_button_pin: power-button-pin { > + marvell,pins = "mpp44"; > + marvell,function = "gpio"; > + }; > + > + reset_button_pin: reset-button-pin { > + marvell,pins = "mpp45"; > + marvell,function = "gpio"; > + }; > + select_button_pin: select-button-pin { > + marvell,pins = "mpp41"; > + marvell,function = "gpio"; > + }; > + > + scroll_button_pin: scroll-button-pin { > + marvell,pins = "mpp42"; > + marvell,function = "gpio"; > + }; > + hdd_led_pin: hdd-led-pin { > + marvell,pins = "mpp26"; > + marvell,function = "gpio"; > + }; More leading tabs issues in this block. > + }; > + > + serial@12000 { > + clocks = <&coreclk 0>; > + status = "okay"; > + }; > + > + mdio { > + phy0: ethernet-phy@0 { /* Marvell 88E1318 */ > + reg = <0>; > + }; > + > + phy1: ethernet-phy@1 { /* Marvell 88E1318 */ > + reg = <1>; > + }; > + }; > + > + ethernet@70000 { > + status = "okay"; > + phy = <&phy0>; > + phy-mode = "rgmii-id"; > + }; > + > + ethernet@74000 { > + status = "okay"; > + phy = <&phy1>; > + phy-mode = "rgmii-id"; > + }; > + > + usb@50000 { > + status = "okay"; > + }; > + > + usb@51000 { > + status = "okay"; > + }; And here. > + > + i2c@11000 { > + compatible = "marvell,mv78230-a0-i2c", "marvell,mv64xxx-i2c"; > + clock-frequency = <400000>; And here. > + status = "okay"; > + > + adt7473@2e { > + compatible = "adt7473"; > + reg = <0x2e>; > + }; > + > + pcf8563@51 { > + compatible = "pcf8563"; > + reg = <0x51>; > + }; > + > + }; Need an empty line here. > + nand@d0000 { > + status = "okay"; > + num-cs = <1>; > + marvell,nand-keep-config; > + marvell,nand-enable-arbiter; > + nand-on-flash-bbt; > + > + partition@0 { > + label = "u-boot"; > + reg = <0x0000000 0xe0000>; > + read-only; > + }; > + > + partition@e0000 { > + label = "u-boot-env"; > + reg = <0xe0000 0x20000>; > + read-only; > + }; > + > + partition@100000 { > + label = "u-boot-env2"; > + reg = <0x100000 0x20000>; > + read-only; > + }; > + > + partition@120000 { > + label = "zImage"; > + reg = <0x120000 0x400000>; > + }; > + > + partition@520000 { > + label = "initrd"; > + reg = <0x520000 0x400000>; > + }; > + partition@xE00000 { > + label = "boot"; > + reg = <0xE00000 0x3F200000>; > + }; > + partition@flash { > + label = "flash"; > + reg = <0x0 0x40000000>; > + }; > + }; > + Don't need this empty line. > + }; > + }; Empty line here. > + gpio-keys { > + compatible = "gpio-keys"; > + pinctrl-0 = <&power_button_pin &reset_button_pin &select_button_pin &scroll_button_pin>; Yeah... I think you get the point ;-) please check the rest of the patch. I'll stop mentioning it. > + pinctrl-names = "default"; > + > + power-button { > + label = "Power Button"; > + linux,code = <KEY_POWER>; > + gpios = <&gpio1 12 GPIO_ACTIVE_HIGH>; > + }; > + reset-button { > + label = "Reset Button"; > + linux,code = <KEY_RESTART>; > + gpios = <&gpio1 13 GPIO_ACTIVE_LOW>; > + }; > + select-button { > + label = "Select Button"; > + linux,code = <BTN_SELECT>; > + gpios = <&gpio1 9 GPIO_ACTIVE_LOW>; > + }; > + scroll-button { > + label = "Scroll Button"; > + linux,code = <KEY_SCROLLDOWN>; > + gpios = <&gpio1 10 GPIO_ACTIVE_LOW>; > + }; > + }; > + > + spi3 { > + compatible = "spi-gpio"; > + status = "okay"; > + gpio-sck = <&gpio0 25 0>; > + gpio-mosi = <&gpio1 15 0>; /*gpio 47*/ > + cs-gpios = <&gpio0 27 0 >; I know no one else does it, but please use GPIO_ACTIVE_HIGH for these three. > + num-chipselects = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + gpio2: gpio2@0 { > + compatible = "fairchild,74hc595"; > + gpio-controller; > + #gpio-cells = <2>; > + reg = <0>; > + registers-number = <2>; > + spi-max-frequency = <100000>; > + }; > + }; > + > + gpio-leds { > + compatible = "gpio-leds"; > + pinctrl-0 = <&hdd_led_pin>; > + pinctrl-names = "default"; > + > + hdd-led { > + label = "ix4300d:blue:hdd"; > + gpios = <&gpio0 26 GPIO_ACTIVE_HIGH>; > + default-state = "off"; > + }; > + > + power-led { > + label = "ix4300d:power"; > + gpios = <&gpio2 1 GPIO_ACTIVE_LOW>; > + linux,default-trigger = "timer"; /* init blinking while booting */ Watch >80 char lines. > + default-state = "on"; > + }; > + > + sysfail-led { > + label = "ix4300d:sysfail:red"; > + gpios = <&gpio2 2 GPIO_ACTIVE_HIGH>; > + default-state = "off"; > + }; > + sys-led { > + label = "ix4300d:sys:blue"; > + gpios = <&gpio2 3 GPIO_ACTIVE_HIGH>; > + default-state = "off"; > + }; > + hddfail-led { > + label = "ix4300d:hddfail:red"; > + gpios = <&gpio2 4 GPIO_ACTIVE_HIGH>; > + default-state = "off"; > + }; > + > + }; > + /* warning: you need both eth1 & 0 to be initialize for poweroff to shutdown otherwise it reboots */ Same here, please convert to multi-line comment. > + gpio-poweroff { > + compatible = "gpio-poweroff"; > + pinctrl-0 = <&poweroff>; > + pinctrl-names = "default"; > + gpios = <&gpio0 24 GPIO_ACTIVE_HIGH>; > + }; > + > +}; This is a great first version, it really just a bunch of minor details to fixup for v2. Please Cc myself, Andrew, Gregory, and Sebastian when you send v2. I've included them in the Cc. thx, Jason. -- 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