[PATCH 1/2] dt-bindings: net: gmii2rgmii: Fix typos

[PATCH 1/2] dt-bindings: net: gmii2rgmii: Fix typos

[Brandon Maier]

The wording of 'provides the RGMII' is unclear, and should be 'is a
converter' to be consistent with the rest of this Doc. Also fix up
assorted spelling and capitalization typos.

Signed-off-by: Brandon Maier <brandon.maier@gmail.com>
---
 Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
index 038dda48b8e6..b2258ce5004d 100644
--- a/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
+++ b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
@@ -2,15 +2,15 @@ XILINX GMIITORGMII Converter Driver Device Tree Bindings
 --------------------------------------------------------
 
 The Gigabit Media Independent Interface (GMII) to Reduced Gigabit Media
-Independent Interface (RGMII) core provides the RGMII between RGMII-compliant
-Ethernet physical media devices (PHY) and the Gigabit Ethernet controller.
+Independent Interface (RGMII) core is a converter between RGMII-compliant
+Ethernet physical media devices (PHY) and GMII Ethernet controllers.
 This core can be used in all three modes of operation(10/100/1000 Mb/s).
 The Management Data Input/Output (MDIO) interface is used to configure the
-Speed of operation. This core can switch dynamically between the three
-Different speed modes by configuring the conveter register through mdio write.
+speed of operation. This core can switch dynamically between the three
+different speed modes by configuring the converter register through mdio writes.
 
-This converter sits between the ethernet MAC and the external phy.
-MAC <==> GMII2RGMII <==> RGMII_PHY
+This converter sits between the ethernet MAC and the external PHY.
+GMII_MAC <==> GMII2RGMII <==> RGMII_PHY
 
 For more details about mdio please refer phy.txt file in the same directory.
 
-- 
2.11.0

[PATCH 2/2] dt-bindings: net: gmii2rgmii: Clarify proper usage

[Brandon Maier]

The devicetree example is written for setups where the Ethernet
controller automatically attaches to the first device on the mdio bus,
in this example ethernet-phy@0. But for more exotic setups where a
phy-handle must be used, it isn't immediately clear which device to
attach too. Some developers may be misled by the hardware diagram to
assume that the Ethernet controller should attach to the gmii2rgmii
device. Clarify this to save future users headache.

Signed-off-by: Brandon Maier <brandon.maier@gmail.com>
---
 .../devicetree/bindings/net/xilinx_gmii2rgmii.txt  | 31 +++++++++++++++-------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
index b2258ce5004d..462a207f3f70 100644
--- a/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
+++ b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
@@ -12,6 +12,11 @@ different speed modes by configuring the converter register through mdio writes.
 This converter sits between the ethernet MAC and the external PHY.
 GMII_MAC <==> GMII2RGMII <==> RGMII_PHY
 
+The driver is a shim between the MAC driver and PHY. It intercepts phy_driver
+calls to handle state changes. It is not a wrapper for the RGMII_PHY, and
+should not be used as an actual PHY device. Ethernet MAC drivers must attach
+directly to the RGMII_PHY driver.
+
 For more details about mdio please refer phy.txt file in the same directory.
 
 Required properties:
@@ -21,15 +26,21 @@ Required properties:
 		  See ethernet.txt file in the same directory.
 
 Example:
