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@xxxxxxxxxxx>
> > ---
> >
> > 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?
