diff mbox series

[net-next,v5,03/16] net: ethtool: Refactor identical get_ts_info implementations.

Message ID 20231009155138.86458-4-kory.maincent@bootlin.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: Make timestamping selectable | expand

Checks

Context Check Description
netdev/series_format fail Series longer than 15 patches (and no cover letter)
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 1815 this patch: 1839
netdev/cc_maintainers warning 2 maintainers not CCed: gal@nvidia.com ehakim@nvidia.com
netdev/build_clang fail Errors and warnings before: 206 this patch: 206
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 2031 this patch: 2031
netdev/checkpatch warning WARNING: line length of 81 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns WARNING: line length of 95 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Kory Maincent Oct. 9, 2023, 3:51 p.m. UTC 
From: Richard Cochran <richardcochran@gmail.com>

The vlan, macvlan and the bonding drivers call their "real" device driver
in order to report the time stamping capabilities.  Provide a core
ethtool helper function to avoid copy/paste in the stack.

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Change in v5:
- Fixe typo
---
 drivers/net/bonding/bond_main.c | 27 ++-------------------------
 drivers/net/macvlan.c           | 14 +-------------
 include/linux/ethtool.h         |  8 ++++++++
 net/8021q/vlan_dev.c            | 15 +--------------
 net/ethtool/common.c            |  6 ++++++
 5 files changed, 18 insertions(+), 52 deletions(-)

Comments

kernel test robot Oct. 9, 2023, 7:56 p.m. UTC | #1 
Hi Köry,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/K-ry-Maincent/net-Convert-PHYs-hwtstamp-callback-to-use-kernel_hwtstamp_config/20231009-235451
base:   net-next/main
patch link:    https://lore.kernel.org/r/20231009155138.86458-4-kory.maincent%40bootlin.com
patch subject: [PATCH net-next v5 03/16] net: ethtool: Refactor identical get_ts_info implementations.
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231010/202310100344.QG4Jg301-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231010/202310100344.QG4Jg301-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310100344.QG4Jg301-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/bonding/bond_main.c: In function 'bond_ethtool_get_ts_info':
>> drivers/net/bonding/bond_main.c:5755:28: warning: unused variable 'phydev' [-Wunused-variable]
    5755 |         struct phy_device *phydev;
         |                            ^~~~~~
>> drivers/net/bonding/bond_main.c:5752:35: warning: unused variable 'ops' [-Wunused-variable]
    5752 |         const struct ethtool_ops *ops;
         |                                   ^~~


vim +/phydev +5755 drivers/net/bonding/bond_main.c

