Re: [PATCH RFC net-next] net: phy: add Marvell PHY PTP support

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Wed, Jul 15, 2020 at 07:56:19PM +0100, Russell King - ARM Linux admin wrote:
> On Wed, Jul 15, 2020 at 08:38:43PM +0200, Andrew Lunn wrote:
> > > Getting the Kconfig for this correct has been a struggle - particularly
> > > the combination where PTP support is modular.  It's rather odd to have
> > > the Marvell PTP support asked before the Marvell PHY support.  I
> > > couldn't work out any other reasonable way to ensure that we always
> > > have a valid configuration, without leading to stupidities such as
> > > having the PTP and Marvell PTP support modular, but non-functional
> > > because Marvell PHY is built-in.
> > 
> > Hi Russell
> > 
> > How much object code is this adding? All the other PHYs which support
> > PTP just make it part of the PHY driver, not a standalone module. That
> > i guess simplifies the conditions. 
> 
> Taking an arm64 build, the PHY driver is 16k and the PTP driver comes
> in just under 8k.
> 
> > Looking at DSDT, it lists
> > 
> >         case MAD_88E1340S:
> >         case MAD_88E1340:
> >         case MAD_88E1340M:
> >         case MAD_SWG65G : 
> > 	case MAD_88E151x:
> > 
> > as being MAD_PHY_PTP_TAI_CAPABLE;
> > 
> > and
> > 
> > 	case MAD_88E1548
> >         case MAD_88E1680:
> >         case MAD_88E1680M:
> > 
> > as MAD_PHY_1STEP_PTP_CAPABLE;
> > 
> > So maybe we can wire this up to a few more PHYs to 'lower' the
> > overhead a bit?
> 
> That's interesting, the 1548 information (covering 1543 and 1545 as
> well) I have doesn't mention anything about PTP.
> 
> > > It seems that the Marvell PHY PTP is very similar to that found in
> > > their DSA chips, which suggests maybe we should share the code, but
> > > different access methods would be required.
> > 
> > That makes the Kconfig even more complex :-(
> 
> We already have that complexity due to the fact that we are interacting
> with two subsystems.  The 88e6xxx Kconfig entry has:
> 
> config NET_DSA_MV88E6XXX_PTP
>         bool "PTP support for Marvell 88E6xxx"
>         default n
>         depends on NET_DSA_MV88E6XXX_GLOBAL2
>         depends on PTP_1588_CLOCK
>         imply NETWORK_PHY_TIMESTAMPING
> 
> and I've been wondering what that means when PTP_1588_CLOCK=m while
> NET_DSA_MV88E6XXX_GLOBAL2=y and NET_DSA_MV88E6XXX=y.  If this is
> selectable, then it seems to be misleading the user - you can't have
> the PTP subsystem modular, and have PTP drivers built-in to the
> kernel.
> 
> Yes, we have the inteligence to be able to make the various PTP calls
> be basically no-ops, but it's not nice.

Sure enough:

CONFIG_NET_DSA_MV88E6XXX=y
CONFIG_NET_DSA_MV88E6XXX_GLOBAL2=y
CONFIG_NET_DSA_MV88E6XXX_PTP=y
CONFIG_PTP_1588_CLOCK=m

is a possible configuration, but all the PTP calls from mv88e6xxx are
stubbed out (since the IS_REACHABLE() in linux/ptp_clock_kernel.h is
false.)

The DP83640 PHY works around this by introducing a hard dependency on
PTP:

config DP83640_PHY
	tristate "Driver for the National Semiconductor DP83640 PHYTER"
	depends on NETWORK_PHY_TIMESTAMPING
	depends on PHYLIB
	depends on PTP_1588_CLOCK

A possible solution to this would be for the PTP core code to be a
separate Kconfig symbol that is not offered to the user, but is
selected by the various drivers and Kconfig entries that need that
support.  That way, it would automatically be modular or built-in
as required by its users.

PTP_1588_CLOCK could then be used as a global enable for PTP support
where appropriate.  We can also avoid all the Kconfig complexity
introduced by having two independent subsystems either of which could
be modular.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!