All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Add Armada 385 General Purpose Development Board support
@ 2014-12-27 11:00 ` Gregory CLEMENT
  0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2014-12-27 11:00 UTC (permalink / raw)
  To: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT
  Cc: Thomas Petazzoni, Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi,

this patch set adds the support for the Armada 385 GP board, this board
is pretty close to the Armada 385 RD one. It comes with a new revision
of the Armada 385 SoC (B0).

I added a patch from Maxime's serie to support the AP board to this
series.

Thanks,

Grégory


Gregory CLEMENT (2):
  ARM: mvebu: a38x: Add the pinctrl alias
  ARM: mvebu: Add Armada 385 General Purpose Development Board support

Maxime Ripard (1):
  ARM: mvebu: a38x: Fix node names

 arch/arm/boot/dts/Makefile          |   1 +
 arch/arm/boot/dts/armada-385-db.dts |   2 +-
 arch/arm/boot/dts/armada-385-gp.dts | 380 ++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/armada-385-rd.dts |   2 +-
 arch/arm/boot/dts/armada-385.dtsi   |   2 +-
 arch/arm/boot/dts/armada-38x.dtsi   |   4 +-
 6 files changed, 386 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/boot/dts/armada-385-gp.dts

-- 
1.9.1

--
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] 30+ messages in thread

* [PATCH 0/3] Add Armada 385 General Purpose Development Board support
@ 2014-12-27 11:00 ` Gregory CLEMENT
  0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2014-12-27 11:00 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

this patch set adds the support for the Armada 385 GP board, this board
is pretty close to the Armada 385 RD one. It comes with a new revision
of the Armada 385 SoC (B0).

I added a patch from Maxime's serie to support the AP board to this
series.

Thanks,

Gr?gory


Gregory CLEMENT (2):
  ARM: mvebu: a38x: Add the pinctrl alias
  ARM: mvebu: Add Armada 385 General Purpose Development Board support

Maxime Ripard (1):
  ARM: mvebu: a38x: Fix node names

 arch/arm/boot/dts/Makefile          |   1 +
 arch/arm/boot/dts/armada-385-db.dts |   2 +-
 arch/arm/boot/dts/armada-385-gp.dts | 380 ++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/armada-385-rd.dts |   2 +-
 arch/arm/boot/dts/armada-385.dtsi   |   2 +-
 arch/arm/boot/dts/armada-38x.dtsi   |   4 +-
 6 files changed, 386 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/boot/dts/armada-385-gp.dts

-- 
1.9.1

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

* [PATCH 1/3] ARM: mvebu: a38x: Fix node names
  2014-12-27 11:00 ` Gregory CLEMENT
@ 2014-12-27 11:00     ` Gregory CLEMENT
  -1 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2014-12-27 11:00 UTC (permalink / raw)
  To: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT
  Cc: Thomas Petazzoni, Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

From: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

Some nodes in the DTs have a reg property but no unit name in their node name.

This contradicts the way the ePAPR defines the node names. Fix this.

Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/armada-385-db.dts | 2 +-
 arch/arm/boot/dts/armada-385-rd.dts | 2 +-
 arch/arm/boot/dts/armada-385.dtsi   | 2 +-
 arch/arm/boot/dts/armada-38x.dtsi   | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/armada-385-db.dts b/arch/arm/boot/dts/armada-385-db.dts
index 2aaa9d2ac284..212605ccc7b6 100644
--- a/arch/arm/boot/dts/armada-385-db.dts
+++ b/arch/arm/boot/dts/armada-385-db.dts
@@ -74,7 +74,7 @@
 				phy-mode = "rgmii-id";
 			};
 
