diff mbox series

[net-next,v2,03/35] dt-bindings: net: fman: Add additional interface properties

Message ID 20220628221404.1444200-4-sean.anderson@seco.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series net: dpaa: Convert to phylink | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count fail Series longer than 15 patches (and no cover letter)
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 9 of 9 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 88 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Sean Anderson June 28, 2022, 10:13 p.m. UTC
At the moment, mEMACs are configured almost completely based on the
phy-connection-type. That is, if the phy interface is RGMII, it assumed
that RGMII is supported. For some interfaces, it is assumed that the
RCW/bootloader has set up the SerDes properly. This is generally OK, but
restricts runtime reconfiguration. The actual link state is never
reported.

To address these shortcomings, the driver will need additional
information. First, it needs to know how to access the PCS/PMAs (in
order to configure them and get the link status). The SGMII PCS/PMA is
the only currently-described PCS/PMA. Add the XFI and QSGMII PCS/PMAs as
well. The XFI (and 1GBase-KR) PCS/PMA is a c45 "phy" which sits on the
same MDIO bus as SGMII PCS/PMA. By default they will have conflicting
addresses, but they are also not enabled at the same time by default.
Therefore, we can let the XFI PCS/PMA be the default when
phy-connection-type is xgmii. This will allow for
backwards-compatibility.

QSGMII, however, cannot work with the current binding. This is because
the QSGMII PCS/PMAs are only present on one MAC's MDIO bus. At the
moment this is worked around by having every MAC write to the PCS/PMA
addresses (without checking if they are present). This only works if
each MAC has the same configuration, and only if we don't need to know
the status. Because the QSGMII PCS/PMA will typically be located on a
different MDIO bus than the MAC's SGMII PCS/PMA, there is no fallback
for the QSGMII PCS/PMA.

mEMACs (across all SoCs) support the following protocols:

- MII
- RGMII
- SGMII, 1000Base-X, and 1000Base-KX
- 2500Base-X (aka 2.5G SGMII)
- QSGMII
- 10GBase-R (aka XFI) and 10GBase-KR
- XAUI and HiGig

Each line documents a set of orthogonal protocols (e.g. XAUI is
supported if and only if HiGig is supported). Additionally,

- XAUI implies support for 10GBase-R
- 10GBase-R is supported if and only if RGMII is not supported
- 2500Base-X implies support for 1000Base-X
- MII implies support for RGMII

To switch between different protocols, we must reconfigure the SerDes.
This is done by using the standard phys property. We can also use it to
validate whether different protocols are supported (e.g. using
phy_validate). This will work for serial protocols, but not RGMII or
MII. Additionally, we still need to be compatible when there is no
SerDes.

While we can detect 10G support by examining the port speed (as set by
fsl,fman-10g-port), we cannot determine support for any of the other
protocols based on the existing binding. In fact, the binding works
against us in some respects, because pcsphy-handle is required even if
there is no possible PCS/PMA for that MAC. To allow for backwards-
compatibility, we use a boolean-style property for RGMII (instead of
presence/absence-style). When the property for RGMII is missing, we will
assume that it is supported. The exception is MII, since no existing
device trees use it (as far as I could tell).

Unfortunately, QSGMII support will be broken for old device trees. There
is nothing we can do about this because of the PCS/PMA situation (as
described above).

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

Changes in v2:
- Better document how we select which PCS to use in the default case

 .../bindings/net/fsl,fman-dtsec.yaml          | 52 +++++++++++++++++--
 .../devicetree/bindings/net/fsl-fman.txt      |  5 +-
 2 files changed, 51 insertions(+), 6 deletions(-)

Comments

