[RFC net-next PATCH 00/16] Add support for Xilinx PCS

[RFC net-next PATCH 00/16] Add support for Xilinx PCS

[Sean Anderson]

This series adds experimental support for Xilinx PCS devices. It is
experimental because while I believe I have the Linux side mostly
sorted, I have yet to achieve any data transfer. Adding support for this
device has required some plumbing work related to PCSs in general, and I
would appreciate feedback in that area. In general, I have not tested
these changes outside of my particular setup, though I do have the
ability to test the macb changes using the internal PCS in the future.


Sean Anderson (16):
  dt-bindings: net: Add pcs property
  dt-bindings: net: Add binding for Xilinx PCS
  net: sfp: Fix typo in state machine debug string
  net: phylink: Move phylink_set_pcs before phylink_create
  net: phylink: Automatically attach PCS devices
  net: phylink: Add function for optionally adding a PCS
  net: phylink: Add helpers for c22 registers without MDIO
  net: macb: Clean up macb_validate
  net: macb: Move most of mac_prepare to mac_config
  net: macb: Move PCS settings to PCS callbacks
  net: macb: Support restarting PCS autonegotiation
  net: macb: Support external PCSs
  net: phy: Export get_phy_c22_id
  net: mdio: Add helper functions for accessing MDIO devices
  net: pcs: Add Xilinx PCS driver
  net: sfp: Add quirk to ignore PHYs

 .../bindings/net/ethernet-controller.yaml     |   5 +
 .../devicetree/bindings/net/xilinx,pcs.yaml   |  83 ++++
 MAINTAINERS                                   |   6 +
 drivers/net/ethernet/cadence/macb_main.c      | 375 +++++++++++-------
 drivers/net/pcs/Kconfig                       |  19 +
 drivers/net/pcs/Makefile                      |   1 +
 drivers/net/pcs/pcs-xilinx.c                  | 326 +++++++++++++++
 drivers/net/phy/phy_device.c                  |   3 +-
 drivers/net/phy/phylink.c                     | 335 ++++++++++++----
 drivers/net/phy/sfp-bus.c                     |  12 +-
 drivers/net/phy/sfp.c                         |   5 +-
 include/linux/mdio.h                          |  17 +
 include/linux/phy.h                           |   1 +
 include/linux/phylink.h                       |  17 +-
 14 files changed, 963 insertions(+), 242 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/xilinx,pcs.yaml
 create mode 100644 drivers/net/pcs/pcs-xilinx.c

-- 
2.25.1

[RFC net-next PATCH 01/16] dt-bindings: net: Add pcs property

[Sean Anderson]

Add a property for associating PCS devices with ethernet controllers.
Because PCS has no generic analogue like PHY, I have left off the
-handle suffix.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

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

diff --git a/Documentation/devicetree/bindings/net/ethernet-controller.yaml b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
index b0933a8c295a..def95fa6a315 100644
--- a/Documentation/devicetree/bindings/net/ethernet-controller.yaml
+++ b/Documentation/devicetree/bindings/net/ethernet-controller.yaml
@@ -116,6 +116,11 @@ properties:
     $ref: "#/properties/phy-handle"
     deprecated: true
 
+  pcs:
+    $ref: /schemas/types.yaml#definitions/phandle
+    description:
+      Specifies a reference to a node representing a PCS device.
+
   rx-fifo-depth:
     $ref: /schemas/types.yaml#/definitions/uint32
     description:
-- 
2.25.1

Re: [RFC net-next PATCH 01/16] dt-bindings: net: Add pcs property

[Russell King (Oracle)]

On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
> Add a property for associating PCS devices with ethernet controllers.
> Because PCS has no generic analogue like PHY, I have left off the
> -handle suffix.

For PHYs, we used to have phy and phy-device as property names, but the
modern name is "phy-handle". I think we should do the same here, so I
would suggest using "pcs-handle".

We actually already have LX2160A platforms using "pcs-handle", (see
Documentation/devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml) so we're
in danger of ending up with multiple property names describing the same
thing.

-- 
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 PATCH 01/16] dt-bindings: net: Add pcs property

[Sean Anderson]