-			mdio {
+			mdio@72004 {
 				phy0: ethernet-phy@0 {
 					reg = <0>;
 				};
diff --git a/arch/arm/boot/dts/armada-385-rd.dts b/arch/arm/boot/dts/armada-385-rd.dts
index aaca2861dc87..74a3bfe6efd7 100644
--- a/arch/arm/boot/dts/armada-385-rd.dts
+++ b/arch/arm/boot/dts/armada-385-rd.dts
@@ -67,7 +67,7 @@
 			};
 
 
-			mdio {
+			mdio@72004 {
 				phy0: ethernet-phy@0 {
 					reg = <0>;
 				};
diff --git a/arch/arm/boot/dts/armada-385.dtsi b/arch/arm/boot/dts/armada-385.dtsi
index 6283d7912f71..5249a4d3c207 100644
--- a/arch/arm/boot/dts/armada-385.dtsi
+++ b/arch/arm/boot/dts/armada-385.dtsi
@@ -37,7 +37,7 @@
 
 	soc {
 		internal-regs {
-			pinctrl {
+			pinctrl@18000 {
 				compatible = "marvell,mv88f6820-pinctrl";
 				reg = <0x18000 0x20>;
 			};
diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 74391dace9e7..ada1f206028b 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -193,7 +193,7 @@
 				status = "disabled";
 			};
 
-			pinctrl {
+			pinctrl@18000 {
 				compatible = "marvell,mv88f6820-pinctrl";
 				reg = <0x18000 0x20>;
 			};
@@ -373,7 +373,7 @@
 				status = "disabled";
 			};
 
-			mdio {
+			mdio@72004 {
 				#address-cells = <1>;
 				#size-cells = <0>;
 				compatible = "marvell,orion-mdio";
-- 
1.9.1

--
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 related	[flat|nested] 30+ messages in thread

* [PATCH 1/3] ARM: mvebu: a38x: Fix node names
@ 2014-12-27 11:00     ` Gregory CLEMENT
  0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2014-12-27 11:00 UTC (permalink / raw)
  To: linux-arm-kernel

From: Maxime Ripard <maxime.ripard@free-electrons.com>

Some nodes in the DTs have a reg property but no unit name in their node name.

This contradicts the way the ePAPR defines the node names. Fix this.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/armada-385-db.dts | 2 +-
 arch/arm/boot/dts/armada-385-rd.dts | 2 +-
 arch/arm/boot/dts/armada-385.dtsi   | 2 +-
 arch/arm/boot/dts/armada-38x.dtsi   | 4 ++--
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/armada-385-db.dts b/arch/arm/boot/dts/armada-385-db.dts
index 2aaa9d2ac284..212605ccc7b6 100644
--- a/arch/arm/boot/dts/armada-385-db.dts
+++ b/arch/arm/boot/dts/armada-385-db.dts
@@ -74,7 +74,7 @@
 				phy-mode = "rgmii-id";
 			};
 
-			mdio {
+			mdio at 72004 {
 				phy0: ethernet-phy at 0 {
 					reg = <0>;
 				};
diff --git a/arch/arm/boot/dts/armada-385-rd.dts b/arch/arm/boot/dts/armada-385-rd.dts
index aaca2861dc87..74a3bfe6efd7 100644
--- a/arch/arm/boot/dts/armada-385-rd.dts
+++ b/arch/arm/boot/dts/armada-385-rd.dts
@@ -67,7 +67,7 @@
 			};
 
 
-			mdio {
+			mdio at 72004 {
 				phy0: ethernet-phy at 0 {
 					reg = <0>;
 				};
diff --git a/arch/arm/boot/dts/armada-385.dtsi b/arch/arm/boot/dts/armada-385.dtsi
index 6283d7912f71..5249a4d3c207 100644
--- a/arch/arm/boot/dts/armada-385.dtsi
+++ b/arch/arm/boot/dts/armada-385.dtsi
@@ -37,7 +37,7 @@
 
 	soc {
 		internal-regs {
-			pinctrl {
+			pinctrl at 18000 {
 				compatible = "marvell,mv88f6820-pinctrl";
 				reg = <0x18000 0x20>;
 			};
diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index 74391dace9e7..ada1f206028b 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -193,7 +193,7 @@
 				status = "disabled";
 			};
 
-			pinctrl {
+			pinctrl at 18000 {
 				compatible = "marvell,mv88f6820-pinctrl";
 				reg = <0x18000 0x20>;
 			};
@@ -373,7 +373,7 @@
 				status = "disabled";
 			};
 
-			mdio {
+			mdio at 72004 {
 				#address-cells = <1>;
 				#size-cells = <0>;
 				compatible = "marvell,orion-mdio";
-- 
1.9.1

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

* [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias
  2014-12-27 11:00 ` Gregory CLEMENT
@ 2014-12-27 11:00     ` Gregory CLEMENT
  -1 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2014-12-27 11:00 UTC (permalink / raw)
  To: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT
  Cc: Thomas Petazzoni, Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/armada-38x.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index ada1f206028b..f579a8929b29 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -193,7 +193,7 @@
 				status = "disabled";
 			};
 
-			pinctrl@18000 {
+			pinctrl: pinctrl@18000 {
 				compatible = "marvell,mv88f6820-pinctrl";
 				reg = <0x18000 0x20>;
 			};
-- 
1.9.1

--
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 related	[flat|nested] 30+ messages in thread

* [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias
@ 2014-12-27 11:00     ` Gregory CLEMENT
  0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2014-12-27 11:00 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/boot/dts/armada-38x.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
index ada1f206028b..f579a8929b29 100644
--- a/arch/arm/boot/dts/armada-38x.dtsi
+++ b/arch/arm/boot/dts/armada-38x.dtsi
@@ -193,7 +193,7 @@
 				status = "disabled";
 			};
 
-			pinctrl at 18000 {
+			pinctrl: pinctrl at 18000 {
 				compatible = "marvell,mv88f6820-pinctrl";
 				reg = <0x18000 0x20>;
 			};
-- 
1.9.1

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

* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
  2014-12-27 11:00 ` Gregory CLEMENT
@ 2014-12-27 11:00     ` Gregory CLEMENT
  -1 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2014-12-27 11:00 UTC (permalink / raw)
  To: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT
  Cc: Thomas Petazzoni, Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

The A385-GP is a board produced by Marvell that holds

- 1 PCIe slot
- 2 mini PCIe slot (one of them is multiplexed with the PCIe slot,
  muxing is selected through the GPIO expander)
- 1 16MB SPI-NOR
- 2 Gigabit Ethernet ports
- 4 SATA ports (2 of them are multiplexed with the mini PCIe slots,
  muxing is selected through the GPIO expander)
- 1 SDIO slot
- 1 USB3 port
- 2 USB2 port
- 2 GPIO/interrupts expander on I2C

Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/boot/dts/Makefile          |   1 +
 arch/arm/boot/dts/armada-385-gp.dts | 380 ++++++++++++++++++++++++++++++++++++
 2 files changed, 381 insertions(+)
 create mode 100644 arch/arm/boot/dts/armada-385-gp.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 91bd5bd62857..d6d7bafac127 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -535,6 +535,7 @@ dtb-$(CONFIG_MACH_ARMADA_375) += \
 	armada-375-db.dtb
 dtb-$(CONFIG_MACH_ARMADA_38X) += \
 	armada-385-db.dtb \
+	armada-385-gp.dtb \
 	armada-385-rd.dtb
 dtb-$(CONFIG_MACH_ARMADA_XP) += \
 	armada-xp-axpwifiap.dtb \
diff --git a/arch/arm/boot/dts/armada-385-gp.dts b/arch/arm/boot/dts/armada-385-gp.dts
new file mode 100644
index 000000000000..c9a375674e7f
--- /dev/null
+++ b/arch/arm/boot/dts/armada-385-gp.dts
@@ -0,0 +1,380 @@
+/*
+ * Device Tree file for Marvell Armada 385 development board
+ * (RD-88F6820-GP)
+ *
+ * Copyright (C) 2014 Marvell
+ *
+ * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is licensed under the terms of the GNU General Public
+ *     License version 2.  This program is licensed "as is" without
+ *     any warranty of any kind, whether express or implied.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "armada-385.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	model = "Marvell Armada 385 GP";
+	compatible = "marvell,a385-gp", "marvell,armada385", "marvell,armada380";
+
+	chosen {
+		bootargs = "console=ttyS0,115200 earlyprintk";
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x80000000>; /* 2 GB */
+	};
+
+	soc {
+		ranges = <MBUS_ID(0xf0, 0x01) 0 0xf1000000 0x100000
+			  MBUS_ID(0x01, 0x1d) 0 0xfff00000 0x100000>;
+
+		internal-regs {
+			spi@10600 {
+				status = "okay";
+
+				spi-flash@0 {
+					#address-cells = <1>;
+					#size-cells = <1>;
+					compatible = "st,m25p128";
+					reg = <0>; /* Chip select 0 */
+					spi-max-frequency = <50000000>;
+					m25p,fast-read;
+				};
+			};
+
+			i2c@11000 {
+				status = "okay";
+				clock-frequency = <100000>;
+
+				pca9555_0: pca9555@20 {
+					compatible = "nxp,pca9555";
+					pinctrl-names = "default";
+					pinctrl-0 = <&pca0_pins>;
+					interrupt-parent = <&gpio0>;
+					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
+					gpio-controller;
+					#gpio-cells = <2>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+					reg = <0x20>;
+				};
+
+				pca9555_1: pca9555@21 {
+					compatible = "nxp,pca9555";
+					pinctrl-names = "default";
+					interrupt-parent = <&gpio0>;
+					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
+
+					gpio-controller;
+					#gpio-cells = <2>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+
+					reg = <0x21>;
+				};
+
+			};
+
+			serial@12000 {
+				status = "okay";
+			};
+
+			ethernet@30000 {
+				status = "okay";
+				phy = <&phy0>;
+				phy-mode = "rgmii-id";
+			};
+
+			usb@50000 {
+				vcc-supply = <&reg_usb2_0_vbus>;
+				status = "okay";
+			};
+
+			ethernet@70000 {
+				status = "okay";
+				phy = <&phy1>;
+				phy-mode = "rgmii-id";
+			};
+
+
+			mdio@72004 {
+				phy0: ethernet-phy@0 {
+					reg = <0>;
+				};
+
+				phy1: ethernet-phy@1 {
+					reg = <1>;
+				};
+			};
+
+			sata@a8000 {
+				nr-ports = <2>;
+				status = "okay";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				sata0: sata-port@0 {
+					reg = <0>;
+					target-supply = <&reg_5v_sata0>;
+				};
+
+				sata1: sata-port@1 {
+					reg = <1>;
+					target-supply = <&reg_5v_sata1>;
+				};
+			};
+
+			sata@e0000 {
+				nr-ports = <2>;
+				status = "okay";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				sata2: sata-port@0 {
+					reg = <0>;
+					target-supply = <&reg_5v_sata2>;
+				};
+
+				sata3: sata-port@1 {
+					reg = <1>;
+					target-supply = <&reg_5v_sata3>;
+				};
+			};
+
+			sdhci@d8000 {
+				clock-frequency = <200000000>;
+				cd-gpios = <&pca9555_0 5 GPIO_ACTIVE_LOW>;
+				no-1-8-v;
+				wp-inverted;
+				bus-width = <8>;
+				status = "okay";
+			};
+
+			usb3@f0000 {
+				vcc-supply = <&reg_usb2_1_vbus>;
+				status = "okay";
+			};
+
+			usb3@f8000 {
+				vcc-supply = <&reg_usb3_vbus>;
+				status = "okay";
+			};
+		};
+
+		pcie-controller {
+			status = "okay";
+			/*
+			 * One PCIe units is accessible through
+			 * standard PCIe slot on the board.
+			 */
+			pcie@1,0 {
+				/* Port 0, Lane 0 */
+				status = "okay";
+			};
+
+			/*
+			 * The two other PCIe units are accessible
+			 * through mini PCIe slot on the board.
+			 */
+			pcie@2,0 {
+				/* Port 1, Lane 0 */
+				status = "okay";
+			};
+			pcie@3,0 {
+				/* Port 2, Lane 0 */
+				status = "okay";
+			};
+		};
+
+		gpio-fan {
+			compatible = "gpio-fan";
+			gpios = <&pca9555_1 3 GPIO_ACTIVE_HIGH>;
+			gpio-fan,speed-map = <0 0 3000 1>;
+		};
+	};
+
+	reg_usb3_vbus: usb3-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "usb3-vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_1 15 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_usb2_0_vbus: v5-vbus0 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-vbus0";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_1 14 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_usb2_1_vbus: v5-vbus1 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-vbus1";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_0 4 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_usb2_1_vbus: v5-vbus1 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-vbus1";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_0 4 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_sata0: pwr-sata0 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwr_en_sata0";
+		enable-active-high;
+		regulator-always-on;
+
+	};
+
+	reg_5v_sata0: v5-sata0 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-sata0";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata0>;
+	};
+
+	reg_12v_sata0: v12-sata0 {
+		compatible = "regulator-fixed";
+		regulator-name = "v12.0-sata0";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata0>;
+	};
+
+	reg_sata1: pwr-sata1 {
+		regulator-name = "pwr_en_sata1";
+		compatible = "regulator-fixed";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_0 3 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_5v_sata1: v5-sata1 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-sata1";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata1>;
+	};
+
+	reg_12v_sata1: v12-sata1 {
+		compatible = "regulator-fixed";
+		regulator-name = "v12.0-sata1";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata1>;
+	};
+
+	reg_sata2: pwr-sata2 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwr_en_sata2";
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_0 11 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_5v_sata2: v5-sata2 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-sata2";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata2>;
+	};
+
+	reg_12v_sata2: v12-sata2 {
+		compatible = "regulator-fixed";
+		regulator-name = "v12.0-sata2";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata2>;
+	};
+
+	reg_sata3: pwr-sata3 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwr_en_sata3";
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_0 12 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_5v_sata3: v5-sata3 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-sata3";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata3>;
+	};
+
+	reg_12v_sata3: v12-sata3 {
+		compatible = "regulator-fixed";
+		regulator-name = "v12.0-sata3";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata3>;
+	};
+};
+
+&pinctrl {
+	pca0_pins: pca0_pins {
+		marvell,pins = "mpp18";
+		marvell,function = "gpio";
+	};
+};
-- 
1.9.1

--
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 related	[flat|nested] 30+ messages in thread

* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
@ 2014-12-27 11:00     ` Gregory CLEMENT
  0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2014-12-27 11:00 UTC (permalink / raw)
  To: linux-arm-kernel

The A385-GP is a board produced by Marvell that holds

- 1 PCIe slot
- 2 mini PCIe slot (one of them is multiplexed with the PCIe slot,
  muxing is selected through the GPIO expander)
- 1 16MB SPI-NOR
- 2 Gigabit Ethernet ports
- 4 SATA ports (2 of them are multiplexed with the mini PCIe slots,
  muxing is selected through the GPIO expander)
- 1 SDIO slot
- 1 USB3 port
- 2 USB2 port
- 2 GPIO/interrupts expander on I2C

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
 arch/arm/boot/dts/Makefile          |   1 +
 arch/arm/boot/dts/armada-385-gp.dts | 380 ++++++++++++++++++++++++++++++++++++
 2 files changed, 381 insertions(+)
 create mode 100644 arch/arm/boot/dts/armada-385-gp.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 91bd5bd62857..d6d7bafac127 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -535,6 +535,7 @@ dtb-$(CONFIG_MACH_ARMADA_375) += \
 	armada-375-db.dtb
 dtb-$(CONFIG_MACH_ARMADA_38X) += \
 	armada-385-db.dtb \
+	armada-385-gp.dtb \
 	armada-385-rd.dtb
 dtb-$(CONFIG_MACH_ARMADA_XP) += \
 	armada-xp-axpwifiap.dtb \
diff --git a/arch/arm/boot/dts/armada-385-gp.dts b/arch/arm/boot/dts/armada-385-gp.dts
new file mode 100644
index 000000000000..c9a375674e7f
--- /dev/null
+++ b/arch/arm/boot/dts/armada-385-gp.dts
@@ -0,0 +1,380 @@
+/*
+ * Device Tree file for Marvell Armada 385 development board
+ * (RD-88F6820-GP)
+ *
+ * Copyright (C) 2014 Marvell
+ *
+ * Gregory CLEMENT <gregory.clement@free-electrons.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is licensed under the terms of the GNU General Public
+ *     License version 2.  This program is licensed "as is" without
+ *     any warranty of any kind, whether express or implied.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "armada-385.dtsi"
+#include <dt-bindings/gpio/gpio.h>
+
+/ {
+	model = "Marvell Armada 385 GP";
+	compatible = "marvell,a385-gp", "marvell,armada385", "marvell,armada380";
+
+	chosen {
+		bootargs = "console=ttyS0,115200 earlyprintk";
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0x00000000 0x80000000>; /* 2 GB */
+	};
+
+	soc {
+		ranges = <MBUS_ID(0xf0, 0x01) 0 0xf1000000 0x100000
+			  MBUS_ID(0x01, 0x1d) 0 0xfff00000 0x100000>;
+
+		internal-regs {
+			spi at 10600 {
+				status = "okay";
+
+				spi-flash at 0 {
+					#address-cells = <1>;
+					#size-cells = <1>;
+					compatible = "st,m25p128";
+					reg = <0>; /* Chip select 0 */
+					spi-max-frequency = <50000000>;
+					m25p,fast-read;
+				};
+			};
+
+			i2c at 11000 {
+				status = "okay";
+				clock-frequency = <100000>;
+
+				pca9555_0: pca9555 at 20 {
+					compatible = "nxp,pca9555";
+					pinctrl-names = "default";
+					pinctrl-0 = <&pca0_pins>;
+					interrupt-parent = <&gpio0>;
+					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
+					gpio-controller;
+					#gpio-cells = <2>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+					reg = <0x20>;
+				};
+
+				pca9555_1: pca9555 at 21 {
+					compatible = "nxp,pca9555";
+					pinctrl-names = "default";
+					interrupt-parent = <&gpio0>;
+					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
+
+					gpio-controller;
+					#gpio-cells = <2>;
+					interrupt-controller;
+					#interrupt-cells = <2>;
+
+					reg = <0x21>;
+				};
+
+			};
+
+			serial at 12000 {
+				status = "okay";
+			};
+
+			ethernet at 30000 {
+				status = "okay";
+				phy = <&phy0>;
+				phy-mode = "rgmii-id";
+			};
+
+			usb at 50000 {
+				vcc-supply = <&reg_usb2_0_vbus>;
+				status = "okay";
+			};
+
+			ethernet at 70000 {
+				status = "okay";
+				phy = <&phy1>;
+				phy-mode = "rgmii-id";
+			};
+
+
+			mdio at 72004 {
+				phy0: ethernet-phy at 0 {
+					reg = <0>;
+				};
+
+				phy1: ethernet-phy at 1 {
+					reg = <1>;
+				};
+			};
+
+			sata at a8000 {
+				nr-ports = <2>;
+				status = "okay";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				sata0: sata-port at 0 {
+					reg = <0>;
+					target-supply = <&reg_5v_sata0>;
+				};
+
+				sata1: sata-port at 1 {
+					reg = <1>;
+					target-supply = <&reg_5v_sata1>;
+				};
+			};
+
+			sata at e0000 {
+				nr-ports = <2>;
+				status = "okay";
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				sata2: sata-port at 0 {
+					reg = <0>;
+					target-supply = <&reg_5v_sata2>;
+				};
+
+				sata3: sata-port at 1 {
+					reg = <1>;
+					target-supply = <&reg_5v_sata3>;
+				};
+			};
+
+			sdhci at d8000 {
+				clock-frequency = <200000000>;
+				cd-gpios = <&pca9555_0 5 GPIO_ACTIVE_LOW>;
+				no-1-8-v;
+				wp-inverted;
+				bus-width = <8>;
+				status = "okay";
+			};
+
+			usb3 at f0000 {
+				vcc-supply = <&reg_usb2_1_vbus>;
+				status = "okay";
+			};
+
+			usb3 at f8000 {
+				vcc-supply = <&reg_usb3_vbus>;
+				status = "okay";
+			};
+		};
+
+		pcie-controller {
+			status = "okay";
+			/*
+			 * One PCIe units is accessible through
+			 * standard PCIe slot on the board.
+			 */
+			pcie at 1,0 {
+				/* Port 0, Lane 0 */
+				status = "okay";
+			};
+
+			/*
+			 * The two other PCIe units are accessible
+			 * through mini PCIe slot on the board.
+			 */
+			pcie at 2,0 {
+				/* Port 1, Lane 0 */
+				status = "okay";
+			};
+			pcie at 3,0 {
+				/* Port 2, Lane 0 */
+				status = "okay";
+			};
+		};
+
+		gpio-fan {
+			compatible = "gpio-fan";
+			gpios = <&pca9555_1 3 GPIO_ACTIVE_HIGH>;
+			gpio-fan,speed-map = <0 0 3000 1>;
+		};
+	};
+
+	reg_usb3_vbus: usb3-vbus {
+		compatible = "regulator-fixed";
+		regulator-name = "usb3-vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_1 15 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_usb2_0_vbus: v5-vbus0 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-vbus0";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_1 14 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_usb2_1_vbus: v5-vbus1 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-vbus1";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_0 4 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_usb2_1_vbus: v5-vbus1 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-vbus1";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_0 4 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_sata0: pwr-sata0 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwr_en_sata0";
+		enable-active-high;
+		regulator-always-on;
+
+	};
+
+	reg_5v_sata0: v5-sata0 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-sata0";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata0>;
+	};
+
+	reg_12v_sata0: v12-sata0 {
+		compatible = "regulator-fixed";
+		regulator-name = "v12.0-sata0";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata0>;
+	};
+
+	reg_sata1: pwr-sata1 {
+		regulator-name = "pwr_en_sata1";
+		compatible = "regulator-fixed";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_0 3 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_5v_sata1: v5-sata1 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-sata1";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata1>;
+	};
+
+	reg_12v_sata1: v12-sata1 {
+		compatible = "regulator-fixed";
+		regulator-name = "v12.0-sata1";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata1>;
+	};
+
+	reg_sata2: pwr-sata2 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwr_en_sata2";
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_0 11 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_5v_sata2: v5-sata2 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-sata2";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata2>;
+	};
+
+	reg_12v_sata2: v12-sata2 {
+		compatible = "regulator-fixed";
+		regulator-name = "v12.0-sata2";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata2>;
+	};
+
+	reg_sata3: pwr-sata3 {
+		compatible = "regulator-fixed";
+		regulator-name = "pwr_en_sata3";
+		enable-active-high;
+		regulator-always-on;
+		gpio = <&pca9555_0 12 GPIO_ACTIVE_HIGH>;
+	};
+
+	reg_5v_sata3: v5-sata3 {
+		compatible = "regulator-fixed";
+		regulator-name = "v5.0-sata3";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata3>;
+	};
+
+	reg_12v_sata3: v12-sata3 {
+		compatible = "regulator-fixed";
+		regulator-name = "v12.0-sata3";
+		regulator-min-microvolt = <12000000>;
+		regulator-max-microvolt = <12000000>;
+		regulator-always-on;
+		vin-supply = <&reg_sata3>;
+	};
+};
+
+&pinctrl {
+	pca0_pins: pca0_pins {
+		marvell,pins = "mpp18";
+		marvell,function = "gpio";
+	};
+};
-- 
1.9.1

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

* Re: [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias
  2014-12-27 11:00     ` Gregory CLEMENT
@ 2014-12-27 11:29         ` Andrew Lunn
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2014-12-27 11:29 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Jason Cooper, Sebastian Hesselbarth, Thomas Petazzoni,
	Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

On Sat, Dec 27, 2014 at 12:00:34PM +0100, Gregory CLEMENT wrote:

Hi Gregory

A one line changelog would be nice.

  Andrew

> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  arch/arm/boot/dts/armada-38x.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
> index ada1f206028b..f579a8929b29 100644
> --- a/arch/arm/boot/dts/armada-38x.dtsi
> +++ b/arch/arm/boot/dts/armada-38x.dtsi
> @@ -193,7 +193,7 @@
>  				status = "disabled";
>  			};
>  
> -			pinctrl@18000 {
> +			pinctrl: pinctrl@18000 {
>  				compatible = "marvell,mv88f6820-pinctrl";
>  				reg = <0x18000 0x20>;
>  			};
> -- 
> 1.9.1
> 
--
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] 30+ messages in thread

* [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias
@ 2014-12-27 11:29         ` Andrew Lunn
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2014-12-27 11:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Dec 27, 2014 at 12:00:34PM +0100, Gregory CLEMENT wrote:

Hi Gregory

A one line changelog would be nice.

  Andrew

> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  arch/arm/boot/dts/armada-38x.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/armada-38x.dtsi b/arch/arm/boot/dts/armada-38x.dtsi
> index ada1f206028b..f579a8929b29 100644
> --- a/arch/arm/boot/dts/armada-38x.dtsi
> +++ b/arch/arm/boot/dts/armada-38x.dtsi
> @@ -193,7 +193,7 @@
>  				status = "disabled";
>  			};
>  
> -			pinctrl at 18000 {
> +			pinctrl: pinctrl at 18000 {
>  				compatible = "marvell,mv88f6820-pinctrl";
>  				reg = <0x18000 0x20>;
>  			};
> -- 
> 1.9.1
> 

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

* Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
  2014-12-27 11:00     ` Gregory CLEMENT
@ 2014-12-27 11:34         ` Andrew Lunn
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2014-12-27 11:34 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Jason Cooper, Sebastian Hesselbarth, Thomas Petazzoni,
	Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

On Sat, Dec 27, 2014 at 12:00:35PM +0100, Gregory CLEMENT wrote:
> The A385-GP is a board produced by Marvell that holds
> 
> - 1 PCIe slot
> - 2 mini PCIe slot (one of them is multiplexed with the PCIe slot,
>   muxing is selected through the GPIO expander)
> - 1 16MB SPI-NOR
> - 2 Gigabit Ethernet ports
> - 4 SATA ports (2 of them are multiplexed with the mini PCIe slots,
>   muxing is selected through the GPIO expander)
> - 1 SDIO slot
> - 1 USB3 port
> - 2 USB2 port
> - 2 GPIO/interrupts expander on I2C
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile          |   1 +
>  arch/arm/boot/dts/armada-385-gp.dts | 380 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 381 insertions(+)
>  create mode 100644 arch/arm/boot/dts/armada-385-gp.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 91bd5bd62857..d6d7bafac127 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -535,6 +535,7 @@ dtb-$(CONFIG_MACH_ARMADA_375) += \
>  	armada-375-db.dtb
>  dtb-$(CONFIG_MACH_ARMADA_38X) += \
>  	armada-385-db.dtb \
> +	armada-385-gp.dtb \
>  	armada-385-rd.dtb
>  dtb-$(CONFIG_MACH_ARMADA_XP) += \
>  	armada-xp-axpwifiap.dtb \
> diff --git a/arch/arm/boot/dts/armada-385-gp.dts b/arch/arm/boot/dts/armada-385-gp.dts
> new file mode 100644
> index 000000000000..c9a375674e7f
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-385-gp.dts
> @@ -0,0 +1,380 @@
> +/*
> + * Device Tree file for Marvell Armada 385 development board
> + * (RD-88F6820-GP)
> + *
> + * Copyright (C) 2014 Marvell
> + *
> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.

Hi Gregory

I'm guessing here...

The file says Copyright Marvell. I guess Marvell released it GPL? Not
Dual license? So it would be nice to have a Signed-off-by: from
somebody in Marvell that they are O.K. with the dual license.

Thanks
	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] 30+ messages in thread

* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
@ 2014-12-27 11:34         ` Andrew Lunn
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2014-12-27 11:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Dec 27, 2014 at 12:00:35PM +0100, Gregory CLEMENT wrote:
> The A385-GP is a board produced by Marvell that holds
> 
> - 1 PCIe slot
> - 2 mini PCIe slot (one of them is multiplexed with the PCIe slot,
>   muxing is selected through the GPIO expander)
> - 1 16MB SPI-NOR
> - 2 Gigabit Ethernet ports
> - 4 SATA ports (2 of them are multiplexed with the mini PCIe slots,
>   muxing is selected through the GPIO expander)
> - 1 SDIO slot
> - 1 USB3 port
> - 2 USB2 port
> - 2 GPIO/interrupts expander on I2C
> 
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
>  arch/arm/boot/dts/Makefile          |   1 +
>  arch/arm/boot/dts/armada-385-gp.dts | 380 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 381 insertions(+)
>  create mode 100644 arch/arm/boot/dts/armada-385-gp.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 91bd5bd62857..d6d7bafac127 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -535,6 +535,7 @@ dtb-$(CONFIG_MACH_ARMADA_375) += \
>  	armada-375-db.dtb
>  dtb-$(CONFIG_MACH_ARMADA_38X) += \
>  	armada-385-db.dtb \
> +	armada-385-gp.dtb \
>  	armada-385-rd.dtb
>  dtb-$(CONFIG_MACH_ARMADA_XP) += \
>  	armada-xp-axpwifiap.dtb \
> diff --git a/arch/arm/boot/dts/armada-385-gp.dts b/arch/arm/boot/dts/armada-385-gp.dts
> new file mode 100644
> index 000000000000..c9a375674e7f
> --- /dev/null
> +++ b/arch/arm/boot/dts/armada-385-gp.dts
> @@ -0,0 +1,380 @@
> +/*
> + * Device Tree file for Marvell Armada 385 development board
> + * (RD-88F6820-GP)
> + *
> + * Copyright (C) 2014 Marvell
> + *
> + * Gregory CLEMENT <gregory.clement@free-electrons.com>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.

Hi Gregory

I'm guessing here...

The file says Copyright Marvell. I guess Marvell released it GPL? Not
Dual license? So it would be nice to have a Signed-off-by: from
somebody in Marvell that they are O.K. with the dual license.

Thanks
	Andrew

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

* Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
  2014-12-27 11:00     ` Gregory CLEMENT
@ 2014-12-27 11:50         ` Andrew Lunn
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2014-12-27 11:50 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Jason Cooper, Sebastian Hesselbarth, Thomas Petazzoni,
	Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

> @@ -0,0 +1,380 @@
> +/*
> + * Device Tree file for Marvell Armada 385 development board
> + * (RD-88F6820-GP)
> + *
> + * Copyright (C) 2014 Marvell
> + *
> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is licensed under the terms of the GNU General Public
> + *     License version 2.  This program is licensed "as is" without
> + *     any warranty of any kind, whether express or implied.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include "armada-385.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	model = "Marvell Armada 385 GP";
> +	compatible = "marvell,a385-gp", "marvell,armada385", "marvell,armada380";
> +
> +	chosen {
> +		bootargs = "console=ttyS0,115200 earlyprintk";
> +	};
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x00000000 0x80000000>; /* 2 GB */
> +	};
> +
> +	soc {
> +		ranges = <MBUS_ID(0xf0, 0x01) 0 0xf1000000 0x100000
> +			  MBUS_ID(0x01, 0x1d) 0 0xfff00000 0x100000>;
> +
> +		internal-regs {
> +			spi@10600 {
> +				status = "okay";
> +
> +				spi-flash@0 {
> +					#address-cells = <1>;
> +					#size-cells = <1>;
> +					compatible = "st,m25p128";
> +					reg = <0>; /* Chip select 0 */
> +					spi-max-frequency = <50000000>;
> +					m25p,fast-read;
> +				};
> +			};
> +
> +			i2c@11000 {
> +				status = "okay";
> +				clock-frequency = <100000>;
> +
> +				pca9555_0: pca9555@20 {

I think Sebastian will come along and ask this is called gpio, not
pca9555_0. See the review he made of the Seagate Black NAS box.

> +					compatible = "nxp,pca9555";
> +					pinctrl-names = "default";
> +					pinctrl-0 = <&pca0_pins>;
> +					interrupt-parent = <&gpio0>;
> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +					gpio-controller;
> +					#gpio-cells = <2>;
> +					interrupt-controller;
> +					#interrupt-cells = <2>;
> +					reg = <0x20>;
> +				};
> +
> +				pca9555_1: pca9555@21 {
> +					compatible = "nxp,pca9555";
> +					pinctrl-names = "default";
> +					interrupt-parent = <&gpio0>;
> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +
> +					gpio-controller;
> +					#gpio-cells = <2>;
> +					interrupt-controller;
> +					#interrupt-cells = <2>;
> +

Maybe remove the blank lines here, to make it similar to the previous
one?

> +					reg = <0x21>;
> +				};
> +

Maybe remove this blank line?

> +			};
> +
> +			serial@12000 {
> +				status = "okay";
> +			};

It would be nice if you can document the connector number and the
pinout of the serial port, if it is not on the silk screen.

> +
> +			ethernet@30000 {
> +				status = "okay";
> +				phy = <&phy0>;
> +				phy-mode = "rgmii-id";
> +			};
> +
> +			usb@50000 {
> +				vcc-supply = <&reg_usb2_0_vbus>;
> +				status = "okay";
> +			};
> +
> +			ethernet@70000 {
> +				status = "okay";
> +				phy = <&phy1>;
> +				phy-mode = "rgmii-id";
> +			};
> +
> +
> +			mdio@72004 {
> +				phy0: ethernet-phy@0 {
> +					reg = <0>;
> +				};
> +
> +				phy1: ethernet-phy@1 {
> +					reg = <1>;
> +				};
> +			};
> +
> +			sata@a8000 {
> +				nr-ports = <2>;
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				sata0: sata-port@0 {
> +					reg = <0>;
> +					target-supply = <&reg_5v_sata0>;
> +				};
> +
> +				sata1: sata-port@1 {
> +					reg = <1>;
> +					target-supply = <&reg_5v_sata1>;
> +				};
> +			};
> +
> +			sata@e0000 {
> +				nr-ports = <2>;
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				sata2: sata-port@0 {
> +					reg = <0>;
> +					target-supply = <&reg_5v_sata2>;
> +				};
> +
> +				sata3: sata-port@1 {
> +					reg = <1>;
> +					target-supply = <&reg_5v_sata3>;
> +				};
> +			};
> +
> +			sdhci@d8000 {
> +				clock-frequency = <200000000>;
> +				cd-gpios = <&pca9555_0 5 GPIO_ACTIVE_LOW>;
> +				no-1-8-v;
> +				wp-inverted;
> +				bus-width = <8>;
> +				status = "okay";
> +			};
> +
> +			usb3@f0000 {
> +				vcc-supply = <&reg_usb2_1_vbus>;
> +				status = "okay";
> +			};
> +
> +			usb3@f8000 {
> +				vcc-supply = <&reg_usb3_vbus>;
> +				status = "okay";
> +			};
> +		};
> +
> +		pcie-controller {
> +			status = "okay";
> +			/*
> +			 * One PCIe units is accessible through
> +			 * standard PCIe slot on the board.
> +			 */
> +			pcie@1,0 {
> +				/* Port 0, Lane 0 */
> +				status = "okay";
> +			};
> +
> +			/*
> +			 * The two other PCIe units are accessible
> +			 * through mini PCIe slot on the board.
> +			 */
> +			pcie@2,0 {
> +				/* Port 1, Lane 0 */
> +				status = "okay";
> +			};
> +			pcie@3,0 {
> +				/* Port 2, Lane 0 */
> +				status = "okay";
> +			};
> +		};
> +
> +		gpio-fan {
> +			compatible = "gpio-fan";
> +			gpios = <&pca9555_1 3 GPIO_ACTIVE_HIGH>;
> +			gpio-fan,speed-map = <0 0 3000 1>;

It would be nice to format this with a newline between the two map
entries.

> +		};
> +	};
> +
> +	reg_usb3_vbus: usb3-vbus {
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb3-vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_1 15 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_usb2_0_vbus: v5-vbus0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-vbus0";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_1 14 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_usb2_1_vbus: v5-vbus1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-vbus1";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_0 4 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_usb2_1_vbus: v5-vbus1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-vbus1";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_0 4 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_sata0: pwr-sata0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwr_en_sata0";
> +		enable-active-high;
> +		regulator-always-on;
> +
> +	};
> +
> +	reg_5v_sata0: v5-sata0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-sata0";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata0>;
> +	};
> +
> +	reg_12v_sata0: v12-sata0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v12.0-sata0";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata0>;
> +	};
> +
> +	reg_sata1: pwr-sata1 {
> +		regulator-name = "pwr_en_sata1";
> +		compatible = "regulator-fixed";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_0 3 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_5v_sata1: v5-sata1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-sata1";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata1>;
> +	};
> +
> +	reg_12v_sata1: v12-sata1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v12.0-sata1";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata1>;
> +	};
> +
> +	reg_sata2: pwr-sata2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwr_en_sata2";
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_0 11 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_5v_sata2: v5-sata2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-sata2";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata2>;
> +	};
> +
> +	reg_12v_sata2: v12-sata2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v12.0-sata2";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata2>;
> +	};
> +
> +	reg_sata3: pwr-sata3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwr_en_sata3";
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_0 12 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_5v_sata3: v5-sata3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-sata3";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata3>;
> +	};
> +
> +	reg_12v_sata3: v12-sata3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v12.0-sata3";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata3>;
> +	};
> +};

