[PATCH 2/2] ARM: dts: kirkwood: add dts support for Seagate BlackArmor NAS220

[PATCH 2/2] ARM: dts: kirkwood: add dts support for Seagate BlackArmor NAS220

[Evgeni Dobrev]

This patch adds support for Seagate BlackArmor NAS220.

The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has 32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two USB 2.0 ports, two buttons and three LEDs. There is a serial port available on the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).

The only functionality still not implemented is the bi-color led on the front panel (status). Pins mpp22 and mpp23 control this led. Setting mpp22 to high and mpp23 to low results in orange color. Setting mpp22 to low and mpp23 to high results in blue color.

The third led is wired to show the SATA activity on the two drives.

Signed-off-by: Evgeni Dobrev <evgeni@studio-punkt.com>
---
 arch/arm/boot/dts/Makefile            |    1 +
 arch/arm/boot/dts/kirkwood-nas220.dts |  182 +++++++++++++++++++++++++++++++++
 2 files changed, 183 insertions(+)
 create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 38c89ca..8b9ad1d 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -132,6 +132,7 @@ dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \
 	kirkwood-lsxhl.dtb \
 	kirkwood-mplcec4.dtb \
 	kirkwood-mv88f6281gtw-ge.dtb \
+	kirkwood-nas220.dtb \
 	kirkwood-net2big.dtb \
 	kirkwood-net5big.dtb \
 	kirkwood-netgear_readynas_duo_v2.dtb \
diff --git a/arch/arm/boot/dts/kirkwood-nas220.dts b/arch/arm/boot/dts/kirkwood-nas220.dts
new file mode 100644
index 0000000..1de2ac3
--- /dev/null
+++ b/arch/arm/boot/dts/kirkwood-nas220.dts
@@ -0,0 +1,182 @@
+/*
+ * Device Tree file for Seagate BlackArmor NAS220
+ *
+ * Copyright (C) 2014 Evgeni Dobrev <evgeni@studio-punkt.com>
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "kirkwood.dtsi"
+#include "kirkwood-6192.dtsi"
+
+/ {
+	model = "Seagate NAS 220";
+	compatible = "seagate,nas220", "marvell,kirkwood-88f6192", "marvell,kirkwood";
+
+	memory { /* 128 MB */
+		device_type = "memory";
+		reg = <0x00000000 0x8000000>;
+	};
+
+        chosen {
+                bootargs = "console=ttyS0,115200n8";
+		stdout-path = &uart0;
+        };
+
+	ocp@f1000000 {
+		pinctrl: pin-controller@10000 {
+			pinctrl-0 = < &pmx_uart0
+				&pmx_button_reset
+				&pmx_button_power
+				>;
+			pinctrl-names = "default";
+
+                        pmx_act_sata0: pmx-act-sata0 {
+                                marvell,pins = "mpp15";
+                                marvell,function = "sata0";
+                        };
+                        pmx_act_sata1: pmx-act-sata1 {
+                                marvell,pins = "mpp16";
+                                marvell,function = "sata1";
+                        };
+                        pmx_power_sata0: pmx-power-sata0 {
+                                marvell,pins = "mpp24";
+                                marvell,function = "gpio";
+                        };
+                        pmx_power_sata1: pmx-power-sata1 {
+                                marvell,pins = "mpp28";
+                                marvell,function = "gpio";
+                        };
+                        pmx_button_reset: pmx-button-reset {
+                                marvell,pins = "mpp29";
+                                marvell,function = "gpio";
+                        };
+                        pmx_button_power: pmx-button-power {
+                                marvell,pins = "mpp26";
+                                marvell,function = "gpio";
+                        };
+		};
+
+		serial@12000 {
+			status = "okay";
+		};
+
+                sata@80000 {
+                        nr-ports = <2>;
+                        status = "okay";
+                };
+
+		i2c@11000 {
+			status = "okay";
+			adt7476: adt7476a@2e {
+				compatible = "adi,adt7476";
+				reg = <0x2e>;
+			};
+		};
+	};
+
+	gpio_poweroff {
+		compatible = "gpio-poweroff";
+		gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
+	};
+
+	gpio_keys {
+		compatible = "gpio-keys";
+
+		button@1{
+			label = "Reset push button";
+			linux,code = <KEY_POWER>;
+			gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>;
+		};
+		button@2{
+			label = "Power push button";
+			linux,code = <KEY_SLEEP>;
+			gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		blue-power {
+			label = "nas220:blue:power";
+			gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "default-on";
+		};
+	};
+
+        regulators {
+                compatible = "simple-bus";
+                #address-cells = <1>;
+                #size-cells = <0>;
+                pinctrl-0 = <&pmx_power_sata0 &pmx_power_sata1>;
+                pinctrl-names = "default";
+
+                sata0_power: regulator@1 {
+                        compatible = "regulator-fixed";
+                        reg = <1>;
+                        regulator-name = "SATA0 Power";
+                        regulator-min-microvolt = <5000000>;
+                        regulator-max-microvolt = <5000000>;
+                        enable-active-high;
+                        regulator-always-on;
+                        regulator-boot-on;
+                        gpio = <&gpio0 24 0>;
+                };
+
+                sata1_power: regulator@2 {
+                        compatible = "regulator-fixed";
+                        reg = <2>;
+                        regulator-name = "SATA1 Power";
+                        regulator-min-microvolt = <5000000>;
+                        regulator-max-microvolt = <5000000>;
+                        enable-active-high;
+                        regulator-always-on;
+                        regulator-boot-on;
+                        gpio = <&gpio0 28 0>;
+                };
+        };
+};
+
+&nand {
+	status = "okay";
+	partition@0 {
+		label = "uboot";
+		reg = <0x0 0xa0000>;
+		read-only;
+	};
+
+	partition@a0000 {
+		label = "env";
+		reg = <0xa0000 0x010000>;
+	};
+
+	partition@c0000 {
+		label = "uimage";
+		reg = <0xc0000 0x500000>;
+	};
+
+	partition@5c0000 {
+		label = "rootfs";
+		reg = <0x5c0000 0x1a40000>;
+	};
+};
+
+&mdio {
+        status = "okay";
+        ethphy0: ethernet-phy@8 {
+                reg = <8>;
+        };
+};
+
+&eth0 {
+        status = "okay";
+        ethernet0-port@0 {
+                phy-handle = <&ethphy0>;
+        };
+};
+
-- 
1.7.10.4

Re: [PATCH 2/2] ARM: dts: kirkwood: add dts support for Seagate BlackArmor NAS220

[Andrew Lunn]

On Mon, Dec 15, 2014 at 09:38:55PM +0100, Evgeni Dobrev wrote:
> This patch adds support for Seagate BlackArmor NAS220.

Hi Evgeni

Thanks for the patch. It looks good apart from a few minor comments.

Please could you also Cc: the mvebu maintainers, otherwise you patch
might net be seen by the right people. I added them.

> 
> The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has 32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two USB 2.0 ports, two buttons and three LEDs. There is a serial port available on the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).
> 

> The only functionality still not implemented is the bi-color led on
> the front panel (status). Pins mpp22 and mpp23 control this
> led. Setting mpp22 to high and mpp23 to low results in orange
> color. Setting mpp22 to low and mpp23 to high results in blue color.

Any thoughts how you are going to handle this? Can you have both blue
and orange at the same time be setting both high? Or are both then
off?

> The third led is wired to show the SATA activity on the two drives.
> 
> Signed-off-by: Evgeni Dobrev <evgeni-c3HRhe8yoGxdzLpqerNdyw@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile            |    1 +
>  arch/arm/boot/dts/kirkwood-nas220.dts |  182 +++++++++++++++++++++++++++++++++
>  2 files changed, 183 insertions(+)
>  create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 38c89ca..8b9ad1d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -132,6 +132,7 @@ dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \
>  	kirkwood-lsxhl.dtb \
>  	kirkwood-mplcec4.dtb \
>  	kirkwood-mv88f6281gtw-ge.dtb \
> +	kirkwood-nas220.dtb \
>  	kirkwood-net2big.dtb \
>  	kirkwood-net5big.dtb \
>  	kirkwood-netgear_readynas_duo_v2.dtb \
> diff --git a/arch/arm/boot/dts/kirkwood-nas220.dts b/arch/arm/boot/dts/kirkwood-nas220.dts
> new file mode 100644
> index 0000000..1de2ac3
> --- /dev/null
> +++ b/arch/arm/boot/dts/kirkwood-nas220.dts
> @@ -0,0 +1,182 @@
> +/*
> + * Device Tree file for Seagate BlackArmor NAS220
> + *
> + * Copyright (C) 2014 Evgeni Dobrev <evgeni-c3HRhe8yoGxdzLpqerNdyw@public.gmane.org>
> + *
> + * Licensed under GPLv2 or later.
> + */

Gregory is in the process of changing the license for the Marvell
Armada SoCs. He is adding X11 license as well, so making it easier for
other systems to reuse the DT blob. I want to see how easy it goes
with Armada, but we may also do the same for Kirkwood sometime in the
future. So maybe you can think about this and follow the discussion.

> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include "kirkwood.dtsi"
> +#include "kirkwood-6192.dtsi"
> +
> +/ {
> +	model = "Seagate NAS 220";
> +	compatible = "seagate,nas220", "marvell,kirkwood-88f6192", "marvell,kirkwood";
> +
> +	memory { /* 128 MB */
> +		device_type = "memory";
> +		reg = <0x00000000 0x8000000>;
> +	};
> +
> +        chosen {
> +                bootargs = "console=ttyS0,115200n8";
> +		stdout-path = &uart0;
> +        };

It looks like there is a mix of spaces and tabs. Please only use tabs
in this file.

> +
> +	ocp@f1000000 {
> +		pinctrl: pin-controller@10000 {
> +			pinctrl-0 = < &pmx_uart0
> +				&pmx_button_reset
> +				&pmx_button_power
> +				>;
> +			pinctrl-names = "default";
> +
> +                        pmx_act_sata0: pmx-act-sata0 {
> +                                marvell,pins = "mpp15";
> +                                marvell,function = "sata0";
> +                        };
> +                        pmx_act_sata1: pmx-act-sata1 {
> +                                marvell,pins = "mpp16";
> +                                marvell,function = "sata1";
> +                        };
> +                        pmx_power_sata0: pmx-power-sata0 {
> +                                marvell,pins = "mpp24";
> +                                marvell,function = "gpio";
> +                        };
> +                        pmx_power_sata1: pmx-power-sata1 {
> +                                marvell,pins = "mpp28";
> +                                marvell,function = "gpio";
> +                        };
> +                        pmx_button_reset: pmx-button-reset {
> +                                marvell,pins = "mpp29";
> +                                marvell,function = "gpio";
> +                        };
> +                        pmx_button_power: pmx-button-power {
> +                                marvell,pins = "mpp26";
> +                                marvell,function = "gpio";
> +                        };
> +		};
> +
> +		serial@12000 {
> +			status = "okay";
> +		};
> +
> +                sata@80000 {
> +                        nr-ports = <2>;
> +                        status = "okay";
> +                };
> +
> +		i2c@11000 {
> +			status = "okay";
> +			adt7476: adt7476a@2e {
> +				compatible = "adi,adt7476";
> +				reg = <0x2e>;
> +			};
> +		};
> +	};
> +
> +	gpio_poweroff {
> +		compatible = "gpio-poweroff";
> +		gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	gpio_keys {
> +		compatible = "gpio-keys";
> +
> +		button@1{
> +			label = "Reset push button";
> +			linux,code = <KEY_POWER>;
> +			gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>;
> +		};
> +		button@2{
> +			label = "Power push button";
> +			linux,code = <KEY_SLEEP>;
> +			gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +	gpio-leds {
> +		compatible = "gpio-leds";
> +
> +		blue-power {
> +			label = "nas220:blue:power";
> +			gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "default-on";
> +		};
> +	};
> +
> +        regulators {
> +                compatible = "simple-bus";
> +                #address-cells = <1>;
> +                #size-cells = <0>;
> +                pinctrl-0 = <&pmx_power_sata0 &pmx_power_sata1>;
> +                pinctrl-names = "default";
> +
> +                sata0_power: regulator@1 {
> +                        compatible = "regulator-fixed";
> +                        reg = <1>;
> +                        regulator-name = "SATA0 Power";
> +                        regulator-min-microvolt = <5000000>;
> +                        regulator-max-microvolt = <5000000>;
> +                        enable-active-high;
> +                        regulator-always-on;
> +                        regulator-boot-on;
> +                        gpio = <&gpio0 24 0>;
> +                };
> +
> +                sata1_power: regulator@2 {
> +                        compatible = "regulator-fixed";
> +                        reg = <2>;
> +                        regulator-name = "SATA1 Power";
> +                        regulator-min-microvolt = <5000000>;
> +                        regulator-max-microvolt = <5000000>;
> +                        enable-active-high;
> +                        regulator-always-on;
> +                        regulator-boot-on;
> +                        gpio = <&gpio0 28 0>;
> +                };
> +        };
> +};
> +
> +&nand {
> +	status = "okay";
> +	partition@0 {
> +		label = "uboot";
> +		reg = <0x0 0xa0000>;
> +		read-only;
> +	};
> +
> +	partition@a0000 {
> +		label = "env";
> +		reg = <0xa0000 0x010000>;
> +	};

Is there supposed to be a gap here?

> +
> +	partition@c0000 {
> +		label = "uimage";
> +		reg = <0xc0000 0x500000>;
> +	};
> +
> +	partition@5c0000 {
> +		label = "rootfs";
> +		reg = <0x5c0000 0x1a40000>;
> +	};
> +};
> +
> +&mdio {
> +        status = "okay";
> +        ethphy0: ethernet-phy@8 {
> +                reg = <8>;
> +        };
> +};
> +
> +&eth0 {
> +        status = "okay";
> +        ethernet0-port@0 {
> +                phy-handle = <&ethphy0>;
> +        };
> +};
> +
> -- 

  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 2/2] ARM: dts: kirkwood: add dts support for Seagate BlackArmor NAS220

[Evgeni Dobrev]

Hi Andrew,

On Tue, Dec 16, 2014 at 09:56:45AM +0100, Andrew Lunn wrote:
> On Mon, Dec 15, 2014 at 09:38:55PM +0100, Evgeni Dobrev wrote:
> > This patch adds support for Seagate BlackArmor NAS220.
> 
> Hi Evgeni
> 
> Thanks for the patch. It looks good apart from a few minor comments.
> 
> Please could you also Cc: the mvebu maintainers, otherwise you patch
> might net be seen by the right people. I added them.
> 

Thank you!

> > 
> > The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has 32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two USB 2.0 ports, two buttons and three LEDs. There is a serial port available on the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).
> > 
> 
> > The only functionality still not implemented is the bi-color led on
> > the front panel (status). Pins mpp22 and mpp23 control this
> > led. Setting mpp22 to high and mpp23 to low results in orange
> > color. Setting mpp22 to low and mpp23 to high results in blue color.
> 
> Any thoughts how you are going to handle this? Can you have both blue
> and orange at the same time be setting both high? Or are both then
> off?
> 

The LED can be either one of the two colors or off. Swapping the polarity 
on the two LED pins changes the color.

I am currently playing with the idea to create a multicolor LED kernel 
driver which is being controlled via GPIOs, but I still have not found 
the proper abstraction for it. The point is, that even though this 
functionality must currently be handled by user space (some
combination of sysfs writes and a shell script) at the moment, the rest
of the system is usable.

> > The third led is wired to show the SATA activity on the two drives.
> > 
> > Signed-off-by: Evgeni Dobrev <evgeni@studio-punkt.com>
> > ---
> >  arch/arm/boot/dts/Makefile            |    1 +
> >  arch/arm/boot/dts/kirkwood-nas220.dts |  182 +++++++++++++++++++++++++++++++++
> >  2 files changed, 183 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts
> > 
> > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> > index 38c89ca..8b9ad1d 100644
> > --- a/arch/arm/boot/dts/Makefile
> > +++ b/arch/arm/boot/dts/Makefile
> > @@ -132,6 +132,7 @@ dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \
> >  	kirkwood-lsxhl.dtb \
> >  	kirkwood-mplcec4.dtb \
> >  	kirkwood-mv88f6281gtw-ge.dtb \
> > +	kirkwood-nas220.dtb \
> >  	kirkwood-net2big.dtb \
> >  	kirkwood-net5big.dtb \
> >  	kirkwood-netgear_readynas_duo_v2.dtb \
> > diff --git a/arch/arm/boot/dts/kirkwood-nas220.dts b/arch/arm/boot/dts/kirkwood-nas220.dts
> > new file mode 100644
> > index 0000000..1de2ac3
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/kirkwood-nas220.dts
> > @@ -0,0 +1,182 @@
> > +/*
> > + * Device Tree file for Seagate BlackArmor NAS220
> > + *
> > + * Copyright (C) 2014 Evgeni Dobrev <evgeni@studio-punkt.com>
> > + *
> > + * Licensed under GPLv2 or later.
> > + */
> 
> Gregory is in the process of changing the license for the Marvell
> Armada SoCs. He is adding X11 license as well, so making it easier for
> other systems to reuse the DT blob. I want to see how easy it goes
> with Armada, but we may also do the same for Kirkwood sometime in the
> future. So maybe you can think about this and follow the discussion.
> 

I will do that.

> > +
> > +/dts-v1/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +#include "kirkwood.dtsi"
> > +#include "kirkwood-6192.dtsi"
> > +
> > +/ {
> > +	model = "Seagate NAS 220";
> > +	compatible = "seagate,nas220", "marvell,kirkwood-88f6192", "marvell,kirkwood";
> > +
> > +	memory { /* 128 MB */
> > +		device_type = "memory";
> > +		reg = <0x00000000 0x8000000>;
> > +	};
> > +
> > +        chosen {
> > +                bootargs = "console=ttyS0,115200n8";
> > +		stdout-path = &uart0;
> > +        };
> 
> It looks like there is a mix of spaces and tabs. Please only use tabs
> in this file.
> 

I will fix this.

> > +
> > +	ocp@f1000000 {
> > +		pinctrl: pin-controller@10000 {
> > +			pinctrl-0 = < &pmx_uart0
> > +				&pmx_button_reset
> > +				&pmx_button_power
> > +				>;
> > +			pinctrl-names = "default";
> > +
> > +                        pmx_act_sata0: pmx-act-sata0 {
> > +                                marvell,pins = "mpp15";
> > +                                marvell,function = "sata0";
> > +                        };
> > +                        pmx_act_sata1: pmx-act-sata1 {
> > +                                marvell,pins = "mpp16";
> > +                                marvell,function = "sata1";
> > +                        };
> > +                        pmx_power_sata0: pmx-power-sata0 {
> > +                                marvell,pins = "mpp24";
> > +                                marvell,function = "gpio";
> > +                        };
> > +                        pmx_power_sata1: pmx-power-sata1 {
> > +                                marvell,pins = "mpp28";
> > +                                marvell,function = "gpio";
> > +                        };
> > +                        pmx_button_reset: pmx-button-reset {
> > +                                marvell,pins = "mpp29";
> > +                                marvell,function = "gpio";
> > +                        };
> > +                        pmx_button_power: pmx-button-power {
> > +                                marvell,pins = "mpp26";
> > +                                marvell,function = "gpio";
> > +                        };
> > +		};
> > +
> > +		serial@12000 {
> > +			status = "okay";
> > +		};
> > +
> > +                sata@80000 {
> > +                        nr-ports = <2>;
> > +                        status = "okay";
> > +                };
> > +
> > +		i2c@11000 {
> > +			status = "okay";
> > +			adt7476: adt7476a@2e {
> > +				compatible = "adi,adt7476";
> > +				reg = <0x2e>;
> > +			};
> > +		};
> > +	};
> > +
> > +	gpio_poweroff {
> > +		compatible = "gpio-poweroff";
> > +		gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
> > +	};
> > +
> > +	gpio_keys {
> > +		compatible = "gpio-keys";
> > +
> > +		button@1{
> > +			label = "Reset push button";
> > +			linux,code = <KEY_POWER>;
> > +			gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>;
> > +		};
> > +		button@2{
> > +			label = "Power push button";
> > +			linux,code = <KEY_SLEEP>;
> > +			gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
> > +		};
> > +	};
> > +
> > +	gpio-leds {
> > +		compatible = "gpio-leds";
> > +
> > +		blue-power {
> > +			label = "nas220:blue:power";
> > +			gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
> > +			linux,default-trigger = "default-on";
> > +		};
> > +	};
> > +
> > +        regulators {
> > +                compatible = "simple-bus";
> > +                #address-cells = <1>;
> > +                #size-cells = <0>;
> > +                pinctrl-0 = <&pmx_power_sata0 &pmx_power_sata1>;
> > +                pinctrl-names = "default";
> > +
> > +                sata0_power: regulator@1 {
> > +                        compatible = "regulator-fixed";
> > +                        reg = <1>;
> > +                        regulator-name = "SATA0 Power";
> > +                        regulator-min-microvolt = <5000000>;
> > +                        regulator-max-microvolt = <5000000>;
> > +                        enable-active-high;
> > +                        regulator-always-on;
> > +                        regulator-boot-on;
> > +                        gpio = <&gpio0 24 0>;
> > +                };
> > +
> > +                sata1_power: regulator@2 {
> > +                        compatible = "regulator-fixed";
> > +                        reg = <2>;
> > +                        regulator-name = "SATA1 Power";
> > +                        regulator-min-microvolt = <5000000>;
> > +                        regulator-max-microvolt = <5000000>;
> > +                        enable-active-high;
> > +                        regulator-always-on;
> > +                        regulator-boot-on;
> > +                        gpio = <&gpio0 28 0>;
> > +                };
> > +        };
> > +};
> > +
> > +&nand {
> > +	status = "okay";
> > +	partition@0 {
> > +		label = "uboot";
> > +		reg = <0x0 0xa0000>;
> > +		read-only;
> > +	};
> > +
> > +	partition@a0000 {
> > +		label = "env";
> > +		reg = <0xa0000 0x010000>;
> > +	};
> 
> Is there supposed to be a gap here?
> 

I mirror the layout of the stock firmware here. Actually I was not sure
how to handle the NAND layout. On one hand it has some merit if I keep
the original layout (and a lot of the *plug device trees do it), 
so that the system can be booted with it. On the other hand one can
pass the NAND layout relevant to his installation via the bootloader 
(either updating the DT blob, or via cmdline). Now that you ask I would 
prefer to remove the layout and leave it for the user to decide.

What would you recommend?

> > +
> > +	partition@c0000 {
> > +		label = "uimage";
> > +		reg = <0xc0000 0x500000>;
> > +	};
> > +
> > +	partition@5c0000 {
> > +		label = "rootfs";
> > +		reg = <0x5c0000 0x1a40000>;
> > +	};
> > +};
> > +
> > +&mdio {
> > +        status = "okay";
> > +        ethphy0: ethernet-phy@8 {
> > +                reg = <8>;
> > +        };
> > +};
> > +
> > +&eth0 {
> > +        status = "okay";
> > +        ethernet0-port@0 {
> > +                phy-handle = <&ethphy0>;
> > +        };
> > +};
> > +
> > -- 
> 
>   Andrew
> --

Thanks for reviewing!

evgeni

Re: [PATCH 2/2] ARM: dts: kirkwood: add dts support for Seagate BlackArmor NAS220

[Andrew Lunn]

> The LED can be either one of the two colors or off. Swapping the
> polarity on the two LED pins changes the color.

O.K, so it is using the diode property of a Light Emitting Diode, to
support two colours.

> I am currently playing with the idea to create a multicolor LED kernel 
> driver which is being controlled via GPIOs, but I still have not found 
> the proper abstraction for it.

Maybe multi-colour is too far. Just support bi-colour via two gpios.
Still, it is not so simple. As you said, the led class does not give
too good an abstraction. I guess the best you can do is that when you
turn one on, it automagically turns the other off.

> The point is, that even though this functionality must currently be
> handled by user space (some combination of sysfs writes and a shell
> script) at the moment, the rest of the system is usable.

Sure, we will take the DT file without support for the bicolour
LED. It can be added later.

      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

[PATCH v2 0/1] add support for Seagate BlackArmor NAS220

[Evgeni Dobrev]

v2:	indent with tabs only
	removed NAND layout

Evgeni Dobrev (1):
  add support for Seagate BlackArmor NAS220

 arch/arm/boot/dts/Makefile            |    1 +
 arch/arm/boot/dts/kirkwood-nas220.dts |  158 +++++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+)
 create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts

-- 
1.7.10.4

--
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 v2 1/1] add support for Seagate BlackArmor NAS220

[Evgeni Dobrev]

This patch adds support for Seagate BlackArmor NAS220.

The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has
32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two
USB 2.0 ports, two buttons and three LEDs. There is a serial port available on
the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).

The only functionality still not implemented is the bi-color led on the front
panel (status). Pins mpp22 and mpp23 control this led. Setting mpp22 to high and
mpp23 to low results in orange color. Setting mpp22 to low and mpp23 to high
results in blue color.

The third led is wired to show the SATA activity on the two drives.

Signed-off-by: Evgeni Dobrev <evgeni@studio-punkt.com>
---
 arch/arm/boot/dts/Makefile            |    1 +
 arch/arm/boot/dts/kirkwood-nas220.dts |  158 +++++++++++++++++++++++++++++++++
 2 files changed, 159 insertions(+)
 create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 38c89ca..8b9ad1d 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -132,6 +132,7 @@ dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \
 	kirkwood-lsxhl.dtb \
 	kirkwood-mplcec4.dtb \
 	kirkwood-mv88f6281gtw-ge.dtb \
+	kirkwood-nas220.dtb \
 	kirkwood-net2big.dtb \
 	kirkwood-net5big.dtb \
 	kirkwood-netgear_readynas_duo_v2.dtb \
diff --git a/arch/arm/boot/dts/kirkwood-nas220.dts b/arch/arm/boot/dts/kirkwood-nas220.dts
new file mode 100644
index 0000000..43c9bbc
--- /dev/null
+++ b/arch/arm/boot/dts/kirkwood-nas220.dts
@@ -0,0 +1,158 @@
+/*
+ * Device Tree file for Seagate BlackArmor NAS220
+ *
+ * Copyright (C) 2014 Evgeni Dobrev <evgeni@studio-punkt.com>
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "kirkwood.dtsi"
+#include "kirkwood-6192.dtsi"
+
+/ {
+	model = "Seagate NAS 220";
+	compatible = "seagate,nas220","marvell,kirkwood-88f6192","marvell,kirkwood";
+
+	memory { /* 128 MB */
+		device_type = "memory";
+		reg = <0x00000000 0x8000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttyS0,115200n8";
+		stdout-path = &uart0;
+	};
+
+	ocp@f1000000 {
+		pinctrl: pin-controller@10000 {
+			pinctrl-0 = <&pmx_uart0
+				     &pmx_button_reset
+				     &pmx_button_power>;
+			pinctrl-names = "default";
+
+			pmx_act_sata0: pmx-act-sata0 {
+				marvell,pins = "mpp15";
+				marvell,function = "sata0";
+			};
+			pmx_act_sata1: pmx-act-sata1 {
+				marvell,pins = "mpp16";
+				marvell,function = "sata1";
+			};
+			pmx_power_sata0: pmx-power-sata0 {
+				marvell,pins = "mpp24";
+				marvell,function = "gpio";
+			};
+			pmx_power_sata1: pmx-power-sata1 {
+				marvell,pins = "mpp28";
+				marvell,function = "gpio";
+			};
+			pmx_button_reset: pmx-button-reset {
+				marvell,pins = "mpp29";
+				marvell,function = "gpio";
+			};
+			pmx_button_power: pmx-button-power {
+				marvell,pins = "mpp26";
+				marvell,function = "gpio";
+			};
+		};
+
+		serial@12000 {
+			status = "okay";
+		};
+
+		sata@80000 {
+			status = "okay";
+			nr-ports = <2>;
+		};
+
+		i2c@11000 {
+			status = "okay";
+			adt7476: adt7476a@2e {
+				compatible = "adi,adt7476";
+				reg = <0x2e>;
+			};
+		};
+	};
+
+	gpio_poweroff {
+		compatible = "gpio-poweroff";
+		gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
+	};
+
+	gpio_keys {
+		compatible = "gpio-keys";
+		button@1{
+			label = "Reset push button";
+			linux,code = <KEY_POWER>;
+			gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>;
+		};
+		button@2{
+			label = "Power push button";
+			linux,code = <KEY_SLEEP>;
+			gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+		blue-power {
+			label = "nas220:blue:power";
+			gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "default-on";
+		};
+	};
+
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-0 = <&pmx_power_sata0 &pmx_power_sata1>;
+		pinctrl-names = "default";
+
+		sata0_power: regulator@1 {
+			compatible = "regulator-fixed";
+			reg = <1>;
+			regulator-name = "SATA0 Power";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			enable-active-high;
+			regulator-always-on;
+			regulator-boot-on;
+			gpio = <&gpio0 24 0>;
+		};
+
+		sata1_power: regulator@2 {
+			compatible = "regulator-fixed";
+			reg = <2>;
+			regulator-name = "SATA1 Power";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			enable-active-high;
+			regulator-always-on;
+			regulator-boot-on;
+			gpio = <&gpio0 28 0>;
+		};
+	};
+};
+
+&nand {
+	status = "okay";
+};
+
+&mdio {
+	status = "okay";
+	ethphy0: ethernet-phy@8 {
+		 reg = <8>;
+	 };
+};
+
+&eth0 {
+	status = "okay";
+	ethernet0-port@0 {
+		phy-handle = <&ethphy0>;
+	};
+};
-- 
1.7.10.4

Re: [PATCH v2 1/1] add support for Seagate BlackArmor NAS220

[Andrew Lunn]