Rob Herring (Arm) June 30, 2022, 4:01 p.m. UTC | #1
On Tue, Jun 28, 2022 at 06:13:32PM -0400, Sean Anderson wrote:
> At the moment, mEMACs are configured almost completely based on the
> phy-connection-type. That is, if the phy interface is RGMII, it assumed
> that RGMII is supported. For some interfaces, it is assumed that the
> RCW/bootloader has set up the SerDes properly. This is generally OK, but
> restricts runtime reconfiguration. The actual link state is never
> reported.
> 
> To address these shortcomings, the driver will need additional
> information. First, it needs to know how to access the PCS/PMAs (in
> order to configure them and get the link status). The SGMII PCS/PMA is
> the only currently-described PCS/PMA. Add the XFI and QSGMII PCS/PMAs as
> well. The XFI (and 1GBase-KR) PCS/PMA is a c45 "phy" which sits on the
> same MDIO bus as SGMII PCS/PMA. By default they will have conflicting
> addresses, but they are also not enabled at the same time by default.
> Therefore, we can let the XFI PCS/PMA be the default when
> phy-connection-type is xgmii. This will allow for
> backwards-compatibility.
> 
> QSGMII, however, cannot work with the current binding. This is because
> the QSGMII PCS/PMAs are only present on one MAC's MDIO bus. At the
> moment this is worked around by having every MAC write to the PCS/PMA
> addresses (without checking if they are present). This only works if
> each MAC has the same configuration, and only if we don't need to know
> the status. Because the QSGMII PCS/PMA will typically be located on a
> different MDIO bus than the MAC's SGMII PCS/PMA, there is no fallback
> for the QSGMII PCS/PMA.
> 
> mEMACs (across all SoCs) support the following protocols:
> 
> - MII
> - RGMII
> - SGMII, 1000Base-X, and 1000Base-KX
> - 2500Base-X (aka 2.5G SGMII)
> - QSGMII
> - 10GBase-R (aka XFI) and 10GBase-KR
> - XAUI and HiGig
> 
> Each line documents a set of orthogonal protocols (e.g. XAUI is
> supported if and only if HiGig is supported). Additionally,
> 
> - XAUI implies support for 10GBase-R
> - 10GBase-R is supported if and only if RGMII is not supported
> - 2500Base-X implies support for 1000Base-X
> - MII implies support for RGMII
> 
> To switch between different protocols, we must reconfigure the SerDes.
> This is done by using the standard phys property. We can also use it to
> validate whether different protocols are supported (e.g. using
> phy_validate). This will work for serial protocols, but not RGMII or
> MII. Additionally, we still need to be compatible when there is no
> SerDes.
> 
> While we can detect 10G support by examining the port speed (as set by
> fsl,fman-10g-port), we cannot determine support for any of the other
> protocols based on the existing binding. In fact, the binding works
> against us in some respects, because pcsphy-handle is required even if
> there is no possible PCS/PMA for that MAC. To allow for backwards-
> compatibility, we use a boolean-style property for RGMII (instead of
> presence/absence-style). When the property for RGMII is missing, we will
> assume that it is supported. The exception is MII, since no existing
> device trees use it (as far as I could tell).
> 
> Unfortunately, QSGMII support will be broken for old device trees. There
> is nothing we can do about this because of the PCS/PMA situation (as
> described above).
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
> 
> Changes in v2:
> - Better document how we select which PCS to use in the default case
> 
>  .../bindings/net/fsl,fman-dtsec.yaml          | 52 +++++++++++++++++--
>  .../devicetree/bindings/net/fsl-fman.txt      |  5 +-
>  2 files changed, 51 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
> index 809df1589f20..ecb772258164 100644
> --- a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
> +++ b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
> @@ -85,9 +85,41 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/phandle
>      description: A reference to the IEEE1588 timer
>  
> +  phys:
> +    description: A reference to the SerDes lane(s)
> +    maxItems: 1
> +
> +  phy-names:
> +    items:
> +      - const: serdes
> +
>    pcsphy-handle:
> -    $ref: /schemas/types.yaml#/definitions/phandle
> -    description: A reference to the PCS (typically found on the SerDes)
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    minItems: 1
> +    maxItems: 3

What determines how many entries?

> +    description: |
> +      A reference to the various PCSs (typically found on the SerDes). If
> +      pcs-names is absent, and phy-connection-type is "xgmii", then the first
> +      reference will be assumed to be for "xfi". Otherwise, if pcs-names is
> +      absent, then the first reference will be assumed to be for "sgmii".
> +
> +  pcs-names:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    minItems: 1
> +    maxItems: 3
> +    contains:
> +      enum:
> +        - sgmii
> +        - qsgmii
> +        - xfi

This means '"foo", "xfi", "bar"' is valid. I think you want to 
s/contains/items/.

> +    description: The type of each PCS in pcsphy-handle.
> +

> +  rgmii:
> +    enum: [0, 1]
> +    description: 1 indicates RGMII is supported, and 0 indicates it is not.
> +
> +  mii:
> +    description: If present, indicates that MII is supported.

Types? Need vendor prefixes.

Are these board specific or SoC specific? Properties are appropriate for 
the former. The latter case should be implied by the compatible string.

>  
>    tbi-handle:
>      $ref: /schemas/types.yaml#/definitions/phandle
> @@ -100,6 +132,10 @@ required:
>    - fsl,fman-ports
>    - ptp-timer
>  
> +dependencies:
> +  pcs-names: [pcsphy-handle]
> +  mii: [rgmii]
> +
>  allOf:
>    - $ref: "ethernet-controller.yaml#"
>    - if:
> @@ -117,7 +153,11 @@ allOf:
>              const: fsl,fman-memac
>      then:
>        required:
> -        - pcsphy-handle
> +        - rgmii
> +    else:
> +      properties:
> +        rgmii: false
> +        mii: false
>  
>  unevaluatedProperties: false
>  
> @@ -138,7 +178,11 @@ examples:
>              reg = <0xe8000 0x1000>;
>              fsl,fman-ports = <&fman0_rx_0x0c &fman0_tx_0x2c>;
>              ptp-timer = <&ptp_timer0>;
> -            pcsphy-handle = <&pcsphy4>;
> +            pcsphy-handle = <&pcsphy4>, <&qsgmiib_pcs1>;
> +            pcs-names = "sgmii", "qsgmii";
> +            rgmii = <0>;
>              phy-handle = <&sgmii_phy1>;
>              phy-connection-type = "sgmii";
> +            phys = <&serdes1 1>;
> +            phy-names = "serdes";
>      };
> diff --git a/Documentation/devicetree/bindings/net/fsl-fman.txt b/Documentation/devicetree/bindings/net/fsl-fman.txt
> index b9055335db3b..bda4b41af074 100644
> --- a/Documentation/devicetree/bindings/net/fsl-fman.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-fman.txt
> @@ -320,8 +320,9 @@ For internal PHY device on internal mdio bus, a PHY node should be created.
>  See the definition of the PHY node in booting-without-of.txt for an
>  example of how to define a PHY (Internal PHY has no interrupt line).
>  - For "fsl,fman-mdio" compatible internal mdio bus, the PHY is TBI PHY.
> -- For "fsl,fman-memac-mdio" compatible internal mdio bus, the PHY is PCS PHY,
> -  PCS PHY addr must be '0'.
> +- For "fsl,fman-memac-mdio" compatible internal mdio bus, the PHY is PCS PHY.
> +  The PCS PHY address should correspond to the value of the appropriate
> +  MDEV_PORT.
>  
>  EXAMPLE
>  
> -- 
> 2.35.1.1320.gc452695387.dirty
> 
>
Sean Anderson June 30, 2022, 4:11 p.m. UTC | #2
Hi Rob,

