Re: [PATCH V2 net-next 2/5] net: Introduce a new MII time stamping interface.

On Sun, Oct 07, 2018 at 12:15:51PM -0700, Richard Cochran wrote:

On Sun, Oct 07, 2018 at 08:27:51PM +0200, Andrew Lunn wrote:

quoted

The mii_timestamper is generic, in the same why hwmon is generic. It
does not matter where the time stamper is. So i'm wondering if we
should remove the special case for a PHY timestamper, remove all the
phylib support, etc.

This implementation is (to the best of my understanding) what you were
asking for in your review of v1:

Sure, but things have moved on since then.
 

quoted

So i really think you need to cleanly integrate into phylib and
phylink.

quoted

Use a phandle, and have
of_mdiobus_register_phy() follow the phandle to get the device.

quoted

To keep lifecycle issues simple, i would also keep it in phydev, not
netdev.

This present series is a reasonable, incremental improvement to the
existing PHY time stamping support.  It will handle any use case that
I can think of, and I would like to avoid over-engineering this.

I can think of three obvious use cases where this does not work:

1) phylink, not phdev. We have been pushing some MAC drivers towards
phylink, especially those which support >1Gbp.

2) When an SFP is connected to the MAC, not a copper PHY. The class of
device you are adding a driver for will work just as well for an SFP
as for a copper PHY. The SERDES interface remains the same,
independent of if a copper PHY is used, or a SFP. But an SFP does not
have an instance of a phydv.

2a) An SFP which is actually a Copper PHY. There is a phydev for this,
but it is associated to the phylink, not the netdev.

3) Firmware controlled PHYs. phylib/phylink is not used, the MAC turns
all ethtool calls into RPCs to the firmware. I've no numbers about
this, but i have the feeling this is becoming more popular. It does
however tend to be high end devices, and those are more likely to have
timestamping in the MAC. I suppose they could also offload
tomestamping to the firmware, in which case, they might want to make
use of this new API.

    Andrew