[PATCH 0/3] net: stmmac: Detect PHY location with phy-is-integrated

[PATCH 0/3] net: stmmac: Detect PHY location with phy-is-integrated

[Corentin Labbe]

Hello

The current way to find if the phy is internal is to compare DT phy-mode
and emac_variant/internal_phy.
But it will negate a possible future SoC where an external PHY use the
same phy mode than the internal one.

This patchs series adds a new way to find if the PHY is internal, via
the phy-is-integrated DT property.

The first and third patch should go via the net tree.
the second via the sunxi tree.

Thanks
Regards

Corentin Labbe (3):
  Documentation: bindings: Add documentation for phy-is-integrated
  ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal
    PHY
  net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated

 Documentation/devicetree/bindings/net/phy.txt     |  4 ++++
 arch/arm/boot/dts/sunxi-h3-h5.dtsi                |  1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++--------
 3 files changed, 13 insertions(+), 8 deletions(-)

-- 
2.13.0

[PATCH 1/3] Documentation: bindings: Add documentation for phy-is-integrated

[Corentin Labbe]

This patch adds documentation for phy-is-integrated, a boolean property
for PHY which permit to know if the PHY is integrated in the SoC.

For example, Allwinner H3 embeds an internal PHY but still permit to
connect an external PHY.
Since it is possible in theory to have the same PHY model both internal
and external, the only way to detect the location of the PHY is via this property.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 Documentation/devicetree/bindings/net/phy.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index b55857696fc3..6fabc14da432 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -52,6 +52,10 @@ Optional Properties:
   Mark the corresponding energy efficient ethernet mode as broken and
   request the ethernet to stop advertising it.
 
+- phy-is-integrated: If set, indicates that the PHY is integrated in the SoC
+  and so is not an external PHY. (Some SoC embeds a PHY and still provide
+  support for an optional external PHY)
+
 Example:
 
 ethernet-phy@0 {
-- 
2.13.0

[PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Corentin Labbe]

This patch add the new phy-is-integrated property to the internal PHY
node.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 4b599b5d26f6..54fc24e4c569 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -425,6 +425,7 @@
 					reg = <1>;
 					clocks = <&ccu CLK_BUS_EPHY>;
 					resets = <&ccu RST_BUS_EPHY>;
+					phy-is-integrated;
 				};
 			};
 		};
-- 
2.13.0

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Chen-Yu Tsai]

Hi,

On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
> This patch add the new phy-is-integrated property to the internal PHY
> node.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
>  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 4b599b5d26f6..54fc24e4c569 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -425,6 +425,7 @@
>                                         reg = <1>;
>                                         clocks = <&ccu CLK_BUS_EPHY>;
>                                         resets = <&ccu RST_BUS_EPHY>;
> +                                       phy-is-integrated;

You also need to "delete" this property at the board level for
any board that has the external PHY at address <1>. Otherwise
they will stop working. This is due to the internal and external
PHYs having the same path and node name in the device tree, so
they are effectively the same node.

ChenYu

>                                 };
>                         };
>                 };
> --
> 2.13.0
>

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Corentin Labbe]

On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
> <clabbe.montjoie@gmail.com> wrote:
> > This patch add the new phy-is-integrated property to the internal PHY
> > node.
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > ---
> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> > index 4b599b5d26f6..54fc24e4c569 100644
> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> > @@ -425,6 +425,7 @@
> >                                         reg = <1>;
> >                                         clocks = <&ccu CLK_BUS_EPHY>;
> >                                         resets = <&ccu RST_BUS_EPHY>;
> > +                                       phy-is-integrated;
> 
> You also need to "delete" this property at the board level for
> any board that has the external PHY at address <1>. Otherwise
> they will stop working. This is due to the internal and external
> PHYs having the same path and node name in the device tree, so
> they are effectively the same node.
> 
> ChenYu
> 

They have not the same name, ext_rgmii_phy vs int_mii_phy.

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Chen-Yu Tsai]