On 6/30/22 12:01 PM, Rob Herring wrote:
> On Tue, Jun 28, 2022 at 06:13:32PM -0400, Sean Anderson wrote:
>> At the moment, mEMACs are configured almost completely based on the
>> phy-connection-type. That is, if the phy interface is RGMII, it assumed
>> that RGMII is supported. For some interfaces, it is assumed that the
>> RCW/bootloader has set up the SerDes properly. This is generally OK, but
>> restricts runtime reconfiguration. The actual link state is never
>> reported.
>> 
>> To address these shortcomings, the driver will need additional
>> information. First, it needs to know how to access the PCS/PMAs (in
>> order to configure them and get the link status). The SGMII PCS/PMA is
>> the only currently-described PCS/PMA. Add the XFI and QSGMII PCS/PMAs as
>> well. The XFI (and 1GBase-KR) PCS/PMA is a c45 "phy" which sits on the
>> same MDIO bus as SGMII PCS/PMA. By default they will have conflicting
>> addresses, but they are also not enabled at the same time by default.
>> Therefore, we can let the XFI PCS/PMA be the default when
>> phy-connection-type is xgmii. This will allow for
>> backwards-compatibility.
>> 
>> QSGMII, however, cannot work with the current binding. This is because
>> the QSGMII PCS/PMAs are only present on one MAC's MDIO bus. At the
>> moment this is worked around by having every MAC write to the PCS/PMA
>> addresses (without checking if they are present). This only works if
>> each MAC has the same configuration, and only if we don't need to know
>> the status. Because the QSGMII PCS/PMA will typically be located on a
>> different MDIO bus than the MAC's SGMII PCS/PMA, there is no fallback
>> for the QSGMII PCS/PMA.
>> 
>> mEMACs (across all SoCs) support the following protocols:
>> 
>> - MII
>> - RGMII
>> - SGMII, 1000Base-X, and 1000Base-KX
>> - 2500Base-X (aka 2.5G SGMII)
>> - QSGMII
>> - 10GBase-R (aka XFI) and 10GBase-KR
>> - XAUI and HiGig
>> 
>> Each line documents a set of orthogonal protocols (e.g. XAUI is
>> supported if and only if HiGig is supported). Additionally,
>> 
>> - XAUI implies support for 10GBase-R
>> - 10GBase-R is supported if and only if RGMII is not supported
>> - 2500Base-X implies support for 1000Base-X
>> - MII implies support for RGMII
>> 
>> To switch between different protocols, we must reconfigure the SerDes.
>> This is done by using the standard phys property. We can also use it to
>> validate whether different protocols are supported (e.g. using
>> phy_validate). This will work for serial protocols, but not RGMII or
>> MII. Additionally, we still need to be compatible when there is no
>> SerDes.
>> 
>> While we can detect 10G support by examining the port speed (as set by
>> fsl,fman-10g-port), we cannot determine support for any of the other
>> protocols based on the existing binding. In fact, the binding works
>> against us in some respects, because pcsphy-handle is required even if
>> there is no possible PCS/PMA for that MAC. To allow for backwards-
>> compatibility, we use a boolean-style property for RGMII (instead of
>> presence/absence-style). When the property for RGMII is missing, we will
>> assume that it is supported. The exception is MII, since no existing
>> device trees use it (as far as I could tell).
>> 
>> Unfortunately, QSGMII support will be broken for old device trees. There
>> is nothing we can do about this because of the PCS/PMA situation (as
>> described above).
>> 
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> ---
>> 
>> Changes in v2:
>> - Better document how we select which PCS to use in the default case
>> 
>>  .../bindings/net/fsl,fman-dtsec.yaml          | 52 +++++++++++++++++--
>>  .../devicetree/bindings/net/fsl-fman.txt      |  5 +-
>>  2 files changed, 51 insertions(+), 6 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
>> index 809df1589f20..ecb772258164 100644
>> --- a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
>> +++ b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
>> @@ -85,9 +85,41 @@ properties:
>>      $ref: /schemas/types.yaml#/definitions/phandle
>>      description: A reference to the IEEE1588 timer
>>  
>> +  phys:
>> +    description: A reference to the SerDes lane(s)
>> +    maxItems: 1
>> +
>> +  phy-names:
>> +    items:
>> +      - const: serdes
>> +
>>    pcsphy-handle:
>> -    $ref: /schemas/types.yaml#/definitions/phandle
>> -    description: A reference to the PCS (typically found on the SerDes)
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    minItems: 1
>> +    maxItems: 3
> 
> What determines how many entries?

