[PATCH 0/4] ARM: mvebu/kirkwood: NETGEAR ReadyNAS .dts files cleanup

[PATCH 0/4] ARM: mvebu/kirkwood: NETGEAR ReadyNAS .dts files cleanup

[Arnaud Ebalard]

Hi,

This series provides some cleanup for all NETGEAR ReadyNAS .dts files
(Duo v2, 102 and 104). This is mostly based on suggestions from
Sebastian during RN2120 .dts submission. Jason, two remarks:

 - Patch 1/4 applies on top of gpio-poweroff patch for Duo v2 pushed
   earlier today (more precisely v2 of that patch)
 - Patch 3/4 applies on NXP PCA 9554 patch for RN104

As requested, last patch (4/4) contains whitespace cleanups for those
.dts (in fact, mainly Duo v2 and a single hunk for RN102). 

Comments welcome,

Cheers,

a+

Arnaud Ebalard (4):
  ARM: kirkwood: NETGEAR ReadyNAS Duo v2 .dts file cleanup
  ARM: mvebu: NETGEAR ReadyNAS 102 .dts file cleanup
  ARM: mvebu: NETGEAR ReadyNAS 104 .dts file cleanup
  ARM: NETGEAR ReadyNAS Duo v2 and RN102 .dts file whitespace cleanup

 arch/arm/boot/dts/armada-370-netgear-rn102.dts     | 81 +++++++++++-------
 arch/arm/boot/dts/armada-370-netgear-rn104.dts     | 66 +++++++--------
 .../boot/dts/kirkwood-netgear_readynas_duo_v2.dts  | 98 +++++++++++++---------
 3 files changed, 141 insertions(+), 104 deletions(-)

-- 
1.8.4.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/4] kirkwood: NETGEAR ReadyNAS Duo v2 .dts file cleanup

[Arnaud Ebalard]

The patch does some cleanup work on NETGEAR ReadyNAS Duo v2 .dts
file. Changes are listed below:

 - Converted GPIO voltage level from value to macros
 - Converted all numeric input key values to macros
 - Made button node names more explicit
 - Added compatible string for PHY (marvell,mv88e1318s)
 - Added header for the file to describe content and author
 - Made G762 clock node name unique by including g762 in it
 - Fixed all node names and labels to use respectively '-' and '_'
 - Removed useless clocks and gpio-keys properties

Signed-off-by: Arnaud Ebalard <arno-LkuqDEemtHBg9hUCZPvPmw@public.gmane.org>
---
 .../boot/dts/kirkwood-netgear_readynas_duo_v2.dts  | 50 +++++++++++++---------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/arch/arm/boot/dts/kirkwood-netgear_readynas_duo_v2.dts b/arch/arm/boot/dts/kirkwood-netgear_readynas_duo_v2.dts
index 551bfed..65d0587 100644
--- a/arch/arm/boot/dts/kirkwood-netgear_readynas_duo_v2.dts
+++ b/arch/arm/boot/dts/kirkwood-netgear_readynas_duo_v2.dts
@@ -1,5 +1,17 @@
+/*
+ * Device Tree file for NETGEAR ReadyNAS Duo v2
+ *
+ * Copyright (C) 2013, Arnaud EBALARD <arno-LkuqDEemtHBg9hUCZPvPmw@public.gmane.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 <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/gpio.h>
 #include "kirkwood.dtsi"
 #include "kirkwood-6282.dtsi"
@@ -68,10 +80,7 @@
 		};
 
 		clocks {
-		       #address-cells = <1>;
-		       #size-cells = <0>;
-
-		       g762_clk: fixedclk {
+		       g762_clk: g762-oscillator {
 				 compatible = "fixed-clock";
 				 #clock-cells = <0>;
 				 clock-frequency = <8192>;
@@ -117,49 +126,47 @@
 
 		power_led {
 			label = "status:blue:power_led";
-			gpios = <&gpio0 31 1>;   /* GPIO 31 Active Low */
+			gpios = <&gpio0 31 GPIO_ACTIVE_LOW>;
 			linux,default-trigger = "default-on";
 		};
 		activity_led {
 			label = "status:blue:activity_led";
-			gpios = <&gpio1 6 1>;    /* GPIO 38 Active Low */
+			gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
 		};
 		disk1_led {
 			label = "status:blue:disk1_led";
-			gpios = <&gpio0 23 1>;   /* GPIO 23 Active Low */
+			gpios = <&gpio0 23 GPIO_ACTIVE_LOW>;
 		};
 		disk2_led {
 			label = "status:blue:disk2_led";
-			gpios = <&gpio0 22 1>;   /* GPIO 22 Active Low */
+			gpios = <&gpio0 22 GPIO_ACTIVE_LOW>;
 		};
 		backup_led {
 			label = "status:blue:backup_led";
-			gpios = <&gpio0 29 1>;   /* GPIO 29 Active Low*/
+			gpios = <&gpio0 29 GPIO_ACTIVE_LOW>;
 		};
 	};
 
