| Message ID | 87bo1tmibv.fsf@natisbad.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Nov 09, 2013 at 09:27:32PM +0100, Arnaud Ebalard wrote: > > All hardware parts of the (mv78230 Armada XP based) NETGEAR ReadyNAS > 2120 are supported by mainline kernel (USB 3.0 and eSATA rear ports, > USB 2.0 front port, Gigabit controller and PHYs for the two rear ports, > serial port, LEDs, Buttons, 88SE9170 SATA controllers, three G762 fan > controllers, G751 temperature sensor) except for: > + i2c@11000 { > + compatible = "marvell,mv64xxx-i2c"; > + clock-frequency = <400000>; > + status = "okay"; > + > + /* Rear fan #1 of 3 (Protechnic MGT4012XB-O20, > + * 8000RPM) near eSATA port */ > + g762_fan1: g762@3e { > + compatible = "gmt,g762"; > + reg = <0x3e>; > + clocks = <&g762_clk>; /* input clock */ > + fan_gear_mode = <0>; > + fan_startv = <1>; > + pwm_polarity = <0>; > + }; > + > + /* Rear fan #2 of 3 at the center */ > + g762_fan2: g762@48 { > + compatible = "gmt,g762"; > + reg = <0x48>; > + clocks = <&g762_clk>; /* input clock */ > + fan_gear_mode = <0>; > + fan_startv = <1>; > + pwm_polarity = <0>; > + }; > + > + /* Rear fan #3 of 3 */ > + g762_fan3: g762@49 { > + compatible = "gmt,g762"; > + reg = <0x49>; > + clocks = <&g762_clk>; /* input clock */ > + fan_gear_mode = <0>; > + fan_startv = <1>; > + pwm_polarity = <0>; > + }; > + > + g751: g751@4c { > + compatible = "gmt,g751"; > + reg = <0x4c>; > + }; Hi Arnaud Looks good to me. However, if for some reason you need to respin, it would be nice to add a comment about what the g751 is. All the other i2c devices you say what they are. No need to respin just because of this. Andrew
Hi, Andrew Lunn <andrew@lunn.ch> writes: > On Sat, Nov 09, 2013 at 09:27:32PM +0100, Arnaud Ebalard wrote: >> >> All hardware parts of the (mv78230 Armada XP based) NETGEAR ReadyNAS >> 2120 are supported by mainline kernel (USB 3.0 and eSATA rear ports, >> USB 2.0 front port, Gigabit controller and PHYs for the two rear ports, >> serial port, LEDs, Buttons, 88SE9170 SATA controllers, three G762 fan >> controllers, G751 temperature sensor) except for: > >> + i2c@11000 { >> + compatible = "marvell,mv64xxx-i2c"; >> + clock-frequency = <400000>; >> + status = "okay"; >> + >> + /* Rear fan #1 of 3 (Protechnic MGT4012XB-O20, >> + * 8000RPM) near eSATA port */ >> + g762_fan1: g762@3e { >> + compatible = "gmt,g762"; >> + reg = <0x3e>; >> + clocks = <&g762_clk>; /* input clock */ >> + fan_gear_mode = <0>; >> + fan_startv = <1>; >> + pwm_polarity = <0>; >> + }; >> + >> + /* Rear fan #2 of 3 at the center */ >> + g762_fan2: g762@48 { >> + compatible = "gmt,g762"; >> + reg = <0x48>; >> + clocks = <&g762_clk>; /* input clock */ >> + fan_gear_mode = <0>; >> + fan_startv = <1>; >> + pwm_polarity = <0>; >> + }; >> + >> + /* Rear fan #3 of 3 */ >> + g762_fan3: g762@49 { >> + compatible = "gmt,g762"; >> + reg = <0x49>; >> + clocks = <&g762_clk>; /* input clock */ >> + fan_gear_mode = <0>; >> + fan_startv = <1>; >> + pwm_polarity = <0>; >> + }; >> + >> + g751: g751@4c { >> + compatible = "gmt,g751"; >> + reg = <0x4c>; >> + }; > > Hi Arnaud > > Looks good to me. However, if for some reason you need to respin, it > would be nice to add a comment about what the g751 is. All the other > i2c devices you say what they are. No need to respin just because of > this. I'll keep that in mind. Thanks. GMT G751 is an I2C temperature sensor and thermal watchdog chip. It is basically a clone of National Semiconductor LM75 chip: root@thin:/sys# sensors g762-i2c-0-3e Adapter: mv64xxx_i2c adapter fan1: 5461 RPM (div = 1) g762-i2c-0-48 Adapter: mv64xxx_i2c adapter fan1: 5461 RPM (div = 1) g762-i2c-0-49 Adapter: mv64xxx_i2c adapter fan1: 5461 RPM (div = 1) g751-i2c-0-4c Adapter: mv64xxx_i2c adapter temp1: +30.5°C (high = +80.0°C, hyst = +75.0°C) armada_thermal-virtual-0 Adapter: Virtual device temp1: +34.2°C Cheers, a+
On 11/09/2013 09:27 PM, Arnaud Ebalard wrote: > All hardware parts of the (mv78230 Armada XP based) NETGEAR ReadyNAS > 2120 are supported by mainline kernel (USB 3.0 and eSATA rear ports, > USB 2.0 front port, Gigabit controller and PHYs for the two rear ports, > serial port, LEDs, Buttons, 88SE9170 SATA controllers, three G762 fan > controllers, G751 temperature sensor) except for: > > - the Intersil ISL12057 I2C RTC Chip, > - the Armada NAND controller. > > Support for both of those is currently work in progress and does not > prevent boot. > > Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > --- [...] > arch/arm/boot/dts/Makefile | 1 + > arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 289 +++++++++++++++++++++++++ Arnaud, thanks for providing this! I do have some comments below. we recently had a discussion about the naming of new DTS files, which proposes to name those after vendor,board.dts (or vendor-board.dts). I personally prefer vendor,board.dts which would give netgear,readynas-2120.dts based on your compatible below. The final call for ',' vs '-' has not been made, so I suggest Jason makes a call here. [...] > diff --git a/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts b/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts > new file mode 100644 > index 0000000..2651ba3 > --- /dev/null > +++ b/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts > @@ -0,0 +1,289 @@ > +/* > + * Device Tree file for NETGEAR ReadyNAS 2120 > + * > + * Copyright (C) 2013, Arnaud EBALARD <arno@natisbad.org> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * as published by the Free Software Foundation; either version > + * 2 of the License, or (at your option) any later version. > + */ > + > +/dts-v1/; > + > +#include "armada-xp-mv78230.dtsi" > + > +/ { > + model = "NETGEAR ReadyNAS 2120"; > + compatible = "netgear,readynas-2120", "marvell,armadaxp-mv78230", "marvell,armadaxp", "marvell,armada-370-xp"; > + > + chosen { > + bootargs = "console=ttyS0,115200 earlyprintk"; > + }; > + > + memory { > + device_type = "memory"; > + reg = <0 0x00000000 0 0x80000000>; /* 2GB */ > + }; > + > + soc { > + ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xd0000000 0x100000 > + MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000>; > + [...] > + internal-regs { > + pinctrl { > + poweroff: poweroff { > + marvell,pins = "mpp42"; > + marvell,function = "gpio"; > + }; > + > + power_key_pin: power-key-pin { > + marvell,pins = "mpp27"; > + marvell,function = "gpio"; > + }; > + > + reset_key_pin: reset-key-pin { > + marvell,pins = "mpp41"; > + marvell,function = "gpio"; > + }; > + > + sata1_led_pin: sata1-led-pin { > + marvell,pins = "mpp31"; > + marvell,function = "gpio"; > + }; > + > + sata2_led_pin: sata2-led-pin { > + marvell,pins = "mpp40"; > + marvell,function = "gpio"; > + }; > + > + sata3_led_pin: sata3-led-pin { > + marvell,pins = "mpp44"; > + marvell,function = "gpio"; > + }; > + > + sata4_led_pin: sata4-led-pin { > + marvell,pins = "mpp47"; > + marvell,function = "gpio"; > + }; > + > + sata1_power_pin: sata1-power-pin { > + marvell,pins = "mpp24"; > + marvell,function = "gpio"; > + }; > + > + sata2_power_pin: sata2-power-pin { > + marvell,pins = "mpp25"; > + marvell,function = "gpio"; > + }; > + > + sata3_power_pin: sata3-power-pin { > + marvell,pins = "mpp26"; > + marvell,function = "gpio"; > + }; > + > + sata4_power_pin: sata4-power-pin { > + marvell,pins = "mpp28"; > + marvell,function = "gpio"; > + }; > + > + sata1_pres_pin: sata1-pres-pin { > + marvell,pins = "mpp32"; > + marvell,function = "gpio"; > + }; > + > + sata2_pres_pin: sata2-pres-pin { > + marvell,pins = "mpp33"; > + marvell,function = "gpio"; > + }; > + > + sata3_pres_pin: sata3-pres-pin { > + marvell,pins = "mpp34"; > + marvell,function = "gpio"; > + }; > + > + sata4_pres_pin: sata4-pres-pin { > + marvell,pins = "mpp35"; > + marvell,function = "gpio"; > + }; > + > + err_led_pin: err-led-pin { > + marvell,pins = "mpp45"; > + marvell,function = "gpio"; > + }; > + }; > + > + serial@12000 { > + clock-frequency = <250000000>; Where does that 250MHz come from? If it is an internal clock and we already have a DT clock provider for it, please use clocks = <&provider num>; If it is TCLK it can be optained from <&coreclk 0>. > + status = "okay"; > + }; > + > + mdio { > + phy0: ethernet-phy@0 { > + reg = <0>; Can you evaluate PHYs compatible from u-boot or post vendor:prodid from MDIO registers? > + }; > + > + phy1: ethernet-phy@1 { > + reg = <1>; > + }; > + }; > + > + ethernet@70000 { > + status = "okay"; > + phy = <&phy0>; > + phy-mode = "rgmii-id"; > + }; > + > + ethernet@74000 { > + status = "okay"; > + phy = <&phy1>; > + phy-mode = "rgmii-id"; > + }; > + > + /* Front USB 2.0 port */ > + usb@50000 { > + status = "okay"; > + }; > + > + i2c@11000 { > + compatible = "marvell,mv64xxx-i2c"; > + clock-frequency = <400000>; > + status = "okay"; > + > + /* Rear fan #1 of 3 (Protechnic MGT4012XB-O20, > + * 8000RPM) near eSATA port */ > + g762_fan1: g762@3e { > + compatible = "gmt,g762"; > + reg = <0x3e>; > + clocks = <&g762_clk>; /* input clock */ > + fan_gear_mode = <0>; > + fan_startv = <1>; > + pwm_polarity = <0>; > + }; > + > + /* Rear fan #2 of 3 at the center */ > + g762_fan2: g762@48 { > + compatible = "gmt,g762"; > + reg = <0x48>; > + clocks = <&g762_clk>; /* input clock */ > + fan_gear_mode = <0>; > + fan_startv = <1>; > + pwm_polarity = <0>; > + }; > + > + /* Rear fan #3 of 3 */ > + g762_fan3: g762@49 { > + compatible = "gmt,g762"; > + reg = <0x49>; > + clocks = <&g762_clk>; /* input clock */ > + fan_gear_mode = <0>; > + fan_startv = <1>; > + pwm_polarity = <0>; > + }; > + > + g751: g751@4c { > + compatible = "gmt,g751"; > + reg = <0x4c>; > + }; > + }; > + }; > + }; > + > + clocks { > + #address-cells = <1>; > + #size-cells = <0>; Your nodes below neither have a reg property nor can they be addressed in any way. Both properties above are not required, so please remove them. > + g762_clk: fixedclk { s/fixedclk/oscillator/ ? > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32768>; > + }; > + }; > + > + gpio_leds { > + compatible = "gpio-leds"; > + pinctrl-0 = <&sata1_led_pin &sata2_led_pin &err_led_pin > + &sata3_led_pin &sata4_led_pin>; > + pinctrl-names = "default"; > + > + red_sata1_led { > + label = "rn2120:red:sata1"; > + gpios = <&gpio0 31 0>; /* GPIO 31 Active High */ You can #include <dt-bindings/gpio/gpio.h> then use gpios = <&gpio0 31 GPIO_ACTIVE_HIGH> and get rid of the comments. > + default-state = "off"; > + }; > + > + red_sata2_led { > + label = "rn2120:red:sata2"; > + gpios = <&gpio1 8 0>; /* GPIO 40 Active High */ > + default-state = "off"; > + }; > + > + red_sata3_led { > + label = "rn2120:red:sata3"; > + gpios = <&gpio1 12 0>; /* GPIO 44 Active High */ > + default-state = "off"; > + }; > + > + red_sata4_led { > + label = "rn2120:red:sata4"; > + gpios = <&gpio1 15 0>; /* GPIO 47 Active High */ > + default-state = "off"; > + }; > + > + red_err_led { > + label = "rn2120:red:err"; > + gpios = <&gpio1 13 1>; /* GPIO 45 Active Low */ > + default-state = "off"; > + }; > + }; > + > + gpio_keys { > + compatible = "gpio-keys"; > + #address-cells = <1>; > + #size-cells = <0>; Again, no addressing of those buttons possible. > + pinctrl-0 = <&power_key_pin > + &reset_key_pin>; > + pinctrl-names = "default"; > + > + button@1 { Instead of button@n you can name the nodes after their function, e.g. power_button, reset_button, ... I know both comments above are in the current binding documentation (at least for gpio-keys-polled.txt, there is no gpio-keys.txt); but IMHO both "bus-like" structure of gpio_keys and numbering of buttons just looks so wrong. Sebastian > + label = "Power Button"; > + linux,code = <116>; /* KEY_POWER */ > + gpios = <&gpio0 27 0>; > + }; > + > + button@2 { > + label = "Reset Button"; > + linux,code = <0x198>; /* KEY_RESTART */ > + gpios = <&gpio1 9 1>; > + }; > + }; > + > + gpio_poweroff { > + compatible = "gpio-poweroff"; > + pinctrl-0 = <&poweroff>; > + pinctrl-names = "default"; > + gpios = <&gpio1 10 1>; > + }; > +}; >
Hi Sebastian, Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> writes: > On 11/09/2013 09:27 PM, Arnaud Ebalard wrote: >> All hardware parts of the (mv78230 Armada XP based) NETGEAR ReadyNAS >> 2120 are supported by mainline kernel (USB 3.0 and eSATA rear ports, >> USB 2.0 front port, Gigabit controller and PHYs for the two rear ports, >> serial port, LEDs, Buttons, 88SE9170 SATA controllers, three G762 fan >> controllers, G751 temperature sensor) except for: >> >> - the Intersil ISL12057 I2C RTC Chip, >> - the Armada NAND controller. >> >> Support for both of those is currently work in progress and does not >> prevent boot. >> >> Signed-off-by: Arnaud Ebalard <arno@natisbad.org> >> --- > [...] >> arch/arm/boot/dts/Makefile | 1 + >> arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 289 +++++++++++++++++++++++++ > > Arnaud, > > thanks for providing this! I do have some comments below. > > we recently had a discussion about the naming of new DTS files, which > proposes to name those after vendor,board.dts (or vendor-board.dts). > > I personally prefer vendor,board.dts which would give > netgear,readynas-2120.dts based on your compatible below. > The final call for ',' vs '-' has not been made, so I suggest Jason > makes a call here. ok, will wait for a decision before resending. I a am not in a hurry. >> + >> + serial@12000 { >> + clock-frequency = <250000000>; > > Where does that 250MHz come from? If it is an internal clock > and we already have a DT clock provider for it, please use > clocks = <&provider num>; > > If it is TCLK it can be optained from <&coreclk 0>. Well, no innovation here: to be honest - it's not a valid argument but - I copy pasted the line from other armada-xp-*.dts files: $ grep 250000000 armada-xp-*.dts armada-xp-axpwifiap.dts: clock-frequency = <250000000>; armada-xp-axpwifiap.dts: clock-frequency = <250000000>; armada-xp-db.dts: clock-frequency = <250000000>; armada-xp-db.dts: clock-frequency = <250000000>; armada-xp-db.dts: clock-frequency = <250000000>; armada-xp-db.dts: clock-frequency = <250000000>; armada-xp-gp.dts: clock-frequency = <250000000>; armada-xp-gp.dts: clock-frequency = <250000000>; armada-xp-gp.dts: clock-frequency = <250000000>; armada-xp-gp.dts: clock-frequency = <250000000>; armada-xp-openblocks-ax3-4.dts: clock-frequency = <250000000>; armada-xp-openblocks-ax3-4.dts: clock-frequency = <250000000>; I will use <&coreclk 0> in next resend. > >> + status = "okay"; >> + }; >> + >> + mdio { >> + phy0: ethernet-phy@0 { >> + reg = <0>; > > Can you evaluate PHYs compatible from u-boot or post vendor:prodid > from MDIO registers? Looking at the PCB, 88E1318-NNB2. I will dig into Documentation/ the associated compatible. Out of curiosity, how do you get the PHY model from u-boot or userland? >> + >> + clocks { >> + #address-cells = <1>; >> + #size-cells = <0>; > > Your nodes below neither have a reg property nor can they be > addressed in any way. I think I do not understand that comment. Can you clarify, possibly w/ an example of you think is wrong and how it could be corrected? > Both properties above are not required, so please remove them. > >> + g762_clk: fixedclk { > > s/fixedclk/oscillator/ ? > >> + compatible = "fixed-clock"; >> + #clock-cells = <0>; >> + clock-frequency = <32768>; >> + }; >> + }; >> + Something like the following? clocks { g762_clk: oscillator { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <32768>; }; }; I think I could then backport that change to all readynas .dts and the Documentation/devicetree/bindings/hwmon/g762.txt at some point. >> + gpio_leds { >> + compatible = "gpio-leds"; >> + pinctrl-0 = <&sata1_led_pin &sata2_led_pin &err_led_pin >> + &sata3_led_pin &sata4_led_pin>; >> + pinctrl-names = "default"; >> + >> + red_sata1_led { >> + label = "rn2120:red:sata1"; >> + gpios = <&gpio0 31 0>; /* GPIO 31 Active High */ > > You can > > #include <dt-bindings/gpio/gpio.h> > > then use > > gpios = <&gpio0 31 GPIO_ACTIVE_HIGH> > > and get rid of the comments. ok, will do. >> + default-state = "off"; >> + }; >> + >> + red_sata2_led { >> + label = "rn2120:red:sata2"; >> + gpios = <&gpio1 8 0>; /* GPIO 40 Active High */ >> + default-state = "off"; >> + }; >> + >> + red_sata3_led { >> + label = "rn2120:red:sata3"; >> + gpios = <&gpio1 12 0>; /* GPIO 44 Active High */ >> + default-state = "off"; >> + }; >> + >> + red_sata4_led { >> + label = "rn2120:red:sata4"; >> + gpios = <&gpio1 15 0>; /* GPIO 47 Active High */ >> + default-state = "off"; >> + }; >> + >> + red_err_led { >> + label = "rn2120:red:err"; >> + gpios = <&gpio1 13 1>; /* GPIO 45 Active Low */ >> + default-state = "off"; >> + }; >> + }; >> + >> + gpio_keys { >> + compatible = "gpio-keys"; >> + #address-cells = <1>; >> + #size-cells = <0>; > > Again, no addressing of those buttons possible. > >> + pinctrl-0 = <&power_key_pin >> + &reset_key_pin>; >> + pinctrl-names = "default"; >> + >> + button@1 { > > Instead of button@n you can name the nodes after their function, > e.g. power_button, reset_button, ... ok, will change that. > I know both comments above are in the current binding documentation > (at least for gpio-keys-polled.txt, there is no gpio-keys.txt); but > IMHO both "bus-like" structure of gpio_keys and numbering of buttons > just looks so wrong. Thanks for your feedback. Cheers, a+
On 11/10/2013 01:42 AM, Arnaud Ebalard wrote: > Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> writes: >> On 11/09/2013 09:27 PM, Arnaud Ebalard wrote: >>> All hardware parts of the (mv78230 Armada XP based) NETGEAR ReadyNAS >>> 2120 are supported by mainline kernel (USB 3.0 and eSATA rear ports, >>> USB 2.0 front port, Gigabit controller and PHYs for the two rear ports, >>> serial port, LEDs, Buttons, 88SE9170 SATA controllers, three G762 fan >>> controllers, G751 temperature sensor) except for: >>> >>> - the Intersil ISL12057 I2C RTC Chip, >>> - the Armada NAND controller. >>> >>> Support for both of those is currently work in progress and does not >>> prevent boot. >>> >>> Signed-off-by: Arnaud Ebalard <arno@natisbad.org> >>> --- >> [...] >>> arch/arm/boot/dts/Makefile | 1 + >>> arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 289 +++++++++++++++++++++++++ >> >> Arnaud, >> >> thanks for providing this! I do have some comments below. >> >> we recently had a discussion about the naming of new DTS files, which >> proposes to name those after vendor,board.dts (or vendor-board.dts). >> >> I personally prefer vendor,board.dts which would give >> netgear,readynas-2120.dts based on your compatible below. >> The final call for ',' vs '-' has not been made, so I suggest Jason >> makes a call here. > > ok, will wait for a decision before resending. I a am not in a hurry. > >>> + >>> + serial@12000 { >>> + clock-frequency = <250000000>; >> >> Where does that 250MHz come from? If it is an internal clock >> and we already have a DT clock provider for it, please use >> clocks = <&provider num>; >> >> If it is TCLK it can be optained from <&coreclk 0>. > > Well, no innovation here: to be honest - it's not a valid argument but - > I copy pasted the line from other armada-xp-*.dts files: > > $ grep 250000000 armada-xp-*.dts > armada-xp-axpwifiap.dts: clock-frequency = <250000000>; > armada-xp-axpwifiap.dts: clock-frequency = <250000000>; > armada-xp-db.dts: clock-frequency = <250000000>; > armada-xp-db.dts: clock-frequency = <250000000>; > armada-xp-db.dts: clock-frequency = <250000000>; > armada-xp-db.dts: clock-frequency = <250000000>; > armada-xp-gp.dts: clock-frequency = <250000000>; > armada-xp-gp.dts: clock-frequency = <250000000>; > armada-xp-gp.dts: clock-frequency = <250000000>; > armada-xp-gp.dts: clock-frequency = <250000000>; > armada-xp-openblocks-ax3-4.dts: clock-frequency = <250000000>; > armada-xp-openblocks-ax3-4.dts: clock-frequency = <250000000>; > > I will use <&coreclk 0> in next resend. Yeah, I never said that existing dts are perfect ;) If you feel like fixing this, I suggest to put clocks property in SoC dtsi. I am sure Gregory can confirm this, but there is no other clock possible for Armada 370/XP's serial IP. But anyway, I checked drivers/clk/mvebu/armada-xp.c and 250MHz is TCLK which can be referenced as <&coreclk 0>. >>> + status = "okay"; >>> + }; >>> + >>> + mdio { >>> + phy0: ethernet-phy@0 { >>> + reg = <0>; >> >> Can you evaluate PHYs compatible from u-boot or post vendor:prodid >> from MDIO registers? > > Looking at the PCB, 88E1318-NNB2. I will dig into Documentation/ the > associated compatible. Out of curiosity, how do you get the PHY model > from u-boot or userland? If it is 88e1318 the compatible is "marvell,88e1318" then. I usually look at u-boot messages which sometimes reveal the PHY used. If that is not sufficient, you can read PHY ID 1/2 (SMI address 2 and 3) registers and go looking for model numbers. >>> + >>> + clocks { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >> >> Your nodes below neither have a reg property nor can they be >> addressed in any way. > > I think I do not understand that comment. Can you clarify, possibly w/ > an example of you think is wrong and how it could be corrected? Above two properties define that reg properties below "clocks" node have 1 address cell and no size. As it is usually just an unsorted list of oscillators and clock generators and no "bus", there is no "addressing" of that devices. No addressing, no reg property, no #address-cells/#size-cells required. Sebastian >> Both properties above are not required, so please remove them. >> >>> + g762_clk: fixedclk { >> >> s/fixedclk/oscillator/ ? >> >>> + compatible = "fixed-clock"; >>> + #clock-cells = <0>; >>> + clock-frequency = <32768>; >>> + }; >>> + }; >>> + > > Something like the following? > > clocks { > g762_clk: oscillator { > compatible = "fixed-clock"; > #clock-cells = <0>; > clock-frequency = <32768>; > }; > }; > > I think I could then backport that change to all readynas .dts and > the Documentation/devicetree/bindings/hwmon/g762.txt at some point. Ah, it is just a name. No need to backport this. >>> + gpio_leds { >>> + compatible = "gpio-leds"; >>> + pinctrl-0 = <&sata1_led_pin &sata2_led_pin &err_led_pin >>> + &sata3_led_pin &sata4_led_pin>; >>> + pinctrl-names = "default"; >>> + >>> + red_sata1_led { >>> + label = "rn2120:red:sata1"; >>> + gpios = <&gpio0 31 0>; /* GPIO 31 Active High */ >> >> You can >> >> #include <dt-bindings/gpio/gpio.h> >> >> then use >> >> gpios = <&gpio0 31 GPIO_ACTIVE_HIGH> >> >> and get rid of the comments. > > ok, will do. > > >>> + default-state = "off"; >>> + }; >>> + >>> + red_sata2_led { >>> + label = "rn2120:red:sata2"; >>> + gpios = <&gpio1 8 0>; /* GPIO 40 Active High */ >>> + default-state = "off"; >>> + }; >>> + >>> + red_sata3_led { >>> + label = "rn2120:red:sata3"; >>> + gpios = <&gpio1 12 0>; /* GPIO 44 Active High */ >>> + default-state = "off"; >>> + }; >>> + >>> + red_sata4_led { >>> + label = "rn2120:red:sata4"; >>> + gpios = <&gpio1 15 0>; /* GPIO 47 Active High */ >>> + default-state = "off"; >>> + }; >>> + >>> + red_err_led { >>> + label = "rn2120:red:err"; >>> + gpios = <&gpio1 13 1>; /* GPIO 45 Active Low */ >>> + default-state = "off"; >>> + }; >>> + }; >>> + >>> + gpio_keys { >>> + compatible = "gpio-keys"; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >> >> Again, no addressing of those buttons possible. >> >>> + pinctrl-0 = <&power_key_pin >>> + &reset_key_pin>; >>> + pinctrl-names = "default"; >>> + >>> + button@1 { >> >> Instead of button@n you can name the nodes after their function, >> e.g. power_button, reset_button, ... > > ok, will change that. > >> I know both comments above are in the current binding documentation >> (at least for gpio-keys-polled.txt, there is no gpio-keys.txt); but >> IMHO both "bus-like" structure of gpio_keys and numbering of buttons >> just looks so wrong.
Dear Sebastian Hesselbarth, On Sun, 10 Nov 2013 00:57:17 +0100, Sebastian Hesselbarth wrote: > > arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 289 +++++++++++++++++++++++++ > > Arnaud, > > thanks for providing this! I do have some comments below. > > we recently had a discussion about the naming of new DTS files, which > proposes to name those after vendor,board.dts (or vendor-board.dts). > > I personally prefer vendor,board.dts which would give > netgear,readynas-2120.dts based on your compatible below. > The final call for ',' vs '-' has not been made, so I suggest Jason > makes a call here. Hum, do we really want that? The current naming scheme is really, really great, because whenever you want to grep through all Armada 370/XP DTS, you just have to use armada-*, for all Kirkwood, kirkwood-*. I've found this very very convenient to verify the coherency between .dts/.dtsi for a given family of SoCs. Thomas
Hi, I thought it would be a simple update but one of your comment gave me a run for my money ;-) Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> writes: >>>> + status = "okay"; >>>> + }; >>>> + >>>> + mdio { >>>> + phy0: ethernet-phy@0 { >>>> + reg = <0>; >>> >>> Can you evaluate PHYs compatible from u-boot or post vendor:prodid >>> from MDIO registers? >> >> Looking at the PCB, 88E1318-NNB2. I will dig into Documentation/ the >> associated compatible. Out of curiosity, how do you get the PHY model >> from u-boot or userland? > > If it is 88e1318 the compatible is "marvell,88e1318" then. I usually > look at u-boot messages which sometimes reveal the PHY used. If that > is not sufficient, you can read PHY ID 1/2 (SMI address 2 and 3) > registers and go looking for model numbers. For the records, another solution is to look under /sys/: root@thin:/sys/bus/mdio_bus/drivers# find -name '*mdio*' ./Marvell 88E1318S/d0072004.mdio-mi:00 ./Marvell 88E1318S/d0072004.mdio-mi:01 I guess the driver just looks at the PHY ID to detect the flavour. Now regarding the compatible string you propose, did some grep calls and was unable to find a reference to marvell,88e1318 or even 88e138 and was unable to find any. How does the kernel do something with it to somehow inflect the selection of the right driver/flavour? >>> Both properties above are not required, so please remove them. >>> >>>> + g762_clk: fixedclk { >>> >>> s/fixedclk/oscillator/ ? In fact, if I change 'fixedclk' for 'oscillator', then I am unable to boot: my SATA disk get unrecognized in a storm of insane messages. Obviously, because I had modified the whole .dts to handle all the points in your revieuw, I spent some time finding the root cause. But that was interesting on many aspects and the continuation of a debugging week end ;-) Can you confirm the following: g762_clk is a *label for the node*, which is referenced via specific properties of g762@xx nodes, so that they get a clock frequency (a property of g762_clk). And fixedclk is *the name of the node*. In that specific case, fixedclk is ok because it does not collide w/ any existing node. But 'oscillator' is already defined in armada-xp.dtsi (included via armada-370-xp.dtsi): clocks { /* 25 MHz reference crystal */ refclk: oscillator { compatible = "fixed-clock"; #clock-cells = <0>; clock-frequency = <25000000>; }; }; So I guess the renaming I did from 'fixedclk' to 'oscillator' just somehow overloaded the armada-xp.dtsi 25Mhz clock for a 32KHz one, hence the inability to boot. If I am correct, I guess it would be nice to have some checks from dtc when compiling the .dts to verify the uniqueness of nodes in order to avoid such things. Comments welcome. Cheers, a+
On 11/11/2013 01:32 AM, Arnaud Ebalard wrote: > Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com> writes: >>>>> + status = "okay"; >>>>> + }; >>>>> + >>>>> + mdio { >>>>> + phy0: ethernet-phy@0 { >>>>> + reg = <0>; >>>> >>>> Can you evaluate PHYs compatible from u-boot or post vendor:prodid >>>> from MDIO registers? >>> >>> Looking at the PCB, 88E1318-NNB2. I will dig into Documentation/ the >>> associated compatible. Out of curiosity, how do you get the PHY model >>> from u-boot or userland? >> >> If it is 88e1318 the compatible is "marvell,88e1318" then. I usually >> look at u-boot messages which sometimes reveal the PHY used. If that >> is not sufficient, you can read PHY ID 1/2 (SMI address 2 and 3) >> registers and go looking for model numbers. > > For the records, another solution is to look under /sys/: > > root@thin:/sys/bus/mdio_bus/drivers# find -name '*mdio*' > ./Marvell 88E1318S/d0072004.mdio-mi:00 > ./Marvell 88E1318S/d0072004.mdio-mi:01 Well, the compatible is "marvell,88e1318s" then. > I guess the driver just looks at the PHY ID to detect the flavour. > > Now regarding the compatible string you propose, did some grep calls > and was unable to find a reference to marvell,88e1318 or even 88e138 and > was unable to find any. How does the kernel do something with it to > somehow inflect the selection of the right driver/flavour? You are right, it uses PHYID registers. The compatible is not used by the driver. Anyway, it is nice to have it in there. >>>> Both properties above are not required, so please remove them. >>>> >>>>> + g762_clk: fixedclk { >>>> >>>> s/fixedclk/oscillator/ ? > > In fact, if I change 'fixedclk' for 'oscillator', then I am unable to > boot: my SATA disk get unrecognized in a storm of insane > messages. Obviously, because I had modified the whole .dts to handle all > the points in your revieuw, I spent some time finding the root > cause. But that was interesting on many aspects and the continuation of > a debugging week end ;-) Ah, sorry for that. You can try to make the node name unique by either choosing a different meaningful name or try to add @1 after the node name. G762 datasheet might have a good name for it, like clock-crystal or something. > Can you confirm the following: g762_clk is a *label for the node*, which > is referenced via specific properties of g762@xx nodes, so that they get > a clock frequency (a property of g762_clk). And fixedclk is *the name of > the node*. In that specific case, fixedclk is ok because it does not > collide w/ any existing node. But 'oscillator' is already defined in > armada-xp.dtsi (included via armada-370-xp.dtsi): > > clocks { > /* 25 MHz reference crystal */ > refclk: oscillator { > compatible = "fixed-clock"; > #clock-cells = <0>; > clock-frequency = <25000000>; > }; > }; Confirmed. > So I guess the renaming I did from 'fixedclk' to 'oscillator' just > somehow overloaded the armada-xp.dtsi 25Mhz clock for a 32KHz one, hence > the inability to boot. If I am correct, I guess it would be nice to have > some checks from dtc when compiling the .dts to verify the uniqueness of > nodes in order to avoid such things. Hmm, that's difficult because dtc cannot know if you intentionally want to overwrite that node or I gave you the wrong name for it. The different node label could be a hint for the latter though. Sorry for the extra work that comment caused. Sebastian
On Sun, Nov 10, 2013 at 12:57:17AM +0100, Sebastian Hesselbarth wrote: > On 11/09/2013 09:27 PM, Arnaud Ebalard wrote: > >All hardware parts of the (mv78230 Armada XP based) NETGEAR ReadyNAS > >2120 are supported by mainline kernel (USB 3.0 and eSATA rear ports, > >USB 2.0 front port, Gigabit controller and PHYs for the two rear ports, > >serial port, LEDs, Buttons, 88SE9170 SATA controllers, three G762 fan > >controllers, G751 temperature sensor) except for: > > > > - the Intersil ISL12057 I2C RTC Chip, > > - the Armada NAND controller. > > > >Support for both of those is currently work in progress and does not > >prevent boot. > > > >Signed-off-by: Arnaud Ebalard <arno@natisbad.org> > >--- > [...] > > arch/arm/boot/dts/Makefile | 1 + > > arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 289 +++++++++++++++++++++++++ > > Arnaud, > > thanks for providing this! I do have some comments below. > > we recently had a discussion about the naming of new DTS files, which > proposes to name those after vendor,board.dts (or vendor-board.dts). > > I personally prefer vendor,board.dts which would give > netgear,readynas-2120.dts based on your compatible below. > The final call for ',' vs '-' has not been made, so I suggest Jason > makes a call here. It's my understanding that Olof's primary concern is for the naming of the dtbs. I might have some time today to spin a patch against dtc to create the symlink target that we kicked around: $ dtc -L -o kirkwood-dreamplug.dtb kirkwood-dreamplug.dts $ ls -l ... globalscale,dreamplug-003-ds2001.dtb -> kirkwood-dreamplug.dtb ... kirkwood-dreamplug.dtb kirkwood-dreamplug.dts Which would solve the problem neatly since the compatible string is globally unique, and refers to exactly one board. The dts naming can then remain whatever works best for us in the kernel without affecting userspace installers accessing the build products. In short, Arnaud, it's fine for the moment. Let's see if my patch gets shot down. ;-) One more comment near the bottom. I'm too lazy to trim today, sorry. :( > > [...] > >diff --git a/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts b/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts > >new file mode 100644 > >index 0000000..2651ba3 > >--- /dev/null > >+++ b/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts > >@@ -0,0 +1,289 @@ > >+/* > >+ * Device Tree file for NETGEAR ReadyNAS 2120 > >+ * > >+ * Copyright (C) 2013, Arnaud EBALARD <arno@natisbad.org> > >+ * > >+ * This program is free software; you can redistribute it and/or > >+ * modify it under the terms of the GNU General Public License > >+ * as published by the Free Software Foundation; either version > >+ * 2 of the License, or (at your option) any later version. > >+ */ > >+ > >+/dts-v1/; > >+ > >+#include "armada-xp-mv78230.dtsi" > >+ > >+/ { > >+ model = "NETGEAR ReadyNAS 2120"; > >+ compatible = "netgear,readynas-2120", "marvell,armadaxp-mv78230", "marvell,armadaxp", "marvell,armada-370-xp"; > >+ > >+ chosen { > >+ bootargs = "console=ttyS0,115200 earlyprintk"; > >+ }; > >+ > >+ memory { > >+ device_type = "memory"; > >+ reg = <0 0x00000000 0 0x80000000>; /* 2GB */ > >+ }; > >+ > >+ soc { > >+ ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xd0000000 0x100000 > >+ MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000>; > >+ > [...] > >+ internal-regs { > >+ pinctrl { > >+ poweroff: poweroff { > >+ marvell,pins = "mpp42"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ power_key_pin: power-key-pin { > >+ marvell,pins = "mpp27"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ reset_key_pin: reset-key-pin { > >+ marvell,pins = "mpp41"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata1_led_pin: sata1-led-pin { > >+ marvell,pins = "mpp31"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata2_led_pin: sata2-led-pin { > >+ marvell,pins = "mpp40"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata3_led_pin: sata3-led-pin { > >+ marvell,pins = "mpp44"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata4_led_pin: sata4-led-pin { > >+ marvell,pins = "mpp47"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata1_power_pin: sata1-power-pin { > >+ marvell,pins = "mpp24"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata2_power_pin: sata2-power-pin { > >+ marvell,pins = "mpp25"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata3_power_pin: sata3-power-pin { > >+ marvell,pins = "mpp26"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata4_power_pin: sata4-power-pin { > >+ marvell,pins = "mpp28"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata1_pres_pin: sata1-pres-pin { > >+ marvell,pins = "mpp32"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata2_pres_pin: sata2-pres-pin { > >+ marvell,pins = "mpp33"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata3_pres_pin: sata3-pres-pin { > >+ marvell,pins = "mpp34"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ sata4_pres_pin: sata4-pres-pin { > >+ marvell,pins = "mpp35"; > >+ marvell,function = "gpio"; > >+ }; > >+ > >+ err_led_pin: err-led-pin { > >+ marvell,pins = "mpp45"; > >+ marvell,function = "gpio"; > >+ }; > >+ }; > >+ > >+ serial@12000 { > >+ clock-frequency = <250000000>; > > Where does that 250MHz come from? If it is an internal clock > and we already have a DT clock provider for it, please use > clocks = <&provider num>; > > If it is TCLK it can be optained from <&coreclk 0>. > > >+ status = "okay"; > >+ }; > >+ > >+ mdio { > >+ phy0: ethernet-phy@0 { > >+ reg = <0>; > > Can you evaluate PHYs compatible from u-boot or post vendor:prodid > from MDIO registers? > > >+ }; > >+ > >+ phy1: ethernet-phy@1 { > >+ reg = <1>; > >+ }; > >+ }; > >+ > >+ ethernet@70000 { > >+ status = "okay"; > >+ phy = <&phy0>; > >+ phy-mode = "rgmii-id"; > >+ }; > >+ > >+ ethernet@74000 { > >+ status = "okay"; > >+ phy = <&phy1>; > >+ phy-mode = "rgmii-id"; > >+ }; > >+ > >+ /* Front USB 2.0 port */ > >+ usb@50000 { > >+ status = "okay"; > >+ }; > >+ > >+ i2c@11000 { > >+ compatible = "marvell,mv64xxx-i2c"; > >+ clock-frequency = <400000>; > >+ status = "okay"; > >+ > >+ /* Rear fan #1 of 3 (Protechnic MGT4012XB-O20, > >+ * 8000RPM) near eSATA port */ > >+ g762_fan1: g762@3e { > >+ compatible = "gmt,g762"; > >+ reg = <0x3e>; > >+ clocks = <&g762_clk>; /* input clock */ > >+ fan_gear_mode = <0>; > >+ fan_startv = <1>; > >+ pwm_polarity = <0>; > >+ }; > >+ > >+ /* Rear fan #2 of 3 at the center */ > >+ g762_fan2: g762@48 { > >+ compatible = "gmt,g762"; > >+ reg = <0x48>; > >+ clocks = <&g762_clk>; /* input clock */ > >+ fan_gear_mode = <0>; > >+ fan_startv = <1>; > >+ pwm_polarity = <0>; > >+ }; > >+ > >+ /* Rear fan #3 of 3 */ > >+ g762_fan3: g762@49 { > >+ compatible = "gmt,g762"; > >+ reg = <0x49>; > >+ clocks = <&g762_clk>; /* input clock */ > >+ fan_gear_mode = <0>; > >+ fan_startv = <1>; > >+ pwm_polarity = <0>; > >+ }; > >+ > >+ g751: g751@4c { > >+ compatible = "gmt,g751"; > >+ reg = <0x4c>; > >+ }; > >+ }; > >+ }; > >+ }; > >+ > >+ clocks { > >+ #address-cells = <1>; > >+ #size-cells = <0>; > > Your nodes below neither have a reg property nor can they be > addressed in any way. Both properties above are not required, > so please remove them. > > >+ g762_clk: fixedclk { > > s/fixedclk/oscillator/ ? > > >+ compatible = "fixed-clock"; > >+ #clock-cells = <0>; > >+ clock-frequency = <32768>; > >+ }; > >+ }; > >+ > >+ gpio_leds { > >+ compatible = "gpio-leds"; > >+ pinctrl-0 = <&sata1_led_pin &sata2_led_pin &err_led_pin > >+ &sata3_led_pin &sata4_led_pin>; > >+ pinctrl-names = "default"; > >+ > >+ red_sata1_led { > >+ label = "rn2120:red:sata1"; > >+ gpios = <&gpio0 31 0>; /* GPIO 31 Active High */ > > You can > > #include <dt-bindings/gpio/gpio.h> > > then use > > gpios = <&gpio0 31 GPIO_ACTIVE_HIGH> > > and get rid of the comments. > > >+ default-state = "off"; > >+ }; > >+ > >+ red_sata2_led { > >+ label = "rn2120:red:sata2"; > >+ gpios = <&gpio1 8 0>; /* GPIO 40 Active High */ > >+ default-state = "off"; > >+ }; > >+ > >+ red_sata3_led { > >+ label = "rn2120:red:sata3"; > >+ gpios = <&gpio1 12 0>; /* GPIO 44 Active High */ > >+ default-state = "off"; > >+ }; > >+ > >+ red_sata4_led { > >+ label = "rn2120:red:sata4"; > >+ gpios = <&gpio1 15 0>; /* GPIO 47 Active High */ > >+ default-state = "off"; > >+ }; > >+ > >+ red_err_led { > >+ label = "rn2120:red:err"; > >+ gpios = <&gpio1 13 1>; /* GPIO 45 Active Low */ > >+ default-state = "off"; > >+ }; > >+ }; > >+ > >+ gpio_keys { > >+ compatible = "gpio-keys"; > >+ #address-cells = <1>; > >+ #size-cells = <0>; > > Again, no addressing of those buttons possible. > > >+ pinctrl-0 = <&power_key_pin > >+ &reset_key_pin>; > >+ pinctrl-names = "default"; > >+ > >+ button@1 { > > Instead of button@n you can name the nodes after their function, > e.g. power_button, reset_button, ... > > I know both comments above are in the current binding documentation > (at least for gpio-keys-polled.txt, there is no gpio-keys.txt); but > IMHO both "bus-like" structure of gpio_keys and numbering of buttons > just looks so wrong. > > Sebastian > > >+ label = "Power Button"; > >+ linux,code = <116>; /* KEY_POWER */ Similar to the gpio include that Sebastian mentioned, I think there was a patch to add the linux key codes as an include file for the same reason. If it didn't make it in, don't sweat it, just figured while you were doing a V2... thx, Jason. > >+ gpios = <&gpio0 27 0>; > >+ }; > >+ > >+ button@2 { > >+ label = "Reset Button"; > >+ linux,code = <0x198>; /* KEY_RESTART */ > >+ gpios = <&gpio1 9 1>; > >+ }; > >+ }; > >+ > >+ gpio_poweroff { > >+ compatible = "gpio-poweroff"; > >+ pinctrl-0 = <&poweroff>; > >+ pinctrl-names = "default"; > >+ gpios = <&gpio1 10 1>; > >+ }; > >+}; > > >
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 802720e..c6dd4de 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -112,6 +112,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += armada-370-db.dtb \ armada-xp-axpwifiap.dtb \ armada-xp-db.dtb \ armada-xp-gp.dtb \ + armada-xp-netgear-rn2120.dtb \ armada-xp-openblocks-ax3-4.dtb dtb-$(CONFIG_ARCH_MXC) += \ imx25-karo-tx25.dtb \ diff --git a/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts b/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts new file mode 100644 index 0000000..2651ba3 --- /dev/null +++ b/arch/arm/boot/dts/armada-xp-netgear-rn2120.dts @@ -0,0 +1,289 @@ +/* + * Device Tree file for NETGEAR ReadyNAS 2120 + * + * Copyright (C) 2013, Arnaud EBALARD <arno@natisbad.org> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version + * 2 of the License, or (at your option) any later version. + */ + +/dts-v1/; + +#include "armada-xp-mv78230.dtsi" + +/ { + model = "NETGEAR ReadyNAS 2120"; + compatible = "netgear,readynas-2120", "marvell,armadaxp-mv78230", "marvell,armadaxp", "marvell,armada-370-xp"; + + chosen { + bootargs = "console=ttyS0,115200 earlyprintk"; + }; + + memory { + device_type = "memory"; + reg = <0 0x00000000 0 0x80000000>; /* 2GB */ + }; + + soc { + ranges = <MBUS_ID(0xf0, 0x01) 0 0 0xd0000000 0x100000 + MBUS_ID(0x01, 0x1d) 0 0 0xfff00000 0x100000>; + + pcie-controller { + status = "okay"; + + /* Connected to first Marvell 88SE9170 SATA controller */ + pcie@1,0 { + /* Port 0, Lane 0 */ + status = "okay"; + }; + + /* Connected to second Marvell 88SE9170 SATA controller */ + pcie@2,0 { + /* Port 0, Lane 1 */ + status = "okay"; + }; + + /* Connected to Fresco Logic FL1009 USB 3.0 controller */ + pcie@5,0 { + /* Port 1, Lane 0 */ + status = "okay"; + }; + }; + + internal-regs { + pinctrl { + poweroff: poweroff { + marvell,pins = "mpp42"; + marvell,function = "gpio"; + }; + + power_key_pin: power-key-pin { + marvell,pins = "mpp27"; + marvell,function = "gpio"; + }; + + reset_key_pin: reset-key-pin { + marvell,pins = "mpp41"; + marvell,function = "gpio"; + }; + + sata1_led_pin: sata1-led-pin { + marvell,pins = "mpp31"; + marvell,function = "gpio"; + }; + + sata2_led_pin: sata2-led-pin { + marvell,pins = "mpp40"; + marvell,function = "gpio"; + }; + + sata3_led_pin: sata3-led-pin { + marvell,pins = "mpp44"; + marvell,function = "gpio"; + }; + + sata4_led_pin: sata4-led-pin { + marvell,pins = "mpp47"; + marvell,function = "gpio"; + }; + + sata1_power_pin: sata1-power-pin { + marvell,pins = "mpp24"; + marvell,function = "gpio"; + }; + + sata2_power_pin: sata2-power-pin { + marvell,pins = "mpp25"; + marvell,function = "gpio"; + }; + + sata3_power_pin: sata3-power-pin { + marvell,pins = "mpp26"; + marvell,function = "gpio"; + }; + + sata4_power_pin: sata4-power-pin { + marvell,pins = "mpp28"; + marvell,function = "gpio"; + }; + + sata1_pres_pin: sata1-pres-pin { + marvell,pins = "mpp32"; + marvell,function = "gpio"; + }; + + sata2_pres_pin: sata2-pres-pin { + marvell,pins = "mpp33"; + marvell,function = "gpio"; + }; + + sata3_pres_pin: sata3-pres-pin { + marvell,pins = "mpp34"; + marvell,function = "gpio"; + }; + + sata4_pres_pin: sata4-pres-pin { + marvell,pins = "mpp35"; + marvell,function = "gpio"; + }; + + err_led_pin: err-led-pin { + marvell,pins = "mpp45"; + marvell,function = "gpio"; + }; + }; + + serial@12000 { + clock-frequency = <250000000>; + status = "okay"; + }; + + mdio { + phy0: ethernet-phy@0 { + reg = <0>; + }; + + phy1: ethernet-phy@1 { + reg = <1>; + }; + }; + + ethernet@70000 { + status = "okay"; + phy = <&phy0>; + phy-mode = "rgmii-id"; + }; + + ethernet@74000 { + status = "okay"; + phy = <&phy1>; + phy-mode = "rgmii-id"; + }; + + /* Front USB 2.0 port */ + usb@50000 { + status = "okay"; + }; + + i2c@11000 { + compatible = "marvell,mv64xxx-i2c"; + clock-frequency = <400000>; + status = "okay"; + + /* Rear fan #1 of 3 (Protechnic MGT4012XB-O20, + * 8000RPM) near eSATA port */ + g762_fan1: g762@3e { + compatible = "gmt,g762"; + reg = <0x3e>; + clocks = <&g762_clk>; /* input clock */ + fan_gear_mode = <0>; + fan_startv = <1>; + pwm_polarity = <0>; + }; + + /* Rear fan #2 of 3 at the center */ + g762_fan2: g762@48 { + compatible = "gmt,g762"; + reg = <0x48>; + clocks = <&g762_clk>; /* input clock */ + fan_gear_mode = <0>; + fan_startv = <1>; + pwm_polarity = <0>; + }; + + /* Rear fan #3 of 3 */ + g762_fan3: g762@49 { + compatible = "gmt,g762"; + reg = <0x49>; + clocks = <&g762_clk>; /* input clock */ + fan_gear_mode = <0>; + fan_startv = <1>; + pwm_polarity = <0>; + }; + + g751: g751@4c { + compatible = "gmt,g751"; + reg = <0x4c>; + }; + }; + }; + }; + + clocks { + #address-cells = <1>; + #size-cells = <0>; + + g762_clk: fixedclk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <32768>; + }; + }; + + gpio_leds { + compatible = "gpio-leds"; + pinctrl-0 = <&sata1_led_pin &sata2_led_pin &err_led_pin + &sata3_led_pin &sata4_led_pin>; + pinctrl-names = "default"; + + red_sata1_led { + label = "rn2120:red:sata1"; + gpios = <&gpio0 31 0>; /* GPIO 31 Active High */ + default-state = "off"; + }; + + red_sata2_led { + label = "rn2120:red:sata2"; + gpios = <&gpio1 8 0>; /* GPIO 40 Active High */ + default-state = "off"; + }; + + red_sata3_led { + label = "rn2120:red:sata3"; + gpios = <&gpio1 12 0>; /* GPIO 44 Active High */ + default-state = "off"; + }; + + red_sata4_led { + label = "rn2120:red:sata4"; + gpios = <&gpio1 15 0>; /* GPIO 47 Active High */ + default-state = "off"; + }; + + red_err_led { + label = "rn2120:red:err"; + gpios = <&gpio1 13 1>; /* GPIO 45 Active Low */ + default-state = "off"; + }; + }; + + gpio_keys { + compatible = "gpio-keys"; + #address-cells = <1>; + #size-cells = <0>; + pinctrl-0 = <&power_key_pin + &reset_key_pin>; + pinctrl-names = "default"; + + button@1 { + label = "Power Button"; + linux,code = <116>; /* KEY_POWER */ + gpios = <&gpio0 27 0>; + }; + + button@2 { + label = "Reset Button"; + linux,code = <0x198>; /* KEY_RESTART */ + gpios = <&gpio1 9 1>; + }; + }; + + gpio_poweroff { + compatible = "gpio-poweroff"; + pinctrl-0 = <&poweroff>; + pinctrl-names = "default"; + gpios = <&gpio1 10 1>; + }; +};
All hardware parts of the (mv78230 Armada XP based) NETGEAR ReadyNAS 2120 are supported by mainline kernel (USB 3.0 and eSATA rear ports, USB 2.0 front port, Gigabit controller and PHYs for the two rear ports, serial port, LEDs, Buttons, 88SE9170 SATA controllers, three G762 fan controllers, G751 temperature sensor) except for: - the Intersil ISL12057 I2C RTC Chip, - the Armada NAND controller. Support for both of those is currently work in progress and does not prevent boot. Signed-off-by: Arnaud Ebalard <arno@natisbad.org> --- Hi, This one is intended for v3.14. This depends on the recent fix I pushed for mv78230 PCIe and also on a small patch Guenter Roeck just accepted to have lm75 driver support GMT G751 Temperature sensor (see https://lkml.org/lkml/2013/11/9/273). Regarding the SATA presence and power pin definitions under pinctrl node if you wonder, I thought it would not harm to have them for reference even though they are currently not used. FWIW, I am currently looking if a GPIO regulator can make any use of those. Comments welcome, Cheers, a+ arch/arm/boot/dts/Makefile | 1 + arch/arm/boot/dts/armada-xp-netgear-rn2120.dts | 289 +++++++++++++++++++++++++ 2 files changed, 290 insertions(+) create mode 100644 arch/arm/boot/dts/armada-xp-netgear-rn2120.dts