On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
> On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
>> <clabbe.montjoie@gmail.com> wrote:
>> > This patch add the new phy-is-integrated property to the internal PHY
>> > node.
>> >
>> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
>> > ---
>> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> > index 4b599b5d26f6..54fc24e4c569 100644
>> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> > @@ -425,6 +425,7 @@
>> >                                         reg = <1>;
>> >                                         clocks = <&ccu CLK_BUS_EPHY>;
>> >                                         resets = <&ccu RST_BUS_EPHY>;
>> > +                                       phy-is-integrated;
>>
>> You also need to "delete" this property at the board level for
>> any board that has the external PHY at address <1>. Otherwise
>> they will stop working. This is due to the internal and external
>> PHYs having the same path and node name in the device tree, so
>> they are effectively the same node.
>>
>> ChenYu
>>
>
> They have not the same name, ext_rgmii_phy vs int_mii_phy.

That is just the label. The label plays no part in device tree merging. The path

    /soc/ethernet@1c30000/mdio/ethernet-phy@1

is the same. You can look under

    /proc/device-tree/soc/ethernet@1c30000/mdio

on the OrangePI Plus 2E or any other H3 board that uses an
external PHY at address 1.

ChenYu

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Corentin Labbe]

On Fri, Aug 11, 2017 at 04:11:13PM +0800, Chen-Yu Tsai wrote:
> On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
> <clabbe.montjoie@gmail.com> wrote:
> > On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
> >> Hi,
> >>
> >> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
> >> <clabbe.montjoie@gmail.com> wrote:
> >> > This patch add the new phy-is-integrated property to the internal PHY
> >> > node.
> >> >
> >> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> >> > ---
> >> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
> >> >  1 file changed, 1 insertion(+)
> >> >
> >> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> > index 4b599b5d26f6..54fc24e4c569 100644
> >> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> > @@ -425,6 +425,7 @@
> >> >                                         reg = <1>;
> >> >                                         clocks = <&ccu CLK_BUS_EPHY>;
> >> >                                         resets = <&ccu RST_BUS_EPHY>;
> >> > +                                       phy-is-integrated;
> >>
> >> You also need to "delete" this property at the board level for
> >> any board that has the external PHY at address <1>. Otherwise
> >> they will stop working. This is due to the internal and external
> >> PHYs having the same path and node name in the device tree, so
> >> they are effectively the same node.
> >>
> >> ChenYu
> >>
> >
> > They have not the same name, ext_rgmii_phy vs int_mii_phy.
> 
> That is just the label. The label plays no part in device tree merging. The path
> 
>     /soc/ethernet@1c30000/mdio/ethernet-phy@1
> 
> is the same. You can look under
> 
>     /proc/device-tree/soc/ethernet@1c30000/mdio
> 
> on the OrangePI Plus 2E or any other H3 board that uses an
> external PHY at address 1.
> 
> ChenYu

Since we get the phy node by phy-handle and not by path, I think all should be good.

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Chen-Yu Tsai]

On Fri, Aug 11, 2017 at 4:19 PM, Corentin Labbe
<clabbe.montjoie@gmail.com> wrote:
> On Fri, Aug 11, 2017 at 04:11:13PM +0800, Chen-Yu Tsai wrote:
>> On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
>> <clabbe.montjoie@gmail.com> wrote:
>> > On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
>> >> Hi,
>> >>
>> >> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
>> >> <clabbe.montjoie@gmail.com> wrote:
>> >> > This patch add the new phy-is-integrated property to the internal PHY
>> >> > node.
>> >> >
>> >> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
>> >> > ---
>> >> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
>> >> >  1 file changed, 1 insertion(+)
>> >> >
>> >> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> > index 4b599b5d26f6..54fc24e4c569 100644
>> >> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> > @@ -425,6 +425,7 @@
>> >> >                                         reg = <1>;
>> >> >                                         clocks = <&ccu CLK_BUS_EPHY>;
>> >> >                                         resets = <&ccu RST_BUS_EPHY>;
>> >> > +                                       phy-is-integrated;
>> >>
>> >> You also need to "delete" this property at the board level for
>> >> any board that has the external PHY at address <1>. Otherwise
>> >> they will stop working. This is due to the internal and external
>> >> PHYs having the same path and node name in the device tree, so
>> >> they are effectively the same node.
>> >>
>> >> ChenYu
>> >>
>> >
>> > They have not the same name, ext_rgmii_phy vs int_mii_phy.
>>
>> That is just the label. The label plays no part in device tree merging. The path
>>
>>     /soc/ethernet@1c30000/mdio/ethernet-phy@1
>>
>> is the same. You can look under
>>
>>     /proc/device-tree/soc/ethernet@1c30000/mdio
>>
>> on the OrangePI Plus 2E or any other H3 board that uses an
>> external PHY at address 1.
>>
>> ChenYu
>
> Since we get the phy node by phy-handle and not by path, I think all should be good.