On 10/5/21 5:39 AM, Russell King (Oracle) wrote:
> On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
>> Add a property for associating PCS devices with ethernet controllers.
>> Because PCS has no generic analogue like PHY, I have left off the
>> -handle suffix.
> 
> For PHYs, we used to have phy and phy-device as property names, but the
> modern name is "phy-handle". I think we should do the same here, so I
> would suggest using "pcs-handle".
> 
> We actually already have LX2160A platforms using "pcs-handle", (see
> Documentation/devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml) so we're
> in danger of ending up with multiple property names describing the same
> thing.
> 

Either way is fine by me.

--Sean

Re: [RFC net-next PATCH 01/16] dt-bindings: net: Add pcs property

[Rob Herring]

On Tue, Oct 05, 2021 at 10:39:36AM +0100, Russell King (Oracle) wrote:
> On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
> > Add a property for associating PCS devices with ethernet controllers.
> > Because PCS has no generic analogue like PHY, I have left off the
> > -handle suffix.
> 
> For PHYs, we used to have phy and phy-device as property names, but the
> modern name is "phy-handle". I think we should do the same here, so I
> would suggest using "pcs-handle".

On 1G and up ethernet, we have 2 PHYs. There's the external (typically) 
ethernet PHY which is what the above properties are for. Then there's 
the on-chip serdes PHY similar to SATA, PCIe, etc. which includes the 
PCS part. For this part, we should use the generic PHY binding. I think 
we already have bindings doing that.

Rob

Re: [RFC net-next PATCH 01/16] dt-bindings: net: Add pcs property

[Sean Anderson]

Hi Rob,

On 10/12/21 9:16 AM, Rob Herring wrote:
> On Tue, Oct 05, 2021 at 10:39:36AM +0100, Russell King (Oracle) wrote:
>> On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
>> > Add a property for associating PCS devices with ethernet controllers.
>> > Because PCS has no generic analogue like PHY, I have left off the
>> > -handle suffix.
>>
>> For PHYs, we used to have phy and phy-device as property names, but the
>> modern name is "phy-handle". I think we should do the same here, so I
>> would suggest using "pcs-handle".
>
> On 1G and up ethernet, we have 2 PHYs. There's the external (typically)
> ethernet PHY which is what the above properties are for. Then there's
> the on-chip serdes PHY similar to SATA, PCIe, etc. which includes the
> PCS part. For this part, we should use the generic PHY binding. I think
> we already have bindings doing that.

In the 802.3 models, there are several components which convert between
the MII (from the MAC) and the MDI (the physical protocol on the wire).
These are the Physical Coding Sublayer (PCS), Physical Medium Attachment
(PMA) sublayer, and Physical Medium Dependent (PMD) sublayer. The PMD
converts between the physical layer signaling and the on-chip (or
on-board) signalling. The PMA performs clock recovery and converts the
serial data from the PMD into parallel data for the PCS. The PCS handles
autonegotiation, CSMA/CD, and conversion to the apripriate MII for
communicating with the MAC.

In the above model, generic serdes devices generally correspond to the
PMA/PMD sublayers. The PCS is generally a separate device, both
on the hardware and software level. It provides an ethernet-specific
layer on top of the more generic underlying encoding. For this reason,
the PCS should be modeled as its own device, which may then contain a
reference to the appropriate serdes.

The above model describes physical layers such as 1000BASE-X or
10GBASE-X where the PCS/PMA/PMD is the last layer before the physical
medium. In that case, the PCS could be modeled as a traditional PHY.
However, when using (e.g.) SGMII, it is common for the "MDI" to be
SGMII, and for another PHY to convert to 1000BASE-T. To model this
correctly, the PCS/PMA/PMD layer must be considered independently from
the PHY which will ultimately convert the MII to the MDI.

--Sean

Re: [RFC net-next PATCH 01/16] dt-bindings: net: Add pcs property

[Rob Herring]