217df670d9a4da Jay Vosburgh    2005-09-26  5746  
94dd016ae538b1 Hangbin Liu     2021-11-30  5747  static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
94dd016ae538b1 Hangbin Liu     2021-11-30  5748  				    struct ethtool_ts_info *info)
94dd016ae538b1 Hangbin Liu     2021-11-30  5749  {
94dd016ae538b1 Hangbin Liu     2021-11-30  5750  	struct bonding *bond = netdev_priv(bond_dev);
980f0799a15c75 Hangbin Liu     2023-04-18  5751  	struct ethtool_ts_info ts_info;
94dd016ae538b1 Hangbin Liu     2021-11-30 @5752  	const struct ethtool_ops *ops;
94dd016ae538b1 Hangbin Liu     2021-11-30  5753  	struct net_device *real_dev;
980f0799a15c75 Hangbin Liu     2023-04-18  5754  	bool sw_tx_support = false;
94dd016ae538b1 Hangbin Liu     2021-11-30 @5755  	struct phy_device *phydev;
980f0799a15c75 Hangbin Liu     2023-04-18  5756  	struct list_head *iter;
980f0799a15c75 Hangbin Liu     2023-04-18  5757  	struct slave *slave;
9b80ccda233fa6 Hangbin Liu     2022-05-19  5758  	int ret = 0;
94dd016ae538b1 Hangbin Liu     2021-11-30  5759  
9b80ccda233fa6 Hangbin Liu     2022-05-19  5760  	rcu_read_lock();
94dd016ae538b1 Hangbin Liu     2021-11-30  5761  	real_dev = bond_option_active_slave_get_rcu(bond);
9b80ccda233fa6 Hangbin Liu     2022-05-19  5762  	dev_hold(real_dev);
9b80ccda233fa6 Hangbin Liu     2022-05-19  5763  	rcu_read_unlock();
9b80ccda233fa6 Hangbin Liu     2022-05-19  5764  
94dd016ae538b1 Hangbin Liu     2021-11-30  5765  	if (real_dev) {
59b068fe2f41f9 Richard Cochran 2023-10-09  5766  		ret = ethtool_get_ts_info_by_layer(real_dev, info);
980f0799a15c75 Hangbin Liu     2023-04-18  5767  	} else {
980f0799a15c75 Hangbin Liu     2023-04-18  5768  		/* Check if all slaves support software tx timestamping */
980f0799a15c75 Hangbin Liu     2023-04-18  5769  		rcu_read_lock();
980f0799a15c75 Hangbin Liu     2023-04-18  5770  		bond_for_each_slave_rcu(bond, slave, iter) {
59b068fe2f41f9 Richard Cochran 2023-10-09  5771  			ret = ethtool_get_ts_info_by_layer(slave->dev, &ts_info);
980f0799a15c75 Hangbin Liu     2023-04-18  5772  			if (!ret && (ts_info.so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE)) {
980f0799a15c75 Hangbin Liu     2023-04-18  5773  				sw_tx_support = true;
980f0799a15c75 Hangbin Liu     2023-04-18  5774  				continue;
980f0799a15c75 Hangbin Liu     2023-04-18  5775  			}
980f0799a15c75 Hangbin Liu     2023-04-18  5776  
980f0799a15c75 Hangbin Liu     2023-04-18  5777  			sw_tx_support = false;
980f0799a15c75 Hangbin Liu     2023-04-18  5778  			break;
980f0799a15c75 Hangbin Liu     2023-04-18  5779  		}
980f0799a15c75 Hangbin Liu     2023-04-18  5780  		rcu_read_unlock();
94dd016ae538b1 Hangbin Liu     2021-11-30  5781  	}
94dd016ae538b1 Hangbin Liu     2021-11-30  5782  
980f0799a15c75 Hangbin Liu     2023-04-18  5783  	if (sw_tx_support)
980f0799a15c75 Hangbin Liu     2023-04-18  5784  		info->so_timestamping |= SOF_TIMESTAMPING_TX_SOFTWARE;
980f0799a15c75 Hangbin Liu     2023-04-18  5785  
9b80ccda233fa6 Hangbin Liu     2022-05-19  5786  	dev_put(real_dev);
9b80ccda233fa6 Hangbin Liu     2022-05-19  5787  	return ret;
94dd016ae538b1 Hangbin Liu     2021-11-30  5788  }
94dd016ae538b1 Hangbin Liu     2021-11-30  5789
Florian Fainelli Oct. 9, 2023, 9:06 p.m. UTC | #2 
On 10/9/23 08:51, Köry Maincent wrote:
> From: Richard Cochran <richardcochran@gmail.com>
> 
> The vlan, macvlan and the bonding drivers call their "real" device driver
> in order to report the time stamping capabilities.  Provide a core
> ethtool helper function to avoid copy/paste in the stack.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

With the unused variables spotted by the kbuild test robot fixed:

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Jay Vosburgh Oct. 11, 2023, 9:41 p.m. UTC | #3 
Köry Maincent <kory.maincent@bootlin.com> wrote:

>From: Richard Cochran <richardcochran@gmail.com>
>
>The vlan, macvlan and the bonding drivers call their "real" device driver
>in order to report the time stamping capabilities.  Provide a core
>ethtool helper function to avoid copy/paste in the stack.
>
>Signed-off-by: Richard Cochran <richardcochran@gmail.com>
>Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>

	For the bonding portion:

Reviewed-by: Jay Vosburgh <jay.vosburgh@canonical.com>


>---
>
>Change in v5:
>- Fixe typo
>---
> drivers/net/bonding/bond_main.c | 27 ++-------------------------
> drivers/net/macvlan.c           | 14 +-------------
> include/linux/ethtool.h         |  8 ++++++++
> net/8021q/vlan_dev.c            | 15 +--------------
> net/ethtool/common.c            |  6 ++++++
> 5 files changed, 18 insertions(+), 52 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index ed7212e61c54..18af563d20b2 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -5763,29 +5763,12 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
> 	rcu_read_unlock();
> 
> 	if (real_dev) {
>-		ops = real_dev->ethtool_ops;
>-		phydev = real_dev->phydev;
>-
>-		if (phy_has_tsinfo(phydev)) {
>-			ret = phy_ts_info(phydev, info);
>-			goto out;
>-		} else if (ops->get_ts_info) {
>-			ret = ops->get_ts_info(real_dev, info);
>-			goto out;
>-		}
>+		ret = ethtool_get_ts_info_by_layer(real_dev, info);
> 	} else {
> 		/* Check if all slaves support software tx timestamping */
> 		rcu_read_lock();
> 		bond_for_each_slave_rcu(bond, slave, iter) {
>-			ret = -1;
>-			ops = slave->dev->ethtool_ops;
>-			phydev = slave->dev->phydev;
>-
>-			if (phy_has_tsinfo(phydev))
>-				ret = phy_ts_info(phydev, &ts_info);
>-			else if (ops->get_ts_info)
>-				ret = ops->get_ts_info(slave->dev, &ts_info);
>-
>+			ret = ethtool_get_ts_info_by_layer(slave->dev, &ts_info);
> 			if (!ret && (ts_info.so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE)) {
> 				sw_tx_support = true;
> 				continue;
>@@ -5797,15 +5780,9 @@ static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
> 		rcu_read_unlock();
> 	}
> 
>-	ret = 0;
>-	info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
>-				SOF_TIMESTAMPING_SOFTWARE;
> 	if (sw_tx_support)
> 		info->so_timestamping |= SOF_TIMESTAMPING_TX_SOFTWARE;
> 
>-	info->phc_index = -1;
>-
>-out:
> 	dev_put(real_dev);
> 	return ret;
> }
>diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
>index 02bd201bc7e5..759406fbaea8 100644
>--- a/drivers/net/macvlan.c
>+++ b/drivers/net/macvlan.c
>@@ -1086,20 +1086,8 @@ static int macvlan_ethtool_get_ts_info(struct net_device *dev,
> 				       struct ethtool_ts_info *info)
> {
> 	struct net_device *real_dev = macvlan_dev_real_dev(dev);
>-	const struct ethtool_ops *ops = real_dev->ethtool_ops;
>-	struct phy_device *phydev = real_dev->phydev;
> 
>-	if (phy_has_tsinfo(phydev)) {
>-		return phy_ts_info(phydev, info);
>-	} else if (ops->get_ts_info) {
>-		return ops->get_ts_info(real_dev, info);
>-	} else {
>-		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
>-			SOF_TIMESTAMPING_SOFTWARE;
>-		info->phc_index = -1;
>-	}
>-
>-	return 0;
>+	return ethtool_get_ts_info_by_layer(real_dev, info);
> }
> 
> static netdev_features_t macvlan_fix_features(struct net_device *dev,
>diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
>index 62b61527bcc4..1159daac776e 100644
>--- a/include/linux/ethtool.h
>+++ b/include/linux/ethtool.h
>@@ -1043,6 +1043,14 @@ static inline int ethtool_mm_frag_size_min_to_add(u32 val_min, u32 *val_add,
> 	return -EINVAL;
> }
> 
>+/**
>+ * ethtool_get_ts_info_by_layer - Obtains time stamping capabilities from the MAC or PHY layer.
>+ * @dev: pointer to net_device structure
>+ * @info: buffer to hold the result
>+ * Returns zero on success, non-zero otherwise.
>+ */
>+int ethtool_get_ts_info_by_layer(struct net_device *dev, struct ethtool_ts_info *info);
>+
> /**
>  * ethtool_sprintf - Write formatted string to ethtool string data
>  * @data: Pointer to start of string to update
>diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
>index 2a7f1b15714a..407b2335f091 100644
>--- a/net/8021q/vlan_dev.c
>+++ b/net/8021q/vlan_dev.c
>@@ -702,20 +702,7 @@ static int vlan_ethtool_get_ts_info(struct net_device *dev,
> 				    struct ethtool_ts_info *info)
> {
> 	const struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
>-	const struct ethtool_ops *ops = vlan->real_dev->ethtool_ops;
>-	struct phy_device *phydev = vlan->real_dev->phydev;
>-
>-	if (phy_has_tsinfo(phydev)) {
>-		return phy_ts_info(phydev, info);
>-	} else if (ops->get_ts_info) {
>-		return ops->get_ts_info(vlan->real_dev, info);
>-	} else {
>-		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
>-			SOF_TIMESTAMPING_SOFTWARE;
>-		info->phc_index = -1;
>-	}
>-
>-	return 0;
>+	return ethtool_get_ts_info_by_layer(vlan->real_dev, info);
> }
> 
> static void vlan_dev_get_stats64(struct net_device *dev,
>diff --git a/net/ethtool/common.c b/net/ethtool/common.c
>index f5598c5f50de..e2315e24d695 100644
>--- a/net/ethtool/common.c
>+++ b/net/ethtool/common.c
>@@ -661,6 +661,12 @@ int ethtool_get_phc_vclocks(struct net_device *dev, int **vclock_index)
> }
> EXPORT_SYMBOL(ethtool_get_phc_vclocks);
> 
>+int ethtool_get_ts_info_by_layer(struct net_device *dev, struct ethtool_ts_info *info)
>+{
>+	return __ethtool_get_ts_info(dev, info);
>+}
>+EXPORT_SYMBOL(ethtool_get_ts_info_by_layer);
>+
> const struct ethtool_phy_ops *ethtool_phy_ops;
> 
> void ethtool_set_ethtool_phy_ops(const struct ethtool_phy_ops *ops)
>-- 
>2.25.1
>
>
diff mbox series

Patch

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index ed7212e61c54..18af563d20b2 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -5763,29 +5763,12 @@  static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
 	rcu_read_unlock();
 
 	if (real_dev) {
-		ops = real_dev->ethtool_ops;
-		phydev = real_dev->phydev;
-
-		if (phy_has_tsinfo(phydev)) {
-			ret = phy_ts_info(phydev, info);
-			goto out;
-		} else if (ops->get_ts_info) {
-			ret = ops->get_ts_info(real_dev, info);
-			goto out;
-		}
+		ret = ethtool_get_ts_info_by_layer(real_dev, info);
 	} else {
 		/* Check if all slaves support software tx timestamping */
 		rcu_read_lock();
 		bond_for_each_slave_rcu(bond, slave, iter) {
-			ret = -1;
-			ops = slave->dev->ethtool_ops;
-			phydev = slave->dev->phydev;
-
-			if (phy_has_tsinfo(phydev))
-				ret = phy_ts_info(phydev, &ts_info);
-			else if (ops->get_ts_info)
-				ret = ops->get_ts_info(slave->dev, &ts_info);
-
+			ret = ethtool_get_ts_info_by_layer(slave->dev, &ts_info);
 			if (!ret && (ts_info.so_timestamping & SOF_TIMESTAMPING_TX_SOFTWARE)) {
 				sw_tx_support = true;
 				continue;
@@ -5797,15 +5780,9 @@  static int bond_ethtool_get_ts_info(struct net_device *bond_dev,
 		rcu_read_unlock();
 	}
 
-	ret = 0;
-	info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
-				SOF_TIMESTAMPING_SOFTWARE;
 	if (sw_tx_support)
 		info->so_timestamping |= SOF_TIMESTAMPING_TX_SOFTWARE;
 
-	info->phc_index = -1;
-
-out:
 	dev_put(real_dev);
 	return ret;
 }
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 02bd201bc7e5..759406fbaea8 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1086,20 +1086,8 @@  static int macvlan_ethtool_get_ts_info(struct net_device *dev,
 				       struct ethtool_ts_info *info)
 {
 	struct net_device *real_dev = macvlan_dev_real_dev(dev);
-	const struct ethtool_ops *ops = real_dev->ethtool_ops;
-	struct phy_device *phydev = real_dev->phydev;
 
-	if (phy_has_tsinfo(phydev)) {
-		return phy_ts_info(phydev, info);
-	} else if (ops->get_ts_info) {
-		return ops->get_ts_info(real_dev, info);
-	} else {
-		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
-			SOF_TIMESTAMPING_SOFTWARE;
-		info->phc_index = -1;
-	}
-
-	return 0;
+	return ethtool_get_ts_info_by_layer(real_dev, info);
 }
 
 static netdev_features_t macvlan_fix_features(struct net_device *dev,
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 62b61527bcc4..1159daac776e 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -1043,6 +1043,14 @@  static inline int ethtool_mm_frag_size_min_to_add(u32 val_min, u32 *val_add,
 	return -EINVAL;
 }
 
+/**
+ * ethtool_get_ts_info_by_layer - Obtains time stamping capabilities from the MAC or PHY layer.
+ * @dev: pointer to net_device structure
+ * @info: buffer to hold the result
+ * Returns zero on success, non-zero otherwise.
+ */
+int ethtool_get_ts_info_by_layer(struct net_device *dev, struct ethtool_ts_info *info);
+
 /**
  * ethtool_sprintf - Write formatted string to ethtool string data
  * @data: Pointer to start of string to update
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 2a7f1b15714a..407b2335f091 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -702,20 +702,7 @@  static int vlan_ethtool_get_ts_info(struct net_device *dev,
 				    struct ethtool_ts_info *info)
 {
 	const struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
-	const struct ethtool_ops *ops = vlan->real_dev->ethtool_ops;
-	struct phy_device *phydev = vlan->real_dev->phydev;
-
-	if (phy_has_tsinfo(phydev)) {
-		return phy_ts_info(phydev, info);
-	} else if (ops->get_ts_info) {
-		return ops->get_ts_info(vlan->real_dev, info);
-	} else {
-		info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
-			SOF_TIMESTAMPING_SOFTWARE;
-		info->phc_index = -1;
-	}
-
-	return 0;
+	return ethtool_get_ts_info_by_layer(vlan->real_dev, info);
 }
 
 static void vlan_dev_get_stats64(struct net_device *dev,
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index f5598c5f50de..e2315e24d695 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -661,6 +661,12 @@  int ethtool_get_phc_vclocks(struct net_device *dev, int **vclock_index)
 }
 EXPORT_SYMBOL(ethtool_get_phc_vclocks);
 
+int ethtool_get_ts_info_by_layer(struct net_device *dev, struct ethtool_ts_info *info)
+{
+	return __ethtool_get_ts_info(dev, info);
+}
+EXPORT_SYMBOL(ethtool_get_ts_info_by_layer);
+
 const struct ethtool_phy_ops *ethtool_phy_ops;
 
 void ethtool_set_ethtool_phy_ops(const struct ethtool_phy_ops *ops)