On Mon, Dec 22, 2014 at 09:04:45AM +0100, Evgeni Dobrev wrote:
> This patch adds support for Seagate BlackArmor NAS220.
> 
> The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has
> 32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two
> USB 2.0 ports, two buttons and three LEDs. There is a serial port available on
> the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).

Hi Evgeni

This looks good.  I have one minor comment. It would be nice to put
this information about the serial port also in the .dts file, by the
serial node.

       Andrew
> 
> The only functionality still not implemented is the bi-color led on the front
> panel (status). Pins mpp22 and mpp23 control this led. Setting mpp22 to high and
> mpp23 to low results in orange color. Setting mpp22 to low and mpp23 to high
> results in blue color.
> 
> The third led is wired to show the SATA activity on the two drives.
> 
> Signed-off-by: Evgeni Dobrev <evgeni-c3HRhe8yoGxdzLpqerNdyw@public.gmane.org>
> ---
>  arch/arm/boot/dts/Makefile            |    1 +
>  arch/arm/boot/dts/kirkwood-nas220.dts |  158 +++++++++++++++++++++++++++++++++
>  2 files changed, 159 insertions(+)
>  create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 38c89ca..8b9ad1d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -132,6 +132,7 @@ dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \
>  	kirkwood-lsxhl.dtb \
>  	kirkwood-mplcec4.dtb \
>  	kirkwood-mv88f6281gtw-ge.dtb \
> +	kirkwood-nas220.dtb \
>  	kirkwood-net2big.dtb \
>  	kirkwood-net5big.dtb \
>  	kirkwood-netgear_readynas_duo_v2.dtb \
> diff --git a/arch/arm/boot/dts/kirkwood-nas220.dts b/arch/arm/boot/dts/kirkwood-nas220.dts
> new file mode 100644
> index 0000000..43c9bbc
> --- /dev/null
> +++ b/arch/arm/boot/dts/kirkwood-nas220.dts
> @@ -0,0 +1,158 @@
> +/*
> + * Device Tree file for Seagate BlackArmor NAS220
> + *
> + * Copyright (C) 2014 Evgeni Dobrev <evgeni-c3HRhe8yoGxdzLpqerNdyw@public.gmane.org>
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include "kirkwood.dtsi"
> +#include "kirkwood-6192.dtsi"
> +
> +/ {
> +	model = "Seagate NAS 220";
> +	compatible = "seagate,nas220","marvell,kirkwood-88f6192","marvell,kirkwood";
> +
> +	memory { /* 128 MB */
> +		device_type = "memory";
> +		reg = <0x00000000 0x8000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "console=ttyS0,115200n8";
> +		stdout-path = &uart0;
> +	};
> +
> +	ocp@f1000000 {
> +		pinctrl: pin-controller@10000 {
> +			pinctrl-0 = <&pmx_uart0
> +				     &pmx_button_reset
> +				     &pmx_button_power>;
> +			pinctrl-names = "default";
> +
> +			pmx_act_sata0: pmx-act-sata0 {
> +				marvell,pins = "mpp15";
> +				marvell,function = "sata0";
> +			};
> +			pmx_act_sata1: pmx-act-sata1 {
> +				marvell,pins = "mpp16";
> +				marvell,function = "sata1";
> +			};
> +			pmx_power_sata0: pmx-power-sata0 {
> +				marvell,pins = "mpp24";
> +				marvell,function = "gpio";
> +			};
> +			pmx_power_sata1: pmx-power-sata1 {
> +				marvell,pins = "mpp28";
> +				marvell,function = "gpio";
> +			};
> +			pmx_button_reset: pmx-button-reset {
> +				marvell,pins = "mpp29";
> +				marvell,function = "gpio";
> +			};
> +			pmx_button_power: pmx-button-power {
> +				marvell,pins = "mpp26";
> +				marvell,function = "gpio";
> +			};
> +		};
> +
> +		serial@12000 {
> +			status = "okay";
> +		};
> +
> +		sata@80000 {
> +			status = "okay";
> +			nr-ports = <2>;
> +		};
> +
> +		i2c@11000 {
> +			status = "okay";
> +			adt7476: adt7476a@2e {
> +				compatible = "adi,adt7476";
> +				reg = <0x2e>;
> +			};
> +		};
> +	};
> +
> +	gpio_poweroff {
> +		compatible = "gpio-poweroff";
> +		gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	gpio_keys {
> +		compatible = "gpio-keys";
> +		button@1{
> +			label = "Reset push button";
> +			linux,code = <KEY_POWER>;
> +			gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>;
> +		};
> +		button@2{
> +			label = "Power push button";
> +			linux,code = <KEY_SLEEP>;
> +			gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +	gpio-leds {
> +		compatible = "gpio-leds";
> +		blue-power {
> +			label = "nas220:blue:power";
> +			gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "default-on";
> +		};
> +	};
> +
> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pinctrl-0 = <&pmx_power_sata0 &pmx_power_sata1>;
> +		pinctrl-names = "default";
> +
> +		sata0_power: regulator@1 {
> +			compatible = "regulator-fixed";
> +			reg = <1>;
> +			regulator-name = "SATA0 Power";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			enable-active-high;
> +			regulator-always-on;
> +			regulator-boot-on;
> +			gpio = <&gpio0 24 0>;
> +		};
> +
> +		sata1_power: regulator@2 {
> +			compatible = "regulator-fixed";
> +			reg = <2>;
> +			regulator-name = "SATA1 Power";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			enable-active-high;
> +			regulator-always-on;
> +			regulator-boot-on;
> +			gpio = <&gpio0 28 0>;
> +		};
> +	};
> +};
> +
> +&nand {
> +	status = "okay";
> +};
> +
> +&mdio {
> +	status = "okay";
> +	ethphy0: ethernet-phy@8 {
> +		 reg = <8>;
> +	 };
> +};
> +
> +&eth0 {
> +	status = "okay";
> +	ethernet0-port@0 {
> +		phy-handle = <&ethphy0>;
> +	};
> +};
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 0/1] add support for Seagate BlackArmor NAS220

[Evgeni Dobrev]

v3:	documented UART pinout in dts

v2:	indent with tabs only
	removed NAND layout

Evgeni Dobrev (1):
  add support for Seagate BlackArmor NAS220

 arch/arm/boot/dts/Makefile            |    1 +
 arch/arm/boot/dts/kirkwood-nas220.dts |  166 +++++++++++++++++++++++++++++++++
 2 files changed, 167 insertions(+)
 create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts

-- 
1.7.10.4

--
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 v3 1/1] add support for Seagate BlackArmor NAS220

[Evgeni Dobrev]

This patch adds support for Seagate BlackArmor NAS220.

The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has
32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two
USB 2.0 ports, two buttons and three LEDs. There is a serial port available on
the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).

The only functionality still not implemented is the bi-color led on the front
panel (status). Pins mpp22 and mpp23 control this led. Setting mpp22 to high and
mpp23 to low results in orange color. Setting mpp22 to low and mpp23 to high
results in blue color.

The third led is wired to show the SATA activity on the two drives.

Signed-off-by: Evgeni Dobrev <evgeni@studio-punkt.com>
---
 arch/arm/boot/dts/Makefile            |    1 +
 arch/arm/boot/dts/kirkwood-nas220.dts |  166 +++++++++++++++++++++++++++++++++
 2 files changed, 167 insertions(+)
 create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 38c89ca..8b9ad1d 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -132,6 +132,7 @@ dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \
 	kirkwood-lsxhl.dtb \
 	kirkwood-mplcec4.dtb \
 	kirkwood-mv88f6281gtw-ge.dtb \
+	kirkwood-nas220.dtb \
 	kirkwood-net2big.dtb \
 	kirkwood-net5big.dtb \
 	kirkwood-netgear_readynas_duo_v2.dtb \
diff --git a/arch/arm/boot/dts/kirkwood-nas220.dts b/arch/arm/boot/dts/kirkwood-nas220.dts
new file mode 100644
index 0000000..8175f3d
--- /dev/null
+++ b/arch/arm/boot/dts/kirkwood-nas220.dts
@@ -0,0 +1,166 @@
+/*
+ * Device Tree file for Seagate BlackArmor NAS220
+ *
+ * Copyright (C) 2014 Evgeni Dobrev <evgeni@studio-punkt.com>
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "kirkwood.dtsi"
+#include "kirkwood-6192.dtsi"
+
+/ {
+	model = "Seagate NAS 220";
+	compatible = "seagate,nas220","marvell,kirkwood-88f6192","marvell,kirkwood";
+
+	memory { /* 128 MB */
+		device_type = "memory";
+		reg = <0x00000000 0x8000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttyS0,115200n8";
+		stdout-path = &uart0;
+	};
+
+	ocp@f1000000 {
+		pinctrl: pin-controller@10000 {
+			pinctrl-0 = <&pmx_uart0
+				     &pmx_button_reset
+				     &pmx_button_power>;
+			pinctrl-names = "default";
+
+			pmx_act_sata0: pmx-act-sata0 {
+				marvell,pins = "mpp15";
+				marvell,function = "sata0";
+			};
+			pmx_act_sata1: pmx-act-sata1 {
+				marvell,pins = "mpp16";
+				marvell,function = "sata1";
+			};
+			pmx_power_sata0: pmx-power-sata0 {
+				marvell,pins = "mpp24";
+				marvell,function = "gpio";
+			};
+			pmx_power_sata1: pmx-power-sata1 {
+				marvell,pins = "mpp28";
+				marvell,function = "gpio";
+			};
+			pmx_button_reset: pmx-button-reset {
+				marvell,pins = "mpp29";
+				marvell,function = "gpio";
+			};
+			pmx_button_power: pmx-button-power {
+				marvell,pins = "mpp26";
+				marvell,function = "gpio";
+			};
+		};
+
+
+		/*
+		 * Serial port routed to connector CN5
+		 *
+		 * pin 1 - TX
+		 * pin 4 - RX
+		 * pin 6 - GND
+		 */
+		serial@12000 {
+			status = "okay";
+		};
+
+		sata@80000 {
+			status = "okay";
+			nr-ports = <2>;
+		};
+
+		i2c@11000 {
+			status = "okay";
+			adt7476: adt7476a@2e {
+				compatible = "adi,adt7476";
+				reg = <0x2e>;
+			};
+		};
+	};
+
+	gpio_poweroff {
+		compatible = "gpio-poweroff";
+		gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
+	};
+
+	gpio_keys {
+		compatible = "gpio-keys";
+		button@1{
+			label = "Reset push button";
+			linux,code = <KEY_POWER>;
+			gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>;
+		};
+		button@2{
+			label = "Power push button";
+			linux,code = <KEY_SLEEP>;
+			gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+		blue-power {
+			label = "nas220:blue:power";
+			gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "default-on";
+		};
+	};
+
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-0 = <&pmx_power_sata0 &pmx_power_sata1>;
+		pinctrl-names = "default";
+
+		sata0_power: regulator@1 {
+			compatible = "regulator-fixed";
+			reg = <1>;
+			regulator-name = "SATA0 Power";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			enable-active-high;
+			regulator-always-on;
+			regulator-boot-on;
+			gpio = <&gpio0 24 0>;
+		};
+
+		sata1_power: regulator@2 {
+			compatible = "regulator-fixed";
+			reg = <2>;
+			regulator-name = "SATA1 Power";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			enable-active-high;
+			regulator-always-on;
+			regulator-boot-on;
+			gpio = <&gpio0 28 0>;
+		};
+	};
+};
+
+&nand {
+	status = "okay";
+};
+
+&mdio {
+	status = "okay";
+	ethphy0: ethernet-phy@8 {
+		 reg = <8>;
+	 };
+};
+
+&eth0 {
+	status = "okay";
+	ethernet0-port@0 {
+		phy-handle = <&ethphy0>;
+	};
+};
-- 
1.7.10.4