-	mdio {
-		#address-cells = <1>;
-		#size-cells = <0>;
-		phy: ethernet-phy@0 {
-			......
-		};
-		gmiitorgmii: gmiitorgmii@8 {
-			compatible = "xlnx,gmii-to-rgmii-1.0";
-			reg = <8>;
-			phy-handle = <&phy>;
+	gem0: ethernet@e000b000 {
+		compatible = "cdns,zynq-gem", "cdns,gem";
+		phy-handle = <&phy>;
+		......
+
+		mdio {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			phy: ethernet-phy@0 {
+				......
+			};
+			gmiitorgmii: gmiitorgmii@8 {
+				compatible = "xlnx,gmii-to-rgmii-1.0";
+				reg = <8>;
+				phy-handle = <&phy>;
+			};
 		};
 	};
-- 
2.11.0

Re: [PATCH 1/2] dt-bindings: net: gmii2rgmii: Fix typos

[Rob Herring]

On Sat, Apr 21, 2018 at 11:01:31PM -0500, Brandon Maier wrote:
> The wording of 'provides the RGMII' is unclear, and should be 'is a
> converter' to be consistent with the rest of this Doc. Also fix up
> assorted spelling and capitalization typos.
> 
> Signed-off-by: Brandon Maier <brandon.maier@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/2] dt-bindings: net: gmii2rgmii: Clarify proper usage

[Rob Herring]

On Sat, Apr 21, 2018 at 11:01:32PM -0500, Brandon Maier wrote:
> The devicetree example is written for setups where the Ethernet
> controller automatically attaches to the first device on the mdio bus,
> in this example ethernet-phy@0. But for more exotic setups where a
> phy-handle must be used, it isn't immediately clear which device to
> attach too. Some developers may be misled by the hardware diagram to
> assume that the Ethernet controller should attach to the gmii2rgmii
> device. Clarify this to save future users headache.
> 
> Signed-off-by: Brandon Maier <brandon.maier@gmail.com>
> ---
>  .../devicetree/bindings/net/xilinx_gmii2rgmii.txt  | 31 +++++++++++++++-------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> index b2258ce5004d..462a207f3f70 100644
> --- a/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> +++ b/Documentation/devicetree/bindings/net/xilinx_gmii2rgmii.txt
> @@ -12,6 +12,11 @@ different speed modes by configuring the converter register through mdio writes.
>  This converter sits between the ethernet MAC and the external PHY.
>  GMII_MAC <==> GMII2RGMII <==> RGMII_PHY
>  
> +The driver is a shim between the MAC driver and PHY. It intercepts phy_driver
> +calls to handle state changes. It is not a wrapper for the RGMII_PHY, and
> +should not be used as an actual PHY device. Ethernet MAC drivers must attach
> +directly to the RGMII_PHY driver.
> +
>  For more details about mdio please refer phy.txt file in the same directory.
>  
>  Required properties:
> @@ -21,15 +26,21 @@ Required properties:
>  		  See ethernet.txt file in the same directory.
>  
>  Example:
> -	mdio {
> -		#address-cells = <1>;
> -		#size-cells = <0>;
> -		phy: ethernet-phy@0 {
> -			......
> -		};
> -		gmiitorgmii: gmiitorgmii@8 {
> -			compatible = "xlnx,gmii-to-rgmii-1.0";
> -			reg = <8>;
> -			phy-handle = <&phy>;
> +	gem0: ethernet@e000b000 {
> +		compatible = "cdns,zynq-gem", "cdns,gem";
> +		phy-handle = <&phy>;

The phy here and...

> +		......
> +
> +		mdio {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			phy: ethernet-phy@0 {
> +				......
> +			};
> +			gmiitorgmii: gmiitorgmii@8 {
> +				compatible = "xlnx,gmii-to-rgmii-1.0";
> +				reg = <8>;
> +				phy-handle = <&phy>;

...here doesn't look right.

> +			};
>  		};
>  	};
> -- 
> 2.11.0
> 

Re: [PATCH 2/2] dt-bindings: net: gmii2rgmii: Clarify proper usage

[Brandon Maier]

On Fri, Apr 27, 2018 at 4:13 PM Rob Herring <robh@kernel.org> wrote:
...
> > +     gem0: ethernet@e000b000 {
> > +             compatible = "cdns,zynq-gem", "cdns,gem";
> > +             phy-handle = <&phy>;

> The phy here and...
...
> > +                     gmiitorgmii: gmiitorgmii@8 {
> > +                             compatible = "xlnx,gmii-to-rgmii-1.0";
> > +                             reg = <8>;
> > +                             phy-handle = <&phy>;

> ...here doesn't look right.


I don't see what you're referring to, they appear to match the
phy/phy-handle formats in Documentation/devicetree/bindings/net/phy.txt and
ethernet.txt. It's a bit odd that the label is just "phy", when most phy
labels are "phy0" etc., is that what should be changed?

Re: [PATCH 2/2] dt-bindings: net: gmii2rgmii: Clarify proper usage

[Rob Herring]

On Fri, Apr 27, 2018 at 11:20:01PM +0000, Brandon Maier wrote:
> On Fri, Apr 27, 2018 at 4:13 PM Rob Herring <robh@kernel.org> wrote:
> ...
> > > +     gem0: ethernet@e000b000 {
> > > +             compatible = "cdns,zynq-gem", "cdns,gem";
> > > +             phy-handle = <&phy>;
> 
> > The phy here and...
> ...
> > > +                     gmiitorgmii: gmiitorgmii@8 {
> > > +                             compatible = "xlnx,gmii-to-rgmii-1.0";
> > > +                             reg = <8>;
> > > +                             phy-handle = <&phy>;
> 
> > ...here doesn't look right.
> 
> 
> I don't see what you're referring to, they appear to match the
> phy/phy-handle formats in Documentation/devicetree/bindings/net/phy.txt and
> ethernet.txt. It's a bit odd that the label is just "phy", when most phy
> labels are "phy0" etc., is that what should be changed?

You don't need both properties. If the ethernet driver needs the phy, 
then it can decend into the child nodes to find it.

Rob

Re: [PATCH 2/2] dt-bindings: net: gmii2rgmii: Clarify proper usage

[Brandon Maier]

On Tue, May 1, 2018 at 8:16 AM Rob Herring <robh@kernel.org> wrote:
...
> You don't need both properties. If the ethernet driver needs the phy,
> then it can decend into the child nodes to find it.

Ah, in the scenario I hit, the phy was on a separate mdio bus. So the
phy-handle was necessary. Is it common behavior for an Ethernet
controller to attach to the first phy on its own mdio bus? That wasn't
clear to me looking at the documentation, and is where I messed up
and attached the Ethernet to the gmiitorgmii phy.

  I could refactor the example to more closely resemble my scenario, e.g.

ethernet0 {
   mdio {
      phy: ethernet-phy@0 {
        ...
      }
      gmiitorgmii@8 {
         ...
       }
   }
}
ethernet1 {
    phy-handle = <&phy>;
}

But that would make the example too specific. So I'm inclined
to resubmit with just the added description, and drop the example changes.