Re: [PATCH v3 02/11] net: phy: Add 1000BASE-KX interface mode

[Sean Anderson <sean.anderson@seco.com>]

Hi Vladimir,

On 8/18/22 4:40 PM, Sean Anderson wrote:
> On 8/18/22 3:51 PM, Vladimir Oltean wrote:
>> On Thu, Aug 18, 2022 at 01:28:19PM -0400, Sean Anderson wrote:
>>> That's not what's documented:
>>> 
>>> > ``PHY_INTERFACE_MODE_10GBASER``
>>> >     This is the IEEE 802.3 Clause 49 defined 10GBASE-R protocol used with
>>> >     various different mediums. Please refer to the IEEE standard for a
>>> >     definition of this.
>>> > 
>>> >     Note: 10GBASE-R is just one protocol that can be used with XFI and SFI.
>>> >     XFI and SFI permit multiple protocols over a single SERDES lane, and
>>> >     also defines the electrical characteristics of the signals with a host
>>> >     compliance board plugged into the host XFP/SFP connector. Therefore,
>>> >     XFI and SFI are not PHY interface types in their own right.
>>> > 
>>> > ``PHY_INTERFACE_MODE_10GKR``
>>> >     This is the IEEE 802.3 Clause 49 defined 10GBASE-R with Clause 73
>>> >     autonegotiation. Please refer to the IEEE standard for further
>>> >     information.
>>> > 
>>> >     Note: due to legacy usage, some 10GBASE-R usage incorrectly makes
>>> >     use of this definition.
>>> 
>>> so indeed you get a new phy interface mode when you add c73 AN. The
>>> clarification only applies to *incorrect* usage.
>> 
>> I challenge you to the following thought experiment. Open clause 73 from
>> IEEE 802.3, and see what is actually exchanged through auto-negotiation.
>> You'll discover that the *use* of the 10GBase-KR operating mode is
>> *established* through clause 73 AN (the Technology Ability field).
>> 
>> So what sense does it make to define 10GBase-KR as "10Base-R with clause 73 AN"
>> as the document you've quoted does?None whatsoever. The K in KR stands
>> for bacKplane, and typical of this type of PMD are the signaling and
>> link training procedures described in the previous clause, 72.
>> Clause 73 AN is not something that is a property of 10GBase-KR, but
>> something that exists outside of it.
> 
> You should send a patch; this document is Documentation/networking/phy.rst
> 
>> So if clause 73 *establishes* the use of 10GBase-KR (or 1000Base-KX or
>> others) through autonegotiation, then what sense does it have to put
>> phy-mode = "1000base-kx" in the device tree? Does it mean "use C73 AN",
>> or "don't use it, I already know what operating mode I want to use"?
>> 
>> If it means "use C73 AN", then what advertisement do you use for the
>> Technology Ability field? There's a priority resolution function for
>> C73, just like there is one for C28/C40 for the twisted pair medium (aka
>> that thing that allows you to fall back to the highest supported common
>> link speed). So why would you populate just one bit in Technology
>> Ability based on DT, if you can potentially support multiple operating
>> modes? And why would you even create your advertisement based on the
>> device tree, for that matter? Twisted pair PHYs don't do this.
> 
> The problem is that our current model looks something like
> 
> 1. MAC <--               A              --> phy (ethernet) --> B <-- far end
> 2. MAC <-> "PCS" <-> phy (serdes) --> C <-- phy (ethernet) --> B <-- far end
> 3.                                --> C <-- transciever    --> B <-- far end
> 4.                                -->           D                <-- far end
> 
> Where 1 is the traditional MAC+phy architecture, 2 is a MAC connected to
> a phy over a serial link, 3 is a MAC connected to an optical
> transcievber, and 4 is a backplane connection. A is the phy interface
> mode, and B is the ethtool link mode. C is also the "phy interface
> mode", except that sometimes it is highly-dependent on the link mode
> (e.g. 1000BASE-X) and sometimes it is not (SGMII). The problem is case
> 4. Here, there isn't really a phy interface mode; just a link mode.
>
> Consider the serdes driver. It has to know how to configure itself.
> Sometimes this will be the phy mode (cases 2 and 3), and sometimes it
> will be the link mode (case 4). In particular, for a link mode like
> 1000BASE-SX, it should be configured for 1000BASE-X. But for
> 1000BASE-KX, it has to be configured for 1000BASE-KX. I suppose the
> right thing to do here is rework the phy subsystem to use link modes and
> not phy modes for phy_mode_ext, since it seems like there is a
> 1000BASE-X link mode. But what should be passed to mac_prepare and
> mac_select_pcs?
> 
> As another example, consider the AQR113C. It supports the following
> (abbreviated) interfaces on the MAC side:
> 
> - 10GBASE-KR
> - 1000BASE-KX
> - 10GBASE-R
> - 1000BASE-X
> - USXGMII
> - SGMII
> 
> This example of what phy-mode = "1000base-kx" would imply. I would
> expect that selecting -KX over -X would change the electrical settings
> to comply with clause 70 (instead of the SFP spec).

Do you have any comments on the above?

--Sean