You are not getting me. The fact that the two seemingly separate
nodes are merged together means, whatever properties you put in
the internal PHY node, also affect the external PHY node. Once
compiled, they are the SAME node.

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Corentin Labbe]

On Fri, Aug 11, 2017 at 04:22:11PM +0800, Chen-Yu Tsai wrote:
> On Fri, Aug 11, 2017 at 4:19 PM, Corentin Labbe
> <clabbe.montjoie@gmail.com> wrote:
> > On Fri, Aug 11, 2017 at 04:11:13PM +0800, Chen-Yu Tsai wrote:
> >> On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
> >> <clabbe.montjoie@gmail.com> wrote:
> >> > On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
> >> >> Hi,
> >> >>
> >> >> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
> >> >> <clabbe.montjoie@gmail.com> wrote:
> >> >> > This patch add the new phy-is-integrated property to the internal PHY
> >> >> > node.
> >> >> >
> >> >> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> >> >> > ---
> >> >> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
> >> >> >  1 file changed, 1 insertion(+)
> >> >> >
> >> >> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> > index 4b599b5d26f6..54fc24e4c569 100644
> >> >> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> > @@ -425,6 +425,7 @@
> >> >> >                                         reg = <1>;
> >> >> >                                         clocks = <&ccu CLK_BUS_EPHY>;
> >> >> >                                         resets = <&ccu RST_BUS_EPHY>;
> >> >> > +                                       phy-is-integrated;
> >> >>
> >> >> You also need to "delete" this property at the board level for
> >> >> any board that has the external PHY at address <1>. Otherwise
> >> >> they will stop working. This is due to the internal and external
> >> >> PHYs having the same path and node name in the device tree, so
> >> >> they are effectively the same node.
> >> >>
> >> >> ChenYu
> >> >>
> >> >
> >> > They have not the same name, ext_rgmii_phy vs int_mii_phy.
> >>
> >> That is just the label. The label plays no part in device tree merging. The path
> >>
> >>     /soc/ethernet@1c30000/mdio/ethernet-phy@1
> >>
> >> is the same. You can look under
> >>
> >>     /proc/device-tree/soc/ethernet@1c30000/mdio
> >>
> >> on the OrangePI Plus 2E or any other H3 board that uses an
> >> external PHY at address 1.
> >>
> >> ChenYu
> >
> > Since we get the phy node by phy-handle and not by path, I think all should be good.
> 
> You are not getting me. The fact that the two seemingly separate
> nodes are merged together means, whatever properties you put in
> the internal PHY node, also affect the external PHY node. Once
> compiled, they are the SAME node.

So why not changing the internal node name from ethernet-phy to integrated-phy ?

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Corentin Labbe]