What is the quality of the power supply? What you often see if that
SATA drives are spun up sequentially, in order to not strain the power
supply with the current draw of them all starting at the same
time. There is a property, startup-delay-us, which can be used for
this.

	Andrew

> +
> +&pinctrl {
> +	pca0_pins: pca0_pins {
> +		marvell,pins = "mpp18";
> +		marvell,function = "gpio";
> +	};
> +};
> -- 
> 1.9.1
> 
--
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] 30+ messages in thread

* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
@ 2014-12-27 11:50         ` Andrew Lunn
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2014-12-27 11:50 UTC (permalink / raw)
  To: linux-arm-kernel

> @@ -0,0 +1,380 @@
> +/*
> + * Device Tree file for Marvell Armada 385 development board
> + * (RD-88F6820-GP)
> + *
> + * Copyright (C) 2014 Marvell
> + *
> + * Gregory CLEMENT <gregory.clement@free-electrons.com>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is licensed under the terms of the GNU General Public
> + *     License version 2.  This program is licensed "as is" without
> + *     any warranty of any kind, whether express or implied.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + *     obtaining a copy of this software and associated documentation
> + *     files (the "Software"), to deal in the Software without
> + *     restriction, including without limitation the rights to use,
> + *     copy, modify, merge, publish, distribute, sublicense, and/or
> + *     sell copies of the Software, and to permit persons to whom the
> + *     Software is furnished to do so, subject to the following
> + *     conditions:
> + *
> + *     The above copyright notice and this permission notice shall be
> + *     included in all copies or substantial portions of the Software.
> + *
> + *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + *     OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +#include "armada-385.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> +	model = "Marvell Armada 385 GP";
> +	compatible = "marvell,a385-gp", "marvell,armada385", "marvell,armada380";
> +
> +	chosen {
> +		bootargs = "console=ttyS0,115200 earlyprintk";
> +	};
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x00000000 0x80000000>; /* 2 GB */
> +	};
> +
> +	soc {
> +		ranges = <MBUS_ID(0xf0, 0x01) 0 0xf1000000 0x100000
> +			  MBUS_ID(0x01, 0x1d) 0 0xfff00000 0x100000>;
> +
> +		internal-regs {
> +			spi at 10600 {
> +				status = "okay";
> +
> +				spi-flash at 0 {
> +					#address-cells = <1>;
> +					#size-cells = <1>;
> +					compatible = "st,m25p128";
> +					reg = <0>; /* Chip select 0 */
> +					spi-max-frequency = <50000000>;
> +					m25p,fast-read;
> +				};
> +			};
> +
> +			i2c at 11000 {
> +				status = "okay";
> +				clock-frequency = <100000>;
> +
> +				pca9555_0: pca9555 at 20 {

I think Sebastian will come along and ask this is called gpio, not
pca9555_0. See the review he made of the Seagate Black NAS box.

> +					compatible = "nxp,pca9555";
> +					pinctrl-names = "default";
> +					pinctrl-0 = <&pca0_pins>;
> +					interrupt-parent = <&gpio0>;
> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +					gpio-controller;
> +					#gpio-cells = <2>;
> +					interrupt-controller;
> +					#interrupt-cells = <2>;
> +					reg = <0x20>;
> +				};
> +
> +				pca9555_1: pca9555 at 21 {
> +					compatible = "nxp,pca9555";
> +					pinctrl-names = "default";
> +					interrupt-parent = <&gpio0>;
> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +
> +					gpio-controller;
> +					#gpio-cells = <2>;
> +					interrupt-controller;
> +					#interrupt-cells = <2>;
> +

Maybe remove the blank lines here, to make it similar to the previous
one?

> +					reg = <0x21>;
> +				};
> +

Maybe remove this blank line?

> +			};
> +
> +			serial at 12000 {
> +				status = "okay";
> +			};

It would be nice if you can document the connector number and the
pinout of the serial port, if it is not on the silk screen.

> +
> +			ethernet at 30000 {
> +				status = "okay";
> +				phy = <&phy0>;
> +				phy-mode = "rgmii-id";
> +			};
> +
> +			usb at 50000 {
> +				vcc-supply = <&reg_usb2_0_vbus>;
> +				status = "okay";
> +			};
> +
> +			ethernet at 70000 {
> +				status = "okay";
> +				phy = <&phy1>;
> +				phy-mode = "rgmii-id";
> +			};
> +
> +
> +			mdio at 72004 {
> +				phy0: ethernet-phy at 0 {
> +					reg = <0>;
> +				};
> +
> +				phy1: ethernet-phy at 1 {
> +					reg = <1>;
> +				};
> +			};
> +
> +			sata at a8000 {
> +				nr-ports = <2>;
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				sata0: sata-port at 0 {
> +					reg = <0>;
> +					target-supply = <&reg_5v_sata0>;
> +				};
> +
> +				sata1: sata-port at 1 {
> +					reg = <1>;
> +					target-supply = <&reg_5v_sata1>;
> +				};
> +			};
> +
> +			sata at e0000 {
> +				nr-ports = <2>;
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				sata2: sata-port at 0 {
> +					reg = <0>;
> +					target-supply = <&reg_5v_sata2>;
> +				};
> +
> +				sata3: sata-port at 1 {
> +					reg = <1>;
> +					target-supply = <&reg_5v_sata3>;
> +				};
> +			};
> +
> +			sdhci at d8000 {
> +				clock-frequency = <200000000>;
> +				cd-gpios = <&pca9555_0 5 GPIO_ACTIVE_LOW>;
> +				no-1-8-v;
> +				wp-inverted;
> +				bus-width = <8>;
> +				status = "okay";
> +			};
> +
> +			usb3 at f0000 {
> +				vcc-supply = <&reg_usb2_1_vbus>;
> +				status = "okay";
> +			};
> +
> +			usb3 at f8000 {
> +				vcc-supply = <&reg_usb3_vbus>;
> +				status = "okay";
> +			};
> +		};
> +
> +		pcie-controller {
> +			status = "okay";
> +			/*
> +			 * One PCIe units is accessible through
> +			 * standard PCIe slot on the board.
> +			 */
> +			pcie at 1,0 {
> +				/* Port 0, Lane 0 */
> +				status = "okay";
> +			};
> +
> +			/*
> +			 * The two other PCIe units are accessible
> +			 * through mini PCIe slot on the board.
> +			 */
> +			pcie at 2,0 {
> +				/* Port 1, Lane 0 */
> +				status = "okay";
> +			};
> +			pcie at 3,0 {
> +				/* Port 2, Lane 0 */
> +				status = "okay";
> +			};
> +		};
> +
> +		gpio-fan {
> +			compatible = "gpio-fan";
> +			gpios = <&pca9555_1 3 GPIO_ACTIVE_HIGH>;
> +			gpio-fan,speed-map = <0 0 3000 1>;

It would be nice to format this with a newline between the two map
entries.

> +		};
> +	};
> +
> +	reg_usb3_vbus: usb3-vbus {
> +		compatible = "regulator-fixed";
> +		regulator-name = "usb3-vbus";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_1 15 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_usb2_0_vbus: v5-vbus0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-vbus0";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_1 14 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_usb2_1_vbus: v5-vbus1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-vbus1";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_0 4 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_usb2_1_vbus: v5-vbus1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-vbus1";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_0 4 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_sata0: pwr-sata0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwr_en_sata0";
> +		enable-active-high;
> +		regulator-always-on;
> +
> +	};
> +
> +	reg_5v_sata0: v5-sata0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-sata0";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata0>;
> +	};
> +
> +	reg_12v_sata0: v12-sata0 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v12.0-sata0";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata0>;
> +	};
> +
> +	reg_sata1: pwr-sata1 {
> +		regulator-name = "pwr_en_sata1";
> +		compatible = "regulator-fixed";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_0 3 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_5v_sata1: v5-sata1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-sata1";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata1>;
> +	};
> +
> +	reg_12v_sata1: v12-sata1 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v12.0-sata1";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata1>;
> +	};
> +
> +	reg_sata2: pwr-sata2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwr_en_sata2";
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_0 11 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_5v_sata2: v5-sata2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-sata2";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata2>;
> +	};
> +
> +	reg_12v_sata2: v12-sata2 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v12.0-sata2";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata2>;
> +	};
> +
> +	reg_sata3: pwr-sata3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "pwr_en_sata3";
> +		enable-active-high;
> +		regulator-always-on;
> +		gpio = <&pca9555_0 12 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	reg_5v_sata3: v5-sata3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v5.0-sata3";
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata3>;
> +	};
> +
> +	reg_12v_sata3: v12-sata3 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "v12.0-sata3";
> +		regulator-min-microvolt = <12000000>;
> +		regulator-max-microvolt = <12000000>;
> +		regulator-always-on;
> +		vin-supply = <&reg_sata3>;
> +	};
> +};

What is the quality of the power supply? What you often see if that
SATA drives are spun up sequentially, in order to not strain the power
supply with the current draw of them all starting at the same
time. There is a property, startup-delay-us, which can be used for
this.

	Andrew

> +
> +&pinctrl {
> +	pca0_pins: pca0_pins {
> +		marvell,pins = "mpp18";
> +		marvell,function = "gpio";
> +	};
> +};
> -- 
> 1.9.1
> 

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

* Re: [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias
  2014-12-27 11:00     ` Gregory CLEMENT
