ARM: dts: kirkwood: add dts support for Seagate BlackArmor NAS220
PATCH 2/2 ARM: dts: kirkwood: add dts support for Seagate BlackArmor NAS220
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.
--- 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
arch/arm/boot/dts/Makefile +1 -0
@@ -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 \
arch/arm/boot/dts/kirkwood-nas220.dts new file +182 -0
182 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Quoted 1 line · Evgeni Dobrev
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.
Quoted 8 lines
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?
Quoted 27 lines
The third led is wired to show the SATA activity on the two drives.
Signed-off-by Evgeni Dobrev <[email protected]>--- 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.dtsarch/arm/boot/dts/Makefile +1
-0@@ -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 \arch/arm/boot/dts/kirkwood-nas220.dts new file +0
-0
arch/arm/boot/dts/kirkwood-nas220.dts line 7
Quoted 7 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +7
-0@@ -0,0 +1,182 @@ +/* + * Device Tree file for Seagate BlackArmor NAS220 + * + * Copyright (C) 2014 Evgeni Dobrev <[email protected]> + * + * 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.
Quoted 11 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +11
-0+ +/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";
arch/arm/boot/dts/kirkwood-nas220.dts line 28
Quoted 10 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +10
-0+ + 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.
Quoted 118 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +118
-0+ + ocp at f1000000 { + pinctrl: pin-controller at 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 at 12000 { + status = "okay"; + }; + + sata at 80000 { + nr-ports = <2>; + status = "okay"; + }; + + i2c at 11000 { + status = "okay"; + adt7476: adt7476a at 2e { + compatible = "adi,adt7476"; + reg = <0x2e>; + }; + }; + }; + + gpio_poweroff { + compatible = "gpio-poweroff"; + gpios = <&gpio0 14 GPIO_ACTIVE_LOW>; + }; + + gpio_keys { + compatible = "gpio-keys"; + + button at 1{ + label = "Reset push button"; + linux,code = <KEY_POWER>; + gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>; + }; + button at 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 at 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 at 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";
arch/arm/boot/dts/kirkwood-nas220.dts line 156
Quoted 10 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +10
-0+ partition at 0 { + label = "uboot"; + reg = <0x0 0xa0000>; + read-only; + }; + + partition at a0000 { + label = "env"; + reg = <0xa0000 0x010000>; + };
Is there supposed to be a gap here?
Quoted 17 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +16
-0+ + partition at c0000 { + label = "uimage"; + reg = <0xc0000 0x500000>; + }; + + partition at 5c0000 { + label = "rootfs"; + reg = <0x5c0000 0x1a40000>; + }; +}; + +&mdio { + status = "okay"; + ethphy0: ethernet-phy at 8 { + reg = <8>;
arch/arm/boot/dts/kirkwood-nas220.dts line 182
Quoted 10 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +10
-0+ }; +}; + +ð0 { + status = "okay"; + ethernet0-port at 0 { + phy-handle = <ðphy0>; + }; +}; +
Andrew
Hi Andrew,
Quoted 10 lines · Andrew Lunn
Quoted 1 line · Evgeni Dobrev
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!
Quoted 4 lines
Quoted 3 lines
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).
Quoted 9 lines
Quoted 4 lines
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.
Quoted 32 lines
Quoted 32 lines
The third led is wired to show the SATA activity on the two drives.
Signed-off-by Evgeni Dobrev <[email protected]>--- 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.dtsarch/arm/boot/dts/Makefile +1
-0@@ -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 \arch/arm/boot/dts/kirkwood-nas220.dts new file +5
-0@@ -0,0 +1,182 @@ +/* + * Device Tree file for Seagate BlackArmor NAS220 + * + * Copyright (C) 2014 Evgeni Dobrev <[email protected]> + *
Quoted 9 lines
arch/arm/boot/dts/kirkwood-nas220.dts line 7
Quoted 2 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +2
-0+ * 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.
Quoted 16 lines
Quoted 16 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +16
-0+ +/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>; + };
Quoted 9 lines
arch/arm/boot/dts/kirkwood-nas220.dts line 28
Quoted 5 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +5
-0+ + 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.
Quoted 122 lines
Quoted 122 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +122
-0+ + ocp at f1000000 { + pinctrl: pin-controller at 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 at 12000 { + status = "okay"; + }; + + sata at 80000 { + nr-ports = <2>; + status = "okay"; + }; + + i2c at 11000 { + status = "okay"; + adt7476: adt7476a at 2e { + compatible = "adi,adt7476"; + reg = <0x2e>; + }; + }; + }; + + gpio_poweroff { + compatible = "gpio-poweroff"; + gpios = <&gpio0 14 GPIO_ACTIVE_LOW>; + }; + + gpio_keys { + compatible = "gpio-keys"; + + button at 1{ + label = "Reset push button"; + linux,code = <KEY_POWER>; + gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>; + }; + button at 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 at 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 at 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 at 0 { + label = "uboot"; + reg = <0x0 0xa0000>; + read-only;
Quoted 9 lines
arch/arm/boot/dts/kirkwood-nas220.dts line 156
Quoted 6 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +6
-0+ }; + + partition at 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?
Quoted 22 lines
Quoted 22 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +22
-0+ + partition at c0000 { + label = "uimage"; + reg = <0xc0000 0x500000>; + }; + + partition at 5c0000 { + label = "rootfs"; + reg = <0x5c0000 0x1a40000>; + }; +}; + +&mdio { + status = "okay"; + ethphy0: ethernet-phy at 8 { + reg = <8>; + }; +}; + +ð0 { + status = "okay"; + ethernet0-port at 0 {
Quoted 8 lines
arch/arm/boot/dts/kirkwood-nas220.dts line 182
Quoted 5 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +4
-0+ phy-handle = <ðphy0>; + }; +}; +Andrew
--
Thanks for reviewing!
evgeni
Quoted 2 lines
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.
Quoted 3 lines
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.
Quoted 3 lines
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
v2 cover add support for Seagate BlackArmor NAS220
In reply to Evgeni Dobrevv2: 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
Signature · 1 line
1.7.10.4
PATCH v2 1/1 add support for Seagate BlackArmor NAS220
In reply to Evgeni DobrevThis 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.
--- 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
arch/arm/boot/dts/Makefile +1 -0
@@ -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 \
arch/arm/boot/dts/kirkwood-nas220.dts new file +158 -0
158 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Re: [PATCH v2 1/1] add support for Seagate BlackArmor NAS220
Quoted 6 lines · 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).
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
Quoted 199 lines
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 <[email protected]>--- 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.dtsarch/arm/boot/dts/Makefile +1
-0@@ -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 \arch/arm/boot/dts/kirkwood-nas220.dts new file +158
-0@@ -0,0 +1,158 @@ +/* + * Device Tree file for Seagate BlackArmor NAS220 + * + * Copyright (C) 2014 Evgeni Dobrev <[email protected]> + * + * 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 at f1000000 { + pinctrl: pin-controller at 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 at 12000 { + status = "okay"; + }; + + sata at 80000 { + status = "okay"; + nr-ports = <2>; + }; + + i2c at 11000 { + status = "okay"; + adt7476: adt7476a at 2e { + compatible = "adi,adt7476"; + reg = <0x2e>; + }; + }; + }; + + gpio_poweroff { + compatible = "gpio-poweroff"; + gpios = <&gpio0 14 GPIO_ACTIVE_LOW>; + }; + + gpio_keys { + compatible = "gpio-keys"; + button at 1{ + label = "Reset push button"; + linux,code = <KEY_POWER>; + gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>; + }; + button at 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 at 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 at 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 at 8 { + reg = <8>; + }; +}; + +ð0 { + status = "okay"; + ethernet0-port at 0 { + phy-handle = <ðphy0>; + }; +};Signature · 7 lines
1.7.10.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
v3 cover add support for Seagate BlackArmor NAS220
In reply to Evgeni Dobrevv3: 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
Signature · 1 line
1.7.10.4
PATCH v3 1/1 add support for Seagate BlackArmor NAS220
In reply to Evgeni DobrevThis 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.
--- 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
arch/arm/boot/dts/Makefile +1 -0
@@ -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 \
arch/arm/boot/dts/kirkwood-nas220.dts new file +166 -0
166 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Re: [PATCH v3 1/1] add support for Seagate BlackArmor NAS220
Quoted 7 lines · 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).
Quoted 6 lines
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
Quoted 200 lines
Signed-off-by Evgeni Dobrev <[email protected]>--- 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.dtsarch/arm/boot/dts/Makefile +1
-0@@ -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 \arch/arm/boot/dts/kirkwood-nas220.dts new file +166
-0@@ -0,0 +1,166 @@ +/* + * Device Tree file for Seagate BlackArmor NAS220 + * + * Copyright (C) 2014 Evgeni Dobrev <[email protected]> + * + * 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 at f1000000 { + pinctrl: pin-controller at 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 at 12000 { + status = "okay"; + }; + + sata at 80000 { + status = "okay"; + nr-ports = <2>; + }; + + i2c at 11000 { + status = "okay"; + adt7476: adt7476a at 2e { + compatible = "adi,adt7476"; + reg = <0x2e>; + }; + }; + }; + + gpio_poweroff { + compatible = "gpio-poweroff"; + gpios = <&gpio0 14 GPIO_ACTIVE_LOW>; + }; + + gpio_keys { + compatible = "gpio-keys"; + button at 1{ + label = "Reset push button"; + linux,code = <KEY_POWER>; + gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>; + }; + button at 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 at 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 at 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 at 8 { + reg = <8>; + }; +}; + +ð0 { + status = "okay"; + ethernet0-port at 0 { + phy-handle = <ðphy0>; + }; +};Signature · 7 lines
1.7.10.4 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel at lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v3 1/1] add support for Seagate BlackArmor NAS220
In reply to Evgeni DobrevQuoted 7 lines · 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).
Quoted 8 lines
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 <[email protected]>
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.
Quoted 11 lines
--- 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.dtsarch/arm/boot/dts/Makefile +0
-0
arch/arm/boot/dts/Makefile dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \ · line 135
Quoted 4 lines
arch/arm/boot/dts/Makefile +1
-0@@ -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" ?
Quoted 16 lines
arch/arm/boot/dts/Makefile +0
-0kirkwood-net2big.dtb \ kirkwood-net5big.dtb \ kirkwood-netgear_readynas_duo_v2.dtb \arch/arm/boot/dts/kirkwood-nas220.dts new file +7
-0@@ -0,0 +1,166 @@ +/* + * Device Tree file for Seagate BlackArmor NAS220 + * + * Copyright (C) 2014 Evgeni Dobrev <[email protected]> + * + * Licensed under GPLv2 or later. + */
arch/arm/boot/dts/kirkwood-nas220.dts line 17
Quoted 10 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +10
-0+ +/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.
arch/arm/boot/dts/kirkwood-nas220.dts line 18
Quoted 1 line
arch/arm/boot/dts/kirkwood-nas220.dts new file +1
-0+ compatible = "seagate,nas220","marvell,kirkwood-88f6192","marvell,kirkwood";
compatible should reflect the chosen base name above.
arch/arm/boot/dts/kirkwood-nas220.dts line 31
Quoted 13 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +13
-0+ + memory { /* 128 MB */ + device_type = "memory"; + reg = <0x00000000 0x8000000>; + }; + + chosen { + bootargs = "console=ttyS0,115200n8"; + stdout-path = &uart0; + }; + + ocp at f1000000 { + pinctrl: pin-controller at 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.
arch/arm/boot/dts/kirkwood-nas220.dts line 40
Quoted 9 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +9
-0+ 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?
Quoted 13 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +13
-0+ 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 {
arch/arm/boot/dts/kirkwood-nas220.dts line 63
Quoted 10 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +10
-0+ marvell,pins = "mpp29"; + marvell,function = "gpio"; + }; + pmx_button_power: pmx-button-power { + marvell,pins = "mpp26"; + marvell,function = "gpio"; + }; + }; + +
Remove extra blank line.
arch/arm/boot/dts/kirkwood-nas220.dts line 70
Quoted 7 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +7
-0+ /* + * 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?
arch/arm/boot/dts/kirkwood-nas220.dts line 71
Quoted 1 line
arch/arm/boot/dts/kirkwood-nas220.dts new file +1
-0+ serial at 12000 {
Please use node references where possible, IIRC v3.19 should have labels for serial, sata, and i2c. Avoid to replay the bus structure.
arch/arm/boot/dts/kirkwood-nas220.dts line 77
Quoted 6 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +6
-0+ status = "okay"; + }; + + sata at 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.
arch/arm/boot/dts/kirkwood-nas220.dts line 82
Quoted 5 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +5
-0+ }; + + i2c at 11000 { + status = "okay"; + adt7476: adt7476a at 2e {
I know we have a lot of bad examples, but: node names should reflect device function, not device name, i.e.
adt7476: thermal at 2e {
...
};arch/arm/boot/dts/kirkwood-nas220.dts line 95
Quoted 13 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +13
-0+ 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.
arch/arm/boot/dts/kirkwood-nas220.dts line 97
Quoted 2 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +2
-0+ button at 1{ + label = "Reset push button";
Reduce label to "Reset" and "Power" below.
arch/arm/boot/dts/kirkwood-nas220.dts line 109
Quoted 12 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +12
-0+ linux,code = <KEY_POWER>; + gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>; + }; + button at 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.
Quoted 14 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +14
-0+ 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"; +
arch/arm/boot/dts/kirkwood-nas220.dts line 133
Quoted 10 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +10
-0+ sata0_power: regulator at 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.
arch/arm/boot/dts/kirkwood-nas220.dts line 145
Quoted 12 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +12
-0+ }; + + sata1_power: regulator at 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.
arch/arm/boot/dts/kirkwood-nas220.dts line 158
Quoted 13 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +13
-0+ }; + }; +}; + +&nand { + status = "okay"; +}; + +&mdio { + status = "okay"; + ethphy0: ethernet-phy at 8 { + reg = <8>; + };
Remove extra space before brace.
arch/arm/boot/dts/kirkwood-nas220.dts line 166
Quoted 9 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +8
-0+}; + +ð0 { + status = "okay"; + ethernet0-port at 0 { + phy-handle = <ðphy0>; + }; +};
Overall, this look fine to me, with the nits taken care of
Thanks!
Re: [PATCH v3 1/1] add support for Seagate BlackArmor NAS220
Quoted 10 lines · Sebastian Hesselbarth
arch/arm/boot/dts/kirkwood-nas220.dts line 77
Quoted 6 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +6
-0+ status = "okay"; + }; + + sata at 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
Quoted 12 lines · Andrew Lunn
Quoted 10 lines · Sebastian Hesselbarth
arch/arm/boot/dts/kirkwood-nas220.dts line 77
Quoted 6 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +6
-0+ status = "okay"; + }; + + sata at 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.
Quoted 3 lines
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.
Quoted 6 lines
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.
Quoted 6 lines
Optional Properties:
- phys : List of phandles to sata phys
- phy-names : Should be "0", "1", etc, one number per phandleThe 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
Quoted 28 lines
Quoted 3 lines
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.
Quoted 6 lines
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.Quoted 6 lines
Optional Properties:
- phys : List of phandles to sata phys
- phy-names : Should be "0", "1", etc, one number per phandleThe 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
Quoted 4 lines
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
Quoted 28 lines · Andrew Lunn
Quoted 28 lines
Quoted 3 lines
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.
Quoted 6 lines
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.Quoted 6 lines
Optional Properties:
- phys : List of phandles to sata phys
- phy-names : Should be "0", "1", etc, one number per phandleThe 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
Quoted 6 lines
Quoted 4 lines
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.
Quoted 6 lines
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
In reply to Sebastian HesselbarthHello Sebastian,
Quoted 15 lines · Sebastian Hesselbarth
Quoted 14 lines · 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.
Quoted 8 lines
Quoted 1 line
Signed-off-by Evgeni Dobrev <[email protected]>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.
Quoted 258 lines
Quoted 11 lines
--- 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.dtsarch/arm/boot/dts/Makefile +0
-0arch/arm/boot/dts/Makefile dtb-$(CONFIG_MACH_KIRKWOOD) += kirkwood-b3.dtb \ · line 135
Quoted 4 lines
arch/arm/boot/dts/Makefile +1
-0@@ -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" ?
Quoted 16 lines
arch/arm/boot/dts/Makefile +0
-0kirkwood-net2big.dtb \ kirkwood-net5big.dtb \ kirkwood-netgear_readynas_duo_v2.dtb \arch/arm/boot/dts/kirkwood-nas220.dts new file +7
-0@@ -0,0 +1,166 @@ +/* + * Device Tree file for Seagate BlackArmor NAS220 + * + * Copyright (C) 2014 Evgeni Dobrev <[email protected]> + * + * Licensed under GPLv2 or later. + */arch/arm/boot/dts/kirkwood-nas220.dts line 17
Quoted 10 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +10
-0+ +/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.
arch/arm/boot/dts/kirkwood-nas220.dts line 18
Quoted 1 line
arch/arm/boot/dts/kirkwood-nas220.dts new file +1
-0+ compatible = "seagate,nas220","marvell,kirkwood-88f6192","marvell,kirkwood";compatible should reflect the chosen base name above.
arch/arm/boot/dts/kirkwood-nas220.dts line 31
Quoted 13 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +13
-0+ + memory { /* 128 MB */ + device_type = "memory"; + reg = <0x00000000 0x8000000>; + }; + + chosen { + bootargs = "console=ttyS0,115200n8"; + stdout-path = &uart0; + }; + + ocp at f1000000 { + pinctrl: pin-controller at 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.
arch/arm/boot/dts/kirkwood-nas220.dts line 40
Quoted 9 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +9
-0+ 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_foonodes?Quoted 13 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +13
-0+ 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 {arch/arm/boot/dts/kirkwood-nas220.dts line 63
Quoted 10 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +10
-0+ marvell,pins = "mpp29"; + marvell,function = "gpio"; + }; + pmx_button_power: pmx-button-power { + marvell,pins = "mpp26"; + marvell,function = "gpio"; + }; + }; + +Remove extra blank line.
arch/arm/boot/dts/kirkwood-nas220.dts line 70
Quoted 7 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +7
-0+ /* + * 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?
arch/arm/boot/dts/kirkwood-nas220.dts line 71
Quoted 1 line
arch/arm/boot/dts/kirkwood-nas220.dts new file +1
-0+ serial at 12000 {Please use node references where possible, IIRC v3.19 should have labels for serial, sata, and i2c. Avoid to replay the bus structure.
arch/arm/boot/dts/kirkwood-nas220.dts line 77
Quoted 6 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +6
-0+ status = "okay"; + }; + + sata at 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.
arch/arm/boot/dts/kirkwood-nas220.dts line 82
Quoted 5 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +5
-0+ }; + + i2c at 11000 { + status = "okay"; + adt7476: adt7476a at 2e {I know we have a lot of bad examples, but: node names should reflect device function, not device name, i.e.
adt7476: thermal at 2e { ... };arch/arm/boot/dts/kirkwood-nas220.dts line 95
Quoted 13 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +13
-0+ 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.
arch/arm/boot/dts/kirkwood-nas220.dts line 97
Quoted 2 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +2
-0+ button at 1{ + label = "Reset push button";Reduce label to "Reset" and "Power" below.
arch/arm/boot/dts/kirkwood-nas220.dts line 109
Quoted 12 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +12
-0+ linux,code = <KEY_POWER>; + gpios = <&gpio0 29 GPIO_ACTIVE_HIGH>; + }; + button at 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.
Quoted 14 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +14
-0+ 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"; +arch/arm/boot/dts/kirkwood-nas220.dts line 133
Quoted 10 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +10
-0+ sata0_power: regulator at 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.
arch/arm/boot/dts/kirkwood-nas220.dts line 145
Quoted 12 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +12
-0+ }; + + sata1_power: regulator at 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.
arch/arm/boot/dts/kirkwood-nas220.dts line 158
Quoted 13 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +13
-0+ }; + }; +}; + +&nand { + status = "okay"; +}; + +&mdio { + status = "okay"; + ethphy0: ethernet-phy at 8 { + reg = <8>; + };Remove extra space before brace.
arch/arm/boot/dts/kirkwood-nas220.dts line 166
Quoted 9 lines
arch/arm/boot/dts/kirkwood-nas220.dts new file +8
-0+}; + +ð0 { + status = "okay"; + ethernet0-port at 0 { + phy-handle = <ðphy0>; + }; +};Overall, this look fine to me, with the nits taken care of
Quoted 4 lines
Thanks!
Thanks,
evgeni
v4 cover add support for Seagate BlackArmor NAS220
In reply to Evgeni Dobrevv4: 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
Signature · 1 line
1.7.10.4
PATCH v4 1/1 add support for Seagate BlackArmor NAS220
In reply to Evgeni DobrevThis 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.
--- 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
arch/arm/boot/dts/Makefile +1 -0
@@ -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 \
arch/arm/boot/dts/kirkwood-blackarmor-nas220.dts new file +172 -0
172 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Re: [PATCH v4 1/1] add support for Seagate BlackArmor NAS220
Quoted 7 lines · 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).
Quoted 8 lines
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 <[email protected]>
Hi Evgeni
I added this to mvebu/dt and will submit it for the next merge window.
Thanks
Andrew