Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

On Tue, Sep 19, 2017 at 09:49:52PM -0500, Rob Herring wrote:
> On Thu, Sep 14, 2017 at 2:19 PM, Andrew Lunn  wrote:
> >> > Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"?
> >> > If the latter, then I think the node is fine, but then the mux should be
> >> > a child node of it. IOW, the child of an MDIO controller should either
> >> > be a mux node or slave devices.
> >
> > Hi Rob
> >
> > Up until now, children of an MDIO bus have been MDIO devices. Those
> > MDIO devices are either Ethernet PHYs, Ethernet Switches, or the
> > oddball devices that Broadcom iProc has, like generic PHYs.
> >
> > We have never had MDIO-muxes as MDIO children. A Mux is not an MDIO
> > device, and does not have the properties of an MDIO device. It is not
> > addressable on the MDIO bus. The current MUXes are addressed via GPIOs
> > or MMIO.
> 
> The DT parent/child relationship defines the bus topology. We describe
> MDIO buses in that way and if a mux is sitting between the controller
> and the devices, then the DT hierarchy should reflect that. Now
> sometimes we have 2 options for what interface has the parent/child
> relationship (e.g. an I2C controlled USB hub chip), but in this case
> we don't.
> 

Putting mdio-mux as a child of it (the mdio node) give me:
[   18.175338] libphy: stmmac: probed
[   18.175379] mdio_bus stmmac-0: /soc/ethernet@1c3/mdio/mdio-mux has 
invalid PHY address
[   18.175408] mdio_bus stmmac-0: scan phy mdio-mux at address 0
[   18.175450] mdio_bus stmmac-0: scan phy mdio-mux at address 1
[   18.175482] mdio_bus stmmac-0: scan phy mdio-mux at address 2
[   18.175513] mdio_bus stmmac-0: scan phy mdio-mux at address 3
[   18.175544] mdio_bus stmmac-0: scan phy mdio-mux at address 4
[   18.175575] mdio_bus stmmac-0: scan phy mdio-mux at address 5
[   18.175607] mdio_bus stmmac-0: scan phy mdio-mux at address 6
[   18.175638] mdio_bus stmmac-0: scan phy mdio-mux at address 7
[   18.175669] mdio_bus stmmac-0: scan phy mdio-mux at address 8
[   18.175700] mdio_bus stmmac-0: scan phy mdio-mux at address 9
[   18.175731] mdio_bus stmmac-0: scan phy mdio-mux at address 10
[   18.175762] mdio_bus stmmac-0: scan phy mdio-mux at address 11
[   18.175795] mdio_bus stmmac-0: scan phy mdio-mux at address 12
[   18.175827] mdio_bus stmmac-0: scan phy mdio-mux at address 13
[   18.175858] mdio_bus stmmac-0: scan phy mdio-mux at address 14
[   18.175889] mdio_bus stmmac-0: scan phy mdio-mux at address 15
[   18.175919] mdio_bus stmmac-0: scan phy mdio-mux at address 16
[   18.175951] mdio_bus stmmac-0: scan phy mdio-mux at address 17
[   18.175982] mdio_bus stmmac-0: scan phy mdio-mux at address 18
[   18.176014] mdio_bus stmmac-0: scan phy mdio-mux at address 19
[   18.176045] mdio_bus stmmac-0: scan phy mdio-mux at address 20
[   18.176076] mdio_bus stmmac-0: scan phy mdio-mux at address 21
[   18.176107] mdio_bus stmmac-0: scan phy mdio-mux at address 22
[   18.176139] mdio_bus stmmac-0: scan phy mdio-mux at address 23
[   18.176170] mdio_bus stmmac-0: scan phy mdio-mux at address 24
[   18.176202] mdio_bus stmmac-0: scan phy mdio-mux at address 25
[   18.176233] mdio_bus stmmac-0: scan phy mdio-mux at address 26
[   18.176271] mdio_bus stmmac-0: scan phy mdio-mux at address 27
[   18.176320] mdio_bus stmmac-0: scan phy mdio-mux at address 28
[   18.176371] mdio_bus stmmac-0: scan phy mdio-mux at address 29
[   18.176420] mdio_bus stmmac-0: scan phy mdio-mux at address 30
[   18.176452] mdio_bus stmmac-0: scan phy mdio-mux at address 31