@ 2014-12-27 14:46         ` Sergei Shtylyov
  -1 siblings, 0 replies; 30+ messages in thread
From: Sergei Shtylyov @ 2014-12-27 14:46 UTC (permalink / raw)
  To: Gregory CLEMENT, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth
  Cc: Thomas Petazzoni, Boris BREZILLON, Tawfik Bayouk,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Rutland, Nadav Haklai,
	Lior Amsalem, Ezequiel Garcia, Maxime Ripard,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hello.

On 12/27/2014 2:00 PM, Gregory CLEMENT wrote:

    You're not adding an alias. It's a label.

> Signed-off-by: Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>

WBR, Sergei

--
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] 30+ messages in thread

* [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias
@ 2014-12-27 14:46         ` Sergei Shtylyov
  0 siblings, 0 replies; 30+ messages in thread
From: Sergei Shtylyov @ 2014-12-27 14:46 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 12/27/2014 2:00 PM, Gregory CLEMENT wrote:

    You're not adding an alias. It's a label.

> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

WBR, Sergei

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

* Re: [PATCH 0/3] Add Armada 385 General Purpose Development Board support
  2014-12-27 11:00 ` Gregory CLEMENT
@ 2014-12-31  9:57     ` Thomas Petazzoni
  -1 siblings, 0 replies; 30+ messages in thread
