* [PATCH 0/3] Add Armada 385 General Purpose Development Board support
@ 2014-12-27 11:00 Gregory CLEMENT
2014-12-27 11:00 ` [PATCH 1/3] ARM: mvebu: a38x: Fix node names Gregory CLEMENT
` (3 more replies)
0 siblings, 4 replies; 15+ 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] 15+ messages in thread* [PATCH 1/3] ARM: mvebu: a38x: Fix node names 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 2014-12-27 11:00 ` [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias Gregory CLEMENT ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ 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] 15+ messages in thread
* [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias 2014-12-27 11:00 [PATCH 0/3] Add Armada 385 General Purpose Development Board support Gregory CLEMENT 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:29 ` Andrew Lunn 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-31 9:57 ` [PATCH 0/3] " Thomas Petazzoni 3 siblings, 2 replies; 15+ 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] 15+ messages in thread
* [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias 2014-12-27 11:00 ` [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias Gregory CLEMENT @ 2014-12-27 11:29 ` Andrew Lunn 2014-12-27 14:46 ` Sergei Shtylyov 1 sibling, 0 replies; 15+ 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] 15+ messages in thread
* [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias 2014-12-27 11:00 ` [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias Gregory CLEMENT 2014-12-27 11:29 ` Andrew Lunn @ 2014-12-27 14:46 ` Sergei Shtylyov 1 sibling, 0 replies; 15+ 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] 15+ messages in thread
* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support 2014-12-27 11:00 [PATCH 0/3] Add Armada 385 General Purpose Development Board support Gregory CLEMENT 2014-12-27 11:00 ` [PATCH 1/3] ARM: mvebu: a38x: Fix node names 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 2014-12-27 11:34 ` Andrew Lunn ` (2 more replies) 2014-12-31 9:57 ` [PATCH 0/3] " Thomas Petazzoni 3 siblings, 3 replies; 15+ 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 = <®_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"; + }; + 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 = <®_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>; + }; + + 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>; + }; +}; + +&pinctrl { + pca0_pins: pca0_pins { + marvell,pins = "mpp18"; + marvell,function = "gpio"; + }; +}; -- 1.9.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support 2014-12-27 11:00 ` [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support Gregory CLEMENT @ 2014-12-27 11:34 ` Andrew Lunn 2015-01-05 14:03 ` Gregory CLEMENT 2014-12-27 11:50 ` Andrew Lunn 2014-12-31 10:32 ` Thomas Petazzoni 2 siblings, 1 reply; 15+ 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] 15+ messages in thread
* [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 2015-01-05 14:24 ` Andrew Lunn 0 siblings, 1 reply; 15+ 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] 15+ messages in thread
* [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 0 siblings, 0 replies; 15+ 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] 15+ messages in thread
* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support 2014-12-27 11:00 ` [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support Gregory CLEMENT 2014-12-27 11:34 ` Andrew Lunn @ 2014-12-27 11:50 ` Andrew Lunn 2015-01-05 14:34 ` Gregory CLEMENT 2014-12-31 10:32 ` Thomas Petazzoni 2 siblings, 1 reply; 15+ 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 = <®_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"; > + }; > + 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 = <®_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>; > + }; > + > + 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 > + > +&pinctrl { > + pca0_pins: pca0_pins { > + marvell,pins = "mpp18"; > + marvell,function = "gpio"; > + }; > +}; > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [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 0 siblings, 0 replies; 15+ 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 = <®_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>; >> + 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. > 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] 15+ messages in thread
* [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support 2014-12-27 11:00 ` [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support Gregory CLEMENT 2014-12-27 11:34 ` Andrew Lunn 2014-12-27 11:50 ` Andrew Lunn @ 2014-12-31 10:32 ` Thomas Petazzoni 2015-01-05 17:06 ` Gregory CLEMENT 2 siblings, 1 reply; 15+ 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 = <®_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. > + }; > + > + 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>; > + }; > + }; 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] 15+ messages in thread
* [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 0 siblings, 0 replies; 15+ 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 = <®_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). 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 = <®_5v_sata2>; >> + }; >> + >> + sata3: sata-port at 1 { >> + reg = <1>; >> + target-supply = <®_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] 15+ messages in thread
* [PATCH 0/3] Add Armada 385 General Purpose Development Board support 2014-12-27 11:00 [PATCH 0/3] Add Armada 385 General Purpose Development Board support Gregory CLEMENT ` (2 preceding siblings ...) 2014-12-27 11:00 ` [PATCH 3/3] ARM: mvebu: Add Armada 385 General Purpose Development Board support Gregory CLEMENT @ 2014-12-31 9:57 ` Thomas Petazzoni 2015-01-05 14:36 ` Gregory CLEMENT 3 siblings, 1 reply; 15+ 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] 15+ messages in thread
* [PATCH 0/3] Add Armada 385 General Purpose Development Board support 2014-12-31 9:57 ` [PATCH 0/3] " Thomas Petazzoni @ 2015-01-05 14:36 ` Gregory CLEMENT 0 siblings, 0 replies; 15+ 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] 15+ messages in thread
end of thread, other threads:[~2015-01-05 17:06 UTC | newest] Thread overview: 15+ 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 ` [PATCH 1/3] ARM: mvebu: a38x: Fix node names Gregory CLEMENT 2014-12-27 11:00 ` [PATCH 2/3] ARM: mvebu: a38x: Add the pinctrl alias Gregory CLEMENT 2014-12-27 11:29 ` Andrew Lunn 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:34 ` Andrew Lunn 2015-01-05 14:03 ` Gregory CLEMENT 2015-01-05 14:24 ` Andrew Lunn 2014-12-27 11:50 ` Andrew Lunn 2015-01-05 14:34 ` Gregory CLEMENT 2014-12-31 10:32 ` Thomas Petazzoni 2015-01-05 17:06 ` Gregory CLEMENT 2014-12-31 9:57 ` [PATCH 0/3] " Thomas Petazzoni 2015-01-05 14:36 ` Gregory CLEMENT
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).