[PATCH] Documentation: Add MDIO bus node to PHY binding document

[PATCH] Documentation: Add MDIO bus node to PHY binding document

[Jonas Jensen]

Add MDIO bus node segment and update the example,
allowing trivial bindings to break out boilerplate.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Changes per reply from Grant [0]
    
    [0] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/208851.html
    
    Applies to next-20131111

 Documentation/devicetree/bindings/net/phy.txt | 37 +++++++++++++++++++++------
 1 file changed, 29 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 7cd18fb..4e58a5d 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -1,3 +1,13 @@
+MDIO Bus Nodes
+
+MDIO bus nodes describe an MDIO bus. It is a container for PHY nodes as
+described below.
+
+Required properties:
+- #address-cells = <1>;
+- #size-cells = <0>;
+
+
 PHY nodes
 
 Required properties:
@@ -23,13 +33,24 @@ Optional Properties:
   assume clause 22. The compatible list may also contain other
   elements.
 
+
 Example:
 
-ethernet-phy@0 {
-	compatible = "ethernet-phy-ieee802.3-c22";
-	linux,phandle = <2452000>;
-	interrupt-parent = <40000>;
-	interrupts = <35 1>;
-	reg = <0>;
-	device_type = "ethernet-phy";
-};
+mdio {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ethernet-phy@0 {
+		device_type = "ethernet-phy";
+		compatible = "...", "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+		interrupts = <24 0>;
+	}
+
+	ethernet-phy@1 {
+		device_type = "ethernet-phy";
+		compatible = "...";
+		reg = <1>;
+		interrupts = <35 1>;
+	}
+}
-- 
1.8.2.1

Re: [PATCH] Documentation: Add MDIO bus node to PHY binding document

[Mark Rutland]

On Mon, Nov 11, 2013 at 01:00:25PM +0000, Jonas Jensen wrote:
> Add MDIO bus node segment and update the example,
> allowing trivial bindings to break out boilerplate.

Hi, I have a couple of (minor) comments.

> 
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
> 
> Notes:
>     Changes per reply from Grant [0]
>     
>     [0] http://lists.infradead.org/pipermail/linux-arm-kernel/2013-November/208851.html
>     
>     Applies to next-20131111
> 
>  Documentation/devicetree/bindings/net/phy.txt | 37 +++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index 7cd18fb..4e58a5d 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -1,3 +1,13 @@
> +MDIO Bus Nodes
> +
> +MDIO bus nodes describe an MDIO bus. It is a container for PHY nodes as
> +described below.

Jumping between pllural and singular is a bit jarring, and I assume the
node name is important (i.e. it should be named "mdio").

How about something like:

An MDIO bus node describes an MDIO bus, and is a container for PHY nodes
as described below. An MDIO bus node should be named "mdio".

Given it seems that the MDIO node is expected to live under the node for
the MAC, it would be nice to have a statement to that effect here.

> +
> +Required properties:
> +- #address-cells = <1>;
> +- #size-cells = <0>;

It would be nice to say what the address cell represents (the PHY
address on the MDIO bus, I think?). Also this looks like a fragment of
dts rather than a description. How about:

- #address-cells: Should be <1> - the PHY address on the MDIO bus
- #size-cells: Should be <0>

Otherwise, this looks fine to me.

Thanks,
Mark.

[PATCH v2] Documentation: Add MDIO bus node to PHY binding document

[Jonas Jensen]

Add MDIO bus node segment and update the example,
allowing trivial bindings to break out boilerplate.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Thanks Mark,
    
    This should have the changes from your comments. It also adds optional properties "compatible" and "reg", were those overlooked or left out intentionally?
    
    Changes since v1:
    
    1. reformat "MDIO bus node" description and add, node name should be "mdio"
    2. reformat property descriptions, describe what the cells represent
    3. add optional properties
    4. add a description after "PHY nodes"
    
    Applies to next-20131113

 Documentation/devicetree/bindings/net/phy.txt | 48 ++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 7cd18fb..4179a70 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -1,5 +1,26 @@
