Subject: Re: [PATCH 3/3] net: macb: add support for high speed interface

From: Russell King - ARM Linux admin <[email-masked]>
Date: Wed, 11 Dec 2019 09:37:08 +0000

[private email content deleted, added Cc list back since this is
important.]

I'm still not getting a good enough view of what you are doing and
how my understanding of your hardware fits with what you're doing
with the software.

My understanding is it's something like:

	----+
        SOC |             PCS
	MAC --(USXGMII)-- PHY ----- PHY or SFP
	    |
	----+

And you are just modelling the MAC part in phylink, where as phylink
has so far been used on systems which have this model - where phylink
knows about both the MAC and the PCS PHY:

	---------------+
	         PCS   |
	MAC ---- PHY ----- PHY or SFP
	     SOC       |
      	---------------+

This is why I recently renamed mac_link_state() to mac_pcs_get_state()
to make it clearer that it reads from the PCS not from the current
settings of the MAC.  So far, all such setups do not implement the PCS
PHY as an 802.3 register set; they implement it as part of the MAC
register set.

In the former case, if phylink is used to manage the connection between
the MAC and the PCS PHY, phylink has nothing to do with the SFP at all.

In the latter case, phylink is used to manage the connection between the
PCS PHY and external device, controlling the MAC as appropriate.

My problem is I believe your hardware is the former case, but you are
trying to implement the latter case by ignoring in-band mode.  As SFPs
rely on in-band mode, that isn't going to work.

The options for the former case are:

1) implement phylink covering both the MAC and the external PCS PHY
2) implement phylink just for the MAC to PCS PHY connection but not
   SFPs, and implement SFP support separately in the PCS PHY driver.

Maybe phylink needs to split mac_pcs_get_state() so it can be supplied
by a separate driver, or by the MAC driver as appropriate - but that
brings with it other problems; phylink with a directly attached SFP
considers the state of the link between the PCS PHY and the external
device - not only speed but also interface mode for that part of the
link.  What you'd see in the mac_config() callback are interface modes
for that part of the link, not between the MAC and the PCS PHY.

To change that would require reworking almost every driver that has
already converted over to somehow remodel the built-in PCS and
COMPHY as a separate PCS PHY for phylink. I'm not entirely clear
whether that would work though.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up