It depends on what the particular MAC supports. From what I can tell, the following
combinations are valid:

- Neither SGMII, QSGMII, or XFI
- Just SGMII
- Just QSGMII
- SGMII and QSGMII
- SGMII and XFI
- All of SGMII, QSGMII, and XFI

All of these are used on different SoCs.

>> +    description: |
>> +      A reference to the various PCSs (typically found on the SerDes). If
>> +      pcs-names is absent, and phy-connection-type is "xgmii", then the first
>> +      reference will be assumed to be for "xfi". Otherwise, if pcs-names is
>> +      absent, then the first reference will be assumed to be for "sgmii".
>> +
>> +  pcs-names:
>> +    $ref: /schemas/types.yaml#/definitions/string-array
>> +    minItems: 1
>> +    maxItems: 3
>> +    contains:
>> +      enum:
>> +        - sgmii
>> +        - qsgmii
>> +        - xfi
> 
> This means '"foo", "xfi", "bar"' is valid. I think you want to 
> s/contains/items/.
> 
>> +    description: The type of each PCS in pcsphy-handle.
>> +
> 
>> +  rgmii:
>> +    enum: [0, 1]
>> +    description: 1 indicates RGMII is supported, and 0 indicates it is not.
>> +
>> +  mii:
>> +    description: If present, indicates that MII is supported.
> 
> Types? Need vendor prefixes.

OK.

> Are these board specific or SoC specific? Properties are appropriate for 
> the former. The latter case should be implied by the compatible string.

Unfortunately, there are not existing specific compatible strings for each
device in each SoC. I suppose those could be added; however, this basically
reflects how each device is hooked up. E.g. on one SoC a device would be
connected to the RGMII pins, but not on another SoC. The MAC itself still
has hardware support for RGMII, but such a configuration would not function.

--Sean