From: Thomas Petazzoni @ 2014-12-31  9:57 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

Dear Gregory CLEMENT,

On Sat, 27 Dec 2014 12:00:32 +0100, Gregory CLEMENT wrote:

> this patch set adds the support for the Armada 385 GP board, this board
> is pretty close to the Armada 385 RD one. It comes with a new revision
> of the Armada 385 SoC (B0).

Unless I'm wrong, it actually comes with an A0 revision, not a B0. But
it's indeed different from the Z1 revision we have been using until now.

At least, my Armada XP GP says:

Board: DB-88F6820-GP
SoC:   MV88F6828 Rev A0

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 0/3] Add Armada 385 General Purpose Development Board support
@ 2014-12-31  9:57     ` Thomas Petazzoni
  0 siblings, 0 replies; 30+ messages in thread
From: Thomas Petazzoni @ 2014-12-31  9:57 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Gregory CLEMENT,

On Sat, 27 Dec 2014 12:00:32 +0100, Gregory CLEMENT wrote:

> this patch set adds the support for the Armada 385 GP board, this board
> is pretty close to the Armada 385 RD one. It comes with a new revision
> of the Armada 385 SoC (B0).

Unless I'm wrong, it actually comes with an A0 revision, not a B0. But
it's indeed different from the Z1 revision we have been using until now.

At least, my Armada XP GP says:

Board: DB-88F6820-GP
SoC:   MV88F6828 Rev A0

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

* Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
  2014-12-27 11:00     ` Gregory CLEMENT
@ 2014-12-31 10:32         ` Thomas Petazzoni
  -1 siblings, 0 replies; 30+ messages in thread
From: Thomas Petazzoni @ 2014-12-31 10:32 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

Dear Gregory CLEMENT,

On Sat, 27 Dec 2014 12:00:35 +0100, Gregory CLEMENT wrote:

> +				spi-flash@0 {
> +					#address-cells = <1>;
> +					#size-cells = <1>;
> +					compatible = "st,m25p128";
> +					reg = <0>; /* Chip select 0 */
> +					spi-max-frequency = <50000000>;

According to the m25p128 datasheet,  the max frequency is 54 Mhz.

> +			i2c@11000 {
> +				status = "okay";
> +				clock-frequency = <100000>;
> +
> +				pca9555_0: pca9555@20 {
> +					compatible = "nxp,pca9555";
> +					pinctrl-names = "default";
> +					pinctrl-0 = <&pca0_pins>;
> +					interrupt-parent = <&gpio0>;
> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +					gpio-controller;
> +					#gpio-cells = <2>;
> +					interrupt-controller;
> +					#interrupt-cells = <2>;
> +					reg = <0x20>;
> +				};
> +
> +				pca9555_1: pca9555@21 {
> +					compatible = "nxp,pca9555";
> +					pinctrl-names = "default";
> +					interrupt-parent = <&gpio0>;
> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +
> +					gpio-controller;
> +					#gpio-cells = <2>;
> +					interrupt-controller;
> +					#interrupt-cells = <2>;
> +
> +					reg = <0x21>;
> +				};

It would be good to align both description in terms of empty new lines.

Also, in the second controller, you have pinctrl-names, but no
pinctrl-0, so it doesn't make much sense.

> +			ethernet@70000 {
> +				status = "okay";
> +				phy = <&phy1>;
> +				phy-mode = "rgmii-id";
> +			};
> +
> +

One too many empty new line.

> +			mdio@72004 {
> +				phy0: ethernet-phy@0 {
> +					reg = <0>;
> +				};
> +
> +				phy1: ethernet-phy@1 {
> +					reg = <1>;
> +				};
> +			};

Maybe this is confusing, but it seems like the port 0 (i.e the one at
0x70000) is connected to the PHY at address 1, while the port 1 (i.e
the one at 0x30000) is connected to the PHY at address 0. According to
U-Boot:

| egiga0 |   RGMII   |     0x01     |
| egiga1 |   RGMII   |     0x00     |

So maybe we should have:

	ethernet@30000 {
		phy = <&phy1>;
	};

	ethernet@70000 {
		phy = <&phy0>;
	};

	mdio@72004 {
		phy0: ethernet-phy@1 {
			reg = <1>;
		};

		phy1: ethernet-phy@0 {
			reg = <0>;
		};
	};

To reflect this, no?

> +			sata@a8000 {
> +				nr-ports = <2>;
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				sata0: sata-port@0 {
> +					reg = <0>;
> +					target-supply = <&reg_5v_sata0>;
> +				};
> +
> +				sata1: sata-port@1 {
> +					reg = <1>;
> +					target-supply = <&reg_5v_sata1>;
> +				};

Doesn't this part depends on the patches you have submitted to the AHCI
driver? If so, it would be good to state this in the cover letter, so
that the dependencies are known. Or for the moment, to not handle the
SATA part, until the DT binding for describing per-SATA port regulators
is sorted out (I saw that the feedback from Mark Brown on your patches
indicate that some rework will be needed).

Also, having a reg property into a child node that isn't part of a bus
looks wrong.

> +			};
> +
> +			sata@e0000 {
> +				nr-ports = <2>;
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				sata2: sata-port@0 {
> +					reg = <0>;
> +					target-supply = <&reg_5v_sata2>;
> +				};
> +
> +				sata3: sata-port@1 {
> +					reg = <1>;
> +					target-supply = <&reg_5v_sata3>;
> +				};
> +			};

Ditto.

> +			sdhci@d8000 {
> +				clock-frequency = <200000000>;

Why? There is already a 'clocks' property in armada-38x.dtsi. However,
the clock is actually 250 Mhz, not 200 Mhz. Any reason why you forced
200 Mhz here?

> +				cd-gpios = <&pca9555_0 5 GPIO_ACTIVE_LOW>;
> +				no-1-8-v;
> +				wp-inverted;

Why do you have a wp-inverted property, with no wp-gpios property? If I
understand the DT binding correctly, wp-inverted only makes sense when
wp-gpios is used.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
@ 2014-12-31 10:32         ` Thomas Petazzoni
  0 siblings, 0 replies; 30+ messages in thread