Re: [PATCH v3 1/1] add support for Seagate BlackArmor NAS220

[Andrew Lunn]

On Mon, Dec 22, 2014 at 01:57:50PM +0100, Evgeni Dobrev wrote:
> This patch adds support for Seagate BlackArmor NAS220.
> 
> The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has
> 32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two
> USB 2.0 ports, two buttons and three LEDs. There is a serial port available on
> the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).
> 
> The only functionality still not implemented is the bi-color led on the front
> panel (status). Pins mpp22 and mpp23 control this led. Setting mpp22 to high and
> mpp23 to low results in orange color. Setting mpp22 to low and mpp23 to high
> results in blue color.
> 
> The third led is wired to show the SATA activity on the two drives.

H Evgeni

This is looking good now. I will wait a few days to see if anybody
else has any comments, and it not, pull it into mvebu/dt.

Thanks
	Andrew


> 
> Signed-off-by: Evgeni Dobrev <evgeni@studio-punkt.com>
> ---
>  arch/arm/boot/dts/Makefile            |    1 +
>  arch/arm/boot/dts/kirkwood-nas220.dts |  166 +++++++++++++++++++++++++++++++++
>  2 files changed, 167 insertions(+)
>  create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts
> 
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 38c89ca..8b9ad1d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -132,6 +132,7 @@ dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \
>  	kirkwood-lsxhl.dtb \
>  	kirkwood-mplcec4.dtb \
>  	kirkwood-mv88f6281gtw-ge.dtb \
> +	kirkwood-nas220.dtb \
>  	kirkwood-net2big.dtb \
>  	kirkwood-net5big.dtb \
>  	kirkwood-netgear_readynas_duo_v2.dtb \
> diff --git a/arch/arm/boot/dts/kirkwood-nas220.dts b/arch/arm/boot/dts/kirkwood-nas220.dts
> new file mode 100644
> index 0000000..8175f3d
> --- /dev/null
> +++ b/arch/arm/boot/dts/kirkwood-nas220.dts
> @@ -0,0 +1,166 @@
> +/*
> + * Device Tree file for Seagate BlackArmor NAS220
> + *
> + * Copyright (C) 2014 Evgeni Dobrev <evgeni@studio-punkt.com>
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include "kirkwood.dtsi"
> +#include "kirkwood-6192.dtsi"
> +
> +/ {
> +	model = "Seagate NAS 220";
> +	compatible = "seagate,nas220","marvell,kirkwood-88f6192","marvell,kirkwood";
> +
> +	memory { /* 128 MB */
> +		device_type = "memory";
> +		reg = <0x00000000 0x8000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "console=ttyS0,115200n8";
> +		stdout-path = &uart0;
> +	};
> +
> +	ocp@f1000000 {
> +		pinctrl: pin-controller@10000 {
> +			pinctrl-0 = <&pmx_uart0
> +				     &pmx_button_reset
> +				     &pmx_button_power>;
> +			pinctrl-names = "default";
> +
> +			pmx_act_sata0: pmx-act-sata0 {
> +				marvell,pins = "mpp15";
> +				marvell,function = "sata0";
> +			};
> +			pmx_act_sata1: pmx-act-sata1 {
> +				marvell,pins = "mpp16";
> +				marvell,function = "sata1";
> +			};
> +			pmx_power_sata0: pmx-power-sata0 {
> +				marvell,pins = "mpp24";
> +				marvell,function = "gpio";
> +			};
> +			pmx_power_sata1: pmx-power-sata1 {
> +				marvell,pins = "mpp28";
> +				marvell,function = "gpio";
> +			};
> +			pmx_button_reset: pmx-button-reset {
> +				marvell,pins = "mpp29";
> +				marvell,function = "gpio";
> +			};
> +			pmx_button_power: pmx-button-power {
> +				marvell,pins = "mpp26";
> +				marvell,function = "gpio";
> +			};
> +		};
> +
> +
> +		/*
> +		 * Serial port routed to connector CN5
> +		 *
> +		 * pin 1 - TX
> +		 * pin 4 - RX
> +		 * pin 6 - GND
> +		 */
> +		serial@12000 {
> +			status = "okay";
> +		};
> +
> +		sata@80000 {
> +			status = "okay";
> +			nr-ports = <2>;
> +		};
> +
> +		i2c@11000 {
> +			status = "okay";
> +			adt7476: adt7476a@2e {
> +				compatible = "adi,adt7476";
> +				reg = <0x2e>;
> +			};
> +		};
> +	};
> +
> +	gpio_poweroff {
> +		compatible = "gpio-poweroff";
> +		gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	gpio_keys {
> +		compatible = "gpio-keys";
> +		button@1{
> +			label = "Reset push button";
> +			linux,code = <KEY_POWER>;
> +			gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>;
> +		};
> +		button@2{
> +			label = "Power push button";
> +			linux,code = <KEY_SLEEP>;
> +			gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +	gpio-leds {
> +		compatible = "gpio-leds";
> +		blue-power {
> +			label = "nas220:blue:power";
> +			gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "default-on";
> +		};
> +	};
> +
> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pinctrl-0 = <&pmx_power_sata0 &pmx_power_sata1>;
> +		pinctrl-names = "default";
> +
> +		sata0_power: regulator@1 {
> +			compatible = "regulator-fixed";
> +			reg = <1>;
> +			regulator-name = "SATA0 Power";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			enable-active-high;
> +			regulator-always-on;
> +			regulator-boot-on;
> +			gpio = <&gpio0 24 0>;
> +		};
> +
> +		sata1_power: regulator@2 {
> +			compatible = "regulator-fixed";
> +			reg = <2>;
> +			regulator-name = "SATA1 Power";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			enable-active-high;
> +			regulator-always-on;
> +			regulator-boot-on;
> +			gpio = <&gpio0 28 0>;
> +		};
> +	};
> +};
> +
> +&nand {
> +	status = "okay";
> +};
> +
> +&mdio {
> +	status = "okay";
> +	ethphy0: ethernet-phy@8 {
> +		 reg = <8>;
> +	 };
> +};
> +
> +&eth0 {
> +	status = "okay";
> +	ethernet0-port@0 {
> +		phy-handle = <&ethphy0>;
> +	};
> +};
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v3 1/1] add support for Seagate BlackArmor NAS220

[Sebastian Hesselbarth]

On 22.12.2014 13:57, Evgeni Dobrev wrote:
> This patch adds support for Seagate BlackArmor NAS220.
>
> The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has
> 32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two
> USB 2.0 ports, two buttons and three LEDs. There is a serial port available on
> the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).
>
> The only functionality still not implemented is the bi-color led on the front
> panel (status). Pins mpp22 and mpp23 control this led. Setting mpp22 to high and
> mpp23 to low results in orange color. Setting mpp22 to low and mpp23 to high
> results in blue color.
>
> The third led is wired to show the SATA activity on the two drives.
>
> Signed-off-by: Evgeni Dobrev <evgeni-c3HRhe8yoGxdzLpqerNdyw@public.gmane.org>

Evgeni,

sorry for the late review, but I do have some nits that should
remove some inconsistencies. If we don't do it now, we'd never
change that later.

> ---
>   arch/arm/boot/dts/Makefile            |    1 +
>   arch/arm/boot/dts/kirkwood-nas220.dts |  166 +++++++++++++++++++++++++++++++++
>   2 files changed, 167 insertions(+)
>   create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 38c89ca..8b9ad1d 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -132,6 +132,7 @@ dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \
>   	kirkwood-lsxhl.dtb \
>   	kirkwood-mplcec4.dtb \
>   	kirkwood-mv88f6281gtw-ge.dtb \
> +	kirkwood-nas220.dtb \

I am not too happy with choosing "nas220" as the base name for the
board. Can we make it a little bit more specific like
"seagate-nas220" or "blackarmor-nas220" ?

