netdev - Re: [PATCH net-next v10 08/13] ptp: Add phc source and helpers to register specific PTP clock or get information

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: <6617567bd4da7_2dcc3c294c5@willemb.c.googlers.com.notmuch>
Date: Wed, 10 Apr 2024 23:18:19 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 Kory Maincent <kory.maincent@...tlin.com>, 
 Florian Fainelli <florian.fainelli@...adcom.com>, 
 Broadcom internal kernel review list <bcm-kernel-feedback-list@...adcom.com>, 
 Andrew Lunn <andrew@...n.ch>, 
 Heiner Kallweit <hkallweit1@...il.com>, 
 Russell King <linux@...linux.org.uk>, 
 "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, 
 Jakub Kicinski <kuba@...nel.org>, 
 Paolo Abeni <pabeni@...hat.com>, 
 Richard Cochran <richardcochran@...il.com>, 
 Radu Pirea <radu-nicolae.pirea@....nxp.com>, 
 Jay Vosburgh <j.vosburgh@...il.com>, 
 Andy Gospodarek <andy@...yhouse.net>, 
 Nicolas Ferre <nicolas.ferre@...rochip.com>, 
 Claudiu Beznea <claudiu.beznea@...on.dev>, 
 Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
 Jonathan Corbet <corbet@....net>, 
 Horatiu Vultur <horatiu.vultur@...rochip.com>, 
 UNGLinuxDriver@...rochip.com, 
 Simon Horman <horms@...nel.org>, 
 Vladimir Oltean <vladimir.oltean@....com>
Cc: Thomas Petazzoni <thomas.petazzoni@...tlin.com>, 
 netdev@...r.kernel.org, 
 linux-kernel@...r.kernel.org, 
 linux-doc@...r.kernel.org, 
 Maxime Chevallier <maxime.chevallier@...tlin.com>, 
 Rahul Rameshbabu <rrameshbabu@...dia.com>, 
 Kory Maincent <kory.maincent@...tlin.com>
Subject: Re: [PATCH net-next v10 08/13] ptp: Add phc source and helpers to
 register specific PTP clock or get information

Willem de Bruijn wrote:
> Kory Maincent wrote:
> > Prepare for future hardware timestamp selection by adding source and
> > corresponding pointers to ptp_clock structure. Additionally, introduce
> > helpers for registering specific phydev or netdev PTP clocks, retrieving
> > PTP clock information such as hwtstamp source or phydev/netdev pointers,
> > and obtaining the ptp_clock structure from the phc index.
> > 
> > Signed-off-by: Kory Maincent <kory.maincent@...tlin.com>
> > ---
> > 
> > Change in v8:
> > - New patch.
> > 
> > Change in v10:
> > - Add get and put function to avoid unregistering a ptp clock object used.
> > - Fix kdoc issues.
> > ---
> >  drivers/ptp/ptp_clock.c          | 114 +++++++++++++++++++++++++++++++++++++++
> >  drivers/ptp/ptp_private.h        |   5 ++
> >  include/linux/ptp_clock_kernel.h | 104 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 223 insertions(+)
> > 
> > diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> > index c56cd0f63909..f962f460ec9d 100644
> > --- a/drivers/ptp/ptp_clock.c
> > +++ b/drivers/ptp/ptp_clock.c
> > @@ -512,6 +512,120 @@ void ptp_cancel_worker_sync(struct ptp_clock *ptp)
> >  }
> >  EXPORT_SYMBOL(ptp_cancel_worker_sync);
> >  
> > +struct ptp_clock *netdev_ptp_clock_register(struct ptp_clock_info *info,
> > +					    struct net_device *dev)
> > +{
> > +	struct ptp_clock *ptp;
> > +
> > +	ptp = ptp_clock_register(info, &dev->dev);
> > +	if (IS_ERR(ptp))
> > +		return ptp;
> > +
> > +	ptp->phc_source = HWTSTAMP_SOURCE_NETDEV;
> > +	ptp->netdev = dev;
> > +
> > +	return ptp;
> > +}
> > +EXPORT_SYMBOL(netdev_ptp_clock_register);
> > +
> > +struct ptp_clock *phydev_ptp_clock_register(struct ptp_clock_info *info,
> > +					    struct phy_device *phydev)
> > +{
> > +	struct ptp_clock *ptp;
> > +
> > +	ptp = ptp_clock_register(info, &phydev->mdio.dev);
> > +	if (IS_ERR(ptp))
> > +		return ptp;
> > +
> > +	ptp->phc_source = HWTSTAMP_SOURCE_PHYLIB;
> > +	ptp->phydev = phydev;
> > +
> > +	return ptp;
> > +}
> > +EXPORT_SYMBOL(phydev_ptp_clock_register);
> > +
> > +bool ptp_clock_from_phylib(struct ptp_clock *ptp)
> > +{
> > +	return ptp->phc_source == HWTSTAMP_SOURCE_PHYLIB;
> > +}
> > +EXPORT_SYMBOL(ptp_clock_from_phylib);
> > +
> > +bool ptp_clock_from_netdev(struct ptp_clock *ptp)
> > +{
> > +	return ptp->phc_source == HWTSTAMP_SOURCE_NETDEV;
> > +}
> > +EXPORT_SYMBOL(ptp_clock_from_netdev);
> > +
> > +struct net_device *ptp_clock_netdev(struct ptp_clock *ptp)
> > +{
> > +	if (ptp->phc_source != HWTSTAMP_SOURCE_NETDEV)
> > +		return NULL;
> > +
> > +	return ptp->netdev;
> > +}
> > +EXPORT_SYMBOL(ptp_clock_netdev);
> > +
> > +struct phy_device *ptp_clock_phydev(struct ptp_clock *ptp)
> > +{
> > +	if (ptp->phc_source != HWTSTAMP_SOURCE_PHYLIB)
> > +		return NULL;
> > +
> > +	return ptp->phydev;
> > +}
> > +EXPORT_SYMBOL(ptp_clock_phydev);
> 
> IMHO these four helpers just add a layer of indirection without much
> benefit.

Actually, ignore this. This is not important enough to revise when
we're alredy at v10. That also goes for the other points.

> Do we ever expect more than two sources? Else, the phc_source could be
> embedded as the least significant bit of the pointer in the union. In
> that case we would need helpers to return the pointer without that LSB.
> But space in struct ptp_clock is probably not so valuable that we need
> to play such games.
> 
> > +/**
> > + * netdev_ptp_clock_register() - register a PTP hardware clock driver for
> > + *				 a net device
> > + *
> > + * @info: Structure describing the new clock.
> > + * @dev:  Pointer of the net device
> > + */
> > +
> > +extern struct ptp_clock *
> > +netdev_ptp_clock_register(struct ptp_clock_info *info,
> > +			  struct net_device *dev);
> 
> No need for explicit extern?