[PATCH 0/3] Add Armada 385 General Purpose Development Board support

[PATCH 0/3] Add Armada 385 General Purpose Development Board support

[Gregory CLEMENT]

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

[PATCH 1/3] ARM: mvebu: a38x: Fix node names

[Gregory CLEMENT]

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

[PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias

[Gregory CLEMENT]

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

Re: [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias

[Andrew Lunn]

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

Re: [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias

[Sergei Shtylyov]

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

[PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support

[Gregory CLEMENT]

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

Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support

[Andrew Lunn]

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

Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support

[Gregory CLEMENT]

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

Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support

[Andrew Lunn]

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

Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support

[Andrew Lunn]

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

Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support

[Gregory CLEMENT]

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

Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support

[Thomas Petazzoni]

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

Re: [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support

[Gregory CLEMENT]

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

Re: [PATCH 0/3] Add Armada 385 General Purpose Development Board support

[Thomas Petazzoni]

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

Re: [PATCH 0/3] Add Armada 385 General Purpose Development Board support

[Gregory CLEMENT]

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