On Fri, Aug 11, 2017 at 04:22:11PM +0800, Chen-Yu Tsai wrote:
> On Fri, Aug 11, 2017 at 4:19 PM, Corentin Labbe
> <clabbe.montjoie@gmail.com> wrote:
> > On Fri, Aug 11, 2017 at 04:11:13PM +0800, Chen-Yu Tsai wrote:
> >> On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
> >> <clabbe.montjoie@gmail.com> wrote:
> >> > On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
> >> >> Hi,
> >> >>
> >> >> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
> >> >> <clabbe.montjoie@gmail.com> wrote:
> >> >> > This patch add the new phy-is-integrated property to the internal PHY
> >> >> > node.
> >> >> >
> >> >> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> >> >> > ---
> >> >> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
> >> >> >  1 file changed, 1 insertion(+)
> >> >> >
> >> >> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> > index 4b599b5d26f6..54fc24e4c569 100644
> >> >> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> > @@ -425,6 +425,7 @@
> >> >> >                                         reg = <1>;
> >> >> >                                         clocks = <&ccu CLK_BUS_EPHY>;
> >> >> >                                         resets = <&ccu RST_BUS_EPHY>;
> >> >> > +                                       phy-is-integrated;
> >> >>
> >> >> You also need to "delete" this property at the board level for
> >> >> any board that has the external PHY at address <1>. Otherwise
> >> >> they will stop working. This is due to the internal and external
> >> >> PHYs having the same path and node name in the device tree, so
> >> >> they are effectively the same node.
> >> >>
> >> >> ChenYu
> >> >>
> >> >
> >> > They have not the same name, ext_rgmii_phy vs int_mii_phy.
> >>
> >> That is just the label. The label plays no part in device tree merging. The path
> >>
> >>     /soc/ethernet@1c30000/mdio/ethernet-phy@1
> >>
> >> is the same. You can look under
> >>
> >>     /proc/device-tree/soc/ethernet@1c30000/mdio
> >>
> >> on the OrangePI Plus 2E or any other H3 board that uses an
> >> external PHY at address 1.
> >>
> >> ChenYu
> >
> > Since we get the phy node by phy-handle and not by path, I think all should be good.
> 
> You are not getting me. The fact that the two seemingly separate
> nodes are merged together means, whatever properties you put in
> the internal PHY node, also affect the external PHY node. Once
> compiled, they are the SAME node.

Hello Rob, florian, mark

Adding a delete property on all external ethernet-phy@1 is a bit overkill, and I dont like the idea that nodes are merged.
What do you think about other possible solutions:
- Using integrated-phy@1 for the integrated PHY node name
- Using a fake address like 31 (see patch below)

If you have any other solution...

Regards

>From fe39183946f7f4a6e21bce38fd8e4c1413012d68 Mon Sep 17 00:00:00 2001
From: Corentin Labbe <clabbe.montjoie@gmail.com>
Date: Fri, 11 Aug 2017 14:49:54 +0200
Subject: [PATCH] ARM: sun8i: sunxi-h3-h5: Prevent merge of external and
 integrated PHY

Actually, some external and integrated PHY are merged due to same dtnode
name "ethernet-phy@1".

This is problematic when we will want to use the phy-is-integrated
property. (Need to delete it on all external PHY node)

