On 8/18/22 1:12 PM, Vladimir Oltean wrote:

On Thu, Aug 18, 2022 at 01:03:54PM -0400, Sean Anderson wrote:

quoted

Well, I suppose the real reason is that this will cause a merge conflict
(or lack of one), since this series introduces phylink_interface_max_speed
in patch 7, which is supposed to contain all the phy modes. So depending on
what gets merged first, the other series will have to be modified and resent.

To be honest, I had expected that trivial patches like that would have been
applied and merged already.

There's nothing trivial about this patch.

Perhaps "limited in scope and mostly independent" is better, then.

1000Base-KX is not a phy-mode
in exactly the same way that 1000Base-T isn't, either. 

It has different AN from 1000BASE-X (c73 vs c37), and doesn't support half
duplex. This is something the serdes and PCS have to care about. Unfortunately,
we don't have a separate PCS_INTERFACE_MODE so these things become
PHY_INTERFACE_MODEs.

If you want to
bring PHY_INTERFACE_MODE_10GKR as a "yes, but" counterexample, it was
later clarified that 10gbase-r was what was actually meant in that case,
and we keep 10gbase-kr as phy-mode only for compatibility with some
device trees.

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'd suggest resolving the merge conflict without 1000Base-KX and
splitting off a separate discussion about this topic. Otherwise it will
unnecessarily detract from PAUSE-based rate adaptation.

Well, no one is using it yet, so hopefully it will not be a problem...

--Sean