>>  
>>    tbi-handle:
>>      $ref: /schemas/types.yaml#/definitions/phandle
>> @@ -100,6 +132,10 @@ required:
>>    - fsl,fman-ports
>>    - ptp-timer
>>  
>> +dependencies:
>> +  pcs-names: [pcsphy-handle]
>> +  mii: [rgmii]
>> +
>>  allOf:
>>    - $ref: "ethernet-controller.yaml#"
>>    - if:
>> @@ -117,7 +153,11 @@ allOf:
>>              const: fsl,fman-memac
>>      then:
>>        required:
>> -        - pcsphy-handle
>> +        - rgmii
>> +    else:
>> +      properties:
>> +        rgmii: false
>> +        mii: false
>>  
>>  unevaluatedProperties: false
>>  
>> @@ -138,7 +178,11 @@ examples:
>>              reg = <0xe8000 0x1000>;
>>              fsl,fman-ports = <&fman0_rx_0x0c &fman0_tx_0x2c>;
>>              ptp-timer = <&ptp_timer0>;
>> -            pcsphy-handle = <&pcsphy4>;
>> +            pcsphy-handle = <&pcsphy4>, <&qsgmiib_pcs1>;
>> +            pcs-names = "sgmii", "qsgmii";
>> +            rgmii = <0>;
>>              phy-handle = <&sgmii_phy1>;
>>              phy-connection-type = "sgmii";
>> +            phys = <&serdes1 1>;
>> +            phy-names = "serdes";
>>      };
>> diff --git a/Documentation/devicetree/bindings/net/fsl-fman.txt b/Documentation/devicetree/bindings/net/fsl-fman.txt
>> index b9055335db3b..bda4b41af074 100644
>> --- a/Documentation/devicetree/bindings/net/fsl-fman.txt
>> +++ b/Documentation/devicetree/bindings/net/fsl-fman.txt
>> @@ -320,8 +320,9 @@ For internal PHY device on internal mdio bus, a PHY node should be created.
>>  See the definition of the PHY node in booting-without-of.txt for an
>>  example of how to define a PHY (Internal PHY has no interrupt line).
>>  - For "fsl,fman-mdio" compatible internal mdio bus, the PHY is TBI PHY.
>> -- For "fsl,fman-memac-mdio" compatible internal mdio bus, the PHY is PCS PHY,
>> -  PCS PHY addr must be '0'.
>> +- For "fsl,fman-memac-mdio" compatible internal mdio bus, the PHY is PCS PHY.
>> +  The PCS PHY address should correspond to the value of the appropriate
>> +  MDEV_PORT.
>>  
>>  EXAMPLE
>>  
>> -- 
>> 2.35.1.1320.gc452695387.dirty
>> 
>> 
>
Rob Herring (Arm) July 12, 2022, 7:36 p.m. UTC | #3
On Thu, Jun 30, 2022 at 12:11:10PM -0400, Sean Anderson wrote:
> Hi Rob,
> 
> On 6/30/22 12:01 PM, Rob Herring wrote:
> > On Tue, Jun 28, 2022 at 06:13:32PM -0400, Sean Anderson wrote:
> >> At the moment, mEMACs are configured almost completely based on the
> >> phy-connection-type. That is, if the phy interface is RGMII, it assumed
> >> that RGMII is supported. For some interfaces, it is assumed that the
> >> RCW/bootloader has set up the SerDes properly. This is generally OK, but
> >> restricts runtime reconfiguration. The actual link state is never
> >> reported.
> >> 
> >> To address these shortcomings, the driver will need additional
> >> information. First, it needs to know how to access the PCS/PMAs (in
> >> order to configure them and get the link status). The SGMII PCS/PMA is
> >> the only currently-described PCS/PMA. Add the XFI and QSGMII PCS/PMAs as
> >> well. The XFI (and 1GBase-KR) PCS/PMA is a c45 "phy" which sits on the
> >> same MDIO bus as SGMII PCS/PMA. By default they will have conflicting
> >> addresses, but they are also not enabled at the same time by default.
> >> Therefore, we can let the XFI PCS/PMA be the default when
> >> phy-connection-type is xgmii. This will allow for
> >> backwards-compatibility.
> >> 
> >> QSGMII, however, cannot work with the current binding. This is because
> >> the QSGMII PCS/PMAs are only present on one MAC's MDIO bus. At the
> >> moment this is worked around by having every MAC write to the PCS/PMA
> >> addresses (without checking if they are present). This only works if
> >> each MAC has the same configuration, and only if we don't need to know
> >> the status. Because the QSGMII PCS/PMA will typically be located on a
> >> different MDIO bus than the MAC's SGMII PCS/PMA, there is no fallback
> >> for the QSGMII PCS/PMA.
> >> 
> >> mEMACs (across all SoCs) support the following protocols:
> >> 
> >> - MII
> >> - RGMII
> >> - SGMII, 1000Base-X, and 1000Base-KX
> >> - 2500Base-X (aka 2.5G SGMII)
> >> - QSGMII
> >> - 10GBase-R (aka XFI) and 10GBase-KR
> >> - XAUI and HiGig
> >> 
> >> Each line documents a set of orthogonal protocols (e.g. XAUI is
> >> supported if and only if HiGig is supported). Additionally,
> >> 
> >> - XAUI implies support for 10GBase-R
> >> - 10GBase-R is supported if and only if RGMII is not supported
> >> - 2500Base-X implies support for 1000Base-X
> >> - MII implies support for RGMII
> >> 
> >> To switch between different protocols, we must reconfigure the SerDes.
> >> This is done by using the standard phys property. We can also use it to
> >> validate whether different protocols are supported (e.g. using
> >> phy_validate). This will work for serial protocols, but not RGMII or
> >> MII. Additionally, we still need to be compatible when there is no
> >> SerDes.
> >> 
> >> While we can detect 10G support by examining the port speed (as set by
> >> fsl,fman-10g-port), we cannot determine support for any of the other
> >> protocols based on the existing binding. In fact, the binding works
> >> against us in some respects, because pcsphy-handle is required even if
> >> there is no possible PCS/PMA for that MAC. To allow for backwards-
> >> compatibility, we use a boolean-style property for RGMII (instead of
> >> presence/absence-style). When the property for RGMII is missing, we will
> >> assume that it is supported. The exception is MII, since no existing
> >> device trees use it (as far as I could tell).
> >> 
> >> Unfortunately, QSGMII support will be broken for old device trees. There
> >> is nothing we can do about this because of the PCS/PMA situation (as
> >> described above).
> >> 
> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> >> ---
> >> 
> >> Changes in v2:
> >> - Better document how we select which PCS to use in the default case
> >> 
> >>  .../bindings/net/fsl,fman-dtsec.yaml          | 52 +++++++++++++++++--
> >>  .../devicetree/bindings/net/fsl-fman.txt      |  5 +-
> >>  2 files changed, 51 insertions(+), 6 deletions(-)
> >> 
> >> diff --git a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
> >> index 809df1589f20..ecb772258164 100644
> >> --- a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
> >> +++ b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
> >> @@ -85,9 +85,41 @@ properties:
> >>      $ref: /schemas/types.yaml#/definitions/phandle
> >>      description: A reference to the IEEE1588 timer
> >>  
> >> +  phys:
> >> +    description: A reference to the SerDes lane(s)
> >> +    maxItems: 1
> >> +
> >> +  phy-names:
> >> +    items:
> >> +      - const: serdes
> >> +
> >>    pcsphy-handle:
> >> -    $ref: /schemas/types.yaml#/definitions/phandle
> >> -    description: A reference to the PCS (typically found on the SerDes)
> >> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> >> +    minItems: 1
> >> +    maxItems: 3
> > 
> > What determines how many entries?
> 
> It depends on what the particular MAC supports. From what I can tell, the following
> combinations are valid:
> 
> - Neither SGMII, QSGMII, or XFI
> - Just SGMII
> - Just QSGMII
> - SGMII and QSGMII
> - SGMII and XFI
> - All of SGMII, QSGMII, and XFI
> 
> All of these are used on different SoCs.

