All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 12:07 ` benoitm974
  0 siblings, 0 replies; 38+ messages in thread
From: benoitm974 @ 2014-07-23 12:07 UTC (permalink / raw)
  To: benoitm, Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala,
	Russell King, devicetree, linux-arm-kernel, linux-kernel
  Cc: benoitm974

Signed-off-by: benoitm974 <yahoo@perenite.com>
---
 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
 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@perenite.com>
+ *
+ * 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";
+                        };
+
+		};
+
+		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";
+		                };
+			};
+
+			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";
+                        };
+
+			i2c@11000 {
+				compatible = "marvell,mv78230-a0-i2c", "marvell,mv64xxx-i2c";
+                                clock-frequency = <400000>;
+				status = "okay";
+
+				adt7473@2e {
+					compatible = "adt7473";
+					reg = <0x2e>;
+				};
+
+				pcf8563@51 {
+					compatible = "pcf8563";
+					reg = <0x51>;
+				};
+
+			};
+			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>;
+                                };
+                        };
+
+		};
+	};
+	gpio-keys {
+                compatible = "gpio-keys";
+		pinctrl-0 = <&power_button_pin &reset_button_pin &select_button_pin &scroll_button_pin>;
+                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 >;
+                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 */
+                        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 */
+	gpio-poweroff {
+                compatible = "gpio-poweroff";
+                pinctrl-0 = <&poweroff>;
+                pinctrl-names = "default";
+                gpios = <&gpio0 24 GPIO_ACTIVE_HIGH>;
+        };
+
+};
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 12:07 ` benoitm974
  0 siblings, 0 replies; 38+ messages in thread
From: benoitm974 @ 2014-07-23 12:07 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: benoitm974 <yahoo@perenite.com>
---
 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
 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@perenite.com>
+ *
+ * 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 at 1,0 {
+				/* Port 0, Lane 0 */
+				status = "okay";
+			};
+			pcie at 5,0 {
+                                /* Port 1, Lane 0 */
+                                status = "okay";
+                        };
+
+		};
+
+		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";
+		                };
+			};
+
+			serial at 12000 {
+				clocks = <&coreclk 0>;
+				status = "okay";
+			};
+
+			mdio {
+				phy0: ethernet-phy at 0 { /* Marvell 88E1318 */
+					reg = <0>;
+				};
+
+				phy1: ethernet-phy at 1 { /* Marvell 88E1318 */
+					reg = <1>;
+				};
+			};
+
+			ethernet at 70000 {
+				status = "okay";
+				phy = <&phy0>;
+				phy-mode = "rgmii-id";
+			};
+
+			ethernet at 74000 {
+				status = "okay";
+				phy = <&phy1>;
+				phy-mode = "rgmii-id";
+			};
+
+			usb at 50000 {
+                                status = "okay";
+                        };
+
+                        usb at 51000 {
+                                status = "okay";
+                        };
+
+			i2c at 11000 {
+				compatible = "marvell,mv78230-a0-i2c", "marvell,mv64xxx-i2c";
+                                clock-frequency = <400000>;
+				status = "okay";
+
+				adt7473 at 2e {
+					compatible = "adt7473";
+					reg = <0x2e>;
+				};
+
+				pcf8563 at 51 {
+					compatible = "pcf8563";
+					reg = <0x51>;
+				};
+
+			};
+			nand at d0000 {
+                                status = "okay";
+                                num-cs = <1>;
+                                marvell,nand-keep-config;
+                                marvell,nand-enable-arbiter;
+                                nand-on-flash-bbt;
+
+                                partition at 0 {
+                                        label = "u-boot";
+                                        reg = <0x0000000 0xe0000>;
+                                        read-only;
+                                };
+
+                                partition at e0000 {
+                                        label = "u-boot-env";
+                                        reg = <0xe0000 0x20000>;
+                                        read-only;
+                                };
+
+                                partition at 100000 {
+                                        label = "u-boot-env2";
+                                        reg = <0x100000 0x20000>;
+					read-only;
+                                };
+
+                                partition at 120000 {
+                                        label = "zImage";
+                                        reg = <0x120000 0x400000>;
+                                };
+
+                                partition at 520000 {
+                                        label = "initrd";
+                                        reg = <0x520000 0x400000>;
+                                };
+                                partition at xE00000 {
+                                        label = "boot";
+                                        reg = <0xE00000 0x3F200000>;
+                                };
+                                partition at flash {
+                                        label = "flash";
+                                        reg = <0x0 0x40000000>;
+                                };
+                        };
+
+		};
+	};
+	gpio-keys {
+                compatible = "gpio-keys";
+		pinctrl-0 = <&power_button_pin &reset_button_pin &select_button_pin &scroll_button_pin>;
+                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 >;
+                num-chipselects = <1>;
+                #address-cells = <1>;
+                #size-cells = <0>;
+
+                gpio2: gpio2 at 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 */
+                        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 */
+	gpio-poweroff {
+                compatible = "gpio-poweroff";
+                pinctrl-0 = <&poweroff>;
+                pinctrl-names = "default";
+                gpios = <&gpio0 24 GPIO_ACTIVE_HIGH>;
+        };
+
+};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 2/2] Adding lenovo in vendor
  2014-07-23 12:07 ` benoitm974
@ 2014-07-23 12:07   ` benoitm974
  -1 siblings, 0 replies; 38+ messages in thread
From: benoitm974 @ 2014-07-23 12:07 UTC (permalink / raw)
  To: benoitm, Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala,
	Russell King, devicetree, linux-arm-kernel, linux-kernel
  Cc: benoitm974

Signed-off-by: benoitm974 <yahoo@perenite.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 46a311e..de81a87 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -72,6 +72,7 @@ keymile	Keymile GmbH
 lacie	LaCie
 lantiq	Lantiq Semiconductor
 lg	LG Corporation
+lenovo	LENOVO
 linux	Linux-specific binding
 lsi	LSI Corp. (LSI Logic)
 lltc	Linear Technology Corporation
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 12:07   ` benoitm974
  0 siblings, 0 replies; 38+ messages in thread
From: benoitm974 @ 2014-07-23 12:07 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: benoitm974 <yahoo@perenite.com>
---
 Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 46a311e..de81a87 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -72,6 +72,7 @@ keymile	Keymile GmbH
 lacie	LaCie
 lantiq	Lantiq Semiconductor
 lg	LG Corporation
+lenovo	LENOVO
 linux	Linux-specific binding
 lsi	LSI Corp. (LSI Logic)
 lltc	Linear Technology Corporation
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 13:45   ` Jason Cooper
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 13:45 UTC (permalink / raw)
  To: benoitm974
  Cc: benoitm, Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala,
	Russell King, devicetree, linux-arm-kernel, linux-kernel,
	Andrew Lunn, Gregory CLEMENT, Sebastian Hesselbarth

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@perenite.com>

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@perenite.com>
> + *
> + * 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.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 13:45   ` Jason Cooper
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 13:45 UTC (permalink / raw)
  To: benoitm974
  Cc: benoitm-+V3Jd3LB6RBWk0Htik3J/w, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, Russell King,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew Lunn,
	Gregory CLEMENT, Sebastian Hesselbarth

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-+V3Jd3LB6RBWk0Htik3J/w@public.gmane.org>

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-+V3Jd3LB6RBWk0Htik3J/w@public.gmane.org>
> + *
> + * 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 13:45   ` Jason Cooper
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 13:45 UTC (permalink / raw)
  To: linux-arm-kernel

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@perenite.com>

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@perenite.com>
> + *
> + * 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 at 1,0 {
> +				/* Port 0, Lane 0 */
> +				status = "okay";
> +			};
> +			pcie at 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 at 12000 {
> +				clocks = <&coreclk 0>;
> +				status = "okay";
> +			};
> +
> +			mdio {
> +				phy0: ethernet-phy at 0 { /* Marvell 88E1318 */
> +					reg = <0>;
> +				};
> +
> +				phy1: ethernet-phy at 1 { /* Marvell 88E1318 */
> +					reg = <1>;
> +				};
> +			};
> +
> +			ethernet at 70000 {
> +				status = "okay";
> +				phy = <&phy0>;
> +				phy-mode = "rgmii-id";
> +			};
> +
> +			ethernet at 74000 {
> +				status = "okay";
> +				phy = <&phy1>;
> +				phy-mode = "rgmii-id";
> +			};
> +
> +			usb at 50000 {
> +                                status = "okay";
> +                        };
> +
> +                        usb at 51000 {
> +                                status = "okay";
> +                        };