On Tue, Oct 12, 2021 at 11:18 AM Sean Anderson <sean.anderson@seco.com> wrote:
>
> Hi Rob,
>
> On 10/12/21 9:16 AM, Rob Herring wrote:
> > On Tue, Oct 05, 2021 at 10:39:36AM +0100, Russell King (Oracle) wrote:
> >> On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
> >> > Add a property for associating PCS devices with ethernet controllers.
> >> > Because PCS has no generic analogue like PHY, I have left off the
> >> > -handle suffix.
> >>
> >> For PHYs, we used to have phy and phy-device as property names, but the
> >> modern name is "phy-handle". I think we should do the same here, so I
> >> would suggest using "pcs-handle".
> >
> > On 1G and up ethernet, we have 2 PHYs. There's the external (typically)
> > ethernet PHY which is what the above properties are for. Then there's
> > the on-chip serdes PHY similar to SATA, PCIe, etc. which includes the
> > PCS part. For this part, we should use the generic PHY binding. I think
> > we already have bindings doing that.
>
> In the 802.3 models, there are several components which convert between
> the MII (from the MAC) and the MDI (the physical protocol on the wire).
> These are the Physical Coding Sublayer (PCS), Physical Medium Attachment
> (PMA) sublayer, and Physical Medium Dependent (PMD) sublayer. The PMD
> converts between the physical layer signaling and the on-chip (or
> on-board) signalling. The PMA performs clock recovery and converts the
> serial data from the PMD into parallel data for the PCS. The PCS handles
> autonegotiation, CSMA/CD, and conversion to the apripriate MII for
> communicating with the MAC.
>
> In the above model, generic serdes devices generally correspond to the
> PMA/PMD sublayers. The PCS is generally a separate device, both
> on the hardware and software level. It provides an ethernet-specific
> layer on top of the more generic underlying encoding. For this reason,
> the PCS should be modeled as its own device, which may then contain a
> reference to the appropriate serdes.

On the h/w I've worked on, PCS was an additional block instantiated
within the PHY, so it looked like one block to s/w. But that's been
almost 10 years ago now.

If you do have 2 h/w blocks, one option is doing something like this:

phys = <&pcs_phy>, <&sgmii_phy>;

I'm okay with 'pcs-handle', but just want to make sure we're not using
it where 'phys' would work.

> The above model describes physical layers such as 1000BASE-X or
> 10GBASE-X where the PCS/PMA/PMD is the last layer before the physical
> medium. In that case, the PCS could be modeled as a traditional PHY.
> However, when using (e.g.) SGMII, it is common for the "MDI" to be
> SGMII, and for another PHY to convert to 1000BASE-T. To model this
> correctly, the PCS/PMA/PMD layer must be considered independently from
> the PHY which will ultimately convert the MII to the MDI.

Re: [RFC net-next PATCH 01/16] dt-bindings: net: Add pcs property

[Sean Anderson]

On 10/12/21 12:44 PM, Rob Herring wrote:
> On Tue, Oct 12, 2021 at 11:18 AM Sean Anderson <sean.anderson@seco.com> wrote:
>>
>> Hi Rob,
>>
>> On 10/12/21 9:16 AM, Rob Herring wrote:
>> > On Tue, Oct 05, 2021 at 10:39:36AM +0100, Russell King (Oracle) wrote:
>> >> On Mon, Oct 04, 2021 at 03:15:12PM -0400, Sean Anderson wrote:
>> >> > Add a property for associating PCS devices with ethernet controllers.
>> >> > Because PCS has no generic analogue like PHY, I have left off the
>> >> > -handle suffix.
>> >>
>> >> For PHYs, we used to have phy and phy-device as property names, but the
>> >> modern name is "phy-handle". I think we should do the same here, so I
>> >> would suggest using "pcs-handle".
>> >
>> > On 1G and up ethernet, we have 2 PHYs. There's the external (typically)
>> > ethernet PHY which is what the above properties are for. Then there's
>> > the on-chip serdes PHY similar to SATA, PCIe, etc. which includes the
>> > PCS part. For this part, we should use the generic PHY binding. I think
>> > we already have bindings doing that.
>>
>> In the 802.3 models, there are several components which convert between
>> the MII (from the MAC) and the MDI (the physical protocol on the wire).
>> These are the Physical Coding Sublayer (PCS), Physical Medium Attachment
>> (PMA) sublayer, and Physical Medium Dependent (PMD) sublayer. The PMD
>> converts between the physical layer signaling and the on-chip (or
>> on-board) signalling. The PMA performs clock recovery and converts the
>> serial data from the PMD into parallel data for the PCS. The PCS handles
>> autonegotiation, CSMA/CD, and conversion to the apripriate MII for
>> communicating with the MAC.
>>
>> In the above model, generic serdes devices generally correspond to the
>> PMA/PMD sublayers. The PCS is generally a separate device, both
>> on the hardware and software level. It provides an ethernet-specific
>> layer on top of the more generic underlying encoding. For this reason,
>> the PCS should be modeled as its own device, which may then contain a
>> reference to the appropriate serdes.
>
> On the h/w I've worked on, PCS was an additional block instantiated
> within the PHY, so it looked like one block to s/w. But that's been
> almost 10 years ago now.

