[PATCH 1/2] net: mii: make mii_ethtool_gset() return void

[PATCH 1/2] net: mii: make mii_ethtool_gset() return void

[Pavel Skripkin]

mii_ethtool_gset() does not return any errors. We can make it return
void to simplify error checking in drivers, that rely on return value
of this function.

Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/net/mii.c   | 5 +----
 include/linux/mii.h | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mii.c b/drivers/net/mii.c
index 779c3a96dba7..3e7823267a3b 100644
--- a/drivers/net/mii.c
+++ b/drivers/net/mii.c
@@ -50,9 +50,8 @@ static u32 mii_get_an(struct mii_if_info *mii, u16 addr)
  * The @ecmd parameter is expected to have been cleared before calling
  * mii_ethtool_gset().
  *
- * Returns 0 for success, negative on error.
  */
-int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
+void mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 {
 	struct net_device *dev = mii->dev;
 	u16 bmcr, bmsr, ctrl1000 = 0, stat1000 = 0;
@@ -131,8 +130,6 @@ int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 	mii->full_duplex = ecmd->duplex;
 
 	/* ignore maxtxpkt, maxrxpkt for now */
-
-	return 0;
 }
 
 /**
diff --git a/include/linux/mii.h b/include/linux/mii.h
index 219b93cad1dd..12ea29e04293 100644
--- a/include/linux/mii.h
+++ b/include/linux/mii.h
@@ -32,7 +32,7 @@ struct mii_if_info {
 
 extern int mii_link_ok (struct mii_if_info *mii);
 extern int mii_nway_restart (struct mii_if_info *mii);
-extern int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
+extern void mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
 extern void mii_ethtool_get_link_ksettings(
 	struct mii_if_info *mii, struct ethtool_link_ksettings *cmd);
 extern int mii_ethtool_sset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
-- 
2.32.0

[PATCH 2/2] net: pch_gbe: remove mii_ethtool_gset() error handling

[Pavel Skripkin]

Since mii_ethtool_gset() becomes void in previous patch we can omit
cheking return value of this function and make code more simple.

Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 8 +-------
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c  | 4 +---
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index e351f3d1608f..539bddfab2d4 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -1031,13 +1031,7 @@ static void pch_gbe_watchdog(struct timer_list *t)
 		struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
 		netdev->tx_queue_len = adapter->tx_queue_len;
 		/* mii library handles link maintenance tasks */
-		if (mii_ethtool_gset(&adapter->mii, &cmd)) {
-			netdev_err(netdev, "ethtool get setting Error\n");
-			mod_timer(&adapter->watchdog_timer,
-				  round_jiffies(jiffies +
-						PCH_GBE_WATCHDOG_PERIOD));
-			return;
-		}
+		mii_ethtool_gset(&adapter->mii, &cmd);
 		hw->mac.link_speed = ethtool_cmd_speed(&cmd);
 		hw->mac.link_duplex = cmd.duplex;
 		/* Set the RGMII control. */
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
index ed832046216a..3426f6fa2b57 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
@@ -301,9 +301,7 @@ void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw)
 	int ret;
 	u16 mii_reg;
 
-	ret = mii_ethtool_gset(&adapter->mii, &cmd);
-	if (ret)
-		netdev_err(adapter->netdev, "Error: mii_ethtool_gset\n");
+	mii_ethtool_gset(&adapter->mii, &cmd);
 
 	ethtool_cmd_speed_set(&cmd, hw->mac.link_speed);
 	cmd.duplex = hw->mac.link_duplex;
-- 
2.32.0

Re: [PATCH 1/2] net: mii: make mii_ethtool_gset() return void

[Jakub Kicinski]

On Tue, 17 Aug 2021 21:34:42 +0300 Pavel Skripkin wrote:
> mii_ethtool_gset() does not return any errors. We can make it return
> void to simplify error checking in drivers, that rely on return value
> of this function.
> 
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>

This breaks the build and therefore would be a nuisance in bisection.
Please squash the changes or invert the order.

Re: [PATCH 1/2] net: mii: make mii_ethtool_gset() return void

[Andy Shevchenko]

On Tue, Aug 17, 2021 at 05:39:04PM -0700, Jakub Kicinski wrote:
> On Tue, 17 Aug 2021 21:34:42 +0300 Pavel Skripkin wrote:
> > mii_ethtool_gset() does not return any errors. We can make it return
> > void to simplify error checking in drivers, that rely on return value
> > of this function.
> > 
> > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> 
> This breaks the build and therefore would be a nuisance in bisection.
> Please squash the changes or invert the order.

Please invert the order. You will need slightly different justification for the
PCH GBE patch. Feel free to add my

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

to the PCH GBE one.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2 1/2] net: pch_gbe: remove mii_ethtool_gset() error handling

[Pavel Skripkin]

mii_ethtool_gset() does not return any errors, so error handling can be
omitted to make code more simple.

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---

Changes in v2:
	inverted the order of patches