>   	kirkwood-net2big.dtb \
>   	kirkwood-net5big.dtb \
>   	kirkwood-netgear_readynas_duo_v2.dtb \
> diff --git a/arch/arm/boot/dts/kirkwood-nas220.dts b/arch/arm/boot/dts/kirkwood-nas220.dts
> new file mode 100644
> index 0000000..8175f3d
> --- /dev/null
> +++ b/arch/arm/boot/dts/kirkwood-nas220.dts
> @@ -0,0 +1,166 @@
> +/*
> + * Device Tree file for Seagate BlackArmor NAS220
> + *
> + * Copyright (C) 2014 Evgeni Dobrev <evgeni-c3HRhe8yoGxdzLpqerNdyw@public.gmane.org>
> + *
> + * Licensed under GPLv2 or later.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include "kirkwood.dtsi"
> +#include "kirkwood-6192.dtsi"
> +
> +/ {
> +	model = "Seagate NAS 220";

model = "Seagate BlackArmor NAS220";

i.e. choose the same spelling in comment above and model name here.

> +	compatible = "seagate,nas220","marvell,kirkwood-88f6192","marvell,kirkwood";

compatible should reflect the chosen base name above.

> +
> +	memory { /* 128 MB */
> +		device_type = "memory";
> +		reg = <0x00000000 0x8000000>;
> +	};
> +
> +	chosen {
> +		bootargs = "console=ttyS0,115200n8";
> +		stdout-path = &uart0;
> +	};
> +
> +	ocp@f1000000 {
> +		pinctrl: pin-controller@10000 {

v3.19 should already have a label for the common pinctrl node.
Please do not replay the bus structure but use a node reference
like &nand and friends below.

> +			pinctrl-0 = <&pmx_uart0
> +				     &pmx_button_reset
> +				     &pmx_button_power>;
> +			pinctrl-names = "default";
> +
> +			pmx_act_sata0: pmx-act-sata0 {
> +				marvell,pins = "mpp15";
> +				marvell,function = "sata0";
> +			};

Insert a blank line between each of the pmx_foo nodes?

> +			pmx_act_sata1: pmx-act-sata1 {
> +				marvell,pins = "mpp16";
> +				marvell,function = "sata1";
> +			};
> +			pmx_power_sata0: pmx-power-sata0 {
> +				marvell,pins = "mpp24";
> +				marvell,function = "gpio";
> +			};
> +			pmx_power_sata1: pmx-power-sata1 {
> +				marvell,pins = "mpp28";
> +				marvell,function = "gpio";
> +			};
> +			pmx_button_reset: pmx-button-reset {
> +				marvell,pins = "mpp29";
> +				marvell,function = "gpio";
> +			};
> +			pmx_button_power: pmx-button-power {
> +				marvell,pins = "mpp26";
> +				marvell,function = "gpio";
> +			};
> +		};
> +
> +

Remove extra blank line.

> +		/*
> +		 * Serial port routed to connector CN5
> +		 *
> +		 * pin 1 - TX
> +		 * pin 4 - RX
> +		 * pin 6 - GND
> +		 */

Nice! Can you also clarify if TX/RX are as seen from the SoC or
remote end?

> +		serial@12000 {

Please use node references where possible, IIRC v3.19 should have
labels for serial, sata, and i2c. Avoid to replay the bus structure.

> +			status = "okay";
> +		};
> +
> +		sata@80000 {
> +			status = "okay";
> +			nr-ports = <2>;

I need some update from the other mvebu guys here: Do we have SATA
PHY nodes in v3.19 for Kirkwood already? If so, please update to the
new binding.

> +		};
> +
> +		i2c@11000 {
> +			status = "okay";
> +			adt7476: adt7476a@2e {

I know we have a lot of bad examples, but: node names should reflect
device function, not device name, i.e.

adt7476: thermal@2e {
	...
};

> +				compatible = "adi,adt7476";
> +				reg = <0x2e>;
> +			};
> +		};
> +	};
> +
> +	gpio_poweroff {
> +		compatible = "gpio-poweroff";
> +		gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
> +	};
> +
> +	gpio_keys {
> +		compatible = "gpio-keys";

Add a blank line between nodes.

> +		button@1{
> +			label = "Reset push button";

Reduce label to "Reset" and "Power" below.

> +			linux,code = <KEY_POWER>;
> +			gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>;
> +		};
> +		button@2{
> +			label = "Power push button";
> +			linux,code = <KEY_SLEEP>;
> +			gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
> +	gpio-leds {
> +		compatible = "gpio-leds";

Add a blank line between nodes.

> +		blue-power {
> +			label = "nas220:blue:power";
> +			gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "default-on";
> +		};
> +	};
> +
> +	regulators {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		pinctrl-0 = <&pmx_power_sata0 &pmx_power_sata1>;
> +		pinctrl-names = "default";
> +
> +		sata0_power: regulator@1 {
> +			compatible = "regulator-fixed";
> +			reg = <1>;
> +			regulator-name = "SATA0 Power";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			enable-active-high;
> +			regulator-always-on;
> +			regulator-boot-on;
> +			gpio = <&gpio0 24 0>;

Hmm, do you need "regulator-always-on" when it is GPIO controlled?
Also, gpio property could use GPIO_ACTIVE_HIGH/LOW here too.

> +		};
> +
> +		sata1_power: regulator@2 {
> +			compatible = "regulator-fixed";
> +			reg = <2>;
> +			regulator-name = "SATA1 Power";
> +			regulator-min-microvolt = <5000000>;
> +			regulator-max-microvolt = <5000000>;
> +			enable-active-high;
> +			regulator-always-on;
> +			regulator-boot-on;
> +			gpio = <&gpio0 28 0>;

ditto.

> +		};
> +	};
> +};
> +
> +&nand {
> +	status = "okay";
> +};
> +
> +&mdio {
> +	status = "okay";
> +	ethphy0: ethernet-phy@8 {
> +		 reg = <8>;
> +	 };

Remove extra space before brace.

> +};
> +
> +&eth0 {
> +	status = "okay";
> +	ethernet0-port@0 {
> +		phy-handle = <&ethphy0>;
> +	};
> +};
>

Overall, this look fine to me, with the nits taken care of

Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Thanks!

--
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 v3 1/1] add support for Seagate BlackArmor NAS220

[Andrew Lunn]

On Thu, Dec 25, 2014 at 02:08:12PM +0100, Sebastian Hesselbarth wrote:
> >+			status = "okay";
> >+		};
> >+
> >+		sata@80000 {
> >+			status = "okay";
> >+			nr-ports = <2>;
> 
> I need some update from the other mvebu guys here: Do we have SATA
> PHY nodes in v3.19 for Kirkwood already? If so, please update to the
> new binding.

Hi Sebastian

I'm not sure what you mean here. The binding Documentation says:

Required Properties:
- compatibility : "marvell,orion-sata" or "marvell,armada-370-sata"
- reg           : Address range of controller
- interrupts    : Interrupt controller is using
- nr-ports      : Number of SATA ports in use.

Optional Properties:
- phys          : List of phandles to sata phys
- phy-names     : Should be "0", "1", etc, one number per phandle

The optional phys/phy-names have just been added to
kirkwood-6192.dtsi.

	Andrew

Re: [PATCH v3 1/1] add support for Seagate BlackArmor NAS220

[Sebastian Hesselbarth]

On 25.12.2014 14:31, Andrew Lunn wrote:
> On Thu, Dec 25, 2014 at 02:08:12PM +0100, Sebastian Hesselbarth wrote:
>>> +			status = "okay";
>>> +		};
>>> +
>>> +		sata@80000 {
>>> +			status = "okay";
>>> +			nr-ports = <2>;
>>
>> I need some update from the other mvebu guys here: Do we have SATA
>> PHY nodes in v3.19 for Kirkwood already? If so, please update to the
>> new binding.
>
> Hi Sebastian
>
> I'm not sure what you mean here. The binding Documentation says:

I was hoping that using phys/phy-names would allow us to get rid of
nr-ports property. I haven't checked the corresponding code and likely
will not before next year, but we should try to get rid of the nr-ports
property completely.

> Required Properties:
> - compatibility : "marvell,orion-sata" or "marvell,armada-370-sata"
> - reg           : Address range of controller
> - interrupts    : Interrupt controller is using
> - nr-ports      : Number of SATA ports in use.
>
> Optional Properties:
> - phys          : List of phandles to sata phys
> - phy-names     : Should be "0", "1", etc, one number per phandle
>
> The optional phys/phy-names have just been added to
> kirkwood-6192.dtsi.

Yeah, saw that patch after the review.. and my understanding of the
phys/phy-names properties is that
(a) they allow to count the number of _available_ ports
(b) they allow to determine the number of _used_ ports
(c) they allow to e.g. disable port 0 but enable port 1

while nr-ports only allows (a). You can derive (b) only if you use
the first out of two but you cannot do (c) with nr-ports.

Which is why we introduced the phys/phy-names properties.

Sebastian

Re: [PATCH v3 1/1] add support for Seagate BlackArmor NAS220

[Andrew Lunn]

> >Hi Sebastian
> >
> >I'm not sure what you mean here. The binding Documentation says:
> 
> I was hoping that using phys/phy-names would allow us to get rid of
> nr-ports property. I haven't checked the corresponding code and likely
> will not before next year, but we should try to get rid of the nr-ports
> property completely.
> 
> >Required Properties:
> >- compatibility : "marvell,orion-sata" or "marvell,armada-370-sata"
> >- reg           : Address range of controller
> >- interrupts    : Interrupt controller is using
> >- nr-ports      : Number of SATA ports in use.
> >
> >Optional Properties:
> >- phys          : List of phandles to sata phys
> >- phy-names     : Should be "0", "1", etc, one number per phandle
> >
> >The optional phys/phy-names have just been added to
> >kirkwood-6192.dtsi.
> 
> Yeah, saw that patch after the review.. and my understanding of the
> phys/phy-names properties is that
> (a) they allow to count the number of _available_ ports
> (b) they allow to determine the number of _used_ ports
> (c) they allow to e.g. disable port 0 but enable port 1
> 
> while nr-ports only allows (a). You can derive (b) only if you use
> the first out of two but you cannot do (c) with nr-ports.
> 
> Which is why we introduced the phys/phy-names properties.

We introduced them for turning the phys on and off.

We don't do any derivation from this information. It is the same for
all devices, since it is in the kirkwood-*.dtsi file, not per board.

Also, orion5x does not allow such control of the phys, they are always
on. So i don't know if we can derive anything from this information,
since it is not always present.

    Andrew

Re: [PATCH v3 1/1] add support for Seagate BlackArmor NAS220

[Sebastian Hesselbarth]

On 25.12.2014 15:12, Andrew Lunn wrote:
>>> Hi Sebastian
>>>
>>> I'm not sure what you mean here. The binding Documentation says:
>>
>> I was hoping that using phys/phy-names would allow us to get rid of
>> nr-ports property. I haven't checked the corresponding code and likely
>> will not before next year, but we should try to get rid of the nr-ports
>> property completely.
>>
>>> Required Properties:
>>> - compatibility : "marvell,orion-sata" or "marvell,armada-370-sata"
>>> - reg           : Address range of controller
>>> - interrupts    : Interrupt controller is using
>>> - nr-ports      : Number of SATA ports in use.
>>>
>>> Optional Properties:
>>> - phys          : List of phandles to sata phys
>>> - phy-names     : Should be "0", "1", etc, one number per phandle
>>>
>>> The optional phys/phy-names have just been added to
>>> kirkwood-6192.dtsi.
>>
>> Yeah, saw that patch after the review.. and my understanding of the
>> phys/phy-names properties is that
>> (a) they allow to count the number of _available_ ports
>> (b) they allow to determine the number of _used_ ports
>> (c) they allow to e.g. disable port 0 but enable port 1
>>
>> while nr-ports only allows (a). You can derive (b) only if you use
>> the first out of two but you cannot do (c) with nr-ports.
>>
>> Which is why we introduced the phys/phy-names properties.
>
> We introduced them for turning the phys on and off.

Actually, the main reason was to turn the unused phys off. IIRC,
just reducing nr-ports from 2 to 1 allowed the SATA driver to skip
the second port, but left the PHY up and running. It also doesn't
allow to disable the first port and keep the second up.

Using phys/phy-names now allows a more detailed description by having
three devices (SATA + 2 PHY nodes), e.g. SATA controller with 2
potential ports, port 0 unconnected, port 1 connected. PHY0 will be
auto-disabled after device probe, PHY1 will be enabled by SATA driver.

IIRC, this is the setup used by Guruplug, where port1 is connected to
eSATA while port0 is unconnected.

> We don't do any derivation from this information. It is the same for
> all devices, since it is in the kirkwood-*.dtsi file, not per board.
>
> Also, orion5x does not allow such control of the phys, they are always
> on. So i don't know if we can derive anything from this information,
> since it is not always present.

Ok, you made me look it up in the code ;)

