Add Armada 385 General Purpose Development Board support
cover Add Armada 385 General Purpose Development Board support
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
Signature · 1 line
1.9.1
PATCH 1/3 ARM: mvebu: a38x: Fix node names
From: Maxime Ripard <[email protected]>
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.
--- 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(-)
arch/arm/boot/dts/armada-385-db.dts +1 -1
@@ -74,7 +74,7 @@ phy-mode = "rgmii-id"; }; - mdio { + mdio at 72004 { phy0: ethernet-phy at 0 { reg = <0>; };
arch/arm/boot/dts/armada-385-rd.dts +1 -1
@@ -67,7 +67,7 @@ }; - mdio { + mdio at 72004 { phy0: ethernet-phy at 0 { reg = <0>; };
arch/arm/boot/dts/armada-385.dtsi +1 -1
@@ -37,7 +37,7 @@ soc { internal-regs { - pinctrl { + pinctrl at 18000 { compatible = "marvell,mv88f6820-pinctrl"; reg = <0x18000 0x20>; };
arch/arm/boot/dts/armada-38x.dtsi +2 -2
@@ -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";
Signature · 1 line
1.9.1
PATCH 2/3 ARM: mvebu: a38x: Add the pinctrl alias
In reply to Gregory CLEMENT--- arch/arm/boot/dts/armada-38x.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/boot/dts/armada-38x.dtsi +1 -1
@@ -193,7 +193,7 @@ status = "disabled"; }; - pinctrl at 18000 { + pinctrl: pinctrl at 18000 { compatible = "marvell,mv88f6820-pinctrl"; reg = <0x18000 0x20>; };
Signature · 1 line
1.9.1
PATCH 2/3 ARM: mvebu: a38x: Add the pinctrl alias
On Sat, Dec 27, 2014 at 12:00:34PM +0100, Gregory CLEMENT wrote:
Hi Gregory
A one line changelog would be nice.
Andrew
Quoted 21 lines
Signed-off-by Gregory CLEMENT <[email protected]>--- arch/arm/boot/dts/armada-38x.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/arm/boot/dts/armada-38x.dtsi +1
-1@@ -193,7 +193,7 @@ status = "disabled"; }; - pinctrl at 18000 { + pinctrl: pinctrl at 18000 { compatible = "marvell,mv88f6820-pinctrl"; reg = <0x18000 0x20>; };Signature · 1 line
1.9.1
PATCH 2/3 ARM: mvebu: a38x: Add the pinctrl alias
In reply to Gregory CLEMENTHello.
On 12/27/2014 2:00 PM, Gregory CLEMENT wrote:
You're not adding an alias. It's a label.
Quoted 1 line
Signed-off-by Gregory CLEMENT <[email protected]>
WBR, Sergei
PATCH 3/3 ARM: mvebu: Add Armada 385 General Purpose Development Board support
In reply to Gregory CLEMENTThe 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
--- 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
arch/arm/boot/dts/Makefile +1 -0
@@ -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 \
arch/arm/boot/dts/armada-385-gp.dts new file +380 -0
380 lines not on this page · raw message
Signature · 1 line
1.9.1
PATCH 3/3 ARM: mvebu: Add Armada 385 General Purpose Development Board support
Quoted 41 lines · 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 I2CSigned-off-by Gregory CLEMENT <[email protected]>--- 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.dtsarch/arm/boot/dts/Makefile +1
-0@@ -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 \arch/arm/boot/dts/armada-385-gp.dts new file +2
-0@@ -0,0 +1,380 @@ +/* + * Device Tree file for Marvell Armada 385 development board
arch/arm/boot/dts/armada-385-gp.dts line 12
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +10
-0+ * (RD-88F6820-GP) + * + * Copyright (C) 2014 Marvell + * + * Gregory CLEMENT <[email protected]> + * + * 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
PATCH 3/3 ARM: mvebu: Add Armada 385 General Purpose Development Board support
Hi Andrew,
Quoted 12 lines
Quoted 11 lines
+ * Device Tree file for Marvell Armada 385 development board
+ * (RD-88F6820-GP)
+ *
+ * Copyright (C) 2014 Marvell
+ *
+ * Gregory CLEMENT <[email protected]>
+ *
+ * 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.
Quoted 8 lines
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
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 3/3 ARM: mvebu: Add Armada 385 General Purpose Development Board support
Quoted 23 lines · Gregory CLEMENT
Hi Andrew,
Quoted 12 lines
Quoted 11 lines
+ * Device Tree file for Marvell Armada 385 development board
+ * (RD-88F6820-GP)
+ *
+ * Copyright (C) 2014 Marvell
+ *
+ * Gregory CLEMENT <[email protected]>
+ *
+ * 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.Quoted 8 lines
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.
Quoted 4 lines
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
PATCH 3/3 ARM: mvebu: Add Armada 385 General Purpose Development Board support
In reply to Gregory CLEMENTQuoted 72 lines
arch/arm/boot/dts/armada-385-gp.dts new file +71
-0@@ -0,0 +1,380 @@ +/* + * Device Tree file for Marvell Armada 385 development board + * (RD-88F6820-GP) + * + * Copyright (C) 2014 Marvell + * + * Gregory CLEMENT <[email protected]> + * + * 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 */
arch/arm/boot/dts/armada-385-gp.dts line 81
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +10
-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.
Quoted 13 lines
arch/arm/boot/dts/armada-385-gp.dts new file +13
-0+ 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 {
arch/arm/boot/dts/armada-385-gp.dts line 104
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +10
-0+ 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?
arch/arm/boot/dts/armada-385-gp.dts line 107
Quoted 3 lines
arch/arm/boot/dts/armada-385-gp.dts new file +3
-0+ reg = <0x21>; + }; +
Maybe remove this blank line?
arch/arm/boot/dts/armada-385-gp.dts line 112
Quoted 5 lines
arch/arm/boot/dts/armada-385-gp.dts new file +5
-0+ }; + + 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.
Quoted 102 lines
arch/arm/boot/dts/armada-385-gp.dts new file +102
-0+ + ethernet at 30000 { + status = "okay"; + phy = <&phy0>; + phy-mode = "rgmii-id"; + }; + + usb at 50000 { + vcc-supply = <®_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 = <®_5v_sata0>; + }; + + sata1: sata-port at 1 { + reg = <1>; + target-supply = <®_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 = <®_5v_sata2>; + }; + + sata3: sata-port at 1 { + reg = <1>; + target-supply = <®_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 = <®_usb2_1_vbus>; + status = "okay"; + }; + + usb3 at f8000 { + vcc-supply = <®_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"; + };
arch/arm/boot/dts/armada-385-gp.dts line 224
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +10
-0+ 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.
Quoted 139 lines
arch/arm/boot/dts/armada-385-gp.dts new file +139
-0+ }; + }; + + 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 = <®_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 = <®_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 = <®_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 = <®_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 = <®_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 = <®_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 = <®_sata3>; + };
arch/arm/boot/dts/armada-385-gp.dts line 373
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +10
-0+ + 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 = <®_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
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +7
-0+ +&pinctrl { + pca0_pins: pca0_pins { + marvell,pins = "mpp18"; + marvell,function = "gpio"; + }; +};Signature · 1 line
1.9.1
PATCH 3/3 ARM: mvebu: Add Armada 385 General Purpose Development Board support
Hi Andrew,
On 27/12/2014 12:50, Andrew Lunn wrote:
[...]
Quoted 9 lines
arch/arm/boot/dts/armada-385-gp.dts line 81
Quoted 6 lines
arch/arm/boot/dts/armada-385-gp.dts new file +6
-0+ + 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.
Quoted 19 lines
Quoted 19 lines
arch/arm/boot/dts/armada-385-gp.dts new file +18
-0+ 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>; +
Quoted 8 lines
arch/arm/boot/dts/armada-385-gp.dts line 93
Quoted 5 lines
arch/arm/boot/dts/armada-385-gp.dts new file +5
-0+ 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
Quoted 7 lines
arch/arm/boot/dts/armada-385-gp.dts line 107
Quoted 4 lines
arch/arm/boot/dts/armada-385-gp.dts new file +3
-0+ reg = <0x21>; + }; +Maybe remove this blank line?
Quoted 8 lines
arch/arm/boot/dts/armada-385-gp.dts line 112
Quoted 5 lines
arch/arm/boot/dts/armada-385-gp.dts new file +5
-0+ }; + + 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.
[...]
Quoted 7 lines
arch/arm/boot/dts/armada-385-gp.dts line 224
Quoted 4 lines
arch/arm/boot/dts/armada-385-gp.dts new file +4
-0+ 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.
[...]
Quoted 39 lines
Quoted 39 lines
arch/arm/boot/dts/armada-385-gp.dts new file +39
-0+ 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 = <®_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 = <®_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 = <®_sata3>; + }; + + reg_12v_sata3: v12-sata3 { + compatible = "regulator-fixed"; + regulator-name = "v12.0-sata3"; + regulator-min-microvolt = <12000000>;
Quoted 8 lines
arch/arm/boot/dts/armada-385-gp.dts line 372
Quoted 5 lines
arch/arm/boot/dts/armada-385-gp.dts new file +5
-0+ regulator-max-microvolt = <12000000>; + regulator-always-on; + vin-supply = <®_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.
Quoted 3 lines
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
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
PATCH 3/3 ARM: mvebu: Add Armada 385 General Purpose Development Board support
In reply to Gregory CLEMENTDear Gregory CLEMENT,
arch/arm/boot/dts/armada-385-gp.dts line 72
Quoted 6 lines · Gregory CLEMENT
arch/arm/boot/dts/armada-385-gp.dts new file +6
-0+ 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.
Quoted 20 lines
arch/arm/boot/dts/armada-385-gp.dts new file +20
-0+ 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";
arch/arm/boot/dts/armada-385-gp.dts line 106
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +10
-0+ 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.
arch/arm/boot/dts/armada-385-gp.dts line 131
Quoted 7 lines
arch/arm/boot/dts/armada-385-gp.dts new file +7
-0+ ethernet at 70000 { + status = "okay"; + phy = <&phy1>; + phy-mode = "rgmii-id"; + }; + +
One too many empty new line.
arch/arm/boot/dts/armada-385-gp.dts line 140
Quoted 9 lines
arch/arm/boot/dts/armada-385-gp.dts new file +9
-0+ 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?
Quoted 5 lines
arch/arm/boot/dts/armada-385-gp.dts new file +5
-0+ sata at a8000 { + nr-ports = <2>; + status = "okay"; + #address-cells = <1>; + #size-cells = <0>;
arch/arm/boot/dts/armada-385-gp.dts line 156
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +10
-0+ + sata0: sata-port at 0 { + reg = <0>; + target-supply = <®_5v_sata0>; + }; + + sata1: sata-port at 1 { + reg = <1>; + target-supply = <®_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.
Quoted 8 lines
arch/arm/boot/dts/armada-385-gp.dts new file +8
-0+ }; + + sata at e0000 { + nr-ports = <2>; + status = "okay"; + #address-cells = <1>; + #size-cells = <0>; +
arch/arm/boot/dts/armada-385-gp.dts line 174
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +10
-0+ sata2: sata-port at 0 { + reg = <0>; + target-supply = <®_5v_sata2>; + }; + + sata3: sata-port at 1 { + reg = <1>; + target-supply = <®_5v_sata3>; + }; + };
Ditto.
arch/arm/boot/dts/armada-385-gp.dts line 177
Quoted 2 lines
arch/arm/boot/dts/armada-385-gp.dts new file +2
-0+ 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?
arch/arm/boot/dts/armada-385-gp.dts line 180
Quoted 3 lines
arch/arm/boot/dts/armada-385-gp.dts new file +3
-0+ 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
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 3/3 ARM: mvebu: Add Armada 385 General Purpose Development Board support
Hi Thomas,
Quoted 4 lines · Thomas Petazzoni
Dear Gregory CLEMENT,
On Sat, 27 Dec 2014 12:00:35 +0100, Gregory CLEMENT wrote:
Quoted 8 lines
arch/arm/boot/dts/armada-385-gp.dts line 72
Quoted 6 lines
arch/arm/boot/dts/armada-385-gp.dts new file +6
-0+ 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.
Quoted 34 lines
Quoted 21 lines
arch/arm/boot/dts/armada-385-gp.dts new file +20
-0+ 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";arch/arm/boot/dts/armada-385-gp.dts line 106
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +10
-0+ 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.
Quoted 3 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.
Quoted 67 lines
arch/arm/boot/dts/armada-385-gp.dts line 131
Quoted 7 lines
arch/arm/boot/dts/armada-385-gp.dts new file +7
-0+ ethernet at 70000 { + status = "okay"; + phy = <&phy1>; + phy-mode = "rgmii-id"; + }; + +One too many empty new line.
arch/arm/boot/dts/armada-385-gp.dts line 140
Quoted 9 lines
arch/arm/boot/dts/armada-385-gp.dts new file +9
-0+ 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 at0x30000) 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?
Quoted 14 lines
arch/arm/boot/dts/armada-385-gp.dts new file +14
-0+ sata at a8000 { + nr-ports = <2>; + status = "okay"; + #address-cells = <1>; + #size-cells = <0>; + + sata0: sata-port at 0 { + reg = <0>; + target-supply = <®_5v_sata0>; + }; + + sata1: sata-port at 1 { + reg = <1>; + target-supply = <®_5v_sata1>;
Quoted 8 lines
Review
Quoted 1 line
(quoted diff) +1
-0+ };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.
Quoted 3 lines
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.
Quoted 22 lines
Quoted 9 lines
arch/arm/boot/dts/armada-385-gp.dts new file +8
-0+ }; + + sata at e0000 { + nr-ports = <2>; + status = "okay"; + #address-cells = <1>; + #size-cells = <0>; +arch/arm/boot/dts/armada-385-gp.dts line 174
Quoted 10 lines
arch/arm/boot/dts/armada-385-gp.dts new file +10
-0+ sata2: sata-port at 0 { + reg = <0>; + target-supply = <®_5v_sata2>; + }; + + sata3: sata-port at 1 { + reg = <1>; + target-supply = <®_5v_sata3>; + }; + };Ditto.
Quoted 7 lines
arch/arm/boot/dts/armada-385-gp.dts line 177
Quoted 2 lines
arch/arm/boot/dts/armada-385-gp.dts new file +2
-0+ 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
Quoted 7 lines
arch/arm/boot/dts/armada-385-gp.dts line 180
Quoted 3 lines
arch/arm/boot/dts/armada-385-gp.dts new file +3
-0+ 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
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com
cover Add Armada 385 General Purpose Development Board support
In reply to Gregory CLEMENTDear Gregory CLEMENT,
Quoted 3 lines · Gregory CLEMENT
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
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
cover Add Armada 385 General Purpose Development Board support
Hi Thomas,
Quoted 8 lines · Thomas Petazzoni
Dear Gregory CLEMENT,
Quoted 3 lines · Gregory CLEMENT
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).
Quoted 7 lines
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
Signature · 4 lines
Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com