Re: [PATCH net-next 02/28] dt-bindings: net: fman: Add additional interface properties

[Krzysztof Kozlowski]

On 18/06/2022 17:55, Sean Anderson wrote:
> Hi Krzysztof,
> 
> On 6/17/22 9:16 PM, Krzysztof Kozlowski wrote:
>> On 17/06/2022 13:32, 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. 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 default address for the XFI PCS/PMA be the
>>> same as for SGMII. 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@xxxxxxxx>
>>
>> Thanks for the patch but you add too many new properties. The file
>> should be converted to YAML/DT schema first.
> 
> Perhaps. However, conversion to yaml is a non-trivial task, especially for
> a complicated binding such as this one. I am more than happy to rework this
> patch to be based on a yaml conversion, but I do not have the bandwidth to
> do so myself.

I understand. Although since 2020  - since when we expect the bindings
to be in YAML - this file grew by 6 properties, because each person
extends it instead of converting. Each person uses the same excuse...

You add here 5 more, so it would be 11 new properties in total.

> 
> If you have any comments on the binding changes themselves, that would be
> much appreciated.

Maybe Rob will ack it, but for me the change is too big to be accepted
in TXT, so no from me.

Best regards,
Krzysztof