Adding a fake  to mdio-mux remove it, but I found that a bit ugly.
Or perhaps patching of_mdiobus_register() to not scan node with compatible 
"mdio-mux".

What do you think ?

Regards

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Rob Herring]

On Thu, Sep 14, 2017 at 2:19 PM, Andrew Lunn  wrote:
>> > Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"?
>> > If the latter, then I think the node is fine, but then the mux should be
>> > a child node of it. IOW, the child of an MDIO controller should either
>> > be a mux node or slave devices.
>
> Hi Rob
>
> Up until now, children of an MDIO bus have been MDIO devices. Those
> MDIO devices are either Ethernet PHYs, Ethernet Switches, or the
> oddball devices that Broadcom iProc has, like generic PHYs.
>
> We have never had MDIO-muxes as MDIO children. A Mux is not an MDIO
> device, and does not have the properties of an MDIO device. It is not
> addressable on the MDIO bus. The current MUXes are addressed via GPIOs
> or MMIO.

The DT parent/child relationship defines the bus topology. We describe
MDIO buses in that way and if a mux is sitting between the controller
and the devices, then the DT hierarchy should reflect that. Now
sometimes we have 2 options for what interface has the parent/child
relationship (e.g. an I2C controlled USB hub chip), but in this case
we don't.

> There other similar cases. i2c-mux-gpio is not a child of an i2c bus,
> nor i2c-mux-reg or gpio-mux. nxp,pca9548 is however a child of the i2c
> bus, because it is an i2c device itself...

Some are i2c controlled mux devices, but some can be GPIO controlled.

>
> If the MDIO mux was an MDIO device, i would agree with you. Bit it is
> not, so lets not make it a child.
>
> Andrew
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

On Thu, Sep 14, 2017 at 09:19:49PM +0200, Andrew Lunn wrote:
> > > Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"? 
> > > If the latter, then I think the node is fine, but then the mux should be 
> > > a child node of it. IOW, the child of an MDIO controller should either 
> > > be a mux node or slave devices.
> 
> Hi Rob
> 
> Up until now, children of an MDIO bus have been MDIO devices. Those
> MDIO devices are either Ethernet PHYs, Ethernet Switches, or the
> oddball devices that Broadcom iProc has, like generic PHYs.
> 
> We have never had MDIO-muxes as MDIO children. A Mux is not an MDIO
> device, and does not have the properties of an MDIO device. It is not
> addressable on the MDIO bus. The current MUXes are addressed via GPIOs
> or MMIO.
> 
> There other similar cases. i2c-mux-gpio is not a child of an i2c bus,
> nor i2c-mux-reg or gpio-mux. nxp,pca9548 is however a child of the i2c
> bus, because it is an i2c device itself...
> 
> If the MDIO mux was an MDIO device, i would agree with you. Bit it is
> not, so lets not make it a child.
> 
>   Andrew

Hello Rob, could you anwser/confirm please.
I wait on this for sending the next version.

Thanks
Regards
Corentin Labbe

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Andrew Lunn]

> > Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"? 
> > If the latter, then I think the node is fine, but then the mux should be 
> > a child node of it. IOW, the child of an MDIO controller should either 
> > be a mux node or slave devices.

Hi Rob

Up until now, children of an MDIO bus have been MDIO devices. Those
MDIO devices are either Ethernet PHYs, Ethernet Switches, or the
oddball devices that Broadcom iProc has, like generic PHYs.

We have never had MDIO-muxes as MDIO children. A Mux is not an MDIO
device, and does not have the properties of an MDIO device. It is not
addressable on the MDIO bus. The current MUXes are addressed via GPIOs
or MMIO.

There other similar cases. i2c-mux-gpio is not a child of an i2c bus,
nor i2c-mux-reg or gpio-mux. nxp,pca9548 is however a child of the i2c
bus, because it is an i2c device itself...

If the MDIO mux was an MDIO device, i would agree with you. Bit it is
not, so lets not make it a child.

Andrew

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