And here.

> +
> +			i2c at 11000 {
> +				compatible = "marvell,mv78230-a0-i2c", "marvell,mv64xxx-i2c";
> +                                clock-frequency = <400000>;

And here.

> +				status = "okay";
> +
> +				adt7473 at 2e {
> +					compatible = "adt7473";
> +					reg = <0x2e>;
> +				};
> +
> +				pcf8563 at 51 {
> +					compatible = "pcf8563";
> +					reg = <0x51>;
> +				};
> +
> +			};

Need an empty line here.

> +			nand at d0000 {
> +                                status = "okay";
> +                                num-cs = <1>;
> +                                marvell,nand-keep-config;
> +                                marvell,nand-enable-arbiter;
> +                                nand-on-flash-bbt;
> +
> +                                partition at 0 {
> +                                        label = "u-boot";
> +                                        reg = <0x0000000 0xe0000>;
> +                                        read-only;
> +                                };
> +
> +                                partition at e0000 {
> +                                        label = "u-boot-env";
> +                                        reg = <0xe0000 0x20000>;
> +                                        read-only;
> +                                };
> +
> +                                partition at 100000 {
> +                                        label = "u-boot-env2";
> +                                        reg = <0x100000 0x20000>;
> +					read-only;
> +                                };
> +
> +                                partition at 120000 {
> +                                        label = "zImage";
> +                                        reg = <0x120000 0x400000>;
> +                                };
> +
> +                                partition at 520000 {
> +                                        label = "initrd";
> +                                        reg = <0x520000 0x400000>;
> +                                };
> +                                partition at xE00000 {
> +                                        label = "boot";
> +                                        reg = <0xE00000 0x3F200000>;
> +                                };
> +                                partition at 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 at 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.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 13:47     ` Jason Cooper
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 13:47 UTC (permalink / raw)
  To: benoitm974
  Cc: benoitm, Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala,
	Russell King, devicetree, linux-arm-kernel, linux-kernel

Benoit,

The same comments regard patch submission apply to this one as well.

On Wed, Jul 23, 2014 at 05:07:12AM -0700, benoitm974 wrote:

Please add a commit entry here describing Lenovo.  Keep it short and
sweet and maybe a link to their webpage.

> Signed-off-by: benoitm974 <yahoo@perenite.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 46a311e..de81a87 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -72,6 +72,7 @@ keymile	Keymile GmbH
>  lacie	LaCie
>  lantiq	Lantiq Semiconductor
>  lg	LG Corporation
> +lenovo	LENOVO

is this their official, registered company name?

>  linux	Linux-specific binding
>  lsi	LSI Corp. (LSI Logic)
>  lltc	Linear Technology Corporation

thx,

Jason.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 13:47     ` Jason Cooper
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 13:47 UTC (permalink / raw)
  To: benoitm974
  Cc: benoitm-+V3Jd3LB6RBWk0Htik3J/w, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, Russell King,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Benoit,

The same comments regard patch submission apply to this one as well.

On Wed, Jul 23, 2014 at 05:07:12AM -0700, benoitm974 wrote:

Please add a commit entry here describing Lenovo.  Keep it short and
sweet and maybe a link to their webpage.

> Signed-off-by: benoitm974 <yahoo-+V3Jd3LB6RBWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 46a311e..de81a87 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -72,6 +72,7 @@ keymile	Keymile GmbH
>  lacie	LaCie
>  lantiq	Lantiq Semiconductor
>  lg	LG Corporation
> +lenovo	LENOVO

is this their official, registered company name?

>  linux	Linux-specific binding
>  lsi	LSI Corp. (LSI Logic)
>  lltc	Linear Technology Corporation

thx,

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 13:47     ` Jason Cooper
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 13:47 UTC (permalink / raw)
  To: linux-arm-kernel

Benoit,

The same comments regard patch submission apply to this one as well.

On Wed, Jul 23, 2014 at 05:07:12AM -0700, benoitm974 wrote:

Please add a commit entry here describing Lenovo.  Keep it short and
sweet and maybe a link to their webpage.

> Signed-off-by: benoitm974 <yahoo@perenite.com>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 46a311e..de81a87 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -72,6 +72,7 @@ keymile	Keymile GmbH
>  lacie	LaCie
>  lantiq	Lantiq Semiconductor
>  lg	LG Corporation
> +lenovo	LENOVO

is this their official, registered company name?

>  linux	Linux-specific binding
>  lsi	LSI Corp. (LSI Logic)
>  lltc	Linear Technology Corporation

thx,

Jason.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 13:49   ` Jason Cooper
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 13:49 UTC (permalink / raw)
  To: benoitm974
  Cc: benoitm, Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala,
	Russell King, devicetree, linux-arm-kernel, linux-kernel

Benoit,

On Wed, Jul 23, 2014 at 05:07:11AM -0700, benoitm974 wrote:

I forgot to mention, please add a commit message describing this device
and possibly a good link to the company website describing it.

thx,

Jason.

> Signed-off-by: benoitm974 <yahoo@perenite.com>
> ---
>  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
>  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@perenite.com>
> + *
> + * 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";
> +                        };
> +
> +		};
> +
> +		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";
> +		                };
> +			};
> +
> +			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";
> +                        };
> +
> +			i2c@11000 {
> +				compatible = "marvell,mv78230-a0-i2c", "marvell,mv64xxx-i2c";
> +                                clock-frequency = <400000>;
> +				status = "okay";
> +
> +				adt7473@2e {
> +					compatible = "adt7473";
> +					reg = <0x2e>;
> +				};
> +
> +				pcf8563@51 {
> +					compatible = "pcf8563";
> +					reg = <0x51>;
> +				};
> +
> +			};
> +			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>;
> +                                };
> +                        };
> +
> +		};
> +	};
> +	gpio-keys {
> +                compatible = "gpio-keys";
> +		pinctrl-0 = <&power_button_pin &reset_button_pin &select_button_pin &scroll_button_pin>;
> +                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 >;
> +                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 */
> +                        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 */
> +	gpio-poweroff {
> +                compatible = "gpio-poweroff";
> +                pinctrl-0 = <&poweroff>;
> +                pinctrl-names = "default";
> +                gpios = <&gpio0 24 GPIO_ACTIVE_HIGH>;
> +        };
> +
> +};
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 13:49   ` Jason Cooper
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 13:49 UTC (permalink / raw)
  To: benoitm974
  Cc: benoitm-+V3Jd3LB6RBWk0Htik3J/w, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, Russell King,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

Benoit,

On Wed, Jul 23, 2014 at 05:07:11AM -0700, benoitm974 wrote:

I forgot to mention, please add a commit message describing this device
and possibly a good link to the company website describing it.

thx,

Jason.

> Signed-off-by: benoitm974 <yahoo-+V3Jd3LB6RBWk0Htik3J/w@public.gmane.org>
> ---
>  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
>  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-+V3Jd3LB6RBWk0Htik3J/w@public.gmane.org>
> + *
> + * 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";
> +                        };
> +
> +		};
> +
> +		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";
> +		                };
> +			};
> +
> +			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";
> +                        };
> +
> +			i2c@11000 {
> +				compatible = "marvell,mv78230-a0-i2c", "marvell,mv64xxx-i2c";
> +                                clock-frequency = <400000>;
> +				status = "okay";
> +
> +				adt7473@2e {
> +					compatible = "adt7473";
> +					reg = <0x2e>;
> +				};
> +
> +				pcf8563@51 {
> +					compatible = "pcf8563";
> +					reg = <0x51>;
> +				};
> +
> +			};
> +			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>;
> +                                };
> +                        };
> +
> +		};
> +	};
> +	gpio-keys {
> +                compatible = "gpio-keys";
> +		pinctrl-0 = <&power_button_pin &reset_button_pin &select_button_pin &scroll_button_pin>;
> +                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 >;
> +                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 */
> +                        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 */
> +	gpio-poweroff {
> +                compatible = "gpio-poweroff";
> +                pinctrl-0 = <&poweroff>;
> +                pinctrl-names = "default";
> +                gpios = <&gpio0 24 GPIO_ACTIVE_HIGH>;
> +        };
> +
> +};
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 13:49   ` Jason Cooper
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 13:49 UTC (permalink / raw)
  To: linux-arm-kernel

Benoit,

