Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9c0513dd-67ce-0d6a-f2a5-58e981f0d55c@linaro.org>
Date:   Sun, 19 Jun 2022 12:33:22 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.10.0
Subject: Re: [PATCH net-next 02/28] dt-bindings: net: fman: Add additional
 interface properties
Content-Language: en-US
To:     Sean Anderson <sean.anderson@seco.com>,
        "David S . Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Madalin Bucur <madalin.bucur@nxp.com>, netdev@vger.kernel.org
Cc:     linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        Paolo Abeni <pabeni@redhat.com>,
        Russell King <linux@armlinux.org.uk>,
        Eric Dumazet <edumazet@google.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>, devicetree@vger.kernel.org
References: <20220617203312.3799646-1-sean.anderson@seco.com>
 <20220617203312.3799646-3-sean.anderson@seco.com>
 <d483da73-c5a1-2474-4992-f7ce9947d5ba@linaro.org>
 <4b305b67-7bc1-d188-23b8-6e5c7e81813b@seco.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <4b305b67-7bc1-d188-23b8-6e5c7e81813b@seco.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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@seco.com>
>>
>> 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