[RFC net-next 1/2] dt-bindings: ethernet-controller: create a type for PHY interface modes

[RFC net-next 1/2] dt-bindings: ethernet-controller: create a type for PHY interface modes

[Marek Behún]

In order to be able to define a property describing an array of PHY
interface modes, we need to change the current scalar
`phy-connection-type`, which lists the possible PHY interface modes, to
an array of length 1 (otherwise we would need to define the same list at
two different places).

Moreover Rob Herring says that we cannot reuse the values of a property;
we need to $ref a type.

Move the definition of possible PHY interface modes from the
`phy-connection-type` property to an array type definition
`phy-connection-type-array`, and simply reference this type in the
original property.

Signed-off-by: Marek Behún <kabel@kernel.org>
---

Is `phy-connection-type` prefered over `phy-mode`? If not, maybe the
type could be called `phy-modes-array`...

---
 .../bindings/net/ethernet-controller.yaml     | 89 ++++++++++---------
 1 file changed, 48 insertions(+), 41 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index 4b7d1e5d003c..0ee25ecbffde 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -53,50 +53,12 @@ properties:
     const: mac-address
 
   phy-connection-type:
+    $ref: "#/$defs/phy-connection-type-array"
     description:
       Specifies interface type between the Ethernet device and a physical
       layer (PHY) device.
-    enum:
-      # There is not a standard bus between the MAC and the PHY,
-      # something proprietary is being used to embed the PHY in the
-      # MAC.
-      - internal
-      - mii
-      - gmii
-      - sgmii
-      - qsgmii
-      - tbi
-      - rev-mii
-      - rmii
-
-      # RX and TX delays are added by the MAC when required
-      - rgmii
-
-      # RGMII with internal RX and TX delays provided by the PHY,
-      # the MAC should not add the RX or TX delays in this case
-      - rgmii-id
-
-      # RGMII with internal RX delay provided by the PHY, the MAC
-      # should not add an RX delay in this case
-      - rgmii-rxid
-
-      # RGMII with internal TX delay provided by the PHY, the MAC
-      # should not add an TX delay in this case
-      - rgmii-txid
-      - rtbi
-      - smii
-      - xgmii
-      - trgmii
-      - 1000base-x
-      - 2500base-x
-      - 5gbase-r
-      - rxaui
-      - xaui
-
-      # 10GBASE-KR, XFI, SFI
-      - 10gbase-kr
-      - usxgmii
-      - 10gbase-r
+    minItems: 1
+    maxItems: 1
 
   phy-mode:
     $ref: "#/properties/phy-connection-type"
@@ -226,4 +188,49 @@ properties:
 
 additionalProperties: true
 
+'$defs':
+  phy-connection-type-array:
+    items:
+      enum:
+        # There is not a standard bus between the MAC and the PHY,
+        # something proprietary is being used to embed the PHY in the
+        # MAC.
+        - internal
+        - mii
+        - gmii
+        - sgmii
+        - qsgmii
+        - tbi
+        - rev-mii
+        - rmii
+
+        # RX and TX delays are added by the MAC when required
+        - rgmii
+
+        # RGMII with internal RX and TX delays provided by the PHY,
+        # the MAC should not add the RX or TX delays in this case
+        - rgmii-id
+
+        # RGMII with internal RX delay provided by the PHY, the MAC
+        # should not add an RX delay in this case
+        - rgmii-rxid
+
+        # RGMII with internal TX delay provided by the PHY, the MAC
+        # should not add an TX delay in this case
+        - rgmii-txid
+        - rtbi
+        - smii
+        - xgmii
+        - trgmii
+        - 1000base-x
+        - 2500base-x
+        - 5gbase-r
+        - rxaui
+        - xaui
+
+        # 10GBASE-KR, XFI, SFI
+        - 10gbase-kr
+        - usxgmii
+        - 10gbase-r
+
 ...
-- 
2.26.2