From: Thomas Petazzoni @ 2014-12-31 10:32 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Gregory CLEMENT,

On Sat, 27 Dec 2014 12:00:35 +0100, Gregory CLEMENT wrote:

> +				spi-flash at 0 {
> +					#address-cells = <1>;
> +					#size-cells = <1>;
> +					compatible = "st,m25p128";
> +					reg = <0>; /* Chip select 0 */
> +					spi-max-frequency = <50000000>;

According to the m25p128 datasheet,  the max frequency is 54 Mhz.

> +			i2c at 11000 {
> +				status = "okay";
> +				clock-frequency = <100000>;
> +
> +				pca9555_0: pca9555 at 20 {
> +					compatible = "nxp,pca9555";
> +					pinctrl-names = "default";
> +					pinctrl-0 = <&pca0_pins>;
> +					interrupt-parent = <&gpio0>;
> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +					gpio-controller;
> +					#gpio-cells = <2>;
> +					interrupt-controller;
> +					#interrupt-cells = <2>;
> +					reg = <0x20>;
> +				};
> +
> +				pca9555_1: pca9555 at 21 {
> +					compatible = "nxp,pca9555";
> +					pinctrl-names = "default";
> +					interrupt-parent = <&gpio0>;
> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
> +
> +					gpio-controller;
> +					#gpio-cells = <2>;
> +					interrupt-controller;
> +					#interrupt-cells = <2>;
> +
> +					reg = <0x21>;
> +				};

It would be good to align both description in terms of empty new lines.

Also, in the second controller, you have pinctrl-names, but no
pinctrl-0, so it doesn't make much sense.

> +			ethernet at 70000 {
> +				status = "okay";
> +				phy = <&phy1>;
> +				phy-mode = "rgmii-id";
> +			};
> +
> +

One too many empty new line.

> +			mdio at 72004 {
> +				phy0: ethernet-phy at 0 {
> +					reg = <0>;
> +				};
> +
> +				phy1: ethernet-phy at 1 {
> +					reg = <1>;
> +				};
> +			};

Maybe this is confusing, but it seems like the port 0 (i.e the one at
0x70000) is connected to the PHY at address 1, while the port 1 (i.e
the one at 0x30000) is connected to the PHY at address 0. According to
U-Boot:

| egiga0 |   RGMII   |     0x01     |
| egiga1 |   RGMII   |     0x00     |

So maybe we should have:

	ethernet at 30000 {
		phy = <&phy1>;
	};

	ethernet at 70000 {
		phy = <&phy0>;
	};

	mdio at 72004 {
		phy0: ethernet-phy at 1 {
			reg = <1>;
		};

		phy1: ethernet-phy at 0 {
			reg = <0>;
		};
	};

To reflect this, no?

> +			sata at a8000 {
> +				nr-ports = <2>;
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				sata0: sata-port at 0 {
> +					reg = <0>;
> +					target-supply = <&reg_5v_sata0>;
> +				};
> +
> +				sata1: sata-port at 1 {
> +					reg = <1>;
> +					target-supply = <&reg_5v_sata1>;
> +				};

Doesn't this part depends on the patches you have submitted to the AHCI
driver? If so, it would be good to state this in the cover letter, so
that the dependencies are known. Or for the moment, to not handle the
SATA part, until the DT binding for describing per-SATA port regulators
is sorted out (I saw that the feedback from Mark Brown on your patches
indicate that some rework will be needed).

Also, having a reg property into a child node that isn't part of a bus
looks wrong.

> +			};
> +
> +			sata at e0000 {
> +				nr-ports = <2>;
> +				status = "okay";
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +
> +				sata2: sata-port at 0 {
> +					reg = <0>;
> +					target-supply = <&reg_5v_sata2>;
> +				};
> +
> +				sata3: sata-port at 1 {
> +					reg = <1>;
> +					target-supply = <&reg_5v_sata3>;
> +				};
> +			};

Ditto.

> +			sdhci at d8000 {
> +				clock-frequency = <200000000>;

Why? There is already a 'clocks' property in armada-38x.dtsi. However,
the clock is actually 250 Mhz, not 200 Mhz. Any reason why you forced
200 Mhz here?

> +				cd-gpios = <&pca9555_0 5 GPIO_ACTIVE_LOW>;
> +				no-1-8-v;
> +				wp-inverted;

Why do you have a wp-inverted property, with no wp-gpios property? If I
understand the DT binding correctly, wp-inverted only makes sense when
wp-gpios is used.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

* Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
  2014-12-27 11:34         ` Andrew Lunn
@ 2015-01-05 14:03             ` Gregory CLEMENT
  -1 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-01-05 14:03 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jason Cooper, Sebastian Hesselbarth, Thomas Petazzoni,
	Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Andrew,

>> + * Device Tree file for Marvell Armada 385 development board
>> + * (RD-88F6820-GP)
>> + *
>> + * Copyright (C) 2014 Marvell
>> + *
>> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
> 
> Hi Gregory
> 
> I'm guessing here...
> 
> The file says Copyright Marvell. I guess Marvell released it GPL? Not
> Dual license? So it would be nice to have a Signed-off-by: from
> somebody in Marvell that they are O.K. with the dual license.
> 

I wrote it from scratch and in the same time Marvell is aware and agrees
about the Dual license. Until now all the files we wrote have the Marvell
copyright but without any  Signed-off-by: from anyone in Marvell so I don't
think we need it now.

Thanks,

Gregory



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

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

* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
@ 2015-01-05 14:03             ` Gregory CLEMENT
  0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-01-05 14:03 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Andrew,

>> + * Device Tree file for Marvell Armada 385 development board
>> + * (RD-88F6820-GP)
>> + *
>> + * Copyright (C) 2014 Marvell
>> + *
>> + * Gregory CLEMENT <gregory.clement@free-electrons.com>
>> + *
>> + * This file is dual-licensed: you can use it either under the terms
>> + * of the GPL or the X11 license, at your option. Note that this dual
>> + * licensing only applies to this file, and not this project as a
>> + * whole.
> 
> Hi Gregory
> 
> I'm guessing here...
> 
> The file says Copyright Marvell. I guess Marvell released it GPL? Not
> Dual license? So it would be nice to have a Signed-off-by: from
> somebody in Marvell that they are O.K. with the dual license.
> 

I wrote it from scratch and in the same time Marvell is aware and agrees
about the Dual license. Until now all the files we wrote have the Marvell
copyright but without any  Signed-off-by: from anyone in Marvell so I don't
think we need it now.

Thanks,

Gregory



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

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

* Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
  2015-01-05 14:03             ` Gregory CLEMENT
@ 2015-01-05 14:24                 ` Andrew Lunn
  -1 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2015-01-05 14:24 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Jason Cooper, Sebastian Hesselbarth, Thomas Petazzoni,
	Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

On Mon, Jan 05, 2015 at 03:03:38PM +0100, Gregory CLEMENT wrote:
> Hi Andrew,
> 
> >> + * Device Tree file for Marvell Armada 385 development board
> >> + * (RD-88F6820-GP)
> >> + *
> >> + * Copyright (C) 2014 Marvell
> >> + *
> >> + * Gregory CLEMENT <gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> >> + *
> >> + * This file is dual-licensed: you can use it either under the terms
> >> + * of the GPL or the X11 license, at your option. Note that this dual
> >> + * licensing only applies to this file, and not this project as a
> >> + * whole.
> > 
> > Hi Gregory
> > 
> > I'm guessing here...
> > 
> > The file says Copyright Marvell. I guess Marvell released it GPL? Not
> > Dual license? So it would be nice to have a Signed-off-by: from
> > somebody in Marvell that they are O.K. with the dual license.
> > 
> 
> I wrote it from scratch and in the same time Marvell is aware and agrees
> about the Dual license. Until now all the files we wrote have the Marvell
> copyright but without any  Signed-off-by: from anyone in Marvell so I don't
> think we need it now.

Hi Gregory

Thanks for the explanation. I guess wrong about where the Copyright
came from. So this is O.K.

Thanks
	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] 30+ messages in thread

* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
@ 2015-01-05 14:24                 ` Andrew Lunn
  0 siblings, 0 replies; 30+ messages in thread
From: Andrew Lunn @ 2015-01-05 14:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jan 05, 2015 at 03:03:38PM +0100, Gregory CLEMENT wrote:
> Hi Andrew,
> 
> >> + * Device Tree file for Marvell Armada 385 development board
> >> + * (RD-88F6820-GP)
> >> + *
> >> + * Copyright (C) 2014 Marvell
> >> + *
> >> + * Gregory CLEMENT <gregory.clement@free-electrons.com>
> >> + *
> >> + * This file is dual-licensed: you can use it either under the terms
> >> + * of the GPL or the X11 license, at your option. Note that this dual
> >> + * licensing only applies to this file, and not this project as a
> >> + * whole.
> > 
> > Hi Gregory
> > 
> > I'm guessing here...
> > 
> > The file says Copyright Marvell. I guess Marvell released it GPL? Not
> > Dual license? So it would be nice to have a Signed-off-by: from
> > somebody in Marvell that they are O.K. with the dual license.
> > 
> 
> I wrote it from scratch and in the same time Marvell is aware and agrees
> about the Dual license. Until now all the files we wrote have the Marvell
> copyright but without any  Signed-off-by: from anyone in Marvell so I don't
> think we need it now.

Hi Gregory

Thanks for the explanation. I guess wrong about where the Copyright
came from. So this is O.K.

Thanks
	Andrew

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

* Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
  2014-12-27 11:50         ` Andrew Lunn
@ 2015-01-05 14:34             ` Gregory CLEMENT
  -1 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-01-05 14:34 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Jason Cooper, Sebastian Hesselbarth, Thomas Petazzoni,
	Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Andrew,

On 27/12/2014 12:50, Andrew Lunn wrote:
[...]
>> +
>> +			i2c@11000 {
>> +				status = "okay";
>> +				clock-frequency = <100000>;
>> +
>> +				pca9555_0: pca9555@20 {
> 
> I think Sebastian will come along and ask this is called gpio, not
> pca9555_0. See the review he made of the Seagate Black NAS box.

OK, I was not very inspired when I chose it. I will have a look on
Sebastien review.

> 
>> +					compatible = "nxp,pca9555";
>> +					pinctrl-names = "default";
>> +					pinctrl-0 = <&pca0_pins>;
>> +					interrupt-parent = <&gpio0>;
>> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>> +					gpio-controller;
>> +					#gpio-cells = <2>;
>> +					interrupt-controller;
>> +					#interrupt-cells = <2>;
>> +					reg = <0x20>;
>> +				};
>> +
>> +				pca9555_1: pca9555@21 {
>> +					compatible = "nxp,pca9555";
>> +					pinctrl-names = "default";
>> +					interrupt-parent = <&gpio0>;
>> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>> +
>> +					gpio-controller;
>> +					#gpio-cells = <2>;
>> +					interrupt-controller;
>> +					#interrupt-cells = <2>;
>> +
> 
> Maybe remove the blank lines here, to make it similar to the previous
> one?

OK
> 
>> +					reg = <0x21>;
>> +				};
>> +
> 
> Maybe remove this blank line?
> 
>> +			};
>> +
>> +			serial@12000 {
>> +				status = "okay";
>> +			};
> 
> It would be nice if you can document the connector number and the
> pinout of the serial port, if it is not on the silk screen.

The serial port is output through an FTDI on an micro-USB connector, I will
add this information in the comment.

[...]

>> +		gpio-fan {
>> +			compatible = "gpio-fan";
>> +			gpios = <&pca9555_1 3 GPIO_ACTIVE_HIGH>;
>> +			gpio-fan,speed-map = <0 0 3000 1>;
> 
> It would be nice to format this with a newline between the two map
> entries.

I copied and pasted what have been done on armada-370-rd.dts but I have no
problem to change it according to your comment.

[...]

>> +	reg_5v_sata2: v5-sata2 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "v5.0-sata2";
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		regulator-always-on;
>> +		vin-supply = <&reg_sata2>;
>> +	};
>> +
>> +	reg_12v_sata2: v12-sata2 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "v12.0-sata2";
>> +		regulator-min-microvolt = <12000000>;
>> +		regulator-max-microvolt = <12000000>;
>> +		regulator-always-on;
>> +		vin-supply = <&reg_sata2>;
>> +	};
>> +
>> +	reg_sata3: pwr-sata3 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "pwr_en_sata3";
>> +		enable-active-high;
>> +		regulator-always-on;
>> +		gpio = <&pca9555_0 12 GPIO_ACTIVE_HIGH>;
>> +	};
>> +
>> +	reg_5v_sata3: v5-sata3 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "v5.0-sata3";
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		regulator-always-on;
>> +		vin-supply = <&reg_sata3>;
>> +	};
>> +
>> +	reg_12v_sata3: v12-sata3 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "v12.0-sata3";
>> +		regulator-min-microvolt = <12000000>;
>> +		regulator-max-microvolt = <12000000>;
>> +		regulator-always-on;
>> +		vin-supply = <&reg_sata3>;
>> +	};
>> +};
> 
> What is the quality of the power supply? What you often see if that
> SATA drives are spun up sequentially, in order to not strain the power

