| Message ID | 20230817150644.3605105-9-vladimir.oltean@nxp.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add C72/C73 copper backplane support for LX2160 | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 252 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Thu, Aug 17, 2023 at 06:06:44PM +0300, Vladimir Oltean wrote: > Illustrate how the backplane AN/LT blocks can be instantiated on the > LX2160A SoC with SerDes protocol 19. > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > --- > .../devicetree/bindings/net/ethernet-phy.yaml | 8 + > .../bindings/net/fsl,backplane-anlt.yaml | 238 ++++++++++++++++++ > 2 files changed, 246 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml > > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml > index c1241c8a3b77..96fa672e4786 100644 > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml > @@ -49,6 +49,14 @@ properties: > - items: > - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" > - const: ethernet-phy-ieee802.3-c45 > + - items: > + - const: fsl,lx2160a-backplane-anlt > + - const: ethernet-phy-ieee802.3-c45 What's the benefit of having ethernet-phy-ieee802.3-c45? Will it work if the OS only understands that and not fsl,lx2160a-backplane-anlt? > + description: > + Some C45 PHYs have no PHY ID in the standard location, and there is > + also no PHY ID allocated for them to fake. They are identified by the > + primary compatible string, plus the secondary one to distinguish them > + from a raw MDIO device. > > reg: > minimum: 0 > diff --git a/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml > new file mode 100644 > index 000000000000..7282e93b1dd4 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml > @@ -0,0 +1,238 @@ > +# SPDX-License-Identifier: GPL-2.0+ Not the right license. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/fsl,backplane-anlt.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale Backplane Ethernet PHY > + > +maintainers: > + - Vladimir Oltean <vladimir.oltean@nxp.com> > + > +description: | > + Some QorIQ and Layerscape SoCs have an additional block on each SerDes > + networking lane, based on an IP core from MoreThanIP, which performs IEEE > + 802.3 clause 73 base page exchanges (for auto-negotiation) and clause 72 > + training frame exchanges (for link training). > + > + By default, this AN/LT block comes up with auto-negotiation disabled, and > + in that case it allows itself to be quickly bypassed from the data path and > + for the PCS link to come up without its involvement. > + > + Software can optionally make use of it, to turn the PCS, AN/LT block and lane > + (PMA/PMD) into a full copper backplane internal PHY. > + > + As a warning, the binding for the multi-lane link modes (40GBase-KR4) is not > + currently backed up by a driver implementation. > + > +allOf: > + - if: > + properties: > + compatible: > + - items: > + - const: fsl,lx2160a-backplane-anlt > + - const: ethernet-phy-ieee802.3-c45 Use "contains" and drop ethernet-phy-ieee802.3-c45. > + then: > + $ref: ethernet-phy.yaml# > + > +properties: > + compatible: > + oneOf: > + - items: > + - const: fsl,lx2160a-backplane-anlt > + - const: ethernet-phy-ieee802.3-c45 > + - const: fsl,lx2160a-secondary-anlt > + > + reg: > + minimum: 0 > + maximum: 31 > + description: | > + The address of the AN/LT block within the internal MDIO bus of the MAC it > + is attached to. > + > + In the 1000Base-KX and 10GBase-KR link modes, the AN/LT block responds at > + the same MDIO address as the PCS (determined by the SGMIInCR1[MDEV_PORT] > + or SXGMIInCR1[MDEV_PORT] registers of the SerDes block, by default 0). > + The PCS and AN/LT block respond to different MMDs, though. > + > + In the 25GBase-KR and higher link modes, the AN/LT block responds at a > + different MDIO address than the PCS, determined by the > + ANLTnCR1[MDEV_PORT] register of the SerDes block. By default this is 4 > + for lanes A and E, 5 for lanes B and F, 6 for lanes C and G, 7 for lanes > + D and H. > + > + The PCS responds in all cases at the address determined by the MDEV_PORT > + field of the SGMIInCR1, SXGMIIaCR1, E25GaCR1, E40GaCR1, E50GaCR1 or > + E100GaCR1 registers of the SerDes block. > + > + phys: > + maxItems: 1 > + description: > + phandle for the generic PHY (SerDes lane) that acts as PMA/PMD layer > + > + pcs-handle: > + maxItems: 1 > + description: > + phandle for the technology-dependent PCS block corresponding to the > + initial (RCW-based) configuration of the port. Must be omitted for the > + link modes where the PCS and AN/LT block respond at the same MDIO > + address. Must be specified otherwise. > + > + secondary-anlt-handle: Needs a vendor prefix and type. > + maxItems: 1 > + description: > + In case this is the primary (first) lane of a multi-lane link mode, this > + property holds an array of phandles for the other AN/LT blocks, that are > + involved in link training but not in auto-negotiation. These have the > + "fsl,lx2160a-secondary-anlt" compatible string. > + > +required: > + - compatible > + - reg > + - phys > + > +unevaluatedProperties: false > + > +examples: > + > + # LX2160A lanes A, B, C, D with SerDes 1 protocol 19: dpmac2 uses 40GBase-KR4 > + - | > + dpmac2 { > + phy-handle = <&mac2_backplane_anlt>; > + phy-connection-type = "internal"; > + }; > + > + pcs_mdio2 { mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + status = "okay"; Don't need status in examples. > + > + pcs2: ethernet-phy@0 { > + reg = <0>; > + }; > + > + mac2_backplane_anlt: ethernet-phy@7 { > + compatible = "fsl,lx2160a-backplane-anlt", > + "ethernet-phy-ieee802.3-c45"; > + reg = <7>; /* according to ANLTDCR1[MDEV_PORT] */ > + phys = <&serdes_1 3>; /* lane D */ > + max-speed = <40000>; > + pcs-handle = <&pcs2>; > + secondary-anlt-handle = <&mac2_lane2_anlt>, <&mac2_lane3_anlt>, > + <&mac2_lane4_anlt>; > + }; > + > + mac2_lane2_anlt: ethernet-backplane-anlt@6 { > + compatible = "fsl,lx2160a-secondary-anlt"; > + reg = <6>; /* according to ANLTCCR1[MDEV_PORT] */ > + phys = <&serdes_1 2>; /* lane C */ > + }; > + > + mac2_lane3_anlt: ethernet-backplane-anlt@5 { > + compatible = "fsl,lx2160a-secondary-anlt"; > + reg = <5>; /* according to ANLTBCR1[MDEV_PORT] */ > + phys = <&serdes_1 1>; /* lane B */ > + }; > + > + mac2_lane4_anlt: ethernet-backplane-anlt@4 { > + compatible = "fsl,lx2160a-secondary-anlt"; > + reg = <4>; /* according to ANLTACR1[MDEV_PORT] */ > + phys = <&serdes_1 0>; /* lane A */ > + }; > + }; > + > + # LX2160A lane E with SerDes 1 protocol 19: dpmac6 uses 25GBase-KR > + - | > + dpmac6 { > + phy-handle = <&mac6_backplane_anlt>; > + phy-connection-type = "internal"; > + }; > + > + pcs_mdio6 { > + #address-cells = <1>; > + #size-cells = <0>; > + status = "okay"; > + > + pcs6: ethernet-phy@0 { > + reg = <0>; > + }; > + > + mac6_backplane_anlt: ethernet-phy@4 { > + compatible = "fsl,lx2160a-backplane-anlt", > + "ethernet-phy-ieee802.3-c45"; > + reg = <4>; /* according to ANLTFCR1[MDEV_PORT] */ > + phys = <&serdes_1 4>; /* lane E */ > + max-speed = <25000>; > + pcs-handle = <&pcs6>; > + }; > + }; > + > + # LX2160A lane F with SerDes 1 protocol 19: dpmac5 uses 25GBase-KR > + - | > + dpmac5 { > + phy-handle = <&mac5_backplane_anlt>; > + phy-connection-type = "internal"; > + }; > + > + pcs_mdio5 { > + #address-cells = <1>; > + #size-cells = <0>; > + status = "okay"; > + > + pcs5: ethernet-phy@0 { > + reg = <0>; > + }; > + > + mac5_backplane_anlt: ethernet-phy@5 { > + compatible = "fsl,lx2160a-backplane-anlt", > + "ethernet-phy-ieee802.3-c45"; > + reg = <5>; /* according to ANLTFCR1[MDEV_PORT] */ > + phys = <&serdes_1 5>; /* lane F */ > + max-speed = <25000>; > + pcs-handle = <&pcs5>; > + }; > + }; > + > + # LX2160A lane G with SerDes 1 protocol 19: dpmac4 uses 10GBase-KR > + - | > + dpmac4 { > + phy-handle = <&mac4_backplane_anlt>; > + phy-connection-type = "internal"; > + }; > + > + pcs_mdio4 { > + #address-cells = <1>; > + #size-cells = <0>; > + status = "okay"; > + > + mac4_backplane_anlt: ethernet-phy@0 { > + compatible = "fsl,lx2160a-backplane-anlt", > + "ethernet-phy-ieee802.3-c45"; > + reg = <0>; /* merged with PCS SXGMIIGCR1[MDEV_PORT] */ > + phys = <&serdes_1 7>; /* lane G */ > + max-speed = <10000>; > + /* no pcs-handle to &pcs4 */ > + }; > + }; > + > + # LX2160A lane H with SerDes 1 protocol 19: dpmac3 uses 10GBase-KR > + - | > + dpmac3 { > + phy-handle = <&mac3_backplane_anlt>; > + phy-connection-type = "internal"; > + }; > + > + pcs_mdio3 { > + #address-cells = <1>; > + #size-cells = <0>; > + status = "okay"; > + > + mac3_backplane_anlt: ethernet-phy@0 { > + compatible = "fsl,lx2160a-backplane-anlt", > + "ethernet-phy-ieee802.3-c45"; > + reg = <0>; /* merged with PCS SXGMIIHCR1[MDEV_PORT] */ > + phys = <&serdes_1 7>; /* lane H */ > + max-speed = <10000>; > + /* no pcs-handle to &pcs3 */ > + }; > + }; 5 examples yet not one for "fsl,lx2160a-secondary-anlt"
Hi Rob, On Mon, Aug 21, 2023 at 02:58:40PM -0500, Rob Herring wrote: > On Thu, Aug 17, 2023 at 06:06:44PM +0300, Vladimir Oltean wrote: > > Illustrate how the backplane AN/LT blocks can be instantiated on the > > LX2160A SoC with SerDes protocol 19. > > > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> > > --- > > .../devicetree/bindings/net/ethernet-phy.yaml | 8 + > > .../bindings/net/fsl,backplane-anlt.yaml | 238 ++++++++++++++++++ > > 2 files changed, 246 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml > > > > diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml > > index c1241c8a3b77..96fa672e4786 100644 > > --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml > > +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml > > @@ -49,6 +49,14 @@ properties: > > - items: > > - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" > > - const: ethernet-phy-ieee802.3-c45 > > + - items: > > + - const: fsl,lx2160a-backplane-anlt > > + - const: ethernet-phy-ieee802.3-c45 > > What's the benefit of having ethernet-phy-ieee802.3-c45? Will it work if > the OS only understands that and not fsl,lx2160a-backplane-anlt? No. The "is_c45" bool won't get set correctly in fwnode_mdiobus_register_phy(). is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45"); With that bool set incorrectly, the MDIO protocol cannot access the device's registers. > > + description: > > + Some C45 PHYs have no PHY ID in the standard location, and there is > > + also no PHY ID allocated for them to fake. They are identified by the > > + primary compatible string, plus the secondary one to distinguish them > > + from a raw MDIO device. > > > > reg: > > minimum: 0 > > diff --git a/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml > > new file mode 100644 > > index 000000000000..7282e93b1dd4 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml > > @@ -0,0 +1,238 @@ > > +# SPDX-License-Identifier: GPL-2.0+ > > Not the right license. What's wrong with it? > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/fsl,backplane-anlt.yaml > > +$schema: http://devicetree.org/meta-schemas/core.yaml > > + > > +title: Freescale Backplane Ethernet PHY > > + > > +maintainers: > > + - Vladimir Oltean <vladimir.oltean@nxp.com> > > + > > +description: | > > + Some QorIQ and Layerscape SoCs have an additional block on each SerDes > > + networking lane, based on an IP core from MoreThanIP, which performs IEEE > > + 802.3 clause 73 base page exchanges (for auto-negotiation) and clause 72 > > + training frame exchanges (for link training). > > + > > + By default, this AN/LT block comes up with auto-negotiation disabled, and > > + in that case it allows itself to be quickly bypassed from the data path and > > + for the PCS link to come up without its involvement. > > + > > + Software can optionally make use of it, to turn the PCS, AN/LT block and lane > > + (PMA/PMD) into a full copper backplane internal PHY. > > + > > + As a warning, the binding for the multi-lane link modes (40GBase-KR4) is not > > + currently backed up by a driver implementation. > > + > > +allOf: > > + - if: > > + properties: > > + compatible: > > + - items: > > + - const: fsl,lx2160a-backplane-anlt > > + - const: ethernet-phy-ieee802.3-c45 > > Use "contains" and drop ethernet-phy-ieee802.3-c45. Thanks, I'll try it. > > + then: > > + $ref: ethernet-phy.yaml# > > + > > +properties: > > + compatible: > > + oneOf: > > + - items: > > + - const: fsl,lx2160a-backplane-anlt > > + - const: ethernet-phy-ieee802.3-c45 > > + - const: fsl,lx2160a-secondary-anlt > > + > > + reg: > > + minimum: 0 > > + maximum: 31 > > + description: | > > + The address of the AN/LT block within the internal MDIO bus of the MAC it > > + is attached to. > > + > > + In the 1000Base-KX and 10GBase-KR link modes, the AN/LT block responds at > > + the same MDIO address as the PCS (determined by the SGMIInCR1[MDEV_PORT] > > + or SXGMIInCR1[MDEV_PORT] registers of the SerDes block, by default 0). > > + The PCS and AN/LT block respond to different MMDs, though. > > + > > + In the 25GBase-KR and higher link modes, the AN/LT block responds at a > > + different MDIO address than the PCS, determined by the > > + ANLTnCR1[MDEV_PORT] register of the SerDes block. By default this is 4 > > + for lanes A and E, 5 for lanes B and F, 6 for lanes C and G, 7 for lanes > > + D and H. > > + > > + The PCS responds in all cases at the address determined by the MDEV_PORT > > + field of the SGMIInCR1, SXGMIIaCR1, E25GaCR1, E40GaCR1, E50GaCR1 or > > + E100GaCR1 registers of the SerDes block. > > + > > + phys: > > + maxItems: 1 > > + description: > > + phandle for the generic PHY (SerDes lane) that acts as PMA/PMD layer > > + > > + pcs-handle: > > + maxItems: 1 > > + description: > > + phandle for the technology-dependent PCS block corresponding to the > > + initial (RCW-based) configuration of the port. Must be omitted for the > > + link modes where the PCS and AN/LT block respond at the same MDIO > > + address. Must be specified otherwise. > > + > > + secondary-anlt-handle: > > Needs a vendor prefix and type. Ok (assuming this will remain in the final solution, after PHY maintainers' review) > > + maxItems: 1 > > + description: > > + In case this is the primary (first) lane of a multi-lane link mode, this > > + property holds an array of phandles for the other AN/LT blocks, that are > > + involved in link training but not in auto-negotiation. These have the > > + "fsl,lx2160a-secondary-anlt" compatible string. > > + > > +required: > > + - compatible > > + - reg > > + - phys > > + > > +unevaluatedProperties: false > > + > > +examples: > > + > > + # LX2160A lanes A, B, C, D with SerDes 1 protocol 19: dpmac2 uses 40GBase-KR4 > > + - | > > + dpmac2 { > > + phy-handle = <&mac2_backplane_anlt>; > > + phy-connection-type = "internal"; > > + }; > > + > > + pcs_mdio2 { > > mdio { Hmm, is it bad if I can keep the name? (it's actually important for documentary purposes) Actually I wanted to put a label like "pcs_mdio2: mdio {", but I couldn't, and this is what led me to the compromise. > > + #address-cells = <1>; > > + #size-cells = <0>; > > + status = "okay"; > > Don't need status in examples. Ok. > > + > > + pcs2: ethernet-phy@0 { > > + reg = <0>; > > + }; > > + > > + mac2_backplane_anlt: ethernet-phy@7 { > > + compatible = "fsl,lx2160a-backplane-anlt", > > + "ethernet-phy-ieee802.3-c45"; > > + reg = <7>; /* according to ANLTDCR1[MDEV_PORT] */ > > + phys = <&serdes_1 3>; /* lane D */ > > + max-speed = <40000>; > > + pcs-handle = <&pcs2>; > > + secondary-anlt-handle = <&mac2_lane2_anlt>, <&mac2_lane3_anlt>, > > + <&mac2_lane4_anlt>; > > + }; > > + > > + mac2_lane2_anlt: ethernet-backplane-anlt@6 { > > + compatible = "fsl,lx2160a-secondary-anlt"; > > + reg = <6>; /* according to ANLTCCR1[MDEV_PORT] */ > > + phys = <&serdes_1 2>; /* lane C */ > > + }; > > + > > + mac2_lane3_anlt: ethernet-backplane-anlt@5 { > > + compatible = "fsl,lx2160a-secondary-anlt"; > > + reg = <5>; /* according to ANLTBCR1[MDEV_PORT] */ > > + phys = <&serdes_1 1>; /* lane B */ > > + }; > > + > > + mac2_lane4_anlt: ethernet-backplane-anlt@4 { > > + compatible = "fsl,lx2160a-secondary-anlt"; > > + reg = <4>; /* according to ANLTACR1[MDEV_PORT] */ > > + phys = <&serdes_1 0>; /* lane A */ > > + }; > > + }; > > + > > + # LX2160A lane E with SerDes 1 protocol 19: dpmac6 uses 25GBase-KR > > + - | > > + dpmac6 { > > + phy-handle = <&mac6_backplane_anlt>; > > + phy-connection-type = "internal"; > > + }; > > + > > + pcs_mdio6 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + status = "okay"; > > + > > + pcs6: ethernet-phy@0 { > > + reg = <0>; > > + }; > > + > > + mac6_backplane_anlt: ethernet-phy@4 { > > + compatible = "fsl,lx2160a-backplane-anlt", > > + "ethernet-phy-ieee802.3-c45"; > > + reg = <4>; /* according to ANLTFCR1[MDEV_PORT] */ > > + phys = <&serdes_1 4>; /* lane E */ > > + max-speed = <25000>; > > + pcs-handle = <&pcs6>; > > + }; > > + }; > > + > > + # LX2160A lane F with SerDes 1 protocol 19: dpmac5 uses 25GBase-KR > > + - | > > + dpmac5 { > > + phy-handle = <&mac5_backplane_anlt>; > > + phy-connection-type = "internal"; > > + }; > > + > > + pcs_mdio5 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + status = "okay"; > > + > > + pcs5: ethernet-phy@0 { > > + reg = <0>; > > + }; > > + > > + mac5_backplane_anlt: ethernet-phy@5 { > > + compatible = "fsl,lx2160a-backplane-anlt", > > + "ethernet-phy-ieee802.3-c45"; > > + reg = <5>; /* according to ANLTFCR1[MDEV_PORT] */ > > + phys = <&serdes_1 5>; /* lane F */ > > + max-speed = <25000>; > > + pcs-handle = <&pcs5>; > > + }; > > + }; > > + > > + # LX2160A lane G with SerDes 1 protocol 19: dpmac4 uses 10GBase-KR > > + - | > > + dpmac4 { > > + phy-handle = <&mac4_backplane_anlt>; > > + phy-connection-type = "internal"; > > + }; > > + > > + pcs_mdio4 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + status = "okay"; > > + > > + mac4_backplane_anlt: ethernet-phy@0 { > > + compatible = "fsl,lx2160a-backplane-anlt", > > + "ethernet-phy-ieee802.3-c45"; > > + reg = <0>; /* merged with PCS SXGMIIGCR1[MDEV_PORT] */ > > + phys = <&serdes_1 7>; /* lane G */ > > + max-speed = <10000>; > > + /* no pcs-handle to &pcs4 */ > > + }; > > + }; > > + > > + # LX2160A lane H with SerDes 1 protocol 19: dpmac3 uses 10GBase-KR > > + - | > > + dpmac3 { > > + phy-handle = <&mac3_backplane_anlt>; > > + phy-connection-type = "internal"; > > + }; > > + > > + pcs_mdio3 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + status = "okay"; > > + > > + mac3_backplane_anlt: ethernet-phy@0 { > > + compatible = "fsl,lx2160a-backplane-anlt", > > + "ethernet-phy-ieee802.3-c45"; > > + reg = <0>; /* merged with PCS SXGMIIHCR1[MDEV_PORT] */ > > + phys = <&serdes_1 7>; /* lane H */ > > + max-speed = <10000>; > > + /* no pcs-handle to &pcs3 */ > > + }; > > + }; > > 5 examples yet not one for "fsl,lx2160a-secondary-anlt" Are you sure? They're in the first one: "LX2160A lanes A, B, C, D with SerDes 1 protocol 19: dpmac2 uses 40GBase-KR4" (and still not trimmed from this email)
> > > - items: > > > - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" > > > - const: ethernet-phy-ieee802.3-c45 > > > + - items: > > > + - const: fsl,lx2160a-backplane-anlt > > > + - const: ethernet-phy-ieee802.3-c45 > > > > What's the benefit of having ethernet-phy-ieee802.3-c45? Will it work if > > the OS only understands that and not fsl,lx2160a-backplane-anlt? > > No. The "is_c45" bool won't get set correctly in fwnode_mdiobus_register_phy(). > > is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45"); > > With that bool set incorrectly, the MDIO protocol cannot access the device's > registers. > > > > + description: > > > + Some C45 PHYs have no PHY ID in the standard location, and there is > > > + also no PHY ID allocated for them to fake. They are identified by the > > > + primary compatible string, plus the secondary one to distinguish them > > > + from a raw MDIO device. Could you fake ID registers? Is this on any arbitrary MDIO bus, or an internal bus with its own MDIO driver which could trap reads to the ID registers and return well known values? Andrew
On Mon, Aug 21, 2023 at 10:20:43PM +0200, Andrew Lunn wrote: > > > > - items: > > > > - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" > > > > - const: ethernet-phy-ieee802.3-c45 > > > > + - items: > > > > + - const: fsl,lx2160a-backplane-anlt > > > > + - const: ethernet-phy-ieee802.3-c45 > > > > > > What's the benefit of having ethernet-phy-ieee802.3-c45? Will it work if > > > the OS only understands that and not fsl,lx2160a-backplane-anlt? > > > > No. The "is_c45" bool won't get set correctly in fwnode_mdiobus_register_phy(). > > > > is_c45 = fwnode_device_is_compatible(child, "ethernet-phy-ieee802.3-c45"); > > > > With that bool set incorrectly, the MDIO protocol cannot access the device's > > registers. > > > > > > + description: > > > > + Some C45 PHYs have no PHY ID in the standard location, and there is > > > > + also no PHY ID allocated for them to fake. They are identified by the > > > > + primary compatible string, plus the secondary one to distinguish them > > > > + from a raw MDIO device. > > Could you fake ID registers? Is this on any arbitrary MDIO bus, or an > internal bus with its own MDIO driver which could trap reads to the ID > registers and return well known values? > > Andrew The MDIO bus is not arbitrary, the integration choice with this register layout is specific to the LX2160A SoC, and it's an internal PHY there. But, there's already something else at those MDIO registers (where the standard PHY ID location is), in the MMD that the AN/LT block responds to. And that would be: /* Auto-Negotiation Control and Status Registers are on page 0: 0x0 */ static const u16 mtip_lx2160a_an_regs[] = { [AN_CTRL] = 0, [AN_STAT] = 1, [AN_ADV_0] = 2, // overlaps with MII_PHYSID1 [AN_ADV_1] = 3, // overlaps with MII_PHYSID2 [AN_ADV_2] = 4, [AN_LPA_0] = 5, // overlaps with MDIO_DEVS1 [AN_LPA_1] = 6, // overlaps with MDIO_DEVS2 [AN_LPA_2] = 7, [AN_MS_CNT] = 8, [AN_ADV_XNP_0] = 9, [AN_ADV_XNP_1] = 10, [AN_ADV_XNP_2] = 11, [AN_LPA_XNP_0] = 12, [AN_LPA_XNP_1] = 13, [AN_LPA_XNP_2] = 14, [AN_BP_ETH_STAT] = 15, }; The AN advertisement registers are kinda important to the operation of the driver, so I wouldn't want to mask them with fake PHY ID values reported by the MDIO controller. The other option would be to somehow make the mtip_backplane driver remap (and thus, standardize) its own register space as phy_read_mmd() and phy_write_mmd() see it, but it's not clear at all how that would be done, or if it was done before / would be useful generally.
> But, there's already something else at those MDIO registers (where the > standard PHY ID location is), in the MMD that the AN/LT block responds to. > And that would be: > > /* Auto-Negotiation Control and Status Registers are on page 0: 0x0 */ > static const u16 mtip_lx2160a_an_regs[] = { > [AN_CTRL] = 0, > [AN_STAT] = 1, > [AN_ADV_0] = 2, // overlaps with MII_PHYSID1 > [AN_ADV_1] = 3, // overlaps with MII_PHYSID2 > [AN_ADV_2] = 4, > [AN_LPA_0] = 5, // overlaps with MDIO_DEVS1 > [AN_LPA_1] = 6, // overlaps with MDIO_DEVS2 > [AN_LPA_2] = 7, > [AN_MS_CNT] = 8, > [AN_ADV_XNP_0] = 9, > [AN_ADV_XNP_1] = 10, > [AN_ADV_XNP_2] = 11, > [AN_LPA_XNP_0] = 12, > [AN_LPA_XNP_1] = 13, > [AN_LPA_XNP_2] = 14, > [AN_BP_ETH_STAT] = 15, > }; > > The AN advertisement registers are kinda important to the operation of > the driver, so I wouldn't want to mask them with fake PHY ID values > reported by the MDIO controller. O.K, not ideal. For C22, you could just put the ID values in the compatible, which is enough to get a driver loaded which supports that ID. But somebody recently commented that that does not work for C45. I assume NXP has an OUI, and could allocate an ID to this device in retrospect. So maybe it makes sense to make C45 work with an ID in the compatible? And get the driver loaded that way? Andrew
On Mon, Aug 21, 2023 at 11:10:27PM +0200, Andrew Lunn wrote: > > But, there's already something else at those MDIO registers (where the > > standard PHY ID location is), in the MMD that the AN/LT block responds to. > > And that would be: > > > > /* Auto-Negotiation Control and Status Registers are on page 0: 0x0 */ > > static const u16 mtip_lx2160a_an_regs[] = { > > [AN_CTRL] = 0, > > [AN_STAT] = 1, > > [AN_ADV_0] = 2, // overlaps with MII_PHYSID1 > > [AN_ADV_1] = 3, // overlaps with MII_PHYSID2 > > [AN_ADV_2] = 4, > > [AN_LPA_0] = 5, // overlaps with MDIO_DEVS1 > > [AN_LPA_1] = 6, // overlaps with MDIO_DEVS2 > > [AN_LPA_2] = 7, > > [AN_MS_CNT] = 8, > > [AN_ADV_XNP_0] = 9, > > [AN_ADV_XNP_1] = 10, > > [AN_ADV_XNP_2] = 11, > > [AN_LPA_XNP_0] = 12, > > [AN_LPA_XNP_1] = 13, > > [AN_LPA_XNP_2] = 14, > > [AN_BP_ETH_STAT] = 15, > > }; > > > > The AN advertisement registers are kinda important to the operation of > > the driver, so I wouldn't want to mask them with fake PHY ID values > > reported by the MDIO controller. > > O.K, not ideal. For C22, you could just put the ID values in the > compatible, which is enough to get a driver loaded which supports that > ID. But somebody recently commented that that does not work for C45. I > assume NXP has an OUI, and could allocate an ID to this device in > retrospect. So maybe it makes sense to make C45 work with an ID in the > compatible? And get the driver loaded that way? > > Andrew There are 2 clarification questions that I can think of right now. Maybe more later. First: Compatible strings per C45 MMD? Drivers per C45 MMD? Is there supposed to be an interest in that? I might end up needing it (see the problem description in the cover letter, with PCS and AN/LT block merged into the same MDIO address, but responding to separate MMDs) Second: Suppose I add something like "ethernet-phy-ieee802.3-c45-idXXXX.XXXX". Do I replace just this with that: compatible = "fsl,lx2160a-backplane-anlt", "ethernet-phy-ieee802.3-c45"; or also this? compatible = "fsl,lx2160a-secondary-anlt"; I suppose it would be just the first one, but going that route would IMO just increase the dissonance between the description of primary and secondary AN/LT blocks. They're the same IP blocks, don't they also have the same fake PHY ID?
> > O.K, not ideal. For C22, you could just put the ID values in the > > compatible, which is enough to get a driver loaded which supports that > > ID. But somebody recently commented that that does not work for C45. I > > assume NXP has an OUI, and could allocate an ID to this device in > > retrospect. So maybe it makes sense to make C45 work with an ID in the > > compatible? And get the driver loaded that way? > > > > Andrew > > There are 2 clarification questions that I can think of right now. > Maybe more later. > > First: Compatible strings per C45 MMD? Drivers per C45 MMD? Is there > supposed to be an interest in that? I might end up needing it (see the > problem description in the cover letter, with PCS and AN/LT block merged > into the same MDIO address, but responding to separate MMDs) > > Second: Suppose I add something like "ethernet-phy-ieee802.3-c45-idXXXX.XXXX". > Do I replace just this with that: > > compatible = "fsl,lx2160a-backplane-anlt", "ethernet-phy-ieee802.3-c45"; > > or also this? > > compatible = "fsl,lx2160a-secondary-anlt"; > > > I suppose it would be just the first one, but going that route would IMO > just increase the dissonance between the description of primary and > secondary AN/LT blocks. They're the same IP blocks, don't they also have > the same fake PHY ID? For C22 PHYs, the ID registers are only used to ask user space to load a driver which supports that ID, and then used to match a device to a driver. We often say that if the ID registers don't actually contain an ID, or the wrong ID, use ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$ to let the subsystem know the correct ID. The device you are trying to support has the same problem, invalid IDs, but its C45. C45 IDs however work slightly differently. An C45 package can have multiple devices in it, up to 32. Each device has its own ID registers. So there can be up to 32 different IDs for one package. The core will try to determine which of the 32 devices are actually in the package, and if they are, what the ID is. It then asks user space to load a driver for all the IDs it finds. And when matching devices to drivers, it sees if any of the ID of the package matches the IDs the driver says it supports. If a match is found, that one driver is expected to drive all the devices in that one package. I don't see a need for ethernet-phy-ieee802.3-c45-idXXXX.XXXX, ethernet-phy-ieee802.3-idXXXX.XXXX should be sufficient, since all you are doing it matching the ID against the driver. That matching does not differ between C22 and C45. Saying "ethernet-phy-ieee802.3-c45" might be useful, because at the moment we have a mixup between C45 register space and C45 bus transactions. The drive is free to access C22 and/or C45 registers, since it should know what the device actually has. But some of the core might get the wrong idea without "ethernet-phy-ieee802.3-c45". O.K, that the background now: > First: Compatible strings per C45 MMD? Drivers per C45 MMD So far, nobody has needed that. All current drivers are package drivers, they drive all the devices in the package. At least for a PHY, there is close integration between devices in a package. Russell has commented that the Marvell 10G PHY does appear to contain a number of licensed devices, but so far, we have not noticed the same licensed device used by multiple vendors. So there has not been any need to reuse code. However, it sounds like the package you are trying to support does contain multiple independent devices. So from an architecture perspective, having multiple drivers would make sense, even if there is no reuse. But are the devices PHY? Everything i've said so far applies to PHYs. It does not apply to a PCS, etc. Andrew
Hi Andrew and phylib/phylink maintainers in general, On Tue, Aug 22, 2023 at 04:10:45PM +0200, Andrew Lunn wrote: > For C22 PHYs, the ID registers are only used to ask user space to load > a driver which supports that ID, and then used to match a device to a > driver. We often say that if the ID registers don't actually contain > an ID, or the wrong ID, use ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$ > to let the subsystem know the correct ID. > > The device you are trying to support has the same problem, invalid > IDs, but its C45. > > C45 IDs however work slightly differently. An C45 package can have > multiple devices in it, up to 32. Each device has its own ID > registers. So there can be up to 32 different IDs for one package. The > core will try to determine which of the 32 devices are actually in the > package, and if they are, what the ID is. It then asks user space to > load a driver for all the IDs it finds. And when matching devices to > drivers, it sees if any of the ID of the package matches the IDs the > driver says it supports. If a match is found, that one driver is > expected to drive all the devices in that one package. > > I don't see a need for ethernet-phy-ieee802.3-c45-idXXXX.XXXX, > ethernet-phy-ieee802.3-idXXXX.XXXX should be sufficient, since all you > are doing it matching the ID against the driver. That matching does > not differ between C22 and C45. > > Saying "ethernet-phy-ieee802.3-c45" might be useful, because at the > moment we have a mixup between C45 register space and C45 bus > transactions. The drive is free to access C22 and/or C45 registers, > since it should know what the device actually has. But some of the > core might get the wrong idea without "ethernet-phy-ieee802.3-c45". > > O.K, that the background now: > > > First: Compatible strings per C45 MMD? Drivers per C45 MMD > > So far, nobody has needed that. All current drivers are package > drivers, they drive all the devices in the package. At least for a > PHY, there is close integration between devices in a package. Russell > has commented that the Marvell 10G PHY does appear to contain a number > of licensed devices, but so far, we have not noticed the same licensed > device used by multiple vendors. So there has not been any need to > reuse code. > > However, it sounds like the package you are trying to support does > contain multiple independent devices. So from an architecture > perspective, having multiple drivers would make sense, even if there > is no reuse. But are the devices PHY? Everything i've said so far > applies to PHYs. It does not apply to a PCS, etc. > > Andrew I don't know if the devices qualify as phy_device structures according to the opinion of the maintainers, and that is one of the fundamental aspects I would like this RFC to clarify, before I proceed to request NXP to allocate a new PHY ID that I can put in the compatible string. If the backplane AN/LT block is not a phy_device structure, my imagination will need a bit of help on how to integrate it, as a raw mdio_device, with phylink or with the consumer MAC drivers directly.
diff --git a/Documentation/devicetree/bindings/net/ethernet-phy.yaml b/Documentation/devicetree/bindings/net/ethernet-phy.yaml index c1241c8a3b77..96fa672e4786 100644 --- a/Documentation/devicetree/bindings/net/ethernet-phy.yaml +++ b/Documentation/devicetree/bindings/net/ethernet-phy.yaml @@ -49,6 +49,14 @@ properties: - items: - pattern: "^ethernet-phy-id[a-f0-9]{4}\\.[a-f0-9]{4}$" - const: ethernet-phy-ieee802.3-c45 + - items: + - const: fsl,lx2160a-backplane-anlt + - const: ethernet-phy-ieee802.3-c45 + description: + Some C45 PHYs have no PHY ID in the standard location, and there is + also no PHY ID allocated for them to fake. They are identified by the + primary compatible string, plus the secondary one to distinguish them + from a raw MDIO device. reg: minimum: 0 diff --git a/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml new file mode 100644 index 000000000000..7282e93b1dd4 --- /dev/null +++ b/Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml @@ -0,0 +1,238 @@ +# SPDX-License-Identifier: GPL-2.0+ +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/fsl,backplane-anlt.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Freescale Backplane Ethernet PHY + +maintainers: + - Vladimir Oltean <vladimir.oltean@nxp.com> + +description: | + Some QorIQ and Layerscape SoCs have an additional block on each SerDes + networking lane, based on an IP core from MoreThanIP, which performs IEEE + 802.3 clause 73 base page exchanges (for auto-negotiation) and clause 72 + training frame exchanges (for link training). + + By default, this AN/LT block comes up with auto-negotiation disabled, and + in that case it allows itself to be quickly bypassed from the data path and + for the PCS link to come up without its involvement. + + Software can optionally make use of it, to turn the PCS, AN/LT block and lane + (PMA/PMD) into a full copper backplane internal PHY. + + As a warning, the binding for the multi-lane link modes (40GBase-KR4) is not + currently backed up by a driver implementation. + +allOf: + - if: + properties: + compatible: + - items: + - const: fsl,lx2160a-backplane-anlt + - const: ethernet-phy-ieee802.3-c45 + then: + $ref: ethernet-phy.yaml# + +properties: + compatible: + oneOf: + - items: + - const: fsl,lx2160a-backplane-anlt + - const: ethernet-phy-ieee802.3-c45 + - const: fsl,lx2160a-secondary-anlt + + reg: + minimum: 0 + maximum: 31 + description: | + The address of the AN/LT block within the internal MDIO bus of the MAC it + is attached to. + + In the 1000Base-KX and 10GBase-KR link modes, the AN/LT block responds at + the same MDIO address as the PCS (determined by the SGMIInCR1[MDEV_PORT] + or SXGMIInCR1[MDEV_PORT] registers of the SerDes block, by default 0). + The PCS and AN/LT block respond to different MMDs, though. + + In the 25GBase-KR and higher link modes, the AN/LT block responds at a + different MDIO address than the PCS, determined by the + ANLTnCR1[MDEV_PORT] register of the SerDes block. By default this is 4 + for lanes A and E, 5 for lanes B and F, 6 for lanes C and G, 7 for lanes + D and H. + + The PCS responds in all cases at the address determined by the MDEV_PORT + field of the SGMIInCR1, SXGMIIaCR1, E25GaCR1, E40GaCR1, E50GaCR1 or + E100GaCR1 registers of the SerDes block. + + phys: + maxItems: 1 + description: + phandle for the generic PHY (SerDes lane) that acts as PMA/PMD layer + + pcs-handle: + maxItems: 1 + description: + phandle for the technology-dependent PCS block corresponding to the + initial (RCW-based) configuration of the port. Must be omitted for the + link modes where the PCS and AN/LT block respond at the same MDIO + address. Must be specified otherwise. + + secondary-anlt-handle: + maxItems: 1 + description: + In case this is the primary (first) lane of a multi-lane link mode, this + property holds an array of phandles for the other AN/LT blocks, that are + involved in link training but not in auto-negotiation. These have the + "fsl,lx2160a-secondary-anlt" compatible string. + +required: + - compatible + - reg + - phys + +unevaluatedProperties: false + +examples: + + # LX2160A lanes A, B, C, D with SerDes 1 protocol 19: dpmac2 uses 40GBase-KR4 + - | + dpmac2 { + phy-handle = <&mac2_backplane_anlt>; + phy-connection-type = "internal"; + }; + + pcs_mdio2 { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + pcs2: ethernet-phy@0 { + reg = <0>; + }; + + mac2_backplane_anlt: ethernet-phy@7 { + compatible = "fsl,lx2160a-backplane-anlt", + "ethernet-phy-ieee802.3-c45"; + reg = <7>; /* according to ANLTDCR1[MDEV_PORT] */ + phys = <&serdes_1 3>; /* lane D */ + max-speed = <40000>; + pcs-handle = <&pcs2>; + secondary-anlt-handle = <&mac2_lane2_anlt>, <&mac2_lane3_anlt>, + <&mac2_lane4_anlt>; + }; + + mac2_lane2_anlt: ethernet-backplane-anlt@6 { + compatible = "fsl,lx2160a-secondary-anlt"; + reg = <6>; /* according to ANLTCCR1[MDEV_PORT] */ + phys = <&serdes_1 2>; /* lane C */ + }; + + mac2_lane3_anlt: ethernet-backplane-anlt@5 { + compatible = "fsl,lx2160a-secondary-anlt"; + reg = <5>; /* according to ANLTBCR1[MDEV_PORT] */ + phys = <&serdes_1 1>; /* lane B */ + }; + + mac2_lane4_anlt: ethernet-backplane-anlt@4 { + compatible = "fsl,lx2160a-secondary-anlt"; + reg = <4>; /* according to ANLTACR1[MDEV_PORT] */ + phys = <&serdes_1 0>; /* lane A */ + }; + }; + + # LX2160A lane E with SerDes 1 protocol 19: dpmac6 uses 25GBase-KR + - | + dpmac6 { + phy-handle = <&mac6_backplane_anlt>; + phy-connection-type = "internal"; + }; + + pcs_mdio6 { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + pcs6: ethernet-phy@0 { + reg = <0>; + }; + + mac6_backplane_anlt: ethernet-phy@4 { + compatible = "fsl,lx2160a-backplane-anlt", + "ethernet-phy-ieee802.3-c45"; + reg = <4>; /* according to ANLTFCR1[MDEV_PORT] */ + phys = <&serdes_1 4>; /* lane E */ + max-speed = <25000>; + pcs-handle = <&pcs6>; + }; + }; + + # LX2160A lane F with SerDes 1 protocol 19: dpmac5 uses 25GBase-KR + - | + dpmac5 { + phy-handle = <&mac5_backplane_anlt>; + phy-connection-type = "internal"; + }; + + pcs_mdio5 { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + pcs5: ethernet-phy@0 { + reg = <0>; + }; + + mac5_backplane_anlt: ethernet-phy@5 { + compatible = "fsl,lx2160a-backplane-anlt", + "ethernet-phy-ieee802.3-c45"; + reg = <5>; /* according to ANLTFCR1[MDEV_PORT] */ + phys = <&serdes_1 5>; /* lane F */ + max-speed = <25000>; + pcs-handle = <&pcs5>; + }; + }; + + # LX2160A lane G with SerDes 1 protocol 19: dpmac4 uses 10GBase-KR + - | + dpmac4 { + phy-handle = <&mac4_backplane_anlt>; + phy-connection-type = "internal"; + }; + + pcs_mdio4 { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + mac4_backplane_anlt: ethernet-phy@0 { + compatible = "fsl,lx2160a-backplane-anlt", + "ethernet-phy-ieee802.3-c45"; + reg = <0>; /* merged with PCS SXGMIIGCR1[MDEV_PORT] */ + phys = <&serdes_1 7>; /* lane G */ + max-speed = <10000>; + /* no pcs-handle to &pcs4 */ + }; + }; + + # LX2160A lane H with SerDes 1 protocol 19: dpmac3 uses 10GBase-KR + - | + dpmac3 { + phy-handle = <&mac3_backplane_anlt>; + phy-connection-type = "internal"; + }; + + pcs_mdio3 { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + mac3_backplane_anlt: ethernet-phy@0 { + compatible = "fsl,lx2160a-backplane-anlt", + "ethernet-phy-ieee802.3-c45"; + reg = <0>; /* merged with PCS SXGMIIHCR1[MDEV_PORT] */ + phys = <&serdes_1 7>; /* lane H */ + max-speed = <10000>; + /* no pcs-handle to &pcs3 */ + }; + };
Illustrate how the backplane AN/LT blocks can be instantiated on the LX2160A SoC with SerDes protocol 19. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- .../devicetree/bindings/net/ethernet-phy.yaml | 8 + .../bindings/net/fsl,backplane-anlt.yaml | 238 ++++++++++++++++++ 2 files changed, 246 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/fsl,backplane-anlt.yaml