[RFC net-next 2/2] dt-bindings: ethernet-phy: define `unsupported-mac-connection-types` property

[Marek Behún]

An ethernet PHY may support PHY interface modes that are not wired on a
specific board (or are broken for some other reason). In order for the
kernel to know that these modes should not be used, we need to specify
this in device tree.

Define a new ethernet PHY property `unsupported-mac-connection-types`,
which lists these unsupported modes.

Signed-off-by: Marek Behún <kabel@kernel.org>
---

As in the previous patch: we allow both `phy-connection-type` and
`phy-mode` to define PHY interface mode. Should we call this new
property as it is proposed by this patch, or something different,
like `unsupported-mac-phy-modes`?

Also, some PHYs (marvell10g for example) also multiple units (host
unit for connecting to the MAC, fiber unit for connecting for example
via a SFP). Should we also add `unsupported-fiber-connection-types`
property?

Moreover should this property be a member of PHY's node, or the
ethernet controller's node? Were it a member of ethernet controller's
node, we wouldn't need to $ref a definition from another file's $defs
(which Rob Herring says that so far is done only in withing single
file).
On the other hand `unsupported-fiber-connection-types` property should
be a member of PHY's node, if we will add this in the future.

---
 .../devicetree/bindings/net/ethernet-phy.yaml     | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 2766fe45bb98..4c5b8fabbec3 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -136,6 +136,20 @@ properties:
       used. The absence of this property indicates the muxers
       should be configured so that the external PHY is used.
 
+  unsupported-mac-connection-types:
+    $ref: "ethernet-controller.yaml#/$defs/phy-connection-type-array"
+    description:
+      The PHY device may support different interface types for
+      connecting the Ethernet MAC device to the PHY device (i.e.
+      rgmii, sgmii, xaui, ...), but not all of these interface
+      types must necessarily be supported for a specific board
+      (either not all of them are wired, or there is a known bug
+      for a specific mode).
+      This property specifies a list of interface modes are not
+      supported on the board.
+    minItems: 1
+    maxItems: 128
+
   resets:
     maxItems: 1
 
@@ -196,5 +210,6 @@ examples:
             reset-gpios = <&gpio1 4 1>;
             reset-assert-us = <1000>;
             reset-deassert-us = <2000>;
+            unsupported-mac-connection-types = "xaui", "rxaui";
         };
     };
-- 
2.26.2

[RFC net-next 1/2] dt-bindings: ethernet-controller: create a type for PHY interface modes

[Marek Behún]

In order to be able to define a property describing an array of PHY
interface modes, we need to change the current scalar
`phy-connection-type`, which lists the possible PHY interface modes, to
an array of length 1 (otherwise we would need to define the same list at
two different places).

Moreover Rob Herring says that we cannot reuse the values of a property;
we need to $ref a type.

Move the definition of possible PHY interface modes from the
`phy-connection-type` property to an array type definition
`phy-connection-type-array`, and simply reference this type in the
original property.

Signed-off-by: Marek Behún <kabel@kernel.org>
---

Is `phy-connection-type` prefered over `phy-mode`? If not, maybe the
type could be called `phy-modes-array`...

---
 .../bindings/net/ethernet-controller.yaml     | 89 ++++++++++---------
 1 file changed, 48 insertions(+), 41 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index 4b7d1e5d003c..0ee25ecbffde 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -53,50 +53,12 @@ properties:
     const: mac-address
 
   phy-connection-type:
+    $ref: "#/$defs/phy-connection-type-array"
     description:
       Specifies interface type between the Ethernet device and a physical
       layer (PHY) device.