-	gpio_keys {
+	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
 		pinctrl-0 = <&pmx_button_power &pmx_button_backup
 			     &pmx_button_reset>;
 		pinctrl-names = "default";
 
-		button@1 {
+		power-button {
 			label = "Power Button";
-			linux,code = <116>;     /* KEY_POWER */
-			gpios = <&gpio1 15 1>;
+			linux,code = <KEY_POWER>;
+			gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
 		};
-		button@2 {
+		reset-button {
 			label = "Reset Button";
-			linux,code = <0x198>;   /* KEY_RESTART */
-			gpios = <&gpio0 13 1>;
+			linux,code = <KEY_RESTART>;
+			gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
 		};
-		button@3 {
+		backup-button {
 			label = "Backup Button";
-			linux,code = <133>;     /* KEY_COPY */
-			gpios = <&gpio1 13 1>;
+			linux,code = <KEY_COPY>;
+			gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
 		};
 	};
 
@@ -224,6 +231,7 @@
 
 	ethphy0: ethernet-phy@0 {
 		device_type = "ethernet-phy";
+		compatible = "marvell,88e1318s";
 		reg = <0>;
 	};
 };
-- 
1.8.4.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/4] ARM: mvebu: NETGEAR ReadyNAS 102 .dts file cleanup

[Arnaud Ebalard]

The patch does some cleanup work on NETGEAR ReadyNAS Duo v2 .dts
file. Changes are listed below

 - Converted from value to macros for GPIO voltage level
 - Converted all numeric input keys values to macros
 - Added GPIO keys pins to pinctrl
 - Made button names more explicit
 - Added compatible string for PHY (marvell,mv88e1318s)
 - Made G762 clock node name unique by including g762 in it
 - Fixed all node names and labels to use respectively '-' and '_'
 - Removed useless clocks and gpio-keys properties

Signed-off-by: Arnaud Ebalard <arno-LkuqDEemtHBg9hUCZPvPmw@public.gmane.org>
---
 arch/arm/boot/dts/armada-370-netgear-rn102.dts | 80 +++++++++++++++-----------
 1 file changed, 48 insertions(+), 32 deletions(-)

diff --git a/arch/arm/boot/dts/armada-370-netgear-rn102.dts b/arch/arm/boot/dts/armada-370-netgear-rn102.dts
index 8ac2ac1..87cd003 100644
--- a/arch/arm/boot/dts/armada-370-netgear-rn102.dts
+++ b/arch/arm/boot/dts/armada-370-netgear-rn102.dts
@@ -11,6 +11,8 @@
 
 /dts-v1/;
 
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/gpio/gpio.h>
 #include "armada-370.dtsi"
 
 / {
@@ -77,6 +79,21 @@
 					marvell,function = "gpio";
 				};
 
+				backup_button_pin: backup-button-pin {
+					marvell,pins = "mpp58";
+					marvell,function = "gpio";
+				};
+
+				power_button_pin: power-button-pin {
+					marvell,pins = "mpp62";
+					marvell,function = "gpio";
+				};
+
+				reset_button_pin: reset-button-pin {
+					marvell,pins = "mpp6";
+					marvell,function = "gpio";
+				};
+
 				poweroff: poweroff {
 					marvell,pins = "mpp8";
 					marvell,function = "gpio";
@@ -85,6 +102,7 @@
 
 			mdio {
 				phy0: ethernet-phy@0 {
+					compatible = "marvell,88e1318s";
 					reg = <0>;
 				};
 			};
@@ -117,78 +135,76 @@
 	};
 
 	clocks {
-	       #address-cells = <1>;
-	       #size-cells = <0>;
-
-	       g762_clk: fixedclk {
+	       g762_clk: g762-oscillator {
 			 compatible = "fixed-clock";
 			 #clock-cells = <0>;
 			 clock-frequency = <8192>;
 	       };
 	};
 
-	gpio_leds {
+	gpio-leds {
 		compatible = "gpio-leds";
-		pinctrl-0 = < &power_led_pin
-			      &sata1_led_pin
-			      &sata2_led_pin
-			      &backup_led_pin >;
+		pinctrl-0 = <&power_led_pin
+			     &sata1_led_pin
+			     &sata2_led_pin
+			     &backup_led_pin>;
 		pinctrl-names = "default";
 
-		blue_power_led {
+		blue-power-led {
 			label = "rn102:blue:pwr";
-			gpios = <&gpio1 25 1>;  /* GPIO 57 Active Low */
+			gpios = <&gpio1 25 GPIO_ACTIVE_LOW>;
 			linux,default-trigger = "heartbeat";
 		};
 
-		green_sata1_led {
+		green-sata1-led {
 			label = "rn102:green:sata1";
-			gpios = <&gpio0 15 1>;  /* GPIO 15 Active Low */
+			gpios = <&gpio0 15 GPIO_ACTIVE_LOW>;
 			default-state = "on";
 		};
 
-		green_sata2_led {
+		green-sata2-led {
 			label = "rn102:green:sata2";
-			gpios = <&gpio0 14 1>;   /* GPIO 14 Active Low */
+			gpios = <&gpio0 14 GPIO_ACTIVE_LOW>;
 			default-state = "on";
 		};
 
-		green_backup_led {
+		green-backup-led {
 			label = "rn102:green:backup";
-			gpios = <&gpio1 24 1>;   /* GPIO 56 Active Low */
+			gpios = <&gpio1 24 GPIO_ACTIVE_LOW>;
 			default-state = "on";
 		};
 	};
 
-	gpio_keys {
+	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
+		pinctrl-0 = <&power_button_pin
+			     &reset_button_pin
+			     &backup_button_pin>;
+		pinctrl-names = "default";
 
-		button@1 {
+		power-button {
 			label = "Power Button";
-			linux,code = <116>;     /* KEY_POWER */
-			gpios = <&gpio1 30 0>;
+			linux,code = <KEY_POWER>;
+			gpios = <&gpio1 30 GPIO_ACTIVE_HIGH>;
 		};
 
-		button@2 {
+		reset-button {
 			label = "Reset Button";
-			linux,code = <0x198>;   /* KEY_RESTART */
-			gpios = <&gpio0 6 1>;
+			linux,code = <KEY_RESTART>;
+			gpios = <&gpio0 6 GPIO_ACTIVE_LOW>;
 		};
 
-		button@3 {
+		backup-button {
 			label = "Backup Button";
-			linux,code = <133>;     /* KEY_COPY */
-			gpios = <&gpio1 26 1>;
+			linux,code = <KEY_COPY>;
+			gpios = <&gpio1 26 GPIO_ACTIVE_LOW>;
 		};
 	};
 
-	gpio_poweroff {
+	gpio-poweroff {
 		compatible = "gpio-poweroff";
 		pinctrl-0 = <&poweroff>;
 		pinctrl-names = "default";
-		gpios = <&gpio0 8 1>;
+		gpios = <&gpio0 8 GPIO_ACTIVE_LOW>;
 	};
-
 };
-- 
1.8.4.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/4] ARM: mvebu: NETGEAR ReadyNAS 104 .dts file cleanup

[Arnaud Ebalard]

The patch does some cleanup work on NETGEAR ReadyNAS 104 .dts
file. Changes are listed below:

 - Completed conversion from value to macros for GPIO voltage level
 - Converted all numeric input key values to macros
 - Fixed all node names and labels to use respectively '-' and '_'
 - Made button names more explicit
 - Changed order of included files from general to local
 - Removed useless clocks and gpio-keys properties
 - Added compatible string for PHY (marvell,mv88e1318s)
 - Made G762 clock node name unique by including g762 in it
 
Signed-off-by: Arnaud Ebalard <arno-LkuqDEemtHBg9hUCZPvPmw@public.gmane.org>
---
 arch/arm/boot/dts/armada-370-netgear-rn104.dts | 66 +++++++++++++-------------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/arch/arm/boot/dts/armada-370-netgear-rn104.dts b/arch/arm/boot/dts/armada-370-netgear-rn104.dts
index 50b5150..853ddf7 100644
--- a/arch/arm/boot/dts/armada-370-netgear-rn104.dts
+++ b/arch/arm/boot/dts/armada-370-netgear-rn104.dts
@@ -11,8 +11,9 @@
 
 /dts-v1/;
 
-#include "armada-370.dtsi"
+#include <dt-bindings/input/input.h>
 #include <dt-bindings/gpio/gpio.h>
+#include "armada-370.dtsi"
 
 / {
 	model = "NETGEAR ReadyNAS 104";
@@ -59,12 +60,12 @@
 					marvell,function = "gpio";
 				};
 
-				backup_key_pin: backup-key-pin {
+				backup_button_pin: backup-button-pin {
 					marvell,pins = "mpp52";
 					marvell,function = "gpio";
 				};
 
-				power_key_pin: power-key-pin {
+				power_button_pin: power-button-pin {
 					marvell,pins = "mpp62";
 					marvell,function = "gpio";
 				};
@@ -79,7 +80,7 @@
 					marvell,function = "gpio";
 				};
 
-				reset_key_pin: reset-key-pin {
+				reset_button_pin: reset-button-pin {
 					marvell,pins = "mpp65";
 					marvell,function = "gpio";
 				};
@@ -87,10 +88,12 @@
 
 			mdio {
 				phy0: ethernet-phy@0 {
+					compatible = "marvell,88e1318s";
 					reg = <0>;
 				};
 
 				phy1: ethernet-phy@1 {
+					compatible = "marvell,88e1318s";
 					reg = <1>;
 				};
 			};
@@ -136,90 +139,85 @@
 	};
 
 	clocks {
-	       #address-cells = <1>;
-	       #size-cells = <0>;
-
-	       g762_clk: fixedclk {
+	       g762_clk: g762-oscillator {
 			 compatible = "fixed-clock";
 			 #clock-cells = <0>;
 			 clock-frequency = <8192>;
 	       };
 	};
 
-	gpio_leds {
+	gpio-leds {
 		compatible = "gpio-leds";
 		pinctrl-0 = <&backup_led_pin &power_led_pin>;
 		pinctrl-names = "default";
 
-		blue_backup_led {
+		blue-backup-led {
 			label = "rn104:blue:backup";
-			gpios = <&gpio1 31 0>;   /* GPIO 63 Active High */
+			gpios = <&gpio1 31 GPIO_ACTIVE_HIGH>;
 			default-state = "off";
 		};
 
-		blue_power_led {
+		blue-power-led {
 			label = "rn104:blue:pwr";
-			gpios = <&gpio2 0 1>;    /* GPIO 64 Active Low */
+			gpios = <&gpio2 0 GPIO_ACTIVE_LOW>;
 			linux,default-trigger = "keep";
 		};
 
-		sata1_led {
+		blue-sata1-led {
 			label = "rn104:blue:sata1";
 			gpios = <&pca9554 0 GPIO_ACTIVE_LOW>;
 			default-state = "off";
 		};
 
-		sata2_led {
+		blue-sata2-led {
 			label = "rn104:blue:sata2";
 			gpios = <&pca9554 1 GPIO_ACTIVE_LOW>;
 			default-state = "off";
 		};
 
-		sata3_led {
+		blue-sata3-led {
 			label = "rn104:blue:sata3";
 			gpios = <&pca9554 2 GPIO_ACTIVE_LOW>;
 			default-state = "off";
 		};
 
-		sata4_led {
+		blue-sata4-led {
 			label = "rn104:blue:sata4";
 			gpios = <&pca9554 3 GPIO_ACTIVE_LOW>;
 			default-state = "off";
 		};
 	};
 
-	gpio_keys {
+	gpio-keys {
 		compatible = "gpio-keys";
-		#address-cells = <1>;
-		#size-cells = <0>;
-		pinctrl-0 = <&backup_key_pin
-			     &power_key_pin
-			     &reset_key_pin>;
+		pinctrl-0 = <&backup_button_pin
+			     &power_button_pin
+			     &reset_button_pin>;
 		pinctrl-names = "default";
 
-		button@1 {
+		backup-button {
 			label = "Backup Button";
-			linux,code = <133>;     /* KEY_COPY */
-			gpios = <&gpio1 20 1>;
+			linux,code = <KEY_COPY>;
+			gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
 		};
 
-		button@2 {
+		power-button {
 			label = "Power Button";
-			linux,code = <116>;     /* KEY_POWER */
-			gpios = <&gpio1 30 0>;
+			linux,code = <KEY_POWER>;
+			gpios = <&gpio1 30 GPIO_ACTIVE_HIGH>;
 		};
 
-		button@3 {
+		reset-button {
 			label = "Reset Button";
-			linux,code = <0x198>;   /* KEY_RESTART */
-			gpios = <&gpio2 1 1>;
+			linux,code = <KEY_RESTART>;
+			gpios = <&gpio2 1 GPIO_ACTIVE_LOW>;
 		};
 	};
 
-	gpio_poweroff {
+	gpio-poweroff {
 		compatible = "gpio-poweroff";
 		pinctrl-0 = <&poweroff>;
 		pinctrl-names = "default";
-		gpios = <&gpio1 28 1>;
+		gpios = <&gpio1 28 GPIO_ACTIVE_LOW>;
 	};
 };
-- 
1.8.4.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ARM: mvebu: NETGEAR ReadyNAS 104 .dts file cleanup

[Jason Gunthorpe]

On Fri, Nov 22, 2013 at 09:07:01PM +0100, Arnaud Ebalard wrote:
>  			mdio {
>  				phy0: ethernet-phy@0 {
> +					compatible = "marvell,88e1318s";
>  					reg = <0>;
>  				};
>  
>  				phy1: ethernet-phy@1 {
> +					compatible = "marvell,88e1318s";
>  					reg = <1>;
>  				};
>  			};

Just an inquiry, are these compatible strings OK?

Documentation/devicetree/bindings/net/phy.txt doesn't really give
guidance.

For self-discoverable busses I would expect to see a structured
compatible string, or property, that encodes the phy-id - such as what
the PCI binding does with vid/did:

"compatible" Construct a list of names in most-specific to
             least-specific order. The names shall be derived from
	     values of the Vendor ID, Device ID, Subsystem Vendor ID,
	     Subsystem ID, Revision ID and Class Code bytes, and shall
             have the following form, and be placed in the list in the
             following order:

  pciVVVV,DDDD.SSSS.ssss.RR (1)

So, I would think something like this:

compatible = "ethernet-phy-id0141,0e90"; /* Marvell 88E1318 */

?

A big rational for this is that the kernel already has complete
infrastructure for matching drivers to the numeric phy-id.

I have a small patch to make the OF mdio bus scanner get the phy-id
from DT. I didn't use compatible string in this patch, but it would be
easy to change, and this demonstrates what we could do with a
structured compatible string (attached).

Grant? Rob? I could make some patches for this if you agree.

Regards,
Jason

>From 1aab2047b6dfb8b814673718dfa7ef0ea0c7a0ef Mon Sep 17 00:00:00 2001
From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
Date: Thu, 12 Sep 2013 17:36:13 -0600
Subject: [PATCH] of_mdio: Allow the DT to specify the phy ID and avoid
 autoprobing

This makes the generic of_mdiobus_register parse the DT property
'phy-id'. If present it should be the numeric constant that matches the
phy-id register normally readable through MDIO.

When the ID is given the phy autoprobing is defeated and the phy is
created directly.

This is necessary to support phy's that cannot be autoprobed when
of_mdiobus_register is called. Specifically, my case has the phy in
reset at that time, the reset is only released once the ethernet
driver starts.

Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
---
 drivers/of/of_mdio.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index d5a57a9..dbe6932 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -81,7 +81,12 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
 
 		is_c45 = of_device_is_compatible(child,
 						 "ethernet-phy-ieee802.3-c45");
-		phy = get_phy_device(mdio, addr, is_c45);
+		paddr = of_get_property(child, "phy-id", &len);
+		if (paddr && len <= sizeof(*paddr) && !is_c45)
+			phy = phy_device_create(mdio, addr, be32_to_cpup(paddr),
+						0, NULL);
+		else
+			phy = get_phy_device(mdio, addr, is_c45);
 
 		if (!phy || IS_ERR(phy)) {
 			dev_err(&mdio->dev,
-- 
1.8.1.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ARM: mvebu: NETGEAR ReadyNAS 104 .dts file cleanup

[Arnaud Ebalard]

Hi,

Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> writes:

> On Fri, Nov 22, 2013 at 09:07:01PM +0100, Arnaud Ebalard wrote:
>>  			mdio {
>>  				phy0: ethernet-phy@0 {
>> +					compatible = "marvell,88e1318s";
>>  					reg = <0>;
>>  				};
>>  
>>  				phy1: ethernet-phy@1 {
>> +					compatible = "marvell,88e1318s";
>>  					reg = <1>;
>>  				};
>>  			};
>
> Just an inquiry, are these compatible strings OK?
>
> Documentation/devicetree/bindings/net/phy.txt doesn't really give
> guidance.
>
> For self-discoverable busses I would expect to see a structured
> compatible string, or property, that encodes the phy-id - such as what
> the PCI binding does with vid/did:
>
> "compatible" Construct a list of names in most-specific to
>              least-specific order. The names shall be derived from
> 	     values of the Vendor ID, Device ID, Subsystem Vendor ID,
> 	     Subsystem ID, Revision ID and Class Code bytes, and shall
>              have the following form, and be placed in the list in the
>              following order:
>
>   pciVVVV,DDDD.SSSS.ssss.RR (1)
>
> So, I would think something like this:
>
> compatible = "ethernet-phy-id0141,0e90"; /* Marvell 88E1318 */

The compatible string I put is based on this discussion:

 http://www.spinics.net/lists/devicetree/msg11465.html

Now, considering the following:

arno@small:linux$ grep -R -88e1318s . | wc -l
0

and also the fact that *in my case* the kernel does its job
auto-detecting the PHY, I now also wonder how the kernel can
use that info and what I get by adding the compatible string.

Anyway, I'll wait for directions on how this should be fixed
before sending a v2 of the set.

Thanks for catching this, Jason.

Cheers,

a+
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ARM: mvebu: NETGEAR ReadyNAS 104 .dts file cleanup

[Jason Gunthorpe]

On Fri, Nov 22, 2013 at 11:22:33PM +0100, Arnaud Ebalard wrote:

> and also the fact that *in my case* the kernel does its job
> auto-detecting the PHY, I now also wonder how the kernel can
> use that info and what I get by adding the compatible string.

I think the main rational was to document what chip the phy is to
people who don't have the hardware...

You probably shouldn't delay your patch on this issue, document the
phy chip model in a comment?

Also, check if it is actually an S varient or not, they have the same
phy-id.. ;)

Jason
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ARM: mvebu: NETGEAR ReadyNAS 104 .dts file cleanup

[Arnaud Ebalard]

Hi,

Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> writes:

> On Fri, Nov 22, 2013 at 11:22:33PM +0100, Arnaud Ebalard wrote:
>
>> and also the fact that *in my case* the kernel does its job
>> auto-detecting the PHY, I now also wonder how the kernel can
>> use that info and what I get by adding the compatible string.
>
> I think the main rational was to document what chip the phy is to
> people who don't have the hardware...
>
> You probably shouldn't delay your patch on this issue, document the
> phy chip model in a comment?

Will do that.

> Also, check if it is actually an S varient or not, they have the same
> phy-id.. ;)

On the chip, I have 88E1318-NNB2 (PNN1230_3JW 1209 A0P TW). I will see
if I can find more info.

Cheers,

a+
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ARM: mvebu: NETGEAR ReadyNAS 104 .dts file cleanup

[Jason Gunthorpe]

On Mon, Nov 25, 2013 at 11:03:36PM +0100, Arnaud Ebalard wrote:

> > Also, check if it is actually an S varient or not, they have the same
> > phy-id.. ;)
> 
> On the chip, I have 88E1318-NNB2 (PNN1230_3JW 1209 A0P TW). I will see
> if I can find more info.

That is the standard varient - I expect the S version is rare. NNB2
indicates the package option. The rest is the lot code.

Jason
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] ARM: mvebu: NETGEAR ReadyNAS 104 .dts file cleanup

[Rob Herring]

On 11/22/2013 03:36 PM, Jason Gunthorpe wrote:
> On Fri, Nov 22, 2013 at 09:07:01PM +0100, Arnaud Ebalard wrote:
>>  			mdio {
>>  				phy0: ethernet-phy@0 {
>> +					compatible = "marvell,88e1318s";
>>  					reg = <0>;
>>  				};
>>  
>>  				phy1: ethernet-phy@1 {
>> +					compatible = "marvell,88e1318s";
>>  					reg = <1>;
>>  				};
>>  			};
> 
> Just an inquiry, are these compatible strings OK?
> 
> Documentation/devicetree/bindings/net/phy.txt doesn't really give
> guidance.
> 
> For self-discoverable busses I would expect to see a structured
> compatible string, or property, that encodes the phy-id - such as what
> the PCI binding does with vid/did:
> 
> "compatible" Construct a list of names in most-specific to
>              least-specific order. The names shall be derived from
> 	     values of the Vendor ID, Device ID, Subsystem Vendor ID,
> 	     Subsystem ID, Revision ID and Class Code bytes, and shall
>              have the following form, and be placed in the list in the
>              following order:
> 
>   pciVVVV,DDDD.SSSS.ssss.RR (1)
> 
> So, I would think something like this:
> 
> compatible = "ethernet-phy-id0141,0e90"; /* Marvell 88E1318 */
> 
> ?
> 
> A big rational for this is that the kernel already has complete
> infrastructure for matching drivers to the numeric phy-id.
>
> I have a small patch to make the OF mdio bus scanner get the phy-id
> from DT. I didn't use compatible string in this patch, but it would be
> easy to change, and this demonstrates what we could do with a
> structured compatible string (attached).
> 
> Grant? Rob? I could make some patches for this if you agree.

Seems like a reasonable approach.

Rob

> 
> Regards,
> Jason
> 
> From 1aab2047b6dfb8b814673718dfa7ef0ea0c7a0ef Mon Sep 17 00:00:00 2001
> From: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Date: Thu, 12 Sep 2013 17:36:13 -0600
> Subject: [PATCH] of_mdio: Allow the DT to specify the phy ID and avoid
>  autoprobing
> 
> This makes the generic of_mdiobus_register parse the DT property
> 'phy-id'. If present it should be the numeric constant that matches the
> phy-id register normally readable through MDIO.
> 
> When the ID is given the phy autoprobing is defeated and the phy is
> created directly.
> 
> This is necessary to support phy's that cannot be autoprobed when
> of_mdiobus_register is called. Specifically, my case has the phy in
> reset at that time, the reset is only released once the ethernet
> driver starts.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> ---
>  drivers/of/of_mdio.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index d5a57a9..dbe6932 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -81,7 +81,12 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
>  
>  		is_c45 = of_device_is_compatible(child,
>  						 "ethernet-phy-ieee802.3-c45");
> -		phy = get_phy_device(mdio, addr, is_c45);
> +		paddr = of_get_property(child, "phy-id", &len);
> +		if (paddr && len <= sizeof(*paddr) && !is_c45)
> +			phy = phy_device_create(mdio, addr, be32_to_cpup(paddr),
> +						0, NULL);
> +		else
> +			phy = get_phy_device(mdio, addr, is_c45);
>  
>  		if (!phy || IS_ERR(phy)) {
>  			dev_err(&mdio->dev,
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] ARM: NETGEAR ReadyNAS Duo v2 and RN102 .dts file whitespace cleanup

[Arnaud Ebalard]

This patch provides some whitespace cleanup for NETGEAR
ReadyNAS Duo v2 and 102 .dts files:

 - Fixed bad whitespaces
 - Added spaces between nodes to improve readability

Signed-off-by: Arnaud Ebalard <arno-LkuqDEemtHBg9hUCZPvPmw@public.gmane.org>
---
 arch/arm/boot/dts/armada-370-netgear-rn102.dts     |  1 +
 .../boot/dts/kirkwood-netgear_readynas_duo_v2.dts  | 48 ++++++++++++++--------
 2 files changed, 32 insertions(+), 17 deletions(-)

diff --git a/arch/arm/boot/dts/armada-370-netgear-rn102.dts b/arch/arm/boot/dts/armada-370-netgear-rn102.dts
index 87cd003..1cbd338 100644
--- a/arch/arm/boot/dts/armada-370-netgear-rn102.dts
+++ b/arch/arm/boot/dts/armada-370-netgear-rn102.dts
@@ -64,6 +64,7 @@
 					marvell,pins = "mpp57";
 					marvell,function = "gpio";
 				};
+
 				sata1_led_pin: sata1-led-pin {
 					marvell,pins = "mpp15";
 					marvell,function = "gpio";
diff --git a/arch/arm/boot/dts/kirkwood-netgear_readynas_duo_v2.dts b/arch/arm/boot/dts/kirkwood-netgear_readynas_duo_v2.dts
index 65d0587..0961e80 100644
--- a/arch/arm/boot/dts/kirkwood-netgear_readynas_duo_v2.dts
+++ b/arch/arm/boot/dts/kirkwood-netgear_readynas_duo_v2.dts
@@ -45,34 +45,42 @@
 				marvell,pins = "mpp47";
 				marvell,function = "gpio";
 			};
+
 			pmx_button_backup: pmx-button-backup {
 				marvell,pins = "mpp45";
 				marvell,function = "gpio";
 			};
+
 			pmx_button_reset: pmx-button-reset {
 				marvell,pins = "mpp13";
 				marvell,function = "gpio";
 			};
+
 			pmx_led_blue_power: pmx-led-blue-power {
 				marvell,pins = "mpp31";
 				marvell,function = "gpio";
 			};
+
 			pmx_led_blue_activity: pmx-led-blue-activity {
 				marvell,pins = "mpp38";
 				marvell,function = "gpio";
 			};
+
 			pmx_led_blue_disk1: pmx-led-blue-disk1 {
 				marvell,pins = "mpp23";
 				marvell,function = "gpio";
 			};
+
 			pmx_led_blue_disk2: pmx-led-blue-disk2 {
 				marvell,pins = "mpp22";
 				marvell,function = "gpio";
 			};
+
 			pmx_led_blue_backup: pmx-led-blue-backup {
 				marvell,pins = "mpp29";
 				marvell,function = "gpio";
 			};
+
 			pmx_poweroff: pmx-poweroff {
 				marvell,pins = "mpp30";
 				marvell,function = "gpio";
@@ -129,18 +137,22 @@
 			gpios = <&gpio0 31 GPIO_ACTIVE_LOW>;
 			linux,default-trigger = "default-on";
 		};
+
 		activity_led {
 			label = "status:blue:activity_led";
 			gpios = <&gpio1 6 GPIO_ACTIVE_LOW>;
 		};
+
 		disk1_led {
 			label = "status:blue:disk1_led";
 			gpios = <&gpio0 23 GPIO_ACTIVE_LOW>;
 		};
+
 		disk2_led {
 			label = "status:blue:disk2_led";
 			gpios = <&gpio0 22 GPIO_ACTIVE_LOW>;
 		};
+
 		backup_led {
 			label = "status:blue:backup_led";
 			gpios = <&gpio0 29 GPIO_ACTIVE_LOW>;
@@ -158,11 +170,13 @@
 			linux,code = <KEY_POWER>;
 			gpios = <&gpio1 15 GPIO_ACTIVE_LOW>;
 		};
+
 		reset-button {
 			label = "Reset Button";
 			linux,code = <KEY_RESTART>;
 			gpios = <&gpio0 13 GPIO_ACTIVE_LOW>;
 		};
+
 		backup-button {
 			label = "Backup Button";
 			linux,code = <KEY_COPY>;
@@ -177,23 +191,23 @@
 		gpios = <&gpio0 30 GPIO_ACTIVE_LOW>;
 	};
 
-        regulators {
-                compatible = "simple-bus";
-                #address-cells = <1>;
-                #size-cells = <0>;
-
-                usb_power: regulator@1 {
-                        compatible = "regulator-fixed";
-                        reg = <1>;
-                        regulator-name = "USB 3.0 Power";
-                        regulator-min-microvolt = <5000000>;
-                        regulator-max-microvolt = <5000000>;
-                        enable-active-high;
-                        regulator-always-on;
-                        regulator-boot-on;
-                        gpio = <&gpio1 14 0>;
-                };
-        };
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		usb3_regulator: usb3-regulator {
+			compatible = "regulator-fixed";
+			reg = <1>;
+			regulator-name = "USB 3.0 Power";
+			regulator-min-microvolt = <5000000>;
+			regulator-max-microvolt = <5000000>;
+			enable-active-high;
+			regulator-always-on;
+			regulator-boot-on;
+			gpio = <&gpio1 14 GPIO_ACTIVE_HIGH>;
+		};
+	};
 };
 
 &nand {
-- 
1.8.4.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html