AFAIKS, nr-ports is only used for preallocating SATA host, clks, and
phys structs. The code logic implies that #phys/clk has to match
nr-ports property anyway. Unfortunately, n_ports variable and
hpriv->n_ports is mangled irritatingly often in current sata_mv.c.

What I was proposing (hoping we already do) is: remove the redundant
information passed by nr-ports were we can, e.g. count the number of
clocks/phys passed instead of reading nr-ports property.

I do understand that Orion5x neither has clocks nor phys on SATA so
we may either stick with nr-ports there or add fake (&tclk) clocks
instead.

Anyway, for this patch, I agree it could stay the way it is.

Sebastian

Re: [PATCH v3 1/1] add support for Seagate BlackArmor NAS220

[Evgeni Dobrev]

Hello Sebastian,

On Thu, Dec 25, 2014 at 02:08:12PM +0100, Sebastian Hesselbarth wrote:
> On 22.12.2014 13:57, Evgeni Dobrev wrote:
> >This patch adds support for Seagate BlackArmor NAS220.
> >
> >The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has
> >32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two
> >USB 2.0 ports, two buttons and three LEDs. There is a serial port available on
> >the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).
> >
> >The only functionality still not implemented is the bi-color led on the front
> >panel (status). Pins mpp22 and mpp23 control this led. Setting mpp22 to high and
> >mpp23 to low results in orange color. Setting mpp22 to low and mpp23 to high
> >results in blue color.
> >
> >The third led is wired to show the SATA activity on the two drives.
> >
> >Signed-off-by: Evgeni Dobrev <evgeni-c3HRhe8yoGxdzLpqerNdyw@public.gmane.org>
> 
> Evgeni,
> 
> sorry for the late review, but I do have some nits that should
> remove some inconsistencies. If we don't do it now, we'd never
> change that later.
> 

thank you for your review. I will make the changes you suggest and resubmit.

> >---
> >  arch/arm/boot/dts/Makefile            |    1 +
> >  arch/arm/boot/dts/kirkwood-nas220.dts |  166 +++++++++++++++++++++++++++++++++
> >  2 files changed, 167 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/kirkwood-nas220.dts
> >
> >diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> >index 38c89ca..8b9ad1d 100644
> >--- a/arch/arm/boot/dts/Makefile
> >+++ b/arch/arm/boot/dts/Makefile
> >@@ -132,6 +132,7 @@ dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \
> >  	kirkwood-lsxhl.dtb \
> >  	kirkwood-mplcec4.dtb \
> >  	kirkwood-mv88f6281gtw-ge.dtb \
> >+	kirkwood-nas220.dtb \
> 
> I am not too happy with choosing "nas220" as the base name for the
> board. Can we make it a little bit more specific like
> "seagate-nas220" or "blackarmor-nas220" ?
> 
> >  	kirkwood-net2big.dtb \
> >  	kirkwood-net5big.dtb \
> >  	kirkwood-netgear_readynas_duo_v2.dtb \
> >diff --git a/arch/arm/boot/dts/kirkwood-nas220.dts b/arch/arm/boot/dts/kirkwood-nas220.dts
> >new file mode 100644
> >index 0000000..8175f3d
> >--- /dev/null
> >+++ b/arch/arm/boot/dts/kirkwood-nas220.dts
> >@@ -0,0 +1,166 @@
> >+/*
> >+ * Device Tree file for Seagate BlackArmor NAS220
> >+ *
> >+ * Copyright (C) 2014 Evgeni Dobrev <evgeni-c3HRhe8yoGxdzLpqerNdyw@public.gmane.org>
> >+ *
> >+ * Licensed under GPLv2 or later.
> >+ */
> >+
> >+/dts-v1/;
> >+
> >+#include <dt-bindings/gpio/gpio.h>
> >+#include <dt-bindings/input/input.h>
> >+#include "kirkwood.dtsi"
> >+#include "kirkwood-6192.dtsi"
> >+
> >+/ {
> >+	model = "Seagate NAS 220";
> 
> model = "Seagate BlackArmor NAS220";
> 
> i.e. choose the same spelling in comment above and model name here.
> 
> >+	compatible = "seagate,nas220","marvell,kirkwood-88f6192","marvell,kirkwood";
> 
> compatible should reflect the chosen base name above.
> 
> >+
> >+	memory { /* 128 MB */
> >+		device_type = "memory";
> >+		reg = <0x00000000 0x8000000>;
> >+	};
> >+
> >+	chosen {
> >+		bootargs = "console=ttyS0,115200n8";
> >+		stdout-path = &uart0;
> >+	};
> >+
> >+	ocp@f1000000 {
> >+		pinctrl: pin-controller@10000 {
> 
> v3.19 should already have a label for the common pinctrl node.
> Please do not replay the bus structure but use a node reference
> like &nand and friends below.
> 
> >+			pinctrl-0 = <&pmx_uart0
> >+				     &pmx_button_reset
> >+				     &pmx_button_power>;
> >+			pinctrl-names = "default";
> >+
> >+			pmx_act_sata0: pmx-act-sata0 {
> >+				marvell,pins = "mpp15";
> >+				marvell,function = "sata0";
> >+			};
> 
> Insert a blank line between each of the pmx_foo nodes?
> 
> >+			pmx_act_sata1: pmx-act-sata1 {
> >+				marvell,pins = "mpp16";
> >+				marvell,function = "sata1";
> >+			};
> >+			pmx_power_sata0: pmx-power-sata0 {
> >+				marvell,pins = "mpp24";
> >+				marvell,function = "gpio";
> >+			};
> >+			pmx_power_sata1: pmx-power-sata1 {
> >+				marvell,pins = "mpp28";
> >+				marvell,function = "gpio";
> >+			};
> >+			pmx_button_reset: pmx-button-reset {
> >+				marvell,pins = "mpp29";
> >+				marvell,function = "gpio";
> >+			};
> >+			pmx_button_power: pmx-button-power {
> >+				marvell,pins = "mpp26";
> >+				marvell,function = "gpio";
> >+			};
> >+		};
> >+
> >+
> 
> Remove extra blank line.
> 
> >+		/*
> >+		 * Serial port routed to connector CN5
> >+		 *
> >+		 * pin 1 - TX
> >+		 * pin 4 - RX
> >+		 * pin 6 - GND
> >+		 */
> 
> Nice! Can you also clarify if TX/RX are as seen from the SoC or
> remote end?
> 
> >+		serial@12000 {
> 
> Please use node references where possible, IIRC v3.19 should have
> labels for serial, sata, and i2c. Avoid to replay the bus structure.
> 
> >+			status = "okay";
> >+		};
> >+
> >+		sata@80000 {
> >+			status = "okay";
> >+			nr-ports = <2>;
> 
> I need some update from the other mvebu guys here: Do we have SATA
> PHY nodes in v3.19 for Kirkwood already? If so, please update to the
> new binding.
> 
> >+		};
> >+
> >+		i2c@11000 {
> >+			status = "okay";
> >+			adt7476: adt7476a@2e {
> 
> I know we have a lot of bad examples, but: node names should reflect
> device function, not device name, i.e.
> 
> adt7476: thermal@2e {
> 	...
> };
> 
> >+				compatible = "adi,adt7476";
> >+				reg = <0x2e>;
> >+			};
> >+		};
> >+	};
> >+
> >+	gpio_poweroff {
> >+		compatible = "gpio-poweroff";
> >+		gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
> >+	};
> >+
> >+	gpio_keys {
> >+		compatible = "gpio-keys";
> 
> Add a blank line between nodes.
> 
> >+		button@1{
> >+			label = "Reset push button";
> 
> Reduce label to "Reset" and "Power" below.
> 
> >+			linux,code = <KEY_POWER>;
> >+			gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>;
> >+		};
> >+		button@2{
> >+			label = "Power push button";
> >+			linux,code = <KEY_SLEEP>;
> >+			gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
> >+		};
> >+	};
> >+
> >+	gpio-leds {
> >+		compatible = "gpio-leds";
> 
> Add a blank line between nodes.
> 
> >+		blue-power {
> >+			label = "nas220:blue:power";
> >+			gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
> >+			linux,default-trigger = "default-on";
> >+		};
> >+	};
> >+
> >+	regulators {
> >+		compatible = "simple-bus";
> >+		#address-cells = <1>;
> >+		#size-cells = <0>;
> >+		pinctrl-0 = <&pmx_power_sata0 &pmx_power_sata1>;
> >+		pinctrl-names = "default";
> >+
> >+		sata0_power: regulator@1 {
> >+			compatible = "regulator-fixed";
> >+			reg = <1>;
> >+			regulator-name = "SATA0 Power";
> >+			regulator-min-microvolt = <5000000>;
> >+			regulator-max-microvolt = <5000000>;
> >+			enable-active-high;
> >+			regulator-always-on;
> >+			regulator-boot-on;
> >+			gpio = <&gpio0 24 0>;
> 
> Hmm, do you need "regulator-always-on" when it is GPIO controlled?
> Also, gpio property could use GPIO_ACTIVE_HIGH/LOW here too.
> 
> >+		};
> >+
> >+		sata1_power: regulator@2 {
> >+			compatible = "regulator-fixed";
> >+			reg = <2>;
> >+			regulator-name = "SATA1 Power";
> >+			regulator-min-microvolt = <5000000>;
> >+			regulator-max-microvolt = <5000000>;
> >+			enable-active-high;
> >+			regulator-always-on;
> >+			regulator-boot-on;
> >+			gpio = <&gpio0 28 0>;
> 
> ditto.
> 
> >+		};
> >+	};
> >+};
> >+
> >+&nand {
> >+	status = "okay";
> >+};
> >+
> >+&mdio {
> >+	status = "okay";
> >+	ethphy0: ethernet-phy@8 {
> >+		 reg = <8>;
> >+	 };
> 
> Remove extra space before brace.
> 
> >+};
> >+
> >+&eth0 {
> >+	status = "okay";
> >+	ethernet0-port@0 {
> >+		phy-handle = <&ethphy0>;
> >+	};
> >+};
> >
> 
> Overall, this look fine to me, with the nits taken care of
> 
> Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> 
> Thanks!
> 

