diff mbox

Documentation: Add MDIO bus node to PHY binding document

Message ID 1384174825-14249-1-git-send-email-jonas.jensen@gmail.com
State Superseded, archived
Headers show

Commit Message

Jonas Jensen Nov. 11, 2013, 1 p.m. UTC
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(-)

Comments

Mark Rutland Nov. 11, 2013, 2:57 p.m. UTC | #1
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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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>;
+	}
+}