I don't think it is the case here. What I observed is a global 5V and a global
12 V power supply line, and then for each voltage and for each SATA drive there
is a FDC6330L which is more or less a controlled switch.

> supply with the current draw of them all starting at the same
> time. There is a property, startup-delay-us, which can be used for
> this.

According to the datasheet of the FDC6330L I didn't see any startup delay feature.
Unless this property was not to describe the hardware but to configure it, in this
case it could make sens to use it.


Thanks,

Gregory


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

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

* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
@ 2015-01-05 14:34             ` Gregory CLEMENT
  0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-01-05 14:34 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Andrew,

On 27/12/2014 12:50, Andrew Lunn wrote:
[...]
>> +
>> +			i2c at 11000 {
>> +				status = "okay";
>> +				clock-frequency = <100000>;
>> +
>> +				pca9555_0: pca9555 at 20 {
> 
> I think Sebastian will come along and ask this is called gpio, not
> pca9555_0. See the review he made of the Seagate Black NAS box.

OK, I was not very inspired when I chose it. I will have a look on
Sebastien review.

> 
>> +					compatible = "nxp,pca9555";
>> +					pinctrl-names = "default";
>> +					pinctrl-0 = <&pca0_pins>;
>> +					interrupt-parent = <&gpio0>;
>> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>> +					gpio-controller;
>> +					#gpio-cells = <2>;
>> +					interrupt-controller;
>> +					#interrupt-cells = <2>;
>> +					reg = <0x20>;
>> +				};
>> +
>> +				pca9555_1: pca9555 at 21 {
>> +					compatible = "nxp,pca9555";
>> +					pinctrl-names = "default";
>> +					interrupt-parent = <&gpio0>;
>> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>> +
>> +					gpio-controller;
>> +					#gpio-cells = <2>;
>> +					interrupt-controller;
>> +					#interrupt-cells = <2>;
>> +
> 
> Maybe remove the blank lines here, to make it similar to the previous
> one?

OK
> 
>> +					reg = <0x21>;
>> +				};
>> +
> 
> Maybe remove this blank line?
> 
>> +			};
>> +
>> +			serial at 12000 {
>> +				status = "okay";
>> +			};
> 
> It would be nice if you can document the connector number and the
> pinout of the serial port, if it is not on the silk screen.

The serial port is output through an FTDI on an micro-USB connector, I will
add this information in the comment.

[...]

>> +		gpio-fan {
>> +			compatible = "gpio-fan";
>> +			gpios = <&pca9555_1 3 GPIO_ACTIVE_HIGH>;
>> +			gpio-fan,speed-map = <0 0 3000 1>;
> 
> It would be nice to format this with a newline between the two map
> entries.

I copied and pasted what have been done on armada-370-rd.dts but I have no
problem to change it according to your comment.

[...]

>> +	reg_5v_sata2: v5-sata2 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "v5.0-sata2";
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		regulator-always-on;
>> +		vin-supply = <&reg_sata2>;
>> +	};
>> +
>> +	reg_12v_sata2: v12-sata2 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "v12.0-sata2";
>> +		regulator-min-microvolt = <12000000>;
>> +		regulator-max-microvolt = <12000000>;
>> +		regulator-always-on;
>> +		vin-supply = <&reg_sata2>;
>> +	};
>> +
>> +	reg_sata3: pwr-sata3 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "pwr_en_sata3";
>> +		enable-active-high;
>> +		regulator-always-on;
>> +		gpio = <&pca9555_0 12 GPIO_ACTIVE_HIGH>;
>> +	};
>> +
>> +	reg_5v_sata3: v5-sata3 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "v5.0-sata3";
>> +		regulator-min-microvolt = <5000000>;
>> +		regulator-max-microvolt = <5000000>;
>> +		regulator-always-on;
>> +		vin-supply = <&reg_sata3>;
>> +	};
>> +
>> +	reg_12v_sata3: v12-sata3 {
>> +		compatible = "regulator-fixed";
>> +		regulator-name = "v12.0-sata3";
>> +		regulator-min-microvolt = <12000000>;
>> +		regulator-max-microvolt = <12000000>;
>> +		regulator-always-on;
>> +		vin-supply = <&reg_sata3>;
>> +	};
>> +};
> 
> What is the quality of the power supply? What you often see if that
> SATA drives are spun up sequentially, in order to not strain the power

I don't think it is the case here. What I observed is a global 5V and a global
12 V power supply line, and then for each voltage and for each SATA drive there
is a FDC6330L which is more or less a controlled switch.

> supply with the current draw of them all starting at the same
> time. There is a property, startup-delay-us, which can be used for
> this.

According to the datasheet of the FDC6330L I didn't see any startup delay feature.
Unless this property was not to describe the hardware but to configure it, in this
case it could make sens to use it.


Thanks,

Gregory


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

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

* Re: [PATCH 0/3] Add Armada 385 General Purpose Development Board support
  2014-12-31  9:57     ` Thomas Petazzoni
@ 2015-01-05 14:36         ` Gregory CLEMENT
  -1 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-01-05 14:36 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Thomas,

On 31/12/2014 10:57, Thomas Petazzoni wrote:
> Dear Gregory CLEMENT,
> 
> On Sat, 27 Dec 2014 12:00:32 +0100, Gregory CLEMENT wrote:
> 
>> this patch set adds the support for the Armada 385 GP board, this board
>> is pretty close to the Armada 385 RD one. It comes with a new revision
>> of the Armada 385 SoC (B0).
> 
> Unless I'm wrong, it actually comes with an A0 revision, not a B0. But
> it's indeed different from the Z1 revision we have been using until now.
> 
> At least, my Armada XP GP says:
> 
> Board: DB-88F6820-GP
> SoC:   MV88F6828 Rev A0

You're right I wen too far in the revision, it was A0.


Thanks,

Gregory


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

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

* [PATCH 0/3] Add Armada 385 General Purpose Development Board support
@ 2015-01-05 14:36         ` Gregory CLEMENT
  0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-01-05 14:36 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Thomas,

On 31/12/2014 10:57, Thomas Petazzoni wrote:
> Dear Gregory CLEMENT,
> 
> On Sat, 27 Dec 2014 12:00:32 +0100, Gregory CLEMENT wrote:
> 
>> this patch set adds the support for the Armada 385 GP board, this board
>> is pretty close to the Armada 385 RD one. It comes with a new revision
>> of the Armada 385 SoC (B0).
> 
> Unless I'm wrong, it actually comes with an A0 revision, not a B0. But
> it's indeed different from the Z1 revision we have been using until now.
> 
> At least, my Armada XP GP says:
> 
> Board: DB-88F6820-GP
> SoC:   MV88F6828 Rev A0

You're right I wen too far in the revision, it was A0.


Thanks,

Gregory


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

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

* Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
  2014-12-31 10:32         ` Thomas Petazzoni
@ 2015-01-05 17:06             ` Gregory CLEMENT
  -1 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-01-05 17:06 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth,
	Ezequiel Garcia,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Maxime Ripard,
	Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai,
	Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Thomas,