+MDIO Bus Nodes
+
+An MDIO bus node describes an MDIO bus, and is a container for PHY nodes
+as described below. An MDIO bus node should be named "mdio".
+
+Required properties:
+
+- #address-cells = Should be <1>, specifies the number of cells needed
+  to encode the PHY address
+- #size-cells = Should be <0>
+
+Optional Properties:
+
+- compatible : Should contain a specific name for the MDIO bus,
+  if known, followed by "-mdio"
+- reg : Should contain register location and length
+
+
 PHY nodes
 
+Describes the PHY chip. A MAC connecting the PHY may use a phandle to
+this node.
+
 Required properties:
 
  - device_type : Should be "ethernet-phy"
@@ -23,13 +44,24 @@ Optional Properties:
   assume clause 22. The compatible list may also contain other
   elements.
 
+
 Example:
 
-ethernet-phy@0 {
-	compatible = "ethernet-phy-ieee802.3-c22";
-	linux,phandle = <2452000>;
-	interrupt-parent = <40000>;
-	interrupts = <35 1>;
-	reg = <0>;
-	device_type = "ethernet-phy";
-};
+mdio {
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ethernet-phy@0 {
+		device_type = "ethernet-phy";
+		compatible = "...", "ethernet-phy-ieee802.3-c22";
+		reg = <0>;
+		interrupts = <24 0>;
+	}
+
+	ethernet-phy@1 {
+		device_type = "ethernet-phy";
+		compatible = "...";
+		reg = <1>;
+		interrupts = <35 1>;
+	}
+}
-- 
1.8.2.1

Re: [PATCH v2] Documentation: Add MDIO bus node to PHY binding document

[Florian Fainelli]

Le mercredi 13 novembre 2013, 15:07:49 Jonas Jensen a écrit :
> Add MDIO bus node segment and update the example,
> allowing trivial bindings to break out boilerplate.
> 
> Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
> ---
> 
> Notes:
>     Thanks Mark,
> 
>     This should have the changes from your comments. It also adds optional
> properties "compatible" and "reg", were those overlooked or left out
> intentionally?

Please CC netdev@vger.kernel.org too as there might be some interest from 
networking folks not actively following devicetree-discuss.

This does looks good to me, there is not much to be said to the point where I 
wonder if this even deserves such an example, but it cannot hurt.

> 
>     Changes since v1:
> 
>     1. reformat "MDIO bus node" description and add, node name should be
> "mdio" 2. reformat property descriptions, describe what the cells represent
> 3. add optional properties
>     4. add a description after "PHY nodes"
> 
>     Applies to next-20131113
> 
>  Documentation/devicetree/bindings/net/phy.txt | 48
> ++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 8
> deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/phy.txt
> b/Documentation/devicetree/bindings/net/phy.txt index 7cd18fb..4179a70
> 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -1,5 +1,26 @@
> +MDIO Bus Nodes
> +
> +An MDIO bus node describes an MDIO bus, and is a container for PHY nodes
> +as described below. An MDIO bus node should be named "mdio".
> +
> +Required properties:
> +
> +- #address-cells = Should be <1>, specifies the number of cells needed
> +  to encode the PHY address
> +- #size-cells = Should be <0>
> +
> +Optional Properties:
> +
> +- compatible : Should contain a specific name for the MDIO bus,
> +  if known, followed by "-mdio"
> +- reg : Should contain register location and length
> +
> +
>  PHY nodes
> 
> +Describes the PHY chip. A MAC connecting the PHY may use a phandle to
> +this node.
> +
>  Required properties:
> 
>   - device_type : Should be "ethernet-phy"
> @@ -23,13 +44,24 @@ Optional Properties:
>    assume clause 22. The compatible list may also contain other
>    elements.
> 
> +
>  Example:
> 
> -ethernet-phy@0 {
> -	compatible = "ethernet-phy-ieee802.3-c22";
> -	linux,phandle = <2452000>;
> -	interrupt-parent = <40000>;
> -	interrupts = <35 1>;
> -	reg = <0>;
> -	device_type = "ethernet-phy";
> -};
> +mdio {
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ethernet-phy@0 {
> +		device_type = "ethernet-phy";
> +		compatible = "...", "ethernet-phy-ieee802.3-c22";
> +		reg = <0>;
> +		interrupts = <24 0>;
> +	}
> +
> +	ethernet-phy@1 {
> +		device_type = "ethernet-phy";
> +		compatible = "...";
> +		reg = <1>;
> +		interrupts = <35 1>;
> +	}
> +}

-- 
Florian