-    enum:
-      # There is not a standard bus between the MAC and the PHY,
-      # something proprietary is being used to embed the PHY in the
-      # MAC.
-      - internal
-      - mii
-      - gmii
-      - sgmii
-      - qsgmii
-      - tbi
-      - rev-mii
-      - rmii
-
-      # RX and TX delays are added by the MAC when required
-      - rgmii
-
-      # RGMII with internal RX and TX delays provided by the PHY,
-      # the MAC should not add the RX or TX delays in this case
-      - rgmii-id
-
-      # RGMII with internal RX delay provided by the PHY, the MAC
-      # should not add an RX delay in this case
-      - rgmii-rxid
-
-      # RGMII with internal TX delay provided by the PHY, the MAC
-      # should not add an TX delay in this case
-      - rgmii-txid
-      - rtbi
-      - smii
-      - xgmii
-      - trgmii
-      - 1000base-x
-      - 2500base-x
-      - 5gbase-r
-      - rxaui
-      - xaui
-
-      # 10GBASE-KR, XFI, SFI
-      - 10gbase-kr
-      - usxgmii
-      - 10gbase-r
+    minItems: 1
+    maxItems: 1
 
   phy-mode:
     $ref: "#/properties/phy-connection-type"
@@ -226,4 +188,49 @@ properties:
 
 additionalProperties: true
 
+'$defs':
+  phy-connection-type-array:
+    items:
+      enum:
+        # There is not a standard bus between the MAC and the PHY,
+        # something proprietary is being used to embed the PHY in the
+        # MAC.
+        - internal
+        - mii
+        - gmii
+        - sgmii
+        - qsgmii
+        - tbi
+        - rev-mii
+        - rmii
+
+        # RX and TX delays are added by the MAC when required
+        - rgmii
+
+        # RGMII with internal RX and TX delays provided by the PHY,
+        # the MAC should not add the RX or TX delays in this case
+        - rgmii-id
+
+        # RGMII with internal RX delay provided by the PHY, the MAC
+        # should not add an RX delay in this case
+        - rgmii-rxid
+
+        # RGMII with internal TX delay provided by the PHY, the MAC
+        # should not add an TX delay in this case
+        - rgmii-txid
+        - rtbi
+        - smii
+        - xgmii
+        - trgmii
+        - 1000base-x
+        - 2500base-x
+        - 5gbase-r
+        - rxaui
+        - xaui
+
+        # 10GBASE-KR, XFI, SFI
+        - 10gbase-kr
+        - usxgmii
+        - 10gbase-r
+
 ...
-- 
2.26.2

[RFC net-next 2/2] dt-bindings: ethernet-phy: define `unsupported-mac-connection-types` property

[Marek Behún]

An ethernet PHY may support PHY interface modes that are not wired on a
specific board (or are broken for some other reason). In order for the
kernel to know that these modes should not be used, we need to specify
this in device tree.

Define a new ethernet PHY property `unsupported-mac-connection-types`,
which lists these unsupported modes.

Signed-off-by: Marek Behún <kabel@kernel.org>
---

As in the previous patch: we allow both `phy-connection-type` and
`phy-mode` to define PHY interface mode. Should we call this new
property as it is proposed by this patch, or something different,
like `unsupported-mac-phy-modes`?

Also, some PHYs (marvell10g for example) also multiple units (host
unit for connecting to the MAC, fiber unit for connecting for example
via a SFP). Should we also add `unsupported-fiber-connection-types`
property?

Moreover should this property be a member of PHY's node, or the
ethernet controller's node? Were it a member of ethernet controller's
node, we wouldn't need to $ref a definition from another file's $defs
(which Rob Herring says that so far is done only in withing single
file).
On the other hand `unsupported-fiber-connection-types` property should
be a member of PHY's node, if we will add this in the future.

---
 .../devicetree/bindings/net/ethernet-phy.yaml     | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
index 2766fe45bb98..4c5b8fabbec3 100644
--- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
@@ -136,6 +136,20 @@ properties:
       used. The absence of this property indicates the muxers
       should be configured so that the external PHY is used.
 