Well, perhaps the line is not as clear as I made it seem above. The
PCS/PMA/PMD separation is mostly a logical one, so different platforms
divide up the work differently. In addition, the naming may not align
with ethernet's idea of what a PCS or PMA is. For example, on the
ZynqMP, the serdes (GTH) contains its own PCS and PMD (as shown on the
datasheet). However, these blocks both correspond to the PMA layer from
ethernet's POV. The ethernet PCS is a block controlled via registers
within the MAC's address space.

> If you do have 2 h/w blocks, one option is doing something like this:
>
> phys = <&pcs_phy>, <&sgmii_phy>;

Generally, PCSs don't export the same interface as PHYs (see e.g.
drivers/net/pcs and include/linux/phylink.h). IMO it would be an abuse
of the phys property to use it for a PCS as well.

> I'm okay with 'pcs-handle', but just want to make sure we're not using
> it where 'phys' would work.
>
>> The above model describes physical layers such as 1000BASE-X or
>> 10GBASE-X where the PCS/PMA/PMD is the last layer before the physical
>> medium. In that case, the PCS could be modeled as a traditional PHY.
>> However, when using (e.g.) SGMII, it is common for the "MDI" to be
>> SGMII, and for another PHY to convert to 1000BASE-T. To model this
>> correctly, the PCS/PMA/PMD layer must be considered independently from
>> the PHY which will ultimately convert the MII to the MDI.
>

[RFC net-next PATCH 02/16] dt-bindings: net: Add binding for Xilinx PCS

[Sean Anderson]

This adds a binding for the Xilinx 1G/2.5G Ethernet PCS/PMA or SGMII
LogiCORE IP. This device is a soft device typically used to adapt between
GMII and SGMII or 1000BASE-X (in combination with a suitable SERDES). The
standard property is roughly analogous to the interface property of
ethernet controllers, except that it has an additional value used to
indicate that dynamic switching is supported. Note that switching is
supported only between SGMII and 1000BASE-X, and only if the appropriate
parameter is set when the device is synthesized. The property name was
chosen to align with the terminology in the datasheet. I also considered
"mdi", but that is a bit of a misnomer in the case of SGMII.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 .../devicetree/bindings/net/xilinx,pcs.yaml   | 83 +++++++++++++++++++
 1 file changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/xilinx,pcs.yaml

diff --git a/Documentation/devicetree/bindings/net/xilinx,pcs.yaml b/Documentation/devicetree/bindings/net/xilinx,pcs.yaml
new file mode 100644
index 000000000000..43750dcb4b11
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/xilinx,pcs.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/xilinx,pcs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx 1G/2.5G Ethernet PCS/PMA or SGMII LogiCORE IP
+
+maintainers:
+  - Sean Anderson <sean.anderson@seco.com>
+
+description:
+  This is a soft device which converts between GMII and SGMII, 2.5G SGMII,
+  1000BASE-X, or 2500BASE-X. It may have an attached SERDES, or may talk
+  directly to LVDS.
+
+allOf:
+  - $ref: "ethernet-controller.yaml#"
+
+properties:
+  compatible:
+    contains:
+      const:
+        - xilinx,pcs-16.2
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    items:
+      - description: The reference clock for the PMD, which is typically a
+                     SERDES but may be a direct interface to LVDS I/Os.
+                     Depending on your setup, this may be the gtrefclk, refclk,
+                     or clk125m signal.
+
+  clock-names:
+    const: refclk
+
+  resets:
+    maxItems: 1
+
+  reset-names:
+    const: pcs
+
+  standard:
+    description:
+      The interface standard that the PCS supports. The sgmii/1000base-x
+      setting indicates that the PCS supports dynamically switching between
+      SGMII and 1000BASE-X.
+    enum:
+      - sgmii
+      - 1000base-x
+      - sgmii/1000base-x
+      - 2500base-x
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+  - standard
+
+additionalProperties: false
+
+examples:
+  - |
+    mdio {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        pcs0: ethernet-pcs@0 {
+            compatible = "xlnx,pcs-16.2";
+            reg = <0>;
+            clocks = <&si570>;
+            clock-names = "refclk";
+            resets = <&pcs_reset 1>;
+            reset-names = "pcs";
+            standard = "sgmii/1000base-x";
+        };
+    };
-- 
2.25.1