On Wed, Jul 23, 2014 at 05:07:11AM -0700, benoitm974 wrote:

I forgot to mention, please add a commit message describing this device
and possibly a good link to the company website describing it.

thx,

Jason.

> Signed-off-by: benoitm974 <yahoo@perenite.com>
> ---
>  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
>  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@perenite.com>
> + *
> + * 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 at 1,0 {
> +				/* Port 0, Lane 0 */
> +				status = "okay";
> +			};
> +			pcie at 5,0 {
> +                                /* Port 1, Lane 0 */
> +                                status = "okay";
> +                        };
> +
> +		};
> +
> +		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";
> +		                };
> +			};
> +
> +			serial at 12000 {
> +				clocks = <&coreclk 0>;
> +				status = "okay";
> +			};
> +
> +			mdio {
> +				phy0: ethernet-phy at 0 { /* Marvell 88E1318 */
> +					reg = <0>;
> +				};
> +
> +				phy1: ethernet-phy at 1 { /* Marvell 88E1318 */
> +					reg = <1>;
> +				};
> +			};
> +
> +			ethernet at 70000 {
> +				status = "okay";
> +				phy = <&phy0>;
> +				phy-mode = "rgmii-id";
> +			};
> +
> +			ethernet at 74000 {
> +				status = "okay";
> +				phy = <&phy1>;
> +				phy-mode = "rgmii-id";
> +			};
> +
> +			usb at 50000 {
> +                                status = "okay";
> +                        };
> +
> +                        usb at 51000 {
> +                                status = "okay";
> +                        };
> +
> +			i2c at 11000 {
> +				compatible = "marvell,mv78230-a0-i2c", "marvell,mv64xxx-i2c";
> +                                clock-frequency = <400000>;
> +				status = "okay";
> +
> +				adt7473 at 2e {
> +					compatible = "adt7473";
> +					reg = <0x2e>;
> +				};
> +
> +				pcf8563 at 51 {
> +					compatible = "pcf8563";
> +					reg = <0x51>;
> +				};
> +
> +			};
> +			nand at d0000 {
> +                                status = "okay";
> +                                num-cs = <1>;
> +                                marvell,nand-keep-config;
> +                                marvell,nand-enable-arbiter;
> +                                nand-on-flash-bbt;
> +
> +                                partition at 0 {
> +                                        label = "u-boot";
> +                                        reg = <0x0000000 0xe0000>;
> +                                        read-only;
> +                                };
> +
> +                                partition at e0000 {
> +                                        label = "u-boot-env";
> +                                        reg = <0xe0000 0x20000>;
> +                                        read-only;
> +                                };
> +
> +                                partition at 100000 {
> +                                        label = "u-boot-env2";
> +                                        reg = <0x100000 0x20000>;
> +					read-only;
> +                                };
> +
> +                                partition at 120000 {
> +                                        label = "zImage";
> +                                        reg = <0x120000 0x400000>;
> +                                };
> +
> +                                partition at 520000 {
> +                                        label = "initrd";
> +                                        reg = <0x520000 0x400000>;
> +                                };
> +                                partition at xE00000 {
> +                                        label = "boot";
> +                                        reg = <0xE00000 0x3F200000>;
> +                                };
> +                                partition at flash {
> +                                        label = "flash";
> +                                        reg = <0x0 0x40000000>;
> +                                };
> +                        };
> +
> +		};
> +	};
> +	gpio-keys {
> +                compatible = "gpio-keys";
> +		pinctrl-0 = <&power_button_pin &reset_button_pin &select_button_pin &scroll_button_pin>;
> +                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 >;
> +                num-chipselects = <1>;
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +
> +                gpio2: gpio2 at 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 */
> +                        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 */
> +	gpio-poweroff {
> +                compatible = "gpio-poweroff";
> +                pinctrl-0 = <&poweroff>;
> +                pinctrl-names = "default";
> +                gpios = <&gpio0 24 GPIO_ACTIVE_HIGH>;
> +        };
> +
> +};
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 14:14     ` Andrew Lunn
  0 siblings, 0 replies; 38+ messages in thread
From: Andrew Lunn @ 2014-07-23 14:14 UTC (permalink / raw)
  To: Jason Cooper
  Cc: benoitm974, benoitm, Rob Herring, Pawel Moll, Ian Campbell,
	Kumar Gala, Russell King, devicetree, linux-arm-kernel,
	linux-kernel, Andrew Lunn, Gregory CLEMENT,
	Sebastian Hesselbarth

Hi Benoit,

> 
> > +			};
> > +
> > +			serial@12000 {
> > +				clocks = <&coreclk 0>;

I don't think you need the clocks property. It should be already set
in armada-xp.dtsi.

> > +				adt7473@2e {
> > +					compatible = "adt7473";

Please include a vendor prefix here. 


> > +					reg = <0x2e>;
> > +				};
> > +
> > +				pcf8563@51 {
> > +					compatible = "pcf8563";

and a vendor prefix here.

> > +	/* warning: you need both eth1 & 0 to be initialize for poweroff to shutdown otherwise it reboots */

What do you mean by initialized? Driver loaded? Interface up? 

> This is a great first version

I agree with Jason, well done.

  Andrew

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 14:14     ` Andrew Lunn
  0 siblings, 0 replies; 38+ messages in thread
From: Andrew Lunn @ 2014-07-23 14:14 UTC (permalink / raw)
  To: Jason Cooper
  Cc: benoitm974, benoitm-+V3Jd3LB6RBWk0Htik3J/w, Rob Herring,
	Pawel Moll, Ian Campbell, Kumar Gala, Russell King,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew Lunn,
	Gregory CLEMENT, Sebastian Hesselbarth

Hi Benoit,