---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 8 +-------
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c  | 4 +---
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index e351f3d1608f..539bddfab2d4 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -1031,13 +1031,7 @@ static void pch_gbe_watchdog(struct timer_list *t)
 		struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
 		netdev->tx_queue_len = adapter->tx_queue_len;
 		/* mii library handles link maintenance tasks */
-		if (mii_ethtool_gset(&adapter->mii, &cmd)) {
-			netdev_err(netdev, "ethtool get setting Error\n");
-			mod_timer(&adapter->watchdog_timer,
-				  round_jiffies(jiffies +
-						PCH_GBE_WATCHDOG_PERIOD));
-			return;
-		}
+		mii_ethtool_gset(&adapter->mii, &cmd);
 		hw->mac.link_speed = ethtool_cmd_speed(&cmd);
 		hw->mac.link_duplex = cmd.duplex;
 		/* Set the RGMII control. */
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
index ed832046216a..3426f6fa2b57 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
@@ -301,9 +301,7 @@ void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw)
 	int ret;
 	u16 mii_reg;
 
-	ret = mii_ethtool_gset(&adapter->mii, &cmd);
-	if (ret)
-		netdev_err(adapter->netdev, "Error: mii_ethtool_gset\n");
+	mii_ethtool_gset(&adapter->mii, &cmd);
 
 	ethtool_cmd_speed_set(&cmd, hw->mac.link_speed);
 	cmd.duplex = hw->mac.link_duplex;
-- 
2.32.0

[PATCH v2 2/2] net: mii: make mii_ethtool_gset() return void

[Pavel Skripkin]

mii_ethtool_gset() does not return any errors. Since there is no users
of this function that rely on its return value, it can be
made void.

Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---

Changes in v2:
	inverted the order of patches

---
 drivers/net/mii.c   | 5 +----
 include/linux/mii.h | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/net/mii.c b/drivers/net/mii.c
index 779c3a96dba7..3e7823267a3b 100644
--- a/drivers/net/mii.c
+++ b/drivers/net/mii.c
@@ -50,9 +50,8 @@ static u32 mii_get_an(struct mii_if_info *mii, u16 addr)
  * The @ecmd parameter is expected to have been cleared before calling
  * mii_ethtool_gset().
  *
- * Returns 0 for success, negative on error.
  */
-int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
+void mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 {
 	struct net_device *dev = mii->dev;
 	u16 bmcr, bmsr, ctrl1000 = 0, stat1000 = 0;
@@ -131,8 +130,6 @@ int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 	mii->full_duplex = ecmd->duplex;
 
 	/* ignore maxtxpkt, maxrxpkt for now */
-
-	return 0;
 }
 
 /**
diff --git a/include/linux/mii.h b/include/linux/mii.h
index 219b93cad1dd..12ea29e04293 100644
--- a/include/linux/mii.h
+++ b/include/linux/mii.h
@@ -32,7 +32,7 @@ struct mii_if_info {
 
 extern int mii_link_ok (struct mii_if_info *mii);
 extern int mii_nway_restart (struct mii_if_info *mii);
-extern int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
+extern void mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
 extern void mii_ethtool_get_link_ksettings(
 	struct mii_if_info *mii, struct ethtool_link_ksettings *cmd);
 extern int mii_ethtool_sset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
-- 
2.32.0

Re: [PATCH v2 2/2] net: mii: make mii_ethtool_gset() return void

[Andy Shevchenko]

On Wed, Aug 18, 2021 at 05:18:55PM +0300, Pavel Skripkin wrote:
> mii_ethtool_gset() does not return any errors. Since there is no users

there are

> of this function that rely on its return value, it can be
> made void.
> 
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
> 
> Changes in v2:
> 	inverted the order of patches
> 
> ---
>  drivers/net/mii.c   | 5 +----
>  include/linux/mii.h | 2 +-
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/mii.c b/drivers/net/mii.c
> index 779c3a96dba7..3e7823267a3b 100644
> --- a/drivers/net/mii.c
> +++ b/drivers/net/mii.c
> @@ -50,9 +50,8 @@ static u32 mii_get_an(struct mii_if_info *mii, u16 addr)
>   * The @ecmd parameter is expected to have been cleared before calling
>   * mii_ethtool_gset().

>   *

This line should gone as well.

> - * Returns 0 for success, negative on error.
>   */
> -int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
> +void mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
>  {
>  	struct net_device *dev = mii->dev;
>  	u16 bmcr, bmsr, ctrl1000 = 0, stat1000 = 0;
> @@ -131,8 +130,6 @@ int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
>  	mii->full_duplex = ecmd->duplex;
>  
>  	/* ignore maxtxpkt, maxrxpkt for now */
> -
> -	return 0;
>  }
>  
>  /**
> diff --git a/include/linux/mii.h b/include/linux/mii.h
> index 219b93cad1dd..12ea29e04293 100644
> --- a/include/linux/mii.h
> +++ b/include/linux/mii.h
> @@ -32,7 +32,7 @@ struct mii_if_info {
>  
>  extern int mii_link_ok (struct mii_if_info *mii);
>  extern int mii_nway_restart (struct mii_if_info *mii);
> -extern int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
> +extern void mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
>  extern void mii_ethtool_get_link_ksettings(
>  	struct mii_if_info *mii, struct ethtool_link_ksettings *cmd);
>  extern int mii_ethtool_sset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
> -- 
> 2.32.0
> 

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 1/2] net: pch_gbe: remove mii_ethtool_gset() error handling

[Pavel Skripkin]

mii_ethtool_gset() does not return any errors, so error handling can be
omitted to make code more simple.

Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---

Changes in v3:
	No changes

Changes in v2:
	inverted the order of patches

---
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 8 +-------
 drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c  | 4 +---
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index e351f3d1608f..539bddfab2d4 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -1031,13 +1031,7 @@ static void pch_gbe_watchdog(struct timer_list *t)
 		struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
 		netdev->tx_queue_len = adapter->tx_queue_len;
 		/* mii library handles link maintenance tasks */