An easy solution is to set integrated PHY nodeaddresss at a fake one
that would never be used.
Since board makers currently only provides PHY at addresses 1 and 7,
we will use 31.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 54fc24e4c569..2110b0069e33 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -420,7 +420,15 @@
 			mdio: mdio {
 				#address-cells = <1>;
 				#size-cells = <0>;
-				int_mii_phy: ethernet-phy@1 {
+				/*
+				 * Using 31 permits to make a separation between
+				 * this integrated PHY and external ones.
+				 * Without it, external "ethernet-phy@1" will be
+				 * merged with it (due to same dtnode name).
+				 * Board makers currently only provides PHY at
+				 * addresses 1 and 7.
+				 */
+				int_mii_phy: ethernet-phy@31 {
 					compatible = "ethernet-phy-ieee802.3-c22";
 					reg = <1>;
 					clocks = <&ccu CLK_BUS_EPHY>;
-- 
2.13.0

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Florian Fainelli]

On August 11, 2017 6:25:26 AM PDT, Corentin Labbe <clabbe.montjoie@gmail.com> wrote:
>On Fri, Aug 11, 2017 at 04:22:11PM +0800, Chen-Yu Tsai wrote:
>> On Fri, Aug 11, 2017 at 4:19 PM, Corentin Labbe
>> <clabbe.montjoie@gmail.com> wrote:
>> > On Fri, Aug 11, 2017 at 04:11:13PM +0800, Chen-Yu Tsai wrote:
>> >> On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
>> >> <clabbe.montjoie@gmail.com> wrote:
>> >> > On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
>> >> >> Hi,
>> >> >>
>> >> >> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
>> >> >> <clabbe.montjoie@gmail.com> wrote:
>> >> >> > This patch add the new phy-is-integrated property to the
>internal PHY
>> >> >> > node.
>> >> >> >
>> >> >> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
>> >> >> > ---
>> >> >> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
>> >> >> >  1 file changed, 1 insertion(+)
>> >> >> >
>> >> >> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> >> > index 4b599b5d26f6..54fc24e4c569 100644
>> >> >> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> >> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>> >> >> > @@ -425,6 +425,7 @@
>> >> >> >                                         reg = <1>;
>> >> >> >                                         clocks = <&ccu
>CLK_BUS_EPHY>;
>> >> >> >                                         resets = <&ccu
>RST_BUS_EPHY>;
>> >> >> > +                                       phy-is-integrated;
>> >> >>
>> >> >> You also need to "delete" this property at the board level for
>> >> >> any board that has the external PHY at address <1>. Otherwise
>> >> >> they will stop working. This is due to the internal and
>external
>> >> >> PHYs having the same path and node name in the device tree, so
>> >> >> they are effectively the same node.
>> >> >>
>> >> >> ChenYu
>> >> >>
>> >> >
>> >> > They have not the same name, ext_rgmii_phy vs int_mii_phy.
>> >>
>> >> That is just the label. The label plays no part in device tree
>merging. The path
>> >>
>> >>     /soc/ethernet@1c30000/mdio/ethernet-phy@1
>> >>
>> >> is the same. You can look under
>> >>
>> >>     /proc/device-tree/soc/ethernet@1c30000/mdio
>> >>
>> >> on the OrangePI Plus 2E or any other H3 board that uses an
>> >> external PHY at address 1.
>> >>
>> >> ChenYu
>> >
>> > Since we get the phy node by phy-handle and not by path, I think
>all should be good.
>> 
>> You are not getting me. The fact that the two seemingly separate
>> nodes are merged together means, whatever properties you put in
>> the internal PHY node, also affect the external PHY node. Once
>> compiled, they are the SAME node.
>
>Hello Rob, florian, mark
>
>Adding a delete property on all external ethernet-phy@1 is a bit
>overkill, and I dont like the idea that nodes are merged.

This is not exactly up to you that's just how DTC works.

>What do you think about other possible solutions:
>- Using integrated-phy@1 for the integrated PHY node name

That might be okay although you are using now a seemingly non-standard unit name.

>- Using a fake address like 31 (see patch below)

You could also drop the address part in the unit name although we'd probably get a DTC warning for that.

I suspect both of your solutions and what I mentioned above will be producing DTC warnings to some extent... Rob what do you think?


>
>If you have any other solution...
>
>Regards
>
>From fe39183946f7f4a6e21bce38fd8e4c1413012d68 Mon Sep 17 00:00:00 2001
>From: Corentin Labbe <clabbe.montjoie@gmail.com>
>Date: Fri, 11 Aug 2017 14:49:54 +0200
>Subject: [PATCH] ARM: sun8i: sunxi-h3-h5: Prevent merge of external and
> integrated PHY
>
>Actually, some external and integrated PHY are merged due to same
>dtnode
>name "ethernet-phy@1".
>
>This is problematic when we will want to use the phy-is-integrated
>property. (Need to delete it on all external PHY node)
>
>An easy solution is to set integrated PHY nodeaddresss at a fake one
>that would never be used.
>Since board makers currently only provides PHY at addresses 1 and 7,
>we will use 31.
>
>Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
>---
> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>index 54fc24e4c569..2110b0069e33 100644
>--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
>@@ -420,7 +420,15 @@
> 			mdio: mdio {
> 				#address-cells = <1>;
> 				#size-cells = <0>;
>-				int_mii_phy: ethernet-phy@1 {
>+				/*
>+				 * Using 31 permits to make a separation between
>+				 * this integrated PHY and external ones.
>+				 * Without it, external "ethernet-phy@1" will be
>+				 * merged with it (due to same dtnode name).
>+				 * Board makers currently only provides PHY at
>+				 * addresses 1 and 7.
>+				 */
>+				int_mii_phy: ethernet-phy@31 {
> 					compatible = "ethernet-phy-ieee802.3-c22";
> 					reg = <1>;
> 					clocks = <&ccu CLK_BUS_EPHY>;


-- 
Florian

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Corentin Labbe]

On Fri, Aug 11, 2017 at 08:03:29AM -0700, Florian Fainelli wrote:
> On August 11, 2017 6:25:26 AM PDT, Corentin Labbe <clabbe.montjoie@gmail.com> wrote:
> >On Fri, Aug 11, 2017 at 04:22:11PM +0800, Chen-Yu Tsai wrote:
> >> On Fri, Aug 11, 2017 at 4:19 PM, Corentin Labbe
> >> <clabbe.montjoie@gmail.com> wrote:
> >> > On Fri, Aug 11, 2017 at 04:11:13PM +0800, Chen-Yu Tsai wrote:
> >> >> On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
> >> >> <clabbe.montjoie@gmail.com> wrote:
> >> >> > On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
> >> >> >> Hi,
> >> >> >>
> >> >> >> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
> >> >> >> <clabbe.montjoie@gmail.com> wrote:
> >> >> >> > This patch add the new phy-is-integrated property to the
> >internal PHY
> >> >> >> > node.
> >> >> >> >
> >> >> >> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> >> >> >> > ---
> >> >> >> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
> >> >> >> >  1 file changed, 1 insertion(+)
> >> >> >> >
> >> >> >> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> >> > index 4b599b5d26f6..54fc24e4c569 100644
> >> >> >> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> >> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> >> > @@ -425,6 +425,7 @@
> >> >> >> >                                         reg = <1>;
> >> >> >> >                                         clocks = <&ccu
> >CLK_BUS_EPHY>;
> >> >> >> >                                         resets = <&ccu
> >RST_BUS_EPHY>;
> >> >> >> > +                                       phy-is-integrated;
> >> >> >>
> >> >> >> You also need to "delete" this property at the board level for
> >> >> >> any board that has the external PHY at address <1>. Otherwise
> >> >> >> they will stop working. This is due to the internal and
> >external
> >> >> >> PHYs having the same path and node name in the device tree, so
> >> >> >> they are effectively the same node.
> >> >> >>
> >> >> >> ChenYu
> >> >> >>
> >> >> >
> >> >> > They have not the same name, ext_rgmii_phy vs int_mii_phy.
> >> >>
> >> >> That is just the label. The label plays no part in device tree
> >merging. The path
> >> >>
> >> >>     /soc/ethernet@1c30000/mdio/ethernet-phy@1
> >> >>
> >> >> is the same. You can look under
> >> >>
> >> >>     /proc/device-tree/soc/ethernet@1c30000/mdio
> >> >>
> >> >> on the OrangePI Plus 2E or any other H3 board that uses an
> >> >> external PHY at address 1.
> >> >>
> >> >> ChenYu
> >> >
> >> > Since we get the phy node by phy-handle and not by path, I think
> >all should be good.
> >> 
> >> You are not getting me. The fact that the two seemingly separate
> >> nodes are merged together means, whatever properties you put in
> >> the internal PHY node, also affect the external PHY node. Once
> >> compiled, they are the SAME node.
> >
> >Hello Rob, florian, mark
> >
> >Adding a delete property on all external ethernet-phy@1 is a bit
> >overkill, and I dont like the idea that nodes are merged.
> 
> This is not exactly up to you that's just how DTC works.
> 
> >What do you think about other possible solutions:
> >- Using integrated-phy@1 for the integrated PHY node name
> 
> That might be okay although you are using now a seemingly non-standard unit name.
> 
> >- Using a fake address like 31 (see patch below)
> 
> You could also drop the address part in the unit name although we'd probably get a DTC warning for that.
> 
> I suspect both of your solutions and what I mentioned above will be producing DTC warnings to some extent... Rob what do you think?
> 

I think I found an easier solution, putting phy-is-integrated on board DT nodes only.
I will send an updated serie.

Regards

Re: [PATCH 2/3] ARM: sun8i: sunxi-h3-h5: add phy-is-integrated property to internal PHY

[Rob Herring]

On Fri, Aug 11, 2017 at 08:03:29AM -0700, Florian Fainelli wrote:
> On August 11, 2017 6:25:26 AM PDT, Corentin Labbe <clabbe.montjoie@gmail.com> wrote:
> >On Fri, Aug 11, 2017 at 04:22:11PM +0800, Chen-Yu Tsai wrote:
> >> On Fri, Aug 11, 2017 at 4:19 PM, Corentin Labbe
> >> <clabbe.montjoie@gmail.com> wrote:
> >> > On Fri, Aug 11, 2017 at 04:11:13PM +0800, Chen-Yu Tsai wrote:
> >> >> On Fri, Aug 11, 2017 at 4:05 PM, Corentin Labbe
> >> >> <clabbe.montjoie@gmail.com> wrote:
> >> >> > On Fri, Aug 11, 2017 at 10:42:51AM +0800, Chen-Yu Tsai wrote:
> >> >> >> Hi,
> >> >> >>
> >> >> >> On Thu, Aug 10, 2017 at 4:51 PM, Corentin Labbe
> >> >> >> <clabbe.montjoie@gmail.com> wrote:
> >> >> >> > This patch add the new phy-is-integrated property to the
> >internal PHY
> >> >> >> > node.
> >> >> >> >
> >> >> >> > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> >> >> >> > ---
> >> >> >> >  arch/arm/boot/dts/sunxi-h3-h5.dtsi | 1 +
> >> >> >> >  1 file changed, 1 insertion(+)
> >> >> >> >
> >> >> >> > diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> >> > index 4b599b5d26f6..54fc24e4c569 100644
> >> >> >> > --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> >> > +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> >> >> >> > @@ -425,6 +425,7 @@
> >> >> >> >                                         reg = <1>;
> >> >> >> >                                         clocks = <&ccu
> >CLK_BUS_EPHY>;
> >> >> >> >                                         resets = <&ccu
> >RST_BUS_EPHY>;
> >> >> >> > +                                       phy-is-integrated;
> >> >> >>
> >> >> >> You also need to "delete" this property at the board level for
> >> >> >> any board that has the external PHY at address <1>. Otherwise
> >> >> >> they will stop working. This is due to the internal and
> >external
> >> >> >> PHYs having the same path and node name in the device tree, so
> >> >> >> they are effectively the same node.
> >> >> >>
> >> >> >> ChenYu
> >> >> >>
> >> >> >
> >> >> > They have not the same name, ext_rgmii_phy vs int_mii_phy.
> >> >>
> >> >> That is just the label. The label plays no part in device tree
> >merging. The path
> >> >>
> >> >>     /soc/ethernet@1c30000/mdio/ethernet-phy@1
> >> >>
> >> >> is the same. You can look under
> >> >>
> >> >>     /proc/device-tree/soc/ethernet@1c30000/mdio
> >> >>
> >> >> on the OrangePI Plus 2E or any other H3 board that uses an
> >> >> external PHY at address 1.
> >> >>
> >> >> ChenYu
> >> >
> >> > Since we get the phy node by phy-handle and not by path, I think
> >all should be good.
> >> 
> >> You are not getting me. The fact that the two seemingly separate
> >> nodes are merged together means, whatever properties you put in
> >> the internal PHY node, also affect the external PHY node. Once
> >> compiled, they are the SAME node.
> >
> >Hello Rob, florian, mark
> >
> >Adding a delete property on all external ethernet-phy@1 is a bit
> >overkill, and I dont like the idea that nodes are merged.
> 
> This is not exactly up to you that's just how DTC works.
> 
> >What do you think about other possible solutions:
> >- Using integrated-phy@1 for the integrated PHY node name
> 
> That might be okay although you are using now a seemingly non-standard unit name.
> 
> >- Using a fake address like 31 (see patch below)
> 
> You could also drop the address part in the unit name although we'd probably get a DTC warning for that.
> 
> I suspect both of your solutions and what I mentioned above will be producing DTC warnings to some extent... Rob what do you think?

If you have 2 devices at the same address, then there is some mux in the 
middle. Describe that and you problems should be solved. The internal 
phy is always there, so it should be able to always be in the DT.

Rob

[PATCH 3/3] net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated

[Corentin Labbe]

The current way to find if the phy is internal is to compare DT phy-mode
and emac_variant/internal_phy.
But it will negate a possible future SoC where an external PHY use the
same phy mode than the internal one.

This patch adds a new way to find if the PHY is internal, via
the phy-is-integrated property.

Since the internal_phy variable does not need anymore to contain the xMII mode
used by the internal PHY, it is still used for knowing the presence of an
internal PHY, so it is modified to a boolean soc_has_internal_phy.

Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index fffd6d5fc907..672553b652bd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -41,14 +41,14 @@
  *				This value is used for disabling properly EMAC
  *				and used as a good starting value in case of the
  *				boot process(uboot) leave some stuff.
- * @internal_phy:		Does the MAC embed an internal PHY
+ * @soc_has_internal_phy:	Does the MAC embed an internal PHY
  * @support_mii:		Does the MAC handle MII
  * @support_rmii:		Does the MAC handle RMII
  * @support_rgmii:		Does the MAC handle RGMII
  */
 struct emac_variant {
 	u32 default_syscon_value;
-	int internal_phy;
+	bool soc_has_internal_phy;
 	bool support_mii;
 	bool support_rmii;
 	bool support_rgmii;
@@ -75,7 +75,7 @@ struct sunxi_priv_data {
 
 static const struct emac_variant emac_variant_h3 = {
 	.default_syscon_value = 0x58000,
-	.internal_phy = PHY_INTERFACE_MODE_MII,
+	.soc_has_internal_phy = true,
 	.support_mii = true,
 	.support_rmii = true,
 	.support_rgmii = true
@@ -83,20 +83,20 @@ static const struct emac_variant emac_variant_h3 = {
 
 static const struct emac_variant emac_variant_v3s = {
 	.default_syscon_value = 0x38000,
-	.internal_phy = PHY_INTERFACE_MODE_MII,
+	.soc_has_internal_phy = true,
 	.support_mii = true
 };
 
 static const struct emac_variant emac_variant_a83t = {
 	.default_syscon_value = 0,
-	.internal_phy = 0,
+	.soc_has_internal_phy = false,
 	.support_mii = true,
 	.support_rgmii = true
 };
 
 static const struct emac_variant emac_variant_a64 = {
 	.default_syscon_value = 0,
-	.internal_phy = 0,
+	.soc_has_internal_phy = false,
 	.support_mii = true,
 	.support_rmii = true,
 	.support_rgmii = true
@@ -648,7 +648,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 			 "Current syscon value is not the default %x (expect %x)\n",
 			 val, reg);
 
-	if (gmac->variant->internal_phy) {
+	if (gmac->variant->soc_has_internal_phy) {
 		if (!gmac->use_internal_phy) {
 			/* switch to external PHY interface */
 			reg &= ~H3_EPHY_SELECT;
@@ -932,7 +932,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 	}
 
 	plat_dat->interface = of_get_phy_mode(dev->of_node);
-	if (plat_dat->interface == gmac->variant->internal_phy) {
+	if (of_property_read_bool(plat_dat->phy_node, "phy-is-integrated")) {
 		dev_info(&pdev->dev, "Will use internal PHY\n");
 		gmac->use_internal_phy = true;
 		gmac->ephy_clk = of_clk_get(plat_dat->phy_node, 0);
-- 
2.13.0