On 31/12/2014 11:32, Thomas Petazzoni wrote:
> Dear Gregory CLEMENT,
> 
> On Sat, 27 Dec 2014 12:00:35 +0100, Gregory CLEMENT wrote:
> 
>> +				spi-flash@0 {
>> +					#address-cells = <1>;
>> +					#size-cells = <1>;
>> +					compatible = "st,m25p128";
>> +					reg = <0>; /* Chip select 0 */
>> +					spi-max-frequency = <50000000>;
> 
> According to the m25p128 datasheet,  the max frequency is 54 Mhz.

It is the  max frequency for the 65nm devices, but the one on the GP board
is a M25P128-VMF6P. As there was no 'B' in the part number then it was a
130nm device which is limited to 50MHz.

> 
>> +			i2c@11000 {
>> +				status = "okay";
>> +				clock-frequency = <100000>;
>> +
>> +				pca9555_0: pca9555@20 {
>> +					compatible = "nxp,pca9555";
>> +					pinctrl-names = "default";
>> +					pinctrl-0 = <&pca0_pins>;
>> +					interrupt-parent = <&gpio0>;
>> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>> +					gpio-controller;
>> +					#gpio-cells = <2>;
>> +					interrupt-controller;
>> +					#interrupt-cells = <2>;
>> +					reg = <0x20>;
>> +				};
>> +
>> +				pca9555_1: pca9555@21 {
>> +					compatible = "nxp,pca9555";
>> +					pinctrl-names = "default";
>> +					interrupt-parent = <&gpio0>;
>> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>> +
>> +					gpio-controller;
>> +					#gpio-cells = <2>;
>> +					interrupt-controller;
>> +					#interrupt-cells = <2>;
>> +
>> +					reg = <0x21>;
>> +				};
> 
> It would be good to align both description in terms of empty new lines.
> 
> Also, in the second controller, you have pinctrl-names, but no
> pinctrl-0, so it doesn't make much sense.
> 

Yes it was a left over of from a previous version, I will remove it.

>> +			ethernet@70000 {
>> +				status = "okay";
>> +				phy = <&phy1>;
>> +				phy-mode = "rgmii-id";
>> +			};
>> +
>> +
> 
> One too many empty new line.
> 
>> +			mdio@72004 {
>> +				phy0: ethernet-phy@0 {
>> +					reg = <0>;
>> +				};
>> +
>> +				phy1: ethernet-phy@1 {
>> +					reg = <1>;
>> +				};
>> +			};
> 
> Maybe this is confusing, but it seems like the port 0 (i.e the one at
> 0x70000) is connected to the PHY at address 1, while the port 1 (i.e
> the one at 0x30000) is connected to the PHY at address 0. According to
> U-Boot:
> 
> | egiga0 |   RGMII   |     0x01     |
> | egiga1 |   RGMII   |     0x00     |
> 
> So maybe we should have:
> 
> 	ethernet@30000 {
> 		phy = <&phy1>;
> 	};
> 
> 	ethernet@70000 {
> 		phy = <&phy0>;
> 	};
> 
> 	mdio@72004 {
> 		phy0: ethernet-phy@1 {
> 			reg = <1>;
> 		};
> 
> 		phy1: ethernet-phy@0 {
> 			reg = <0>;
> 		};
> 	};
> 
> To reflect this, no?



> 
>> +			sata@a8000 {
>> +				nr-ports = <2>;
>> +				status = "okay";
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +
>> +				sata0: sata-port@0 {
>> +					reg = <0>;
>> +					target-supply = <&reg_5v_sata0>;
>> +				};
>> +
>> +				sata1: sata-port@1 {
>> +					reg = <1>;
>> +					target-supply = <&reg_5v_sata1>;
>> +				};
> 
> Doesn't this part depends on the patches you have submitted to the AHCI
> driver? If so, it would be good to state this in the cover letter, so
> that the dependencies are known. Or for the moment, to not handle the
> SATA part, until the DT binding for describing per-SATA port regulators
> is sorted out (I saw that the feedback from Mark Brown on your patches
> indicate that some rework will be needed).

It was the same kind of DT binding which was used for PHY that I used for
the regulator, I didn't imagine that it couldn't be accepted. But I was
wrong DT bindings seems to be really dependent of each maintainers.

I will move the regulator part of the SATA in a different patch.

> 
> Also, having a reg property into a child node that isn't part of a bus
> looks wrong.

But according to the binding documentation:
Documentation/devicetree/bindings/ata/ahci-platform.txt, the reg property is a
required property. If it is wrong that means that the bindings should be changed,
but it also should be stable.

> 
>> +			};
>> +
>> +			sata@e0000 {
>> +				nr-ports = <2>;
>> +				status = "okay";
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +
>> +				sata2: sata-port@0 {
>> +					reg = <0>;
>> +					target-supply = <&reg_5v_sata2>;
>> +				};
>> +
>> +				sata3: sata-port@1 {
>> +					reg = <1>;
>> +					target-supply = <&reg_5v_sata3>;
>> +				};
>> +			};
> 
> Ditto.
> 
>> +			sdhci@d8000 {
>> +				clock-frequency = <200000000>;
> 
> Why? There is already a 'clocks' property in armada-38x.dtsi. However,
> the clock is actually 250 Mhz, not 200 Mhz. Any reason why you forced
> 200 Mhz here?
> 

It was copied from the 385 DB dts before the patch "ARM: mvebu: remove
clock-frequency from Armada 38x SDHCI Device Tree node" was applied. I will
remove it


>> +				cd-gpios = <&pca9555_0 5 GPIO_ACTIVE_LOW>;
>> +				no-1-8-v;
>> +				wp-inverted;
> 
> Why do you have a wp-inverted property, with no wp-gpios property? If I
> understand the DT binding correctly, wp-inverted only makes sense when
> wp-gpios is used.

This part came also from the 385 DB board as the connector was similar. I
thought you had introduced it on purpose so I kept it.


Thanks,

Gregory



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

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

* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support
@ 2015-01-05 17:06             ` Gregory CLEMENT
  0 siblings, 0 replies; 30+ messages in thread
From: Gregory CLEMENT @ 2015-01-05 17:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Thomas,

On 31/12/2014 11:32, Thomas Petazzoni wrote:
> Dear Gregory CLEMENT,
> 
> On Sat, 27 Dec 2014 12:00:35 +0100, Gregory CLEMENT wrote:
> 
>> +				spi-flash at 0 {
>> +					#address-cells = <1>;
>> +					#size-cells = <1>;
>> +					compatible = "st,m25p128";
>> +					reg = <0>; /* Chip select 0 */
>> +					spi-max-frequency = <50000000>;
> 
> According to the m25p128 datasheet,  the max frequency is 54 Mhz.

It is the  max frequency for the 65nm devices, but the one on the GP board
is a M25P128-VMF6P. As there was no 'B' in the part number then it was a
130nm device which is limited to 50MHz.

> 
>> +			i2c at 11000 {
>> +				status = "okay";
>> +				clock-frequency = <100000>;
>> +
>> +				pca9555_0: pca9555 at 20 {
>> +					compatible = "nxp,pca9555";
>> +					pinctrl-names = "default";
>> +					pinctrl-0 = <&pca0_pins>;
>> +					interrupt-parent = <&gpio0>;
>> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>> +					gpio-controller;
>> +					#gpio-cells = <2>;
>> +					interrupt-controller;
>> +					#interrupt-cells = <2>;
>> +					reg = <0x20>;
>> +				};
>> +
>> +				pca9555_1: pca9555 at 21 {
>> +					compatible = "nxp,pca9555";
>> +					pinctrl-names = "default";
>> +					interrupt-parent = <&gpio0>;
>> +					interrupts = <18 IRQ_TYPE_EDGE_FALLING>;
>> +
>> +					gpio-controller;
>> +					#gpio-cells = <2>;
>> +					interrupt-controller;
>> +					#interrupt-cells = <2>;
>> +
>> +					reg = <0x21>;
>> +				};
> 
> It would be good to align both description in terms of empty new lines.
> 
> Also, in the second controller, you have pinctrl-names, but no
> pinctrl-0, so it doesn't make much sense.
> 

Yes it was a left over of from a previous version, I will remove it.

>> +			ethernet at 70000 {
>> +				status = "okay";
>> +				phy = <&phy1>;
>> +				phy-mode = "rgmii-id";
>> +			};
>> +
>> +
> 
> One too many empty new line.
> 
>> +			mdio at 72004 {
>> +				phy0: ethernet-phy at 0 {
>> +					reg = <0>;
>> +				};
>> +
>> +				phy1: ethernet-phy at 1 {
>> +					reg = <1>;
>> +				};
>> +			};
> 
> Maybe this is confusing, but it seems like the port 0 (i.e the one at
> 0x70000) is connected to the PHY at address 1, while the port 1 (i.e
> the one at 0x30000) is connected to the PHY at address 0. According to
> U-Boot:
> 
> | egiga0 |   RGMII   |     0x01     |
> | egiga1 |   RGMII   |     0x00     |
> 
> So maybe we should have:
> 
> 	ethernet at 30000 {
> 		phy = <&phy1>;
> 	};
> 
> 	ethernet at 70000 {
> 		phy = <&phy0>;
> 	};
> 
> 	mdio at 72004 {
> 		phy0: ethernet-phy at 1 {
> 			reg = <1>;
> 		};
> 
> 		phy1: ethernet-phy at 0 {
> 			reg = <0>;
> 		};
> 	};
> 
> To reflect this, no?



> 
>> +			sata at a8000 {
>> +				nr-ports = <2>;
>> +				status = "okay";
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +
>> +				sata0: sata-port at 0 {
>> +					reg = <0>;
>> +					target-supply = <&reg_5v_sata0>;
>> +				};
>> +
>> +				sata1: sata-port at 1 {
>> +					reg = <1>;
>> +					target-supply = <&reg_5v_sata1>;
>> +				};
> 
> Doesn't this part depends on the patches you have submitted to the AHCI
> driver? If so, it would be good to state this in the cover letter, so
> that the dependencies are known. Or for the moment, to not handle the
> SATA part, until the DT binding for describing per-SATA port regulators
> is sorted out (I saw that the feedback from Mark Brown on your patches
> indicate that some rework will be needed).

It was the same kind of DT binding which was used for PHY that I used for
the regulator, I didn't imagine that it couldn't be accepted. But I was
wrong DT bindings seems to be really dependent of each maintainers.

I will move the regulator part of the SATA in a different patch.

> 
> Also, having a reg property into a child node that isn't part of a bus
> looks wrong.

But according to the binding documentation:
Documentation/devicetree/bindings/ata/ahci-platform.txt, the reg property is a
required property. If it is wrong that means that the bindings should be changed,
but it also should be stable.

> 
>> +			};
>> +
>> +			sata at e0000 {
>> +				nr-ports = <2>;
>> +				status = "okay";
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +
>> +				sata2: sata-port at 0 {
>> +					reg = <0>;
>> +					target-supply = <&reg_5v_sata2>;
>> +				};
>> +
>> +				sata3: sata-port at 1 {
>> +					reg = <1>;
>> +					target-supply = <&reg_5v_sata3>;
>> +				};
>> +			};
> 
> Ditto.
> 
>> +			sdhci at d8000 {
>> +				clock-frequency = <200000000>;
> 
> Why? There is already a 'clocks' property in armada-38x.dtsi. However,
> the clock is actually 250 Mhz, not 200 Mhz. Any reason why you forced
> 200 Mhz here?
> 

It was copied from the 385 DB dts before the patch "ARM: mvebu: remove
clock-frequency from Armada 38x SDHCI Device Tree node" was applied. I will
remove it


>> +				cd-gpios = <&pca9555_0 5 GPIO_ACTIVE_LOW>;
>> +				no-1-8-v;
>> +				wp-inverted;
> 
> Why do you have a wp-inverted property, with no wp-gpios property? If I
> understand the DT binding correctly, wp-inverted only makes sense when
> wp-gpios is used.

This part came also from the 385 DB board as the connector was similar. I
thought you had introduced it on purpose so I kept it.


Thanks,

Gregory



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

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

end of thread, other threads:[~2015-01-05 17:06 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-12-27 11:00 [PATCH 0/3] Add Armada 385 General Purpose Development Board support Gregory CLEMENT
2014-12-27 11:00 ` Gregory CLEMENT
     [not found] ` <1419678035-10658-1-git-send-email-gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2014-12-27 11:00   ` [PATCH 1/3] ARM: mvebu: a38x: Fix node names Gregory CLEMENT
2014-12-27 11:00     ` Gregory CLEMENT
2014-12-27 11:00   ` [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias Gregory CLEMENT
2014-12-27 11:00     ` Gregory CLEMENT
     [not found]     ` <1419678035-10658-3-git-send-email-gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2014-12-27 11:29       ` Andrew Lunn
2014-12-27 11:29         ` Andrew Lunn
2014-12-27 14:46       ` Sergei Shtylyov
2014-12-27 14:46         ` Sergei Shtylyov
2014-12-27 11:00   ` [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support Gregory CLEMENT
2014-12-27 11:00     ` Gregory CLEMENT
     [not found]     ` <1419678035-10658-4-git-send-email-gregory.clement-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2014-12-27 11:34       ` Andrew Lunn
2014-12-27 11:34         ` Andrew Lunn
     [not found]         ` <20141227113401.GC21001-g2DYL2Zd6BY@public.gmane.org>
2015-01-05 14:03           ` Gregory CLEMENT
2015-01-05 14:03             ` Gregory CLEMENT
     [not found]             ` <54AA99BA.9060300-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2015-01-05 14:24               ` Andrew Lunn
2015-01-05 14:24                 ` Andrew Lunn
2014-12-27 11:50       ` Andrew Lunn
2014-12-27 11:50         ` Andrew Lunn
     [not found]         ` <20141227115029.GD21001-g2DYL2Zd6BY@public.gmane.org>
2015-01-05 14:34           ` Gregory CLEMENT
2015-01-05 14:34             ` Gregory CLEMENT
2014-12-31 10:32       ` Thomas Petazzoni
2014-12-31 10:32         ` Thomas Petazzoni
     [not found]         ` <20141231113250.6ae98b19-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2015-01-05 17:06           ` Gregory CLEMENT
2015-01-05 17:06             ` Gregory CLEMENT
2014-12-31  9:57   ` [PATCH 0/3] " Thomas Petazzoni
2014-12-31  9:57     ` Thomas Petazzoni
     [not found]     ` <20141231105720.24e6983f-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
2015-01-05 14:36       ` Gregory CLEMENT
2015-01-05 14:36         ` Gregory CLEMENT

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.