So there will be a different PCS device for SGMII, QSGMII, and XFI 
rather than 1 PCS device that supports those 3 interfaces?


> >> +    description: |
> >> +      A reference to the various PCSs (typically found on the SerDes). If
> >> +      pcs-names is absent, and phy-connection-type is "xgmii", then the first
> >> +      reference will be assumed to be for "xfi". Otherwise, if pcs-names is
> >> +      absent, then the first reference will be assumed to be for "sgmii".
> >> +
> >> +  pcs-names:
> >> +    $ref: /schemas/types.yaml#/definitions/string-array
> >> +    minItems: 1
> >> +    maxItems: 3
> >> +    contains:
> >> +      enum:
> >> +        - sgmii
> >> +        - qsgmii
> >> +        - xfi
> > 
> > This means '"foo", "xfi", "bar"' is valid. I think you want to 
> > s/contains/items/.
> > 
> >> +    description: The type of each PCS in pcsphy-handle.
> >> +
> > 
> >> +  rgmii:
> >> +    enum: [0, 1]
> >> +    description: 1 indicates RGMII is supported, and 0 indicates it is not.
> >> +
> >> +  mii:
> >> +    description: If present, indicates that MII is supported.
> > 
> > Types? Need vendor prefixes.
> 
> OK.
> 
> > Are these board specific or SoC specific? Properties are appropriate for 
> > the former. The latter case should be implied by the compatible string.
> 
> Unfortunately, there are not existing specific compatible strings for each
> device in each SoC. I suppose those could be added; however, this basically
> reflects how each device is hooked up. E.g. on one SoC a device would be
> connected to the RGMII pins, but not on another SoC. The MAC itself still
> has hardware support for RGMII, but such a configuration would not function.

A difference in instances on a given SoC would also be reason for 
properties rather than different compatible strings. However, we already 
have such properties. We have 'phy-connection-type' for which mode to 
use. Do you have some need to know all possible modes? I think there was 
something posted to allow 'phy-connection-type' to be an array of 
supported modes instead.

Rob
Sean Anderson July 12, 2022, 7:56 p.m. UTC | #4
Hi Rob,

