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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200715185619.GJ1551@shell.armlinux.org.uk>
Date:   Wed, 15 Jul 2020 19:56:19 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Richard Cochran <richardcochran@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH RFC net-next] net: phy: add Marvell PHY PTP support

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.

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