+  unsupported-mac-connection-types:
+    $ref: "ethernet-controller.yaml#/$defs/phy-connection-type-array"
+    description:
+      The PHY device may support different interface types for
+      connecting the Ethernet MAC device to the PHY device (i.e.
+      rgmii, sgmii, xaui, ...), but not all of these interface
+      types must necessarily be supported for a specific board
+      (either not all of them are wired, or there is a known bug
+      for a specific mode).
+      This property specifies a list of interface modes are not
+      supported on the board.
+    minItems: 1
+    maxItems: 128
+
   resets:
     maxItems: 1
 
@@ -196,5 +210,6 @@ examples:
             reset-gpios = <&gpio1 4 1>;
             reset-assert-us = <1000>;
             reset-deassert-us = <2000>;
+            unsupported-mac-connection-types = "xaui", "rxaui";
         };
     };
-- 
2.26.2

Re: [RFC net-next 2/2] dt-bindings: ethernet-phy: define `unsupported-mac-connection-types` property

[Russell King - ARM Linux admin]

On Mon, Mar 22, 2021 at 08:49:59PM +0100, Marek Behún wrote:
> diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> index 2766fe45bb98..4c5b8fabbec3 100644
> --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> @@ -136,6 +136,20 @@ properties:
>        used. The absence of this property indicates the muxers
>        should be configured so that the external PHY is used.
>  
> +  unsupported-mac-connection-types:
> +    $ref: "ethernet-controller.yaml#/$defs/phy-connection-type-array"
> +    description:
> +      The PHY device may support different interface types for
> +      connecting the Ethernet MAC device to the PHY device (i.e.
> +      rgmii, sgmii, xaui, ...), but not all of these interface
> +      types must necessarily be supported for a specific board
> +      (either not all of them are wired, or there is a known bug
> +      for a specific mode).
> +      This property specifies a list of interface modes are not
> +      supported on the board.

I think this needs to be clearer. "This property specifies a list
of interface modes supported by the PHY hardware but are not
supported on the board."

I would also suggest having a think about a PHY that supports some
interface types that we don't have support in the kernel for, but
which also are not part of the board. Should these be listed
somehow as well? If not, how do we deal with the kernel later gaining
support for those interface modes, potentially the PHY driver as well,
and then having a load of boards not listing this?

My feeling is that listing negative properties presents something of
a problem, and we ought to stick with boards specifying what they
support, rather than what they don't.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [RFC net-next 1/2] dt-bindings: ethernet-controller: create a type for PHY interface modes

[Andrew Lunn]

On Mon, Mar 22, 2021 at 08:49:58PM +0100, Marek Behún wrote:
> In order to be able to define a property describing an array of PHY
> interface modes, we need to change the current scalar
> `phy-connection-type`, which lists the possible PHY interface modes, to
> an array of length 1 (otherwise we would need to define the same list at
> two different places).

Hi Marek

Please could you include a 0/2 patch which explains the big
picture. It is not clear to me why you need these properties.  What is
the problem you are trying to solve? That should be in the patch
series cover note.

	 Andrew

Re: [RFC net-next 2/2] dt-bindings: ethernet-phy: define `unsupported-mac-connection-types` property

[Marek Behún]

On Mon, 22 Mar 2021 19:56:05 +0000
Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:

> On Mon, Mar 22, 2021 at 08:49:59PM +0100, Marek Behún wrote:
> > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > index 2766fe45bb98..4c5b8fabbec3 100644
> > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml
> > @@ -136,6 +136,20 @@ properties:
> >        used. The absence of this property indicates the muxers
> >        should be configured so that the external PHY is used.
> >  
> > +  unsupported-mac-connection-types:
> > +    $ref: "ethernet-controller.yaml#/$defs/phy-connection-type-array"
> > +    description:
> > +      The PHY device may support different interface types for
> > +      connecting the Ethernet MAC device to the PHY device (i.e.
> > +      rgmii, sgmii, xaui, ...), but not all of these interface
> > +      types must necessarily be supported for a specific board
> > +      (either not all of them are wired, or there is a known bug
> > +      for a specific mode).
> > +      This property specifies a list of interface modes are not
> > +      supported on the board.  
> 
> I think this needs to be clearer. "This property specifies a list
> of interface modes supported by the PHY hardware but are not
> supported on the board."
> 
> I would also suggest having a think about a PHY that supports some
> interface types that we don't have support in the kernel for, but
> which also are not part of the board. Should these be listed
> somehow as well? If not, how do we deal with the kernel later gaining
> support for those interface modes, potentially the PHY driver as well,
> and then having a load of boards not listing this?
> 
> My feeling is that listing negative properties presents something of
> a problem, and we ought to stick with boards specifying what they
> support, rather than what they don't.