On Wed, Sep 13, 2017 at 01:20:04PM -0500, Rob Herring wrote:
> On Fri, Sep 08, 2017 at 09:43:25AM +0200, Corentin Labbe wrote:
> > On Fri, Sep 08, 2017 at 09:25:38AM +0200, Maxime Ripard wrote:
> > > On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> > > > This patch add documentation about the MDIO switch used on sun8i-h3-emac
> > > > for integrated PHY.
> > > > 
> > > > Signed-off-by: Corentin Labbe 
> > > > ---
> > > >  .../devicetree/bindings/net/dwmac-sun8i.txt| 127 
> > > > +++--
> > > >  1 file changed, 120 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
> > > > b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > > index 725f3b187886..3fa0e54825ea 100644
> > > > --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > > +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > > @@ -39,7 +39,7 @@ Optional properties for the following compatibles:
> > > >  - allwinner,leds-active-low: EPHY LEDs are active low
> > > >  
> > > >  Required child node of emac:
> > > > -- mdio bus node: should be named mdio
> > > > +- mdio bus node: should be labelled mdio
> > > 
> > > labels do not end up in the final DT (while the names do) so why are
> > > you making this change?
> > > 
> > 
> > I misunderstood label/name.
> > Anyway, this contrainst should leave due to "snps,dwmac-mdio MDIOs are 
> > automatically registered"
> > 
> > > >  
> > > >  Required properties of the mdio node:
> > > >  - #address-cells: shall be 1
> > > > @@ -48,14 +48,28 @@ Required properties of the mdio node:
> > > >  The device node referenced by "phy" or "phy-handle" should be a child 
> > > > node
> > > >  of the mdio node. See phy.txt for the generic PHY bindings.
> > > >  
> > > > -Required properties of the phy node with the following compatibles:
> > > > +The following compatibles require an mdio-mux node called "mdio-mux":
> > > > +  - "allwinner,sun8i-h3-emac"
> > > > +  - "allwinner,sun8i-v3s-emac":
> > > > +Required properties for the mdio-mux node:
> > > > +  - compatible = "mdio-mux"
> > > > +  - one child mdio for the integrated mdio
> > > > +  - one child mdio for the external mdio if present (V3s have none)
> > > > +Required properties for the mdio-mux children node:
> > > > +  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> > > > +
> > > > +The following compatibles require a PHY node representing the 
> > > > integrated
> > > > +PHY, under the integrated MDIO bus node if an mdio-mux node is used:
> > > >- "allwinner,sun8i-h3-emac",
> > > >- "allwinner,sun8i-v3s-emac":
> > > > +
> > > > +Required properties of the integrated phy node:
> > > >  - clocks: a phandle to the reference clock for the EPHY
> > > >  - resets: a phandle to the reset control for the EPHY
> > > > +- phy-is-integrated
> > > > +- Should be a child of the integrated mdio
> > > 
> > > I'm not sure what you mean by that, you ask that it should (so not
> > > required?) be a child of the integrated mdio...
> > > 
> > 
> > I will change words to "must"
> > 
> > > >  
> > > > -Example:
> > > > -
> > > > +Example with integrated PHY:
> > > >  emac: ethernet@1c0b000 {
> > > > compatible = "allwinner,sun8i-h3-emac";
> > > > syscon = <>;
> > > > @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
> > > > phy-handle = <_mii_phy>;
> > > > phy-mode = "mii";
> > > > allwinner,leds-active-low;
> > > > +
> > > > +   mdio0: mdio {
> > > 
> > > (You don't label it mdio here, unlike what was asked before)
> > > 
> > > > +   #address-cells = <1>;
> > > > +   #size-cells = <0>;
> > > > +   compatible = "snps,dwmac-mdio";
> > > > +   };
> > > 
> > > I think Rob wanted that node gone?
> > > 
> > 
> > MDIO mux does not work without a parent MDIO, either gived by "parent-bus" 
> > or directly via mdio_mux_init() (like it is the case in dwmac-sun8i)
> 
> Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"? 
> If the latter, then I think the node is fine, but then the mux should be 
> a child node of it. IOW, the child of an MDIO controller should either 
> be a mux node or slave devices.
> 

It will be snps,dwmac-mdio but putting mdio-mux as a child of it (the mdio 
node) give me:
[   18.175338] libphy: stmmac: probed
[   18.175379] mdio_bus stmmac-0: /soc/ethernet@1c3/mdio/mdio-mux has 
invalid PHY address
[   18.175408] mdio_bus stmmac-0: scan phy mdio-mux at address 0
[   18.175450] mdio_bus stmmac-0: scan phy mdio-mux at address 1
[   18.175482] mdio_bus stmmac-0: scan phy mdio-mux at address 2
[   18.175513] mdio_bus stmmac-0: scan phy mdio-mux at address 3
[   18.175544] mdio_bus stmmac-0: scan phy mdio-mux at address 4
[   18.175575] mdio_bus stmmac-0: scan phy mdio-mux at address 5
[   18.175607] mdio_bus stmmac-0: scan phy mdio-mux at address 6
[   18.175638] mdio_bus stmmac-0: scan phy mdio-mux 

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Rob Herring]

On Fri, Sep 08, 2017 at 09:43:25AM +0200, Corentin Labbe wrote:
> On Fri, Sep 08, 2017 at 09:25:38AM +0200, Maxime Ripard wrote:
> > On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> > > This patch add documentation about the MDIO switch used on sun8i-h3-emac
> > > for integrated PHY.
> > > 
> > > Signed-off-by: Corentin Labbe 
> > > ---
> > >  .../devicetree/bindings/net/dwmac-sun8i.txt| 127 
> > > +++--
> > >  1 file changed, 120 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
> > > b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > index 725f3b187886..3fa0e54825ea 100644
> > > --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > > @@ -39,7 +39,7 @@ Optional properties for the following compatibles:
> > >  - allwinner,leds-active-low: EPHY LEDs are active low
> > >  
> > >  Required child node of emac:
> > > -- mdio bus node: should be named mdio
> > > +- mdio bus node: should be labelled mdio
> > 
> > labels do not end up in the final DT (while the names do) so why are
> > you making this change?
> > 
> 
> I misunderstood label/name.
> Anyway, this contrainst should leave due to "snps,dwmac-mdio MDIOs are 
> automatically registered"
> 
> > >  
> > >  Required properties of the mdio node:
> > >  - #address-cells: shall be 1
> > > @@ -48,14 +48,28 @@ Required properties of the mdio node:
> > >  The device node referenced by "phy" or "phy-handle" should be a child 
> > > node
> > >  of the mdio node. See phy.txt for the generic PHY bindings.
> > >  
> > > -Required properties of the phy node with the following compatibles:
> > > +The following compatibles require an mdio-mux node called "mdio-mux":
> > > +  - "allwinner,sun8i-h3-emac"
> > > +  - "allwinner,sun8i-v3s-emac":
> > > +Required properties for the mdio-mux node:
> > > +  - compatible = "mdio-mux"
> > > +  - one child mdio for the integrated mdio
> > > +  - one child mdio for the external mdio if present (V3s have none)
> > > +Required properties for the mdio-mux children node:
> > > +  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> > > +
> > > +The following compatibles require a PHY node representing the integrated
> > > +PHY, under the integrated MDIO bus node if an mdio-mux node is used:
> > >- "allwinner,sun8i-h3-emac",
> > >- "allwinner,sun8i-v3s-emac":
> > > +
> > > +Required properties of the integrated phy node:
> > >  - clocks: a phandle to the reference clock for the EPHY
> > >  - resets: a phandle to the reset control for the EPHY
> > > +- phy-is-integrated
> > > +- Should be a child of the integrated mdio
> > 
> > I'm not sure what you mean by that, you ask that it should (so not
> > required?) be a child of the integrated mdio...
> > 
> 
> I will change words to "must"
> 
> > >  
> > > -Example:
> > > -
> > > +Example with integrated PHY:
> > >  emac: ethernet@1c0b000 {
> > >   compatible = "allwinner,sun8i-h3-emac";
> > >   syscon = <>;
> > > @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
> > >   phy-handle = <_mii_phy>;
> > >   phy-mode = "mii";
> > >   allwinner,leds-active-low;
> > > +
> > > + mdio0: mdio {
> > 
> > (You don't label it mdio here, unlike what was asked before)
> > 
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + compatible = "snps,dwmac-mdio";
> > > + };
> > 
> > I think Rob wanted that node gone?
> > 
> 
> MDIO mux does not work without a parent MDIO, either gived by "parent-bus" or 
> directly via mdio_mux_init() (like it is the case in dwmac-sun8i)

Is the MDIO controller "allwinner,sun8i-h3-emac" or "snps,dwmac-mdio"? 
If the latter, then I think the node is fine, but then the mux should be 
a child node of it. IOW, the child of an MDIO controller should either 
be a mux node or slave devices.

> 
> > > + mdio-mux {
> > > + compatible = "mdio-mux";
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > +
> > > + int_mdio: mdio@1 {
> > > + reg = <0>;

unit address of 1 and reg prop of 0 don't match. Build your dtb with 
W=2.

> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + int_mii_phy: ethernet-phy@1 {
> > > + reg = <1>;
> > > + clocks = < CLK_BUS_EPHY>;
> > > + resets = < RST_BUS_EPHY>;
> > > + phy-is-integrated

Missing ;

> > > + };
> > > + };
> > 
> > ... And in your example it's a child of the mdio mux?
> > 
> 
> So I confirm, integrated PHY must be a child of integrated MDIO (that must be 
> a child of mdio-mux).
> The example is good.
> 
> > > + ext_mdio: mdio@0 {
> > > + reg = <1>;

Another unit address mismatch.

Rob

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

On Fri, Sep 08, 2017 at 09:25:38AM +0200, Maxime Ripard wrote:
> On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> > This patch add documentation about the MDIO switch used on sun8i-h3-emac
> > for integrated PHY.
> > 
> > Signed-off-by: Corentin Labbe 
> > ---
> >  .../devicetree/bindings/net/dwmac-sun8i.txt| 127 
> > +++--
> >  1 file changed, 120 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
> > b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > index 725f3b187886..3fa0e54825ea 100644
> > --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > @@ -39,7 +39,7 @@ Optional properties for the following compatibles:
> >  - allwinner,leds-active-low: EPHY LEDs are active low
> >  
> >  Required child node of emac:
> > -- mdio bus node: should be named mdio
> > +- mdio bus node: should be labelled mdio
> 
> labels do not end up in the final DT (while the names do) so why are
> you making this change?
> 

I misunderstood label/name.
Anyway, this contrainst should leave due to "snps,dwmac-mdio MDIOs are 
automatically registered"

> >  
> >  Required properties of the mdio node:
> >  - #address-cells: shall be 1
> > @@ -48,14 +48,28 @@ Required properties of the mdio node:
> >  The device node referenced by "phy" or "phy-handle" should be a child node
> >  of the mdio node. See phy.txt for the generic PHY bindings.
> >  
> > -Required properties of the phy node with the following compatibles:
> > +The following compatibles require an mdio-mux node called "mdio-mux":
> > +  - "allwinner,sun8i-h3-emac"
> > +  - "allwinner,sun8i-v3s-emac":
> > +Required properties for the mdio-mux node:
> > +  - compatible = "mdio-mux"
> > +  - one child mdio for the integrated mdio
> > +  - one child mdio for the external mdio if present (V3s have none)
> > +Required properties for the mdio-mux children node:
> > +  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> > +
> > +The following compatibles require a PHY node representing the integrated
> > +PHY, under the integrated MDIO bus node if an mdio-mux node is used:
> >- "allwinner,sun8i-h3-emac",
> >- "allwinner,sun8i-v3s-emac":
> > +
> > +Required properties of the integrated phy node:
> >  - clocks: a phandle to the reference clock for the EPHY
> >  - resets: a phandle to the reset control for the EPHY
> > +- phy-is-integrated
> > +- Should be a child of the integrated mdio
> 
> I'm not sure what you mean by that, you ask that it should (so not
> required?) be a child of the integrated mdio...
> 

I will change words to "must"

> >  
> > -Example:
> > -
> > +Example with integrated PHY:
> >  emac: ethernet@1c0b000 {
> > compatible = "allwinner,sun8i-h3-emac";
> > syscon = <>;
> > @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
> > phy-handle = <_mii_phy>;
> > phy-mode = "mii";
> > allwinner,leds-active-low;
> > +
> > +   mdio0: mdio {
> 
> (You don't label it mdio here, unlike what was asked before)
> 
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   compatible = "snps,dwmac-mdio";
> > +   };
> 
> I think Rob wanted that node gone?
> 

MDIO mux does not work without a parent MDIO, either gived by "parent-bus" or 
directly via mdio_mux_init() (like it is the case in dwmac-sun8i)

> > +   mdio-mux {
> > +   compatible = "mdio-mux";
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   int_mdio: mdio@1 {
> > +   reg = <0>;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   int_mii_phy: ethernet-phy@1 {
> > +   reg = <1>;
> > +   clocks = < CLK_BUS_EPHY>;
> > +   resets = < RST_BUS_EPHY>;
> > +   phy-is-integrated
> > +   };
> > +   };
> 
> ... And in your example it's a child of the mdio mux?
> 

So I confirm, integrated PHY must be a child of integrated MDIO (that must be a 
child of mdio-mux).
The example is good.

> > +   ext_mdio: mdio@0 {
> > +   reg = <1>;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   };
> > +   };
> > +};
> > +
> > +Example with external PHY:
> > +emac: ethernet@1c0b000 {
> > +   compatible = "allwinner,sun8i-h3-emac";
> > +   syscon = <>;
> > +   reg = <0x01c0b000 0x104>;
> > +   interrupts = ;
> > +   interrupt-names = "macirq";
> > +   resets = < RST_BUS_EMAC>;
> > +   reset-names = "stmmaceth";
> > +   clocks = < CLK_BUS_EMAC>;
> > +   clock-names = "stmmaceth";
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   phy-handle = <_rgmii_phy>;
> > +   phy-mode = "rgmii";
> > +   allwinner,leds-active-low;
> > +
> > +   mdio0: mdio {
> 

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Maxime Ripard]

On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> This patch add documentation about the MDIO switch used on sun8i-h3-emac
> for integrated PHY.
> 
> Signed-off-by: Corentin Labbe 
> ---
>  .../devicetree/bindings/net/dwmac-sun8i.txt| 127 
> +++--
>  1 file changed, 120 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
> b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> index 725f3b187886..3fa0e54825ea 100644
> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> @@ -39,7 +39,7 @@ Optional properties for the following compatibles:
>  - allwinner,leds-active-low: EPHY LEDs are active low
>  
>  Required child node of emac:
> -- mdio bus node: should be named mdio
> +- mdio bus node: should be labelled mdio

labels do not end up in the final DT (while the names do) so why are
you making this change?

>  
>  Required properties of the mdio node:
>  - #address-cells: shall be 1
> @@ -48,14 +48,28 @@ Required properties of the mdio node:
>  The device node referenced by "phy" or "phy-handle" should be a child node
>  of the mdio node. See phy.txt for the generic PHY bindings.
>  
> -Required properties of the phy node with the following compatibles:
> +The following compatibles require an mdio-mux node called "mdio-mux":
> +  - "allwinner,sun8i-h3-emac"
> +  - "allwinner,sun8i-v3s-emac":
> +Required properties for the mdio-mux node:
> +  - compatible = "mdio-mux"
> +  - one child mdio for the integrated mdio
> +  - one child mdio for the external mdio if present (V3s have none)
> +Required properties for the mdio-mux children node:
> +  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> +
> +The following compatibles require a PHY node representing the integrated
> +PHY, under the integrated MDIO bus node if an mdio-mux node is used:
>- "allwinner,sun8i-h3-emac",
>- "allwinner,sun8i-v3s-emac":
> +
> +Required properties of the integrated phy node:
>  - clocks: a phandle to the reference clock for the EPHY
>  - resets: a phandle to the reset control for the EPHY
> +- phy-is-integrated
> +- Should be a child of the integrated mdio

I'm not sure what you mean by that, you ask that it should (so not
required?) be a child of the integrated mdio...

>  
> -Example:
> -
> +Example with integrated PHY:
>  emac: ethernet@1c0b000 {
>   compatible = "allwinner,sun8i-h3-emac";
>   syscon = <>;
> @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
>   phy-handle = <_mii_phy>;
>   phy-mode = "mii";
>   allwinner,leds-active-low;
> +
> + mdio0: mdio {

(You don't label it mdio here, unlike what was asked before)

> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "snps,dwmac-mdio";
> + };

I think Rob wanted that node gone?

> + mdio-mux {
> + compatible = "mdio-mux";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + int_mdio: mdio@1 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = < CLK_BUS_EPHY>;
> + resets = < RST_BUS_EPHY>;
> + phy-is-integrated
> + };
> + };

... And in your example it's a child of the mdio mux?

> + ext_mdio: mdio@0 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +};
> +
> +Example with external PHY:
> +emac: ethernet@1c0b000 {
> + compatible = "allwinner,sun8i-h3-emac";
> + syscon = <>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = ;
> + interrupt-names = "macirq";
> + resets = < RST_BUS_EMAC>;
> + reset-names = "stmmaceth";
> + clocks = < CLK_BUS_EMAC>;
> + clock-names = "stmmaceth";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy-handle = <_rgmii_phy>;
> + phy-mode = "rgmii";
> + allwinner,leds-active-low;
> +
> + mdio0: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "snps,dwmac-mdio";
> + };
> +
> + mdio-mux {
> + compatible = "mdio-mux";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + int_mdio: mdio@1 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = < CLK_BUS_EPHY>;
> + resets = < RST_BUS_EPHY>;
> +

[PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

This patch add documentation about the MDIO switch used on sun8i-h3-emac
for integrated PHY.

Signed-off-by: Corentin Labbe 
---
 .../devicetree/bindings/net/dwmac-sun8i.txt| 127 +++--
 1 file changed, 120 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 725f3b187886..3fa0e54825ea 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -39,7 +39,7 @@ Optional properties for the following compatibles:
 - allwinner,leds-active-low: EPHY LEDs are active low
 
 Required child node of emac:
-- mdio bus node: should be named mdio
+- mdio bus node: should be labelled mdio
 
 Required properties of the mdio node:
 - #address-cells: shall be 1
@@ -48,14 +48,28 @@ Required properties of the mdio node:
 The device node referenced by "phy" or "phy-handle" should be a child node
 of the mdio node. See phy.txt for the generic PHY bindings.
 
-Required properties of the phy node with the following compatibles:
+The following compatibles require an mdio-mux node called "mdio-mux":
+  - "allwinner,sun8i-h3-emac"
+  - "allwinner,sun8i-v3s-emac":
+Required properties for the mdio-mux node:
+  - compatible = "mdio-mux"
+  - one child mdio for the integrated mdio
+  - one child mdio for the external mdio if present (V3s have none)
+Required properties for the mdio-mux children node:
+  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
+
+The following compatibles require a PHY node representing the integrated
+PHY, under the integrated MDIO bus node if an mdio-mux node is used:
   - "allwinner,sun8i-h3-emac",
   - "allwinner,sun8i-v3s-emac":
+
+Required properties of the integrated phy node:
 - clocks: a phandle to the reference clock for the EPHY
 - resets: a phandle to the reset control for the EPHY
+- phy-is-integrated
+- Should be a child of the integrated mdio
 
-Example:
-
+Example with integrated PHY:
 emac: ethernet@1c0b000 {
compatible = "allwinner,sun8i-h3-emac";
syscon = <>;
@@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
phy-handle = <_mii_phy>;
phy-mode = "mii";
allwinner,leds-active-low;
+
+   mdio0: mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   compatible = "snps,dwmac-mdio";
+   };
+
+   mdio-mux {
+   compatible = "mdio-mux";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   int_mdio: mdio@1 {
+   reg = <0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   int_mii_phy: ethernet-phy@1 {
+   reg = <1>;
+   clocks = < CLK_BUS_EPHY>;
+   resets = < RST_BUS_EPHY>;
+   phy-is-integrated
+   };
+   };
+   ext_mdio: mdio@0 {
+   reg = <1>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+   };
+};
+
+Example with external PHY:
+emac: ethernet@1c0b000 {
+   compatible = "allwinner,sun8i-h3-emac";
+   syscon = <>;
+   reg = <0x01c0b000 0x104>;
+   interrupts = ;
+   interrupt-names = "macirq";
+   resets = < RST_BUS_EMAC>;
+   reset-names = "stmmaceth";
+   clocks = < CLK_BUS_EMAC>;
+   clock-names = "stmmaceth";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy-handle = <_rgmii_phy>;
+   phy-mode = "rgmii";
+   allwinner,leds-active-low;
+
+   mdio0: mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   compatible = "snps,dwmac-mdio";
+   };
+
+   mdio-mux {
+   compatible = "mdio-mux";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   int_mdio: mdio@1 {
+   reg = <0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   int_mii_phy: ethernet-phy@1 {
+   reg = <1>;
+   clocks = < CLK_BUS_EPHY>;
+   resets = < RST_BUS_EPHY>;
+   phy-is-integrated
+   };
+   };
+   ext_mdio: mdio@0 {
+   reg = <1>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ext_rgmii_phy: ethernet-phy@1 {
+   reg = <1>;
+   };
+   };
+   };
+};
+
+Example with SoC without integrated PHY
+
+emac: ethernet@1c0b000 {
+   compatible = "allwinner,sun8i-a83t-emac";
+   syscon = <>;
+   reg