On 7/12/22 3:36 PM, Rob Herring wrote:
> On Thu, Jun 30, 2022 at 12:11:10PM -0400, Sean Anderson wrote:
>> Hi Rob,
>> 
>> On 6/30/22 12:01 PM, Rob Herring wrote:
>> > On Tue, Jun 28, 2022 at 06:13:32PM -0400, Sean Anderson wrote:
>> >> At the moment, mEMACs are configured almost completely based on the
>> >> phy-connection-type. That is, if the phy interface is RGMII, it assumed
>> >> that RGMII is supported. For some interfaces, it is assumed that the
>> >> RCW/bootloader has set up the SerDes properly. This is generally OK, but
>> >> restricts runtime reconfiguration. The actual link state is never
>> >> reported.
>> >> 
>> >> To address these shortcomings, the driver will need additional
>> >> information. First, it needs to know how to access the PCS/PMAs (in
>> >> order to configure them and get the link status). The SGMII PCS/PMA is
>> >> the only currently-described PCS/PMA. Add the XFI and QSGMII PCS/PMAs as
>> >> well. The XFI (and 1GBase-KR) PCS/PMA is a c45 "phy" which sits on the
>> >> same MDIO bus as SGMII PCS/PMA. By default they will have conflicting
>> >> addresses, but they are also not enabled at the same time by default.
>> >> Therefore, we can let the XFI PCS/PMA be the default when
>> >> phy-connection-type is xgmii. This will allow for
>> >> backwards-compatibility.
>> >> 
>> >> QSGMII, however, cannot work with the current binding. This is because
>> >> the QSGMII PCS/PMAs are only present on one MAC's MDIO bus. At the
>> >> moment this is worked around by having every MAC write to the PCS/PMA
>> >> addresses (without checking if they are present). This only works if
>> >> each MAC has the same configuration, and only if we don't need to know
>> >> the status. Because the QSGMII PCS/PMA will typically be located on a
>> >> different MDIO bus than the MAC's SGMII PCS/PMA, there is no fallback
>> >> for the QSGMII PCS/PMA.
>> >> 
>> >> mEMACs (across all SoCs) support the following protocols:
>> >> 
>> >> - MII
>> >> - RGMII
>> >> - SGMII, 1000Base-X, and 1000Base-KX
>> >> - 2500Base-X (aka 2.5G SGMII)
>> >> - QSGMII
>> >> - 10GBase-R (aka XFI) and 10GBase-KR
>> >> - XAUI and HiGig
>> >> 
>> >> Each line documents a set of orthogonal protocols (e.g. XAUI is
>> >> supported if and only if HiGig is supported). Additionally,
>> >> 
>> >> - XAUI implies support for 10GBase-R
>> >> - 10GBase-R is supported if and only if RGMII is not supported
>> >> - 2500Base-X implies support for 1000Base-X
>> >> - MII implies support for RGMII
>> >> 
>> >> To switch between different protocols, we must reconfigure the SerDes.
>> >> This is done by using the standard phys property. We can also use it to
>> >> validate whether different protocols are supported (e.g. using
>> >> phy_validate). This will work for serial protocols, but not RGMII or
>> >> MII. Additionally, we still need to be compatible when there is no
>> >> SerDes.
>> >> 
>> >> While we can detect 10G support by examining the port speed (as set by
>> >> fsl,fman-10g-port), we cannot determine support for any of the other
>> >> protocols based on the existing binding. In fact, the binding works
>> >> against us in some respects, because pcsphy-handle is required even if
>> >> there is no possible PCS/PMA for that MAC. To allow for backwards-
>> >> compatibility, we use a boolean-style property for RGMII (instead of
>> >> presence/absence-style). When the property for RGMII is missing, we will
>> >> assume that it is supported. The exception is MII, since no existing
>> >> device trees use it (as far as I could tell).
>> >> 
>> >> Unfortunately, QSGMII support will be broken for old device trees. There
>> >> is nothing we can do about this because of the PCS/PMA situation (as
>> >> described above).
>> >> 
>> >> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> >> ---
>> >> 
>> >> Changes in v2:
>> >> - Better document how we select which PCS to use in the default case
>> >> 
>> >>  .../bindings/net/fsl,fman-dtsec.yaml          | 52 +++++++++++++++++--
>> >>  .../devicetree/bindings/net/fsl-fman.txt      |  5 +-
>> >>  2 files changed, 51 insertions(+), 6 deletions(-)
>> >> 
>> >> diff --git a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
>> >> index 809df1589f20..ecb772258164 100644
>> >> --- a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
>> >> +++ b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
>> >> @@ -85,9 +85,41 @@ properties:
>> >>      $ref: /schemas/types.yaml#/definitions/phandle
>> >>      description: A reference to the IEEE1588 timer
>> >>  
>> >> +  phys:
>> >> +    description: A reference to the SerDes lane(s)
>> >> +    maxItems: 1
>> >> +
>> >> +  phy-names:
>> >> +    items:
>> >> +      - const: serdes
>> >> +
>> >>    pcsphy-handle:
>> >> -    $ref: /schemas/types.yaml#/definitions/phandle
>> >> -    description: A reference to the PCS (typically found on the SerDes)
>> >> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> >> +    minItems: 1
>> >> +    maxItems: 3
>> > 
>> > What determines how many entries?
>> 
>> It depends on what the particular MAC supports. From what I can tell, the following
>> combinations are valid:
>> 
>> - Neither SGMII, QSGMII, or XFI
>> - Just SGMII
>> - Just QSGMII
>> - SGMII and QSGMII
>> - SGMII and XFI
>> - All of SGMII, QSGMII, and XFI
>> 
>> All of these are used on different SoCs.
> 
> So there will be a different PCS device for SGMII, QSGMII, and XFI 
> rather than 1 PCS device that supports those 3 interfaces?

There were always 3 PCSs. There are two things which let the driver get
away with this. The first is that the default address of PCSs is 0, and
the boot hardware would enable only the PCS which was necessary. So you
could pretend that the same PCS would support both SGMII and XFI. In
fact, you can still do this, since the phy driver I add later in this
series is careful to duplicate this. Eventually, I would like to set the
PCS addresses so that both the SGMII and XFI phys will be accessible at
the same time (removing the need to enable/disable them). This is why I
have allowed for a separate XFI PCS.

When QSGMII is enabled, there are 4 QSGMII PCSs on the MDIO bus of one
of the MACs. The base address is, once again, 0. I believe this
overrides the SGMII PCS. When configuring for QSGMII, the driver would
always write to all QSGMII addresses. The MAC with the PCSs on its MDIO
bus would configure the PCSs for the other MACs as well. The other MACs
effectively made dummy writes (since there was nothing listening).
Unfortunately, this only works for writing registers. In order to get
the link status, we need the real QSGMII PCS.