Thanks,

evgeni
--
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 v4 0/1] add support for Seagate BlackArmor NAS220

[Evgeni Dobrev]

v4:	rename basename to blackarmor-nas220
	use "Seagate Blackarmor NAS220" as model name
	use node references instead of repeating the bus structure
	separate nodes with blank lines
	adt7476's node name reflects device function (i.e. thermal)
	reduce button labels to "Reset" and "Power"
	use GPIO_ACTIVE_LOW for regulator's gpio property

v3:	documented UART pinout in dts

v2:	indent with tabs only
	removed NAND layout

Evgeni Dobrev (1):
  add support for Seagate BlackArmor NAS220

 arch/arm/boot/dts/Makefile                       |    1 +
 arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts |  172 ++++++++++++++++++++++
 2 files changed, 173 insertions(+)
 create mode 100644 arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts

-- 
1.7.10.4

--
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 v4 1/1] add support for Seagate BlackArmor NAS220

[Evgeni Dobrev]

This patch adds support for Seagate BlackArmor NAS220.

The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has
32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two
USB 2.0 ports, two buttons and three LEDs. There is a serial port available on
the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).

The only functionality still not implemented is the bi-color led on the front
panel (status). Pins mpp22 and mpp23 control this led. Setting mpp22 to high and
mpp23 to low results in orange color. Setting mpp22 to low and mpp23 to high
results in blue color.

The third led is wired to show the SATA activity on the two drives.

Signed-off-by: Evgeni Dobrev <evgeni-c3HRhe8yoGxdzLpqerNdyw@public.gmane.org>
---
 arch/arm/boot/dts/Makefile                       |    1 +
 arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts |  172 ++++++++++++++++++++++
 2 files changed, 173 insertions(+)
 create mode 100644 arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 38c89ca..780bf29 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -100,6 +100,7 @@ dtb-$(CONFIG_ARCH_KEYSTONE) += k2hk-evm.dtb \
 	k2l-evm.dtb \
 	k2e-evm.dtb
 dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \
+	kirkwood-blackarmor-nas220.dtb \
 	kirkwood-cloudbox.dtb \
 	kirkwood-d2net.dtb \
 	kirkwood-db-88f6281.dtb \
diff --git a/arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts b/arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts
new file mode 100644
index 0000000..27fe2f9
--- /dev/null
+++ b/arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts
@@ -0,0 +1,172 @@
+/*
+ * Device Tree file for Seagate Blackarmor NAS220
+ *
+ * Copyright (C) 2014 Evgeni Dobrev <evgeni-c3HRhe8yoGxdzLpqerNdyw@public.gmane.org>
+ *
+ * Licensed under GPLv2 or later.
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include "kirkwood.dtsi"
+#include "kirkwood-6192.dtsi"
+
+/ {
+	model = "Seagate Blackarmor NAS220";
+	compatible = "seagate,blackarmor-nas220","marvell,kirkwood-88f6192","marvell,kirkwood";
+
+	memory { /* 128 MB */
+		device_type = "memory";
+		reg = <0x00000000 0x8000000>;
+	};
+
+	chosen {
+		bootargs = "console=ttyS0,115200n8";
+		stdout-path = &uart0;
+	};
+
+	gpio_poweroff {
+		compatible = "gpio-poweroff";
+		gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
+	};
+
+	gpio_keys {
+		compatible = "gpio-keys";
+
+		button@1{
+			label = "Reset";
+			linux,code = <KEY_POWER>;
+			gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>;
+		};
+
+		button@2{
+			label = "Power";
+			linux,code = <KEY_SLEEP>;
+			gpios = <&gpio0 26 GPIO_ACTIVE_LOW>;
+		};
+	};
+
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		blue-power {
+			label = "nas220:blue:power";
+			gpios = <&gpio0 12 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "default-on";
+		};
+	};
+
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+		pinctrl-0 = <&pmx_power_sata0 &pmx_power_sata1>;
+		pinctrl-names = "default";
+
+		sata0_power: regulator@1 {
+			compatible = "regulator-fixed";
+			reg = <1>;
+			regulator-name = "SATA0 Power";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			enable-active-high;
+			regulator-always-on;
+			regulator-boot-on;
+			gpio = <&gpio0 24 GPIO_ACTIVE_LOW>;
+		};
+
+		sata1_power: regulator@2 {
+			compatible = "regulator-fixed";
+			reg = <2>;
+			regulator-name = "SATA1 Power";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			enable-active-high;
+			regulator-always-on;
+			regulator-boot-on;
+			gpio = <&gpio0 28 GPIO_ACTIVE_LOW>;
+		};
+	};
+};
+
+/*
+ * Serial port routed to connector CN5
+ *
+ * pin 1 - TX (CPU's TX)
+ * pin 4 - RX (CPU's RX)
+ * pin 6 - GND
+ */
+&uart0 {
+	status = "okay";
+};
+
+&pinctrl {
+	pinctrl-0 = <&pmx_button_reset &pmx_button_power>;
+	pinctrl-names = "default";
+
+	pmx_act_sata0: pmx-act-sata0 {
+		marvell,pins = "mpp15";
+		marvell,function = "sata0";
+	};
+
+	pmx_act_sata1: pmx-act-sata1 {
+		marvell,pins = "mpp16";
+		marvell,function = "sata1";
+	};
+
+	pmx_power_sata0: pmx-power-sata0 {
+		marvell,pins = "mpp24";
+		marvell,function = "gpio";
+	};
+
+	pmx_power_sata1: pmx-power-sata1 {
+		marvell,pins = "mpp28";
+		marvell,function = "gpio";
+	};
+
+	pmx_button_reset: pmx-button-reset {
+		marvell,pins = "mpp29";
+		marvell,function = "gpio";
+	};
+
+	pmx_button_power: pmx-button-power {
+		marvell,pins = "mpp26";
+		marvell,function = "gpio";
+	};
+};
+
+&sata {
+	status = "okay";
+	nr-ports = <2>;
+};
+		
+&i2c0 {
+	status = "okay";
+
+	adt7476: thermal@2e {
+		compatible = "adi,adt7476";
+		reg = <0x2e>;
+	};
+};
+
+&nand {
+	status = "okay";
+};
+
+&mdio {
+	status = "okay";
+
+	ethphy0: ethernet-phy@8 {
+		 reg = <8>;
+	};
+};
+
+&eth0 {
+	status = "okay";
+
+	ethernet0-port@0 {
+		phy-handle = <&ethphy0>;
+	};
+};
-- 
1.7.10.4

--
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 v4 1/1] add support for Seagate BlackArmor NAS220

[Andrew Lunn]

On Sun, Dec 28, 2014 at 11:46:54AM +0100, Evgeni Dobrev wrote:
> This patch adds support for Seagate BlackArmor NAS220.
> 
> The Seagate BlackArmor NAS 220 is a NAS system based on Marvell 88f6192. It has
> 32MB NAND and 128MB DRAM. It has two SATA slots, one Gigabit Ethernet port, two
> USB 2.0 ports, two buttons and three LEDs. There is a serial port available on
> the CN5 connector on the board (1 - TX, 4 - RX, 6 - GND).
> 
> The only functionality still not implemented is the bi-color led on the front
> panel (status). Pins mpp22 and mpp23 control this led. Setting mpp22 to high and
> mpp23 to low results in orange color. Setting mpp22 to low and mpp23 to high
> results in blue color.
> 
> The third led is wired to show the SATA activity on the two drives.
> 
> Signed-off-by: Evgeni Dobrev <evgeni@studio-punkt.com>

Hi Evgeni

I added this to mvebu/dt and will submit it for the next merge window.

Thanks
	Andrew