> 
> > +			};
> > +
> > +			serial@12000 {
> > +				clocks = <&coreclk 0>;

I don't think you need the clocks property. It should be already set
in armada-xp.dtsi.

> > +				adt7473@2e {
> > +					compatible = "adt7473";

Please include a vendor prefix here. 


> > +					reg = <0x2e>;
> > +				};
> > +
> > +				pcf8563@51 {
> > +					compatible = "pcf8563";

and a vendor prefix here.

> > +	/* warning: you need both eth1 & 0 to be initialize for poweroff to shutdown otherwise it reboots */

What do you mean by initialized? Driver loaded? Interface up? 

> This is a great first version

I agree with Jason, well done.

  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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 14:14     ` Andrew Lunn
  0 siblings, 0 replies; 38+ messages in thread
From: Andrew Lunn @ 2014-07-23 14:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Benoit,

> 
> > +			};
> > +
> > +			serial at 12000 {
> > +				clocks = <&coreclk 0>;

I don't think you need the clocks property. It should be already set
in armada-xp.dtsi.

> > +				adt7473 at 2e {
> > +					compatible = "adt7473";

Please include a vendor prefix here. 


> > +					reg = <0x2e>;
> > +				};
> > +
> > +				pcf8563 at 51 {
> > +					compatible = "pcf8563";

and a vendor prefix here.

> > +	/* warning: you need both eth1 & 0 to be initialize for poweroff to shutdown otherwise it reboots */

What do you mean by initialized? Driver loaded? Interface up? 

> This is a great first version

I agree with Jason, well done.

  Andrew

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 15:10       ` Andrew Lunn
  0 siblings, 0 replies; 38+ messages in thread
From: Andrew Lunn @ 2014-07-23 15:10 UTC (permalink / raw)
  To: Jason Cooper
  Cc: benoitm974, devicetree, Russell King, Pawel Moll, Ian Campbell,
	linux-kernel, Rob Herring, Kumar Gala, benoitm, linux-arm-kernel

> > +lenovo	LENOVO
> 
> is this their official, registered company name?

http://www.lenovo.com/ww/lenovo/FAQs.html says

1. What is Lenovo’s stock symbol (ticker)?
HKSE: 992.hk
ADR (Level I): LNVGY

So i think lenovo is probably the more understandable vendor prefix,
even if it is not the official stock ticker.

However there official registered company name appears to be
Lenovo Group Limited.

       Andrew

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 15:10       ` Andrew Lunn
  0 siblings, 0 replies; 38+ messages in thread
From: Andrew Lunn @ 2014-07-23 15:10 UTC (permalink / raw)
  To: Jason Cooper
  Cc: benoitm974, devicetree-u79uwXL29TY76Z2rM5mHXA, Russell King,
	Pawel Moll, Ian Campbell, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Rob Herring, Kumar Gala, benoitm-+V3Jd3LB6RBWk0Htik3J/w,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

> > +lenovo	LENOVO
> 
> is this their official, registered company name?

http://www.lenovo.com/ww/lenovo/FAQs.html says

1. What is Lenovo’s stock symbol (ticker)?
HKSE: 992.hk
ADR (Level I): LNVGY

So i think lenovo is probably the more understandable vendor prefix,
even if it is not the official stock ticker.

However there official registered company name appears to be
Lenovo Group Limited.

       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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 15:10       ` Andrew Lunn
  0 siblings, 0 replies; 38+ messages in thread
From: Andrew Lunn @ 2014-07-23 15:10 UTC (permalink / raw)
  To: linux-arm-kernel

> > +lenovo	LENOVO
> 
> is this their official, registered company name?

http://www.lenovo.com/ww/lenovo/FAQs.html says

1. What is Lenovo?s stock symbol (ticker)?
HKSE: 992.hk
ADR (Level I): LNVGY

So i think lenovo is probably the more understandable vendor prefix,
even if it is not the official stock ticker.

However there official registered company name appears to be
Lenovo Group Limited.

       Andrew

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 15:45     ` Sebastian Hesselbarth
  0 siblings, 0 replies; 38+ messages in thread
From: Sebastian Hesselbarth @ 2014-07-23 15:45 UTC (permalink / raw)
  To: Jason Cooper, benoitm974
  Cc: benoitm, Rob Herring, Pawel Moll, Ian Campbell, Kumar Gala,
	Russell King, devicetree, linux-arm-kernel, linux-kernel,
	Andrew Lunn, Gregory CLEMENT, Sebastian Hesselbarth

On 07/23/2014 03:45 PM, Jason Cooper wrote:
> Thanks for the patch!  A few minor things:

Benoit,

I also have some minor remarks just because I stumble over them
all the time.

>> ---
>>   arch/arm/boot/dts/Makefile                     |   1 +
>>   arch/arm/boot/dts/armada-xp-lenovo-ix4300d.dts | 267 +++++++++++++++++++++++++

I looked it up and Lenovo names it "ix4-300d", maybe you should
rename the dts "armada-xp-lenovo-ix4-300d.dts" then.

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

It is spelled "Lenovo" and the full name is "Lenovo Iomega ix4-300d".
As it is the initial commit, we should be more sensitive on the case
here.

>> + * Copyright (C) 2014, Benoit Masson <yahoo@perenite.com>
>> + *
>> + * 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";

Same comment about "Lenovo Iomega ix4-300d"

>> +	compatible = "lenovo,ix4-300d", "marvell,armadaxp-mv78230", "marvell,armadaxp", "marvell,armada-370-xp";
>> +
>> +	chosen {
>> +		bootargs = "console=ttyS0,115200 earlyprintk";

Consider to add

stdout-path = "/soc/internal-regs/serial@12000";

That way you would not have to fix it up for e.g. Barebox.
Unfortunately, neither Armada 370 nor XP dtsi define node
labels yet, so you have to put the full node path.

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

Any chance you know what is connected to above PCIe ports?
If so, put a comment above the node itself naming the attached
device, e.g.

/* USB 3.0 xHCI controller */
pcie@1,0 {...}

Also, I saw you have a good impression about the GPIOs used on
the ix4-300d. If you also know the PERST# GPIOs, add them with

reset-gpios = <&gpioN M GPIO_ACTIVE_LOW>;

where N, M follow gpio# = (32 * N) + M, i.e. N=1, M=22 for gpio54.

>> +		};
>> +
>> +		internal-regs {
>> +			pinctrl {
>> +				poweroff: poweroff {

nit: poweroff_pin: poweroff-pin {...}
like the ones below?

>> +                                        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";
>> +		                };
[...]
>> +			};
>> +
>> +			serial@12000 {
>> +				clocks = <&coreclk 0>;

As Andrew already said, no need to duplicate the clocks property.

>> +				status = "okay";
>> +			};
[...]
>> +	spi3 {
>> +                compatible = "spi-gpio";
>> +                status = "okay";
>> +                gpio-sck = <&gpio0 25 0>;
>> +                gpio-mosi = <&gpio1 15 0>; /*gpio 47*/
>> +                cs-gpios = <&gpio0 27 0 >;
[...]
>> +                num-chipselects = <1>;
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +
>> +                gpio2: gpio2@0 {
>> +                        compatible = "fairchild,74hc595";

Freaky we have a driver for a 74x595 8-bit shift register ;)

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

Can you get rid of "ix4300d:" or rename it to "ix4-300d:" without
breaking any userspace stuff? If so, please update all LEDs.

>> +                        gpios = <&gpio0 26 GPIO_ACTIVE_HIGH>;
>> +                        default-state = "off";
>> +                };
[...]
>> +	/* warning: you need both eth1 & 0 to be initialize for poweroff to shutdown otherwise it reboots */

I don't get the comment.

> This is a great first version

Yup, Thanks!

Sebastian

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 15:45     ` Sebastian Hesselbarth
  0 siblings, 0 replies; 38+ messages in thread
From: Sebastian Hesselbarth @ 2014-07-23 15:45 UTC (permalink / raw)
  To: Jason Cooper, benoitm974
  Cc: benoitm-+V3Jd3LB6RBWk0Htik3J/w, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, Russell King,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andrew Lunn,
	Gregory CLEMENT, Sebastian Hesselbarth

On 07/23/2014 03:45 PM, Jason Cooper wrote:
> Thanks for the patch!  A few minor things:

Benoit,

I also have some minor remarks just because I stumble over them
all the time.

>> ---
>>   arch/arm/boot/dts/Makefile                     |   1 +
>>   arch/arm/boot/dts/armada-xp-lenovo-ix4300d.dts | 267 +++++++++++++++++++++++++

I looked it up and Lenovo names it "ix4-300d", maybe you should
rename the dts "armada-xp-lenovo-ix4-300d.dts" then.

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

It is spelled "Lenovo" and the full name is "Lenovo Iomega ix4-300d".
As it is the initial commit, we should be more sensitive on the case
here.

>> + * Copyright (C) 2014, Benoit Masson <yahoo-+V3Jd3LB6RBWk0Htik3J/w@public.gmane.org>
>> + *
>> + * 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";

Same comment about "Lenovo Iomega ix4-300d"

>> +	compatible = "lenovo,ix4-300d", "marvell,armadaxp-mv78230", "marvell,armadaxp", "marvell,armada-370-xp";
>> +
>> +	chosen {
>> +		bootargs = "console=ttyS0,115200 earlyprintk";

Consider to add

stdout-path = "/soc/internal-regs/serial@12000";

That way you would not have to fix it up for e.g. Barebox.
Unfortunately, neither Armada 370 nor XP dtsi define node
labels yet, so you have to put the full node path.

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

Any chance you know what is connected to above PCIe ports?
If so, put a comment above the node itself naming the attached
device, e.g.

/* USB 3.0 xHCI controller */
pcie@1,0 {...}

Also, I saw you have a good impression about the GPIOs used on
the ix4-300d. If you also know the PERST# GPIOs, add them with

reset-gpios = <&gpioN M GPIO_ACTIVE_LOW>;

where N, M follow gpio# = (32 * N) + M, i.e. N=1, M=22 for gpio54.

>> +		};
>> +
>> +		internal-regs {
>> +			pinctrl {
>> +				poweroff: poweroff {

nit: poweroff_pin: poweroff-pin {...}
like the ones below?

>> +                                        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";
>> +		                };
[...]
>> +			};
>> +
>> +			serial@12000 {
>> +				clocks = <&coreclk 0>;

As Andrew already said, no need to duplicate the clocks property.

>> +				status = "okay";
>> +			};
[...]
>> +	spi3 {
>> +                compatible = "spi-gpio";
>> +                status = "okay";
>> +                gpio-sck = <&gpio0 25 0>;
>> +                gpio-mosi = <&gpio1 15 0>; /*gpio 47*/
>> +                cs-gpios = <&gpio0 27 0 >;
[...]
>> +                num-chipselects = <1>;
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +
>> +                gpio2: gpio2@0 {
>> +                        compatible = "fairchild,74hc595";

Freaky we have a driver for a 74x595 8-bit shift register ;)

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

Can you get rid of "ix4300d:" or rename it to "ix4-300d:" without
breaking any userspace stuff? If so, please update all LEDs.

>> +                        gpios = <&gpio0 26 GPIO_ACTIVE_HIGH>;
>> +                        default-state = "off";
>> +                };
[...]
>> +	/* warning: you need both eth1 & 0 to be initialize for poweroff to shutdown otherwise it reboots */

I don't get the comment.

> This is a great first version

Yup, Thanks!

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 15:45     ` Sebastian Hesselbarth
  0 siblings, 0 replies; 38+ messages in thread
From: Sebastian Hesselbarth @ 2014-07-23 15:45 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/23/2014 03:45 PM, Jason Cooper wrote:
> Thanks for the patch!  A few minor things:

Benoit,

I also have some minor remarks just because I stumble over them
all the time.

>> ---
>>   arch/arm/boot/dts/Makefile                     |   1 +
>>   arch/arm/boot/dts/armada-xp-lenovo-ix4300d.dts | 267 +++++++++++++++++++++++++

I looked it up and Lenovo names it "ix4-300d", maybe you should
rename the dts "armada-xp-lenovo-ix4-300d.dts" then.

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

It is spelled "Lenovo" and the full name is "Lenovo Iomega ix4-300d".
As it is the initial commit, we should be more sensitive on the case
here.

>> + * Copyright (C) 2014, Benoit Masson <yahoo@perenite.com>
>> + *
>> + * 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";

Same comment about "Lenovo Iomega ix4-300d"

>> +	compatible = "lenovo,ix4-300d", "marvell,armadaxp-mv78230", "marvell,armadaxp", "marvell,armada-370-xp";
>> +
>> +	chosen {
>> +		bootargs = "console=ttyS0,115200 earlyprintk";

Consider to add

stdout-path = "/soc/internal-regs/serial at 12000";

That way you would not have to fix it up for e.g. Barebox.
Unfortunately, neither Armada 370 nor XP dtsi define node
labels yet, so you have to put the full node path.

>> +	};
>> +
[...]
>> +	soc {
>> +		ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xd0000000 0x100000
>> +			  MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000>;
>> +
>> +		pcie-controller {
>> +			status = "okay";
>> +
>> +			pcie at 1,0 {
>> +				/* Port 0, Lane 0 */
>> +				status = "okay";
>> +			};
>> +			pcie at 5,0 {
>> +                                /* Port 1, Lane 0 */
>> +                                status = "okay";
>> +                        };
>> +

Any chance you know what is connected to above PCIe ports?
If so, put a comment above the node itself naming the attached
device, e.g.

/* USB 3.0 xHCI controller */
pcie at 1,0 {...}

Also, I saw you have a good impression about the GPIOs used on
the ix4-300d. If you also know the PERST# GPIOs, add them with

reset-gpios = <&gpioN M GPIO_ACTIVE_LOW>;

where N, M follow gpio# = (32 * N) + M, i.e. N=1, M=22 for gpio54.

>> +		};
>> +
>> +		internal-regs {
>> +			pinctrl {
>> +				poweroff: poweroff {

nit: poweroff_pin: poweroff-pin {...}
like the ones below?

>> +                                        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";
>> +		                };
[...]
>> +			};
>> +
>> +			serial at 12000 {
>> +				clocks = <&coreclk 0>;

As Andrew already said, no need to duplicate the clocks property.

>> +				status = "okay";
>> +			};
[...]
>> +	spi3 {
>> +                compatible = "spi-gpio";
>> +                status = "okay";
>> +                gpio-sck = <&gpio0 25 0>;
>> +                gpio-mosi = <&gpio1 15 0>; /*gpio 47*/
>> +                cs-gpios = <&gpio0 27 0 >;
[...]
>> +                num-chipselects = <1>;
>> +                #address-cells = <1>;
>> +                #size-cells = <0>;
>> +
>> +                gpio2: gpio2 at 0 {
>> +                        compatible = "fairchild,74hc595";

Freaky we have a driver for a 74x595 8-bit shift register ;)

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

Can you get rid of "ix4300d:" or rename it to "ix4-300d:" without
breaking any userspace stuff? If so, please update all LEDs.

>> +                        gpios = <&gpio0 26 GPIO_ACTIVE_HIGH>;
>> +                        default-state = "off";
>> +                };
[...]
>> +	/* warning: you need both eth1 & 0 to be initialize for poweroff to shutdown otherwise it reboots */

I don't get the comment.

> This is a great first version

Yup, Thanks!

Sebastian

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
  2014-07-23 14:14     ` Andrew Lunn
@ 2014-07-23 15:52       ` Benoit Masson
  -1 siblings, 0 replies; 38+ messages in thread
From: Benoit Masson @ 2014-07-23 15:52 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jason Cooper, benoitm974, Rob Herring, Pawel Moll, Ian Campbell,
	Kumar Gala, Russell King, devicetree, linux-arm-kernel,
	linux-kernel, Gregory CLEMENT, Sebastian Hesselbarth

Hi Andrew I've taken your comment into account, yet for the PHY init comment, see my answer below.

Le 23 juil. 2014 à 16:14, Andrew Lunn <andrew@lunn.ch> a écrit :
> 
>>> +	/* warning: you need both eth1 & 0 to be initialize for poweroff to shutdown otherwise it reboots */
> 
> What do you mean by initialized? Driver loaded? Interface up? 
Well actually the PHY need to be initialized (at least 1 mII reg written), which from marvel LSP driver always occurs, while it doesn't with mainline PHY driver (drivers/net/phy/marvell.c), so the only simple way I found to have at least one PHY reg on both interface written is to have both eth up at OS config level.

Probably the best option would be to have a reg-init = <reg offset value> on both phy dts definition but the current armada mii doesn't support this dts config...

> 
>> This is a great first version
> 
> I agree with Jason, well done.
> 
Thanks !
>  Andrew


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 15:52       ` Benoit Masson
  0 siblings, 0 replies; 38+ messages in thread
From: Benoit Masson @ 2014-07-23 15:52 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Andrew I've taken your comment into account, yet for the PHY init comment, see my answer below.

Le 23 juil. 2014 ? 16:14, Andrew Lunn <andrew@lunn.ch> a ?crit :
> 
>>> +	/* warning: you need both eth1 & 0 to be initialize for poweroff to shutdown otherwise it reboots */
> 
> What do you mean by initialized? Driver loaded? Interface up? 
Well actually the PHY need to be initialized (at least 1 mII reg written), which from marvel LSP driver always occurs, while it doesn't with mainline PHY driver (drivers/net/phy/marvell.c), so the only simple way I found to have at least one PHY reg on both interface written is to have both eth up at OS config level.

Probably the best option would be to have a reg-init = <reg offset value> on both phy dts definition but the current armada mii doesn't support this dts config...

> 
>> This is a great first version
> 
> I agree with Jason, well done.
> 
Thanks !
>  Andrew

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
  2014-07-23 15:52       ` Benoit Masson
@ 2014-07-23 16:49         ` Andrew Lunn
  -1 siblings, 0 replies; 38+ messages in thread
From: Andrew Lunn @ 2014-07-23 16:49 UTC (permalink / raw)
  To: Benoit Masson
  Cc: Andrew Lunn, Jason Cooper, benoitm974, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, Russell King, devicetree,
	linux-arm-kernel, linux-kernel, Gregory CLEMENT,
	Sebastian Hesselbarth

> Well actually the PHY need to be initialized (at least 1 mII reg
> written), which from marvel LSP driver always occurs, while it
> doesn't with mainline PHY driver (drivers/net/phy/marvell.c), so the
> only simple way I found to have at least one PHY reg on both
> interface written is to have both eth up at OS config level.

Thanks for the information. This sounds like a wake on LAN feature.
I've seen other Marvell hardware connect a PHY LED output pin to the
circuit controlling the main power supply. When the PHY detects the
magic wake-up packet, it 'blinks' the LED so turning the power back
on.

My guess is, the register write to the PHY is configuring the LED. Do
you have the datasheet for the PHY? Can you check this?

> Probably the best option would be to have a reg-init = <reg offset
> value> on both phy dts definition but the current armada mii doesn't
> support this dts config...

Once we understand what is going on here, we can consider adding
support for this.

	Andrew

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 16:49         ` Andrew Lunn
  0 siblings, 0 replies; 38+ messages in thread
From: Andrew Lunn @ 2014-07-23 16:49 UTC (permalink / raw)
  To: linux-arm-kernel

> Well actually the PHY need to be initialized (at least 1 mII reg
> written), which from marvel LSP driver always occurs, while it
> doesn't with mainline PHY driver (drivers/net/phy/marvell.c), so the
> only simple way I found to have at least one PHY reg on both
> interface written is to have both eth up at OS config level.

Thanks for the information. This sounds like a wake on LAN feature.
I've seen other Marvell hardware connect a PHY LED output pin to the
circuit controlling the main power supply. When the PHY detects the
magic wake-up packet, it 'blinks' the LED so turning the power back
on.

My guess is, the register write to the PHY is configuring the LED. Do
you have the datasheet for the PHY? Can you check this?

> Probably the best option would be to have a reg-init = <reg offset
> value> on both phy dts definition but the current armada mii doesn't
> support this dts config...

Once we understand what is going on here, we can consider adding
support for this.

	Andrew

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/2] Adding lenovo in vendor
  2014-07-23 15:10       ` Andrew Lunn
@ 2014-07-23 17:26         ` Jason Cooper
  -1 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 17:26 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: benoitm974, devicetree, Russell King, Pawel Moll, Ian Campbell,
	linux-kernel, Rob Herring, Kumar Gala, benoitm, linux-arm-kernel

On Wed, Jul 23, 2014 at 05:10:33PM +0200, Andrew Lunn wrote:
> > > +lenovo	LENOVO
> > 
> > is this their official, registered company name?
> 
> http://www.lenovo.com/ww/lenovo/FAQs.html says
> 
> 1. What is Lenovo’s stock symbol (ticker)?
> HKSE: 992.hk
> ADR (Level I): LNVGY
> 
> So i think lenovo is probably the more understandable vendor prefix,
> even if it is not the official stock ticker.
> 
> However there official registered company name appears to be
> Lenovo Group Limited.

Agreed, Benoit, please do 'lenovo  Lenovo Group Limited'

thx,

Jason.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 17:26         ` Jason Cooper
  0 siblings, 0 replies; 38+ messages in thread
From: Jason Cooper @ 2014-07-23 17:26 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jul 23, 2014 at 05:10:33PM +0200, Andrew Lunn wrote:
> > > +lenovo	LENOVO
> > 
> > is this their official, registered company name?
> 
> http://www.lenovo.com/ww/lenovo/FAQs.html says
> 
> 1. What is Lenovo?s stock symbol (ticker)?
> HKSE: 992.hk
> ADR (Level I): LNVGY
> 
> So i think lenovo is probably the more understandable vendor prefix,
> even if it is not the official stock ticker.
> 
> However there official registered company name appears to be
> Lenovo Group Limited.

Agreed, Benoit, please do 'lenovo  Lenovo Group Limited'

thx,

Jason.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 21:03           ` Benoit Masson
  0 siblings, 0 replies; 38+ messages in thread
From: Benoit Masson @ 2014-07-23 21:03 UTC (permalink / raw)
  To: Jason Cooper
  Cc: Andrew Lunn, benoitm974, devicetree, Russell King, Pawel Moll,
	Ian Campbell, linux-kernel, Rob Herring, Kumar Gala,
	linux-arm-kernel

Looking at the vendor list it should then be "Lenovo Group Ltd." since no other vendor except 1 use Limited all other use Ltd.
Fine with this ?
Le 23 juil. 2014 à 19:26, Jason Cooper <jason@lakedaemon.net> a écrit :

> On Wed, Jul 23, 2014 at 05:10:33PM +0200, Andrew Lunn wrote:
>>>> +lenovo	LENOVO
>>> 
>>> is this their official, registered company name?
>> 
>> http://www.lenovo.com/ww/lenovo/FAQs.html says
>> 
>> 1. What is Lenovo’s stock symbol (ticker)?
>> HKSE: 992.hk
>> ADR (Level I): LNVGY
>> 
>> So i think lenovo is probably the more understandable vendor prefix,
>> even if it is not the official stock ticker.
>> 
>> However there official registered company name appears to be
>> Lenovo Group Limited.
> 
> Agreed, Benoit, please do 'lenovo  Lenovo Group Limited'
> 
> thx,
> 
> Jason.


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 21:03           ` Benoit Masson
  0 siblings, 0 replies; 38+ messages in thread
From: Benoit Masson @ 2014-07-23 21:03 UTC (permalink / raw)
  To: Jason Cooper
  Cc: Andrew Lunn, benoitm974, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Russell King, Pawel Moll, Ian Campbell,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Kumar Gala,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Looking at the vendor list it should then be "Lenovo Group Ltd." since no other vendor except 1 use Limited all other use Ltd.
Fine with this ?
Le 23 juil. 2014 à 19:26, Jason Cooper <jason-NLaQJdtUoK4Be96aLqz0jA@public.gmane.org> a écrit :

> On Wed, Jul 23, 2014 at 05:10:33PM +0200, Andrew Lunn wrote:
>>>> +lenovo	LENOVO
>>> 
>>> is this their official, registered company name?
>> 
>> http://www.lenovo.com/ww/lenovo/FAQs.html says
>> 
>> 1. What is Lenovo’s stock symbol (ticker)?
>> HKSE: 992.hk
>> ADR (Level I): LNVGY
>> 
>> So i think lenovo is probably the more understandable vendor prefix,
>> even if it is not the official stock ticker.
>> 
>> However there official registered company name appears to be
>> Lenovo Group Limited.
> 
> Agreed, Benoit, please do 'lenovo  Lenovo Group Limited'
> 
> thx,
> 
> Jason.

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 2/2] Adding lenovo in vendor
@ 2014-07-23 21:03           ` Benoit Masson
  0 siblings, 0 replies; 38+ messages in thread
From: Benoit Masson @ 2014-07-23 21:03 UTC (permalink / raw)
  To: linux-arm-kernel

Looking at the vendor list it should then be "Lenovo Group Ltd." since no other vendor except 1 use Limited all other use Ltd.
Fine with this ?
Le 23 juil. 2014 ? 19:26, Jason Cooper <jason@lakedaemon.net> a ?crit :

> On Wed, Jul 23, 2014 at 05:10:33PM +0200, Andrew Lunn wrote:
>>>> +lenovo	LENOVO
>>> 
>>> is this their official, registered company name?
>> 
>> http://www.lenovo.com/ww/lenovo/FAQs.html says
>> 
>> 1. What is Lenovo?s stock symbol (ticker)?
>> HKSE: 992.hk
>> ADR (Level I): LNVGY
>> 
>> So i think lenovo is probably the more understandable vendor prefix,
>> even if it is not the official stock ticker.
>> 
>> However there official registered company name appears to be
>> Lenovo Group Limited.
> 
> Agreed, Benoit, please do 'lenovo  Lenovo Group Limited'
> 
> thx,
> 
> Jason.

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 21:15           ` Benoit Masson
  0 siblings, 0 replies; 38+ messages in thread
From: Benoit Masson @ 2014-07-23 21:15 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jason Cooper, benoitm974, Rob Herring, Pawel Moll, Ian Campbell,
	Kumar Gala, Russell King, devicetree, linux-arm-kernel,
	linux-kernel, Gregory CLEMENT, Sebastian Hesselbarth

Well after several test with the original BSP driver I found that this is not fully WOL related. It could be that the LED pin is linked to poweron for WOL, yet here the issue is on shutdown pulling up MPP24 should poweroff the deice but only reboots it except is both PHY have some reg written to...

By the way the current nv_neta does not seems to support WOL for 88e1318 and still having them both up is enough to have poweroff to work, which is why I'm putting the WOL as a side track.

Both phy are :
marvell,88e1318

example of a minimal reg write that lead MPP24 to shutdown instead of rebooting on original BSP driver
XXXXX BasicInit
XXXXXXXXXXXXXX phyAdr 0: regOffs: 16 data: 3 caller: mvEthE1310PhyBasicInit+0x2c/0x58
XXXXXXXXXXXXXX phyAdr 0: regOffs: 10 data: 830 caller: mvEthE1310PhyBasicInit+0x3c/0x58
XXXXXXXXXXXXXX phyAdr 0: regOffs: 16 data: 0 caller: mvEthE1310PhyBasicInit+0x4c/0x58
XXXXX BasicInit
XXXXXXXXXXXXXX phyAdr 1: regOffs: 16 data: 3 caller: mvEthE1310PhyBasicInit+0x2c/0x58
XXXXXXXXXXXXXX phyAdr 1: regOffs: 10 data: 830 caller: mvEthE1310PhyBasicInit+0x3c/0x58
XXXXXXXXXXXXXX phyAdr 1: regOffs: 16 data: 0 caller: mvEthE1310PhyBasicInit+0x4c/0x58

I've tried something like:
compatible = "marvell,88e1318";
                                        device_type = "ethernet-phy";
                                        marvell,reg-init =
                                          /* Init led/activity workaround for MP24 shutdown. */
                                          <3 0x10 0 0x830>;

In the dts but without luck

Regards,
Benoit


Le 23 juil. 2014 à 18:49, Andrew Lunn <andrew@lunn.ch> a écrit :

>> Well actually the PHY need to be initialized (at least 1 mII reg
>> written), which from marvel LSP driver always occurs, while it
>> doesn't with mainline PHY driver (drivers/net/phy/marvell.c), so the
>> only simple way I found to have at least one PHY reg on both
>> interface written is to have both eth up at OS config level.
> 
> Thanks for the information. This sounds like a wake on LAN feature.
> I've seen other Marvell hardware connect a PHY LED output pin to the
> circuit controlling the main power supply. When the PHY detects the
> magic wake-up packet, it 'blinks' the LED so turning the power back
> on.
> 
> My guess is, the register write to the PHY is configuring the LED. Do
> you have the datasheet for the PHY? Can you check this?
> 
>> Probably the best option would be to have a reg-init = <reg offset
>> value> on both phy dts definition but the current armada mii doesn't
>> support this dts config...
> 
> Once we understand what is going on here, we can consider adding
> support for this.
> 
> 	Andrew


^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 21:15           ` Benoit Masson
  0 siblings, 0 replies; 38+ messages in thread
From: Benoit Masson @ 2014-07-23 21:15 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jason Cooper, benoitm974, Rob Herring, Pawel Moll, Ian Campbell,
	Kumar Gala, Russell King, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Gregory CLEMENT,
	Sebastian Hesselbarth

Well after several test with the original BSP driver I found that this is not fully WOL related. It could be that the LED pin is linked to poweron for WOL, yet here the issue is on shutdown pulling up MPP24 should poweroff the deice but only reboots it except is both PHY have some reg written to...

By the way the current nv_neta does not seems to support WOL for 88e1318 and still having them both up is enough to have poweroff to work, which is why I'm putting the WOL as a side track.

Both phy are :
marvell,88e1318

example of a minimal reg write that lead MPP24 to shutdown instead of rebooting on original BSP driver
XXXXX BasicInit
XXXXXXXXXXXXXX phyAdr 0: regOffs: 16 data: 3 caller: mvEthE1310PhyBasicInit+0x2c/0x58
XXXXXXXXXXXXXX phyAdr 0: regOffs: 10 data: 830 caller: mvEthE1310PhyBasicInit+0x3c/0x58
XXXXXXXXXXXXXX phyAdr 0: regOffs: 16 data: 0 caller: mvEthE1310PhyBasicInit+0x4c/0x58
XXXXX BasicInit
XXXXXXXXXXXXXX phyAdr 1: regOffs: 16 data: 3 caller: mvEthE1310PhyBasicInit+0x2c/0x58
XXXXXXXXXXXXXX phyAdr 1: regOffs: 10 data: 830 caller: mvEthE1310PhyBasicInit+0x3c/0x58
XXXXXXXXXXXXXX phyAdr 1: regOffs: 16 data: 0 caller: mvEthE1310PhyBasicInit+0x4c/0x58

I've tried something like:
compatible = "marvell,88e1318";
                                        device_type = "ethernet-phy";
                                        marvell,reg-init =
                                          /* Init led/activity workaround for MP24 shutdown. */
                                          <3 0x10 0 0x830>;

In the dts but without luck

Regards,
Benoit


Le 23 juil. 2014 à 18:49, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> a écrit :

>> Well actually the PHY need to be initialized (at least 1 mII reg
>> written), which from marvel LSP driver always occurs, while it
>> doesn't with mainline PHY driver (drivers/net/phy/marvell.c), so the
>> only simple way I found to have at least one PHY reg on both
>> interface written is to have both eth up at OS config level.
> 
> Thanks for the information. This sounds like a wake on LAN feature.
> I've seen other Marvell hardware connect a PHY LED output pin to the
> circuit controlling the main power supply. When the PHY detects the
> magic wake-up packet, it 'blinks' the LED so turning the power back
> on.
> 
> My guess is, the register write to the PHY is configuring the LED. Do
> you have the datasheet for the PHY? Can you check this?
> 
>> Probably the best option would be to have a reg-init = <reg offset
>> value> on both phy dts definition but the current armada mii doesn't
>> support this dts config...
> 
> Once we understand what is going on here, we can consider adding
> support for this.
> 
> 	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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 21:15           ` Benoit Masson
  0 siblings, 0 replies; 38+ messages in thread
From: Benoit Masson @ 2014-07-23 21:15 UTC (permalink / raw)
  To: linux-arm-kernel

Well after several test with the original BSP driver I found that this is not fully WOL related. It could be that the LED pin is linked to poweron for WOL, yet here the issue is on shutdown pulling up MPP24 should poweroff the deice but only reboots it except is both PHY have some reg written to...

By the way the current nv_neta does not seems to support WOL for 88e1318 and still having them both up is enough to have poweroff to work, which is why I'm putting the WOL as a side track.

Both phy are :
marvell,88e1318

example of a minimal reg write that lead MPP24 to shutdown instead of rebooting on original BSP driver
XXXXX BasicInit
XXXXXXXXXXXXXX phyAdr 0: regOffs: 16 data: 3 caller: mvEthE1310PhyBasicInit+0x2c/0x58
XXXXXXXXXXXXXX phyAdr 0: regOffs: 10 data: 830 caller: mvEthE1310PhyBasicInit+0x3c/0x58
XXXXXXXXXXXXXX phyAdr 0: regOffs: 16 data: 0 caller: mvEthE1310PhyBasicInit+0x4c/0x58
XXXXX BasicInit
XXXXXXXXXXXXXX phyAdr 1: regOffs: 16 data: 3 caller: mvEthE1310PhyBasicInit+0x2c/0x58
XXXXXXXXXXXXXX phyAdr 1: regOffs: 10 data: 830 caller: mvEthE1310PhyBasicInit+0x3c/0x58
XXXXXXXXXXXXXX phyAdr 1: regOffs: 16 data: 0 caller: mvEthE1310PhyBasicInit+0x4c/0x58

I've tried something like:
compatible = "marvell,88e1318";
                                        device_type = "ethernet-phy";
                                        marvell,reg-init =
                                          /* Init led/activity workaround for MP24 shutdown. */
                                          <3 0x10 0 0x830>;

In the dts but without luck

Regards,
Benoit


Le 23 juil. 2014 ? 18:49, Andrew Lunn <andrew@lunn.ch> a ?crit :

>> Well actually the PHY need to be initialized (at least 1 mII reg
>> written), which from marvel LSP driver always occurs, while it
>> doesn't with mainline PHY driver (drivers/net/phy/marvell.c), so the
>> only simple way I found to have at least one PHY reg on both
>> interface written is to have both eth up at OS config level.
> 
> Thanks for the information. This sounds like a wake on LAN feature.
> I've seen other Marvell hardware connect a PHY LED output pin to the
> circuit controlling the main power supply. When the PHY detects the
> magic wake-up packet, it 'blinks' the LED so turning the power back
> on.
> 
> My guess is, the register write to the PHY is configuring the LED. Do
> you have the datasheet for the PHY? Can you check this?
> 
>> Probably the best option would be to have a reg-init = <reg offset
>> value> on both phy dts definition but the current armada mii doesn't
>> support this dts config...
> 
> Once we understand what is going on here, we can consider adding
> support for this.
> 
> 	Andrew

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
  2014-07-23 21:15           ` Benoit Masson
@ 2014-07-23 22:26             ` Andrew Lunn
  -1 siblings, 0 replies; 38+ messages in thread
From: Andrew Lunn @ 2014-07-23 22:26 UTC (permalink / raw)
  To: Benoit Masson
  Cc: Andrew Lunn, Jason Cooper, benoitm974, Rob Herring, Pawel Moll,
	Ian Campbell, Kumar Gala, Russell King, devicetree,
	linux-arm-kernel, linux-kernel, Gregory CLEMENT,
	Sebastian Hesselbarth

> Both phy are :
> marvell,88e1318

I don't have the datasheet for this specific phy model, but i do have
the datasheet for another similar phy.
 
> example of a minimal reg write that lead MPP24 to shutdown instead
> of rebooting on original BSP driver

> XXXXX BasicInit

I'm assuming regOffs is decimal, and data is hex?

> phyAdr 0: regOffs: 16 data: 3

Copper Specific control register. 3 means Polarity Reversal Disable &
Jabber function disable.

> phyAdr 0: regOffs: 10 data: 830

Reg 10 is the 1000BASE-T Status register, which is read only!

So this is not making much sense. Are we missing some changes to the
page register? Register 16 of page 3 is the LED control register.

     Andrew


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 22:26             ` Andrew Lunn
  0 siblings, 0 replies; 38+ messages in thread
From: Andrew Lunn @ 2014-07-23 22:26 UTC (permalink / raw)
  To: linux-arm-kernel

> Both phy are :
> marvell,88e1318

I don't have the datasheet for this specific phy model, but i do have
the datasheet for another similar phy.
 
> example of a minimal reg write that lead MPP24 to shutdown instead
> of rebooting on original BSP driver

> XXXXX BasicInit

I'm assuming regOffs is decimal, and data is hex?

> phyAdr 0: regOffs: 16 data: 3

Copper Specific control register. 3 means Polarity Reversal Disable &
Jabber function disable.

> phyAdr 0: regOffs: 10 data: 830

Reg 10 is the 1000BASE-T Status register, which is read only!

So this is not making much sense. Are we missing some changes to the
page register? Register 16 of page 3 is the LED control register.

     Andrew

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
  2014-07-23 22:26             ` Andrew Lunn
@ 2014-07-23 23:26               ` Benoit Masson
  -1 siblings, 0 replies; 38+ messages in thread
From: Benoit Masson @ 2014-07-23 23:26 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jason Cooper, benoitm974, Rob Herring, Pawel Moll, Ian Campbell,
	Kumar Gala, Russell King, devicetree, linux-arm-kernel,
	linux-kernel, Gregory CLEMENT, Sebastian Hesselbarth

Well I'm glad you spend time on this. So I'll explain more in detail where I am on this one.

I have the vendor 3.0.29 kernel patched with LSP drivers from Marvell (which includes the PHY & Net drivers).

With this version (called LSPkernel now on) pulling MPP24 GPIO output up shutdown the power of the device.

At first I though it could be watchdog or wol related so I deep dive into both drivers and start uncommenting register write by register write, in the end the LSPKernel start rebooting instead of poweroff when no phy register were written at all... which is when I start thinking about that its was maybe not really which reg were written but that both phy were to be writtent to at least on reg for the MPP24 to poweroff the device and that worked out.

So as soon as both phy get "m88e1318_config_aneg" fucntion called from "drivers/net/phy/marvell.c" it works fine.

Also maybe you can help me there although the marvell.c has m88e1318_get_wol & m88e1318_set_wol setup as part of the MARVELL_PHY_ID_88E1318S calling "ethtool -s eth0 wol g" from debian return a non-supported error .. any though ?

Benoit
Le 24 juil. 2014 à 00:26, Andrew Lunn <andrew@lunn.ch> a écrit :

>> Both phy are :
>> marvell,88e1318
> 
> I don't have the datasheet for this specific phy model, but i do have
> the datasheet for another similar phy.
> 
>> example of a minimal reg write that lead MPP24 to shutdown instead
>> of rebooting on original BSP driver
> 
>> XXXXX BasicInit
> 
> I'm assuming regOffs is decimal, and data is hex?
> 
>> phyAdr 0: regOffs: 16 data: 3
> 
> Copper Specific control register. 3 means Polarity Reversal Disable &
> Jabber function disable.
> 
>> phyAdr 0: regOffs: 10 data: 830
> 
> Reg 10 is the 1000BASE-T Status register, which is read only!
> 
> So this is not making much sense. Are we missing some changes to the
> page register? Register 16 of page 3 is the LED control register.
> 
>     Andrew
> 


^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas
@ 2014-07-23 23:26               ` Benoit Masson
  0 siblings, 0 replies; 38+ messages in thread
From: Benoit Masson @ 2014-07-23 23:26 UTC (permalink / raw)
  To: linux-arm-kernel

Well I'm glad you spend time on this. So I'll explain more in detail where I am on this one.

I have the vendor 3.0.29 kernel patched with LSP drivers from Marvell (which includes the PHY & Net drivers).

With this version (called LSPkernel now on) pulling MPP24 GPIO output up shutdown the power of the device.

At first I though it could be watchdog or wol related so I deep dive into both drivers and start uncommenting register write by register write, in the end the LSPKernel start rebooting instead of poweroff when no phy register were written at all... which is when I start thinking about that its was maybe not really which reg were written but that both phy were to be writtent to at least on reg for the MPP24 to poweroff the device and that worked out.

So as soon as both phy get "m88e1318_config_aneg" fucntion called from "drivers/net/phy/marvell.c" it works fine.

Also maybe you can help me there although the marvell.c has m88e1318_get_wol & m88e1318_set_wol setup as part of the MARVELL_PHY_ID_88E1318S calling "ethtool -s eth0 wol g" from debian return a non-supported error .. any though ?

Benoit
Le 24 juil. 2014 ? 00:26, Andrew Lunn <andrew@lunn.ch> a ?crit :

>> Both phy are :
>> marvell,88e1318
> 
> I don't have the datasheet for this specific phy model, but i do have
> the datasheet for another similar phy.
> 
>> example of a minimal reg write that lead MPP24 to shutdown instead
>> of rebooting on original BSP driver
> 
>> XXXXX BasicInit
> 
> I'm assuming regOffs is decimal, and data is hex?
> 
>> phyAdr 0: regOffs: 16 data: 3
> 
> Copper Specific control register. 3 means Polarity Reversal Disable &
> Jabber function disable.
> 
>> phyAdr 0: regOffs: 10 data: 830
> 
> Reg 10 is the 1000BASE-T Status register, which is read only!
> 
> So this is not making much sense. Are we missing some changes to the
> page register? Register 16 of page 3 is the LED control register.
> 
>     Andrew
> 

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2014-07-23 23:26 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-23 12:07 [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas benoitm974
2014-07-23 12:07 ` benoitm974
2014-07-23 12:07 ` [PATCH 2/2] Adding lenovo in vendor benoitm974
2014-07-23 12:07   ` benoitm974
2014-07-23 13:47   ` Jason Cooper
2014-07-23 13:47     ` Jason Cooper
2014-07-23 13:47     ` Jason Cooper
2014-07-23 15:10     ` Andrew Lunn
2014-07-23 15:10       ` Andrew Lunn
2014-07-23 15:10       ` Andrew Lunn
2014-07-23 17:26       ` Jason Cooper
2014-07-23 17:26         ` Jason Cooper
2014-07-23 21:03         ` Benoit Masson
2014-07-23 21:03           ` Benoit Masson
2014-07-23 21:03           ` Benoit Masson
2014-07-23 13:45 ` [PATCH 1/2] Added dts defintion for Lenovo ix4-300d nas Jason Cooper
2014-07-23 13:45   ` Jason Cooper
2014-07-23 13:45   ` Jason Cooper
2014-07-23 14:14   ` Andrew Lunn
2014-07-23 14:14     ` Andrew Lunn
2014-07-23 14:14     ` Andrew Lunn
2014-07-23 15:52     ` Benoit Masson
2014-07-23 15:52       ` Benoit Masson
2014-07-23 16:49       ` Andrew Lunn
2014-07-23 16:49         ` Andrew Lunn
2014-07-23 21:15         ` Benoit Masson
2014-07-23 21:15           ` Benoit Masson
2014-07-23 21:15           ` Benoit Masson
2014-07-23 22:26           ` Andrew Lunn
2014-07-23 22:26             ` Andrew Lunn
2014-07-23 23:26             ` Benoit Masson
2014-07-23 23:26               ` Benoit Masson
2014-07-23 15:45   ` Sebastian Hesselbarth
2014-07-23 15:45     ` Sebastian Hesselbarth
2014-07-23 15:45     ` Sebastian Hesselbarth
2014-07-23 13:49 ` Jason Cooper
2014-07-23 13:49   ` Jason Cooper
2014-07-23 13:49   ` Jason Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.