>> >> +    description: |
>> >> +      A reference to the various PCSs (typically found on the SerDes). If
>> >> +      pcs-names is absent, and phy-connection-type is "xgmii", then the first
>> >> +      reference will be assumed to be for "xfi". Otherwise, if pcs-names is
>> >> +      absent, then the first reference will be assumed to be for "sgmii".
>> >> +
>> >> +  pcs-names:
>> >> +    $ref: /schemas/types.yaml#/definitions/string-array
>> >> +    minItems: 1
>> >> +    maxItems: 3
>> >> +    contains:
>> >> +      enum:
>> >> +        - sgmii
>> >> +        - qsgmii
>> >> +        - xfi
>> > 
>> > This means '"foo", "xfi", "bar"' is valid. I think you want to 
>> > s/contains/items/.
>> > 
>> >> +    description: The type of each PCS in pcsphy-handle.
>> >> +
>> > 
>> >> +  rgmii:
>> >> +    enum: [0, 1]
>> >> +    description: 1 indicates RGMII is supported, and 0 indicates it is not.
>> >> +
>> >> +  mii:
>> >> +    description: If present, indicates that MII is supported.
>> > 
>> > Types? Need vendor prefixes.
>> 
>> OK.
>> 
>> > Are these board specific or SoC specific? Properties are appropriate for 
>> > the former. The latter case should be implied by the compatible string.
>> 
>> Unfortunately, there are not existing specific compatible strings for each
>> device in each SoC. I suppose those could be added; however, this basically
>> reflects how each device is hooked up. E.g. on one SoC a device would be
>> connected to the RGMII pins, but not on another SoC. The MAC itself still
>> has hardware support for RGMII, but such a configuration would not function.
> 
> A difference in instances on a given SoC would also be reason for 
> properties rather than different compatible strings. However, we already 
> have such properties. We have 'phy-connection-type' for which mode to 
> use. Do you have some need to know all possible modes? I think there was 
> something posted to allow 'phy-connection-type' to be an array of 
> supported modes instead.

There's no need to know all possible modes. I can drop this if it is
preferred to use phy-connection-type without other checks.

--Sean
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
index 809df1589f20..ecb772258164 100644
--- a/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
+++ b/Documentation/devicetree/bindings/net/fsl,fman-dtsec.yaml
@@ -85,9 +85,41 @@  properties:
     $ref: /schemas/types.yaml#/definitions/phandle
     description: A reference to the IEEE1588 timer
 
+  phys:
+    description: A reference to the SerDes lane(s)
+    maxItems: 1
+
+  phy-names:
+    items:
+      - const: serdes
+
   pcsphy-handle:
-    $ref: /schemas/types.yaml#/definitions/phandle
-    description: A reference to the PCS (typically found on the SerDes)
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    minItems: 1
+    maxItems: 3
+    description: |
+      A reference to the various PCSs (typically found on the SerDes). If
+      pcs-names is absent, and phy-connection-type is "xgmii", then the first
+      reference will be assumed to be for "xfi". Otherwise, if pcs-names is
+      absent, then the first reference will be assumed to be for "sgmii".
+
+  pcs-names:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    minItems: 1
+    maxItems: 3
+    contains:
+      enum:
+        - sgmii
+        - qsgmii
+        - xfi
+    description: The type of each PCS in pcsphy-handle.
+
+  rgmii:
+    enum: [0, 1]
+    description: 1 indicates RGMII is supported, and 0 indicates it is not.
+
+  mii:
+    description: If present, indicates that MII is supported.
 
   tbi-handle:
     $ref: /schemas/types.yaml#/definitions/phandle
@@ -100,6 +132,10 @@  required:
   - fsl,fman-ports
   - ptp-timer
 
+dependencies:
+  pcs-names: [pcsphy-handle]
+  mii: [rgmii]
+
 allOf:
   - $ref: "ethernet-controller.yaml#"
   - if:
@@ -117,7 +153,11 @@  allOf:
             const: fsl,fman-memac
     then:
       required:
-        - pcsphy-handle
+        - rgmii
+    else:
+      properties:
+        rgmii: false
+        mii: false
 
 unevaluatedProperties: false
 
@@ -138,7 +178,11 @@  examples:
             reg = <0xe8000 0x1000>;
             fsl,fman-ports = <&fman0_rx_0x0c &fman0_tx_0x2c>;
             ptp-timer = <&ptp_timer0>;
-            pcsphy-handle = <&pcsphy4>;
+            pcsphy-handle = <&pcsphy4>, <&qsgmiib_pcs1>;
+            pcs-names = "sgmii", "qsgmii";
+            rgmii = <0>;
             phy-handle = <&sgmii_phy1>;
             phy-connection-type = "sgmii";
+            phys = <&serdes1 1>;
+            phy-names = "serdes";
     };
diff --git a/Documentation/devicetree/bindings/net/fsl-fman.txt b/Documentation/devicetree/bindings/net/fsl-fman.txt
index b9055335db3b..bda4b41af074 100644
--- a/Documentation/devicetree/bindings/net/fsl-fman.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fman.txt
@@ -320,8 +320,9 @@  For internal PHY device on internal mdio bus, a PHY node should be created.
 See the definition of the PHY node in booting-without-of.txt for an
 example of how to define a PHY (Internal PHY has no interrupt line).
 - For "fsl,fman-mdio" compatible internal mdio bus, the PHY is TBI PHY.
-- For "fsl,fman-memac-mdio" compatible internal mdio bus, the PHY is PCS PHY,
-  PCS PHY addr must be '0'.
+- For "fsl,fman-memac-mdio" compatible internal mdio bus, the PHY is PCS PHY.
+  The PCS PHY address should correspond to the value of the appropriate
+  MDEV_PORT.
 
 EXAMPLE