Re: [RFC net-next PATCH 02/16] dt-bindings: net: Add binding for Xilinx PCS

[Rob Herring]

On Mon, 04 Oct 2021 15:15:13 -0400, Sean Anderson wrote:
> This adds a binding for the Xilinx 1G/2.5G Ethernet PCS/PMA or SGMII
> LogiCORE IP. This device is a soft device typically used to adapt between
> GMII and SGMII or 1000BASE-X (in combination with a suitable SERDES). The
> standard property is roughly analogous to the interface property of
> ethernet controllers, except that it has an additional value used to
> indicate that dynamic switching is supported. Note that switching is
> supported only between SGMII and 1000BASE-X, and only if the appropriate
> parameter is set when the device is synthesized. The property name was
> chosen to align with the terminology in the datasheet. I also considered
> "mdi", but that is a bit of a misnomer in the case of SGMII.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
> 
>  .../devicetree/bindings/net/xilinx,pcs.yaml   | 83 +++++++++++++++++++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/xilinx,pcs.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/xilinx,pcs.yaml: properties:compatible:contains:const: ['xilinx,pcs-16.2'] is not of type 'string'
	from schema $id: http://devicetree.org/meta-schemas/string-array.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/xilinx,pcs.yaml: properties:clocks: {'maxItems': 1, 'items': [{'description': 'The reference clock for the PMD, which is typically a SERDES but may be a direct interface to LVDS I/Os. Depending on your setup, this may be the gtrefclk, refclk, or clk125m signal.'}]} should not be valid under {'required': ['maxItems']}
	hint: "maxItems" is not needed with an "items" list
	from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/xilinx,pcs.yaml: properties:clocks: 'oneOf' conditional failed, one must be fixed:
	/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/xilinx,pcs.yaml: properties:clocks: 'anyOf' conditional failed, one must be fixed:
		'items' is not one of ['maxItems', 'description', 'deprecated']
			hint: Only "maxItems" is required for a single entry if there are no constraints defined for the values.
		'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref']
		'items' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref']
		1 is less than the minimum of 2
			hint: Arrays must be described with a combination of minItems/maxItems/items
		hint: cell array properties must define how many entries and what the entries are when there is more than one entry.
		from schema $id: http://devicetree.org/meta-schemas/clocks.yaml#
	'maxItems' is not one of ['type', 'description', 'dependencies', 'properties', 'patternProperties', 'additionalProperties', 'unevaluatedProperties', 'deprecated', 'required', 'allOf', 'anyOf', 'oneOf', '$ref']
	'items' is not one of ['type', 'description', 'dependencies', 'properties', 'patternProperties', 'additionalProperties', 'unevaluatedProperties', 'deprecated', 'required', 'allOf', 'anyOf', 'oneOf', '$ref']
	'type' is a required property
		hint: DT nodes ("object" type in schemas) can only use a subset of json-schema keywords
	from schema $id: http://devicetree.org/meta-schemas/clocks.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/xilinx,pcs.yaml: ignoring, error in schema: properties: compatible: contains: const
warning: no schema found in file: ./Documentation/devicetree/bindings/net/xilinx,pcs.yaml
Documentation/devicetree/bindings/net/xilinx,pcs.example.dt.yaml:0:0: /example-0/mdio/ethernet-pcs@0: failed to match any schema with compatible: ['xlnx,pcs-16.2']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1536331

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.