diff mbox series

[net-next,v9,02/13] net: Make dev_get_hwtstamp_phylib accessible

Message ID 20240226-feature_ptp_netnext-v9-2-455611549f21@bootlin.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: Make timestamping selectable | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next, async
netdev/apply fail Patch does not apply to net-next

Commit Message

Kory Maincent Feb. 26, 2024, 1:39 p.m. UTC 
Make the dev_get_hwtstamp_phylib function accessible in prevision to use
it from ethtool to read the hwtstamp current configuration.

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Change in v8:
- New patch
---
 include/linux/netdevice.h | 2 ++
 net/core/dev_ioctl.c      | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

Jakub Kicinski March 5, 2024, 2:40 a.m. UTC | #1 
On Mon, 26 Feb 2024 14:39:53 +0100 Kory Maincent wrote:
> Make the dev_get_hwtstamp_phylib function accessible in prevision to use
> it from ethtool to read the hwtstamp current configuration.

ethtool can't be a module, exports are only needed for code which ends
up being called from modules. 

> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index f07c8374f29c..7f78aef73fe1 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -4005,6 +4005,8 @@ int generic_hwtstamp_set_lower(struct net_device *dev,
>  int dev_set_hwtstamp_phylib(struct net_device *dev,
>  			    struct kernel_hwtstamp_config *cfg,
>  			    struct netlink_ext_ack *extack);
> +int dev_get_hwtstamp_phylib(struct net_device *dev,
> +			    struct kernel_hwtstamp_config *cfg);

since we don't expect modules to call this, how about we move dev_set*
and the new declaration to net/core/dev.h ?
Kory Maincent March 5, 2024, 9:56 a.m. UTC | #2 
On Mon, 4 Mar 2024 18:40:48 -0800
Jakub Kicinski <kuba@kernel.org> wrote:

> On Mon, 26 Feb 2024 14:39:53 +0100 Kory Maincent wrote:
> > Make the dev_get_hwtstamp_phylib function accessible in prevision to use
> > it from ethtool to read the hwtstamp current configuration.  
> 
> ethtool can't be a module, exports are only needed for code which ends
> up being called from modules. 

Ah right!

> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index f07c8374f29c..7f78aef73fe1 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -4005,6 +4005,8 @@ int generic_hwtstamp_set_lower(struct net_device *dev,
> >  int dev_set_hwtstamp_phylib(struct net_device *dev,
> >  			    struct kernel_hwtstamp_config *cfg,
> >  			    struct netlink_ext_ack *extack);
> > +int dev_get_hwtstamp_phylib(struct net_device *dev,
> > +			    struct kernel_hwtstamp_config *cfg);  
> 
> since we don't expect modules to call this, how about we move dev_set*
> and the new declaration to net/core/dev.h ?

Ok for me.

Regards,
Kory Maincent March 5, 2024, 10:02 a.m. UTC | #3 
On Tue, 5 Mar 2024 10:56:27 +0100
Köry Maincent <kory.maincent@bootlin.com> wrote:

> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index f07c8374f29c..7f78aef73fe1 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -4005,6 +4005,8 @@ int generic_hwtstamp_set_lower(struct net_device
> > > *dev, int dev_set_hwtstamp_phylib(struct net_device *dev,
> > >  			    struct kernel_hwtstamp_config *cfg,
> > >  			    struct netlink_ext_ack *extack);
> > > +int dev_get_hwtstamp_phylib(struct net_device *dev,
> > > +			    struct kernel_hwtstamp_config *cfg);    
> > 
> > since we don't expect modules to call this, how about we move dev_set*
> > and the new declaration to net/core/dev.h ?  
> 
> Ok for me.

I replied to quickly.
It seems this header in not include in ethtool part. 
This would imply adding #include "../core/dev.h" in the tsinfo.c file.
Not sure this is what we want.

Regards,
Jakub Kicinski March 5, 2024, 3:05 p.m. UTC | #4 
On Tue, 5 Mar 2024 11:02:33 +0100 Köry Maincent wrote:
> > > since we don't expect modules to call this, how about we move dev_set*
> > > and the new declaration to net/core/dev.h ?    
> > 
> > Ok for me.  
> 
> I replied to quickly.
> It seems this header in not include in ethtool part. 
> This would imply adding #include "../core/dev.h" in the tsinfo.c file.
> Not sure this is what we want.

It's not amazing but I think that's better than putting internal stuff
in netdevice.h. ethtool is separated from core for code organization,
it's not really a separate entity, it controls very core things.
diff mbox series

Patch

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f07c8374f29c..7f78aef73fe1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4005,6 +4005,8 @@  int generic_hwtstamp_set_lower(struct net_device *dev,
 int dev_set_hwtstamp_phylib(struct net_device *dev,
 			    struct kernel_hwtstamp_config *cfg,
 			    struct netlink_ext_ack *extack);
+int dev_get_hwtstamp_phylib(struct net_device *dev,
+			    struct kernel_hwtstamp_config *cfg);
 int dev_ethtool(struct net *net, struct ifreq *ifr, void __user *userdata);
 unsigned int dev_get_flags(const struct net_device *);
 int __dev_change_flags(struct net_device *dev, unsigned int flags,
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 9a66cf5015f2..5d3b169d8f18 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -268,14 +268,15 @@  static int dev_eth_ioctl(struct net_device *dev,
  * -EOPNOTSUPP for phylib for now, which is still more accurate than letting
  * the netdev handle the GET request.
  */
-static int dev_get_hwtstamp_phylib(struct net_device *dev,
-				   struct kernel_hwtstamp_config *cfg)
+int dev_get_hwtstamp_phylib(struct net_device *dev,
+			    struct kernel_hwtstamp_config *cfg)
 {
 	if (phy_has_hwtstamp(dev->phydev))
 		return phy_hwtstamp_get(dev->phydev, cfg);
 
 	return dev->netdev_ops->ndo_hwtstamp_get(dev, cfg);
 }
+EXPORT_SYMBOL_GPL(dev_get_hwtstamp_phylib);
 
 static int dev_get_hwtstamp(struct net_device *dev, struct ifreq *ifr)
 {