That is a good point. And if this alternative `supported-modes` property
is missing, we can just assume that all modes are supported, in order
to be backward compatible.

Re: [RFC net-next 1/2] dt-bindings: ethernet-controller: create a type for PHY interface modes

[Marek Behún]

On Mon, 22 Mar 2021 22:11:04 +0100
Andrew Lunn <andrew@lunn.ch> wrote:

> On Mon, Mar 22, 2021 at 08:49:58PM +0100, Marek Behún wrote:
> > In order to be able to define a property describing an array of PHY
> > interface modes, we need to change the current scalar
> > `phy-connection-type`, which lists the possible PHY interface modes, to
> > an array of length 1 (otherwise we would need to define the same list at
> > two different places).  
> 
> Hi Marek
> 
> Please could you include a 0/2 patch which explains the big
> picture. It is not clear to me why you need these properties.  What is
> the problem you are trying to solve? That should be in the patch
> series cover note.
> 
> 	 Andrew

Hi Andrew,

sorry, I did not add a cover letter because the second patch commit
message basically explains the purpose, but now I realize that a cover
letter could mention a specific example.

I will include a cover letter in v2.

Meanwhile I can explain the purpose here:

Some PHYs support interface modes that must not necessarily be wired
on a board. A good example is Marvell 88x3310 PHY, which supports
several modes via one SerDes lane (10gbase-r, usxgmii, 5gbase-r,
2500base-x, sgmii), but also via 2 or 4 SerDes lanes (rxaui and xaui).

So a board utilizing this PHY can have different constraints:
- the board wiring can have a frequnecy constraint (for example the
  connection can go via a connector that does not support frequencies
  greater than 6 GHz, so for 10g link multiple lanse - xaui or rxaui -
  must be used)

- the board may simply not wire all SerDes lanes

- the MAC does not have to support all these modes

For the first two points it is impossible for the code to know this
without it being specified in device tree. The last one can be solved in
code, and Russell King has experimental patches in his repo for this
(both MAC and PHY driver fill up a supported_interfaces bitmask).

But why can't we just depend on the phy-mode property defined in ethernet
controller's OF node?
Becuase this PHY can change its interface mode to the MAC depending on
the negotiated speed on the copper side. This PHY has several possible
configurations
- communicate with the MAC in USXGMII. This is simplest one since the
  interface mode does not change even if copper speed changes
- 10gbase-r/5gbase-r/2500base-x/sgmii depending on the copper speed
-      xaui/5gbase-r/2500base-x/sgmii depending on the copper speed
-     rxaui/5gbase-r/2500base-x/sgmii depending on the copper speed
- 10gbase-r with rate matching
-      xaui with rate matching
-     rxaui with rate matching

One of these configurations is selected by strapping pins, and can be
read by the marvell10g driver. The driver can then change phy-mode on
the MAC side.

One problem is that the mode selected by the strapping pins may not
be ideal. There are some erratas published for this PHY, which say that
rate-matching is broken in some conditions (speed <= 1000), for example.
Also the rate-matching mode is not optimal if the MAC supports the
lower-speed modes, because of overhead of sending pause-frames.

So in order to select the best possible configuration, we need to know
which PHY interface modes are supported on the board.

The most generic solution is to specify a property which either lists
unsupported PHY modes or supported PHY modes (if missing, assume all
modes are supported).

Marek