-		if (mii_ethtool_gset(&adapter->mii, &cmd)) {
-			netdev_err(netdev, "ethtool get setting Error\n");
-			mod_timer(&adapter->watchdog_timer,
-				  round_jiffies(jiffies +
-						PCH_GBE_WATCHDOG_PERIOD));
-			return;
-		}
+		mii_ethtool_gset(&adapter->mii, &cmd);
 		hw->mac.link_speed = ethtool_cmd_speed(&cmd);
 		hw->mac.link_duplex = cmd.duplex;
 		/* Set the RGMII control. */
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
index ed832046216a..3426f6fa2b57 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_phy.c
@@ -301,9 +301,7 @@ void pch_gbe_phy_init_setting(struct pch_gbe_hw *hw)
 	int ret;
 	u16 mii_reg;
 
-	ret = mii_ethtool_gset(&adapter->mii, &cmd);
-	if (ret)
-		netdev_err(adapter->netdev, "Error: mii_ethtool_gset\n");
+	mii_ethtool_gset(&adapter->mii, &cmd);
 
 	ethtool_cmd_speed_set(&cmd, hw->mac.link_speed);
 	cmd.duplex = hw->mac.link_duplex;
-- 
2.32.0

[PATCH v3 2/2] net: mii: make mii_ethtool_gset() return void

[Pavel Skripkin]

mii_ethtool_gset() does not return any errors. Since there are no users
of this function that rely on its return value, it can be
made void.

Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---

Changes in v3:
	1. removed empty comment line
	2. there is -> there are

Changes in v2:
	inverted the order of patches

---
 drivers/net/mii.c   | 6 +-----
 include/linux/mii.h | 2 +-
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/mii.c b/drivers/net/mii.c
index 779c3a96dba7..22680f47385d 100644
--- a/drivers/net/mii.c
+++ b/drivers/net/mii.c
@@ -49,10 +49,8 @@ static u32 mii_get_an(struct mii_if_info *mii, u16 addr)
  *
  * The @ecmd parameter is expected to have been cleared before calling
  * mii_ethtool_gset().
- *
- * Returns 0 for success, negative on error.
  */
-int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
+void mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 {
 	struct net_device *dev = mii->dev;
 	u16 bmcr, bmsr, ctrl1000 = 0, stat1000 = 0;
@@ -131,8 +129,6 @@ int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd)
 	mii->full_duplex = ecmd->duplex;
 
 	/* ignore maxtxpkt, maxrxpkt for now */
-
-	return 0;
 }
 
 /**
diff --git a/include/linux/mii.h b/include/linux/mii.h
index 219b93cad1dd..12ea29e04293 100644
--- a/include/linux/mii.h
+++ b/include/linux/mii.h
@@ -32,7 +32,7 @@ struct mii_if_info {
 
 extern int mii_link_ok (struct mii_if_info *mii);
 extern int mii_nway_restart (struct mii_if_info *mii);
-extern int mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
+extern void mii_ethtool_gset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
 extern void mii_ethtool_get_link_ksettings(
 	struct mii_if_info *mii, struct ethtool_link_ksettings *cmd);
 extern int mii_ethtool_sset(struct mii_if_info *mii, struct ethtool_cmd *ecmd);
-- 
2.32.0

Re: [PATCH v3 1/2] net: pch_gbe: remove mii_ethtool_gset() error handling

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (refs/heads/master):

On Wed, 18 Aug 2021 18:06:30 +0300 you wrote:
> mii_ethtool_gset() does not return any errors, so error handling can be
> omitted to make code more simple.
> 
> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
> ---
> 
> [...]

Here is the summary with links:
  - [v3,1/2] net: pch_gbe: remove mii_ethtool_gset() error handling
    https://git.kernel.org/netdev/net-next/c/9fcfd0888cb7
  - [v3,2/2] net: mii: make mii_ethtool_gset() return void
    https://git.kernel.org/netdev/net-next/c/2274af1d60fe

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html