[PATCH net-next v2] net: stmmac: drop the ethtool begin() callback

[PATCH net-next v2] net: stmmac: drop the ethtool begin() callback

[Andrew Halaney]

This callback doesn't seem to serve much purpose, and prevents things
like:

    - systemd.link files from disabling autonegotiation
    - carrier detection in NetworkManager
    - any ethtool setting

prior to userspace bringing the link up.

The only fear I can think of is accessing unclocked resources due to
pm_runtime, but ethtool ioctls handle that as of commit
f32a21376573 ("ethtool: runtime-resume netdev parent before ethtool ioctl ops")

Reviewed-by: Dmitry Dolenko <d.dolenko@metrotek.ru>
Tested-by: Dmitry Dolenko <d.dolenko@metrotek.ru>
Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
---
Changes in v2:
- Rebase on next-20240829
- Drop RFC/RFT tags, add Dmitry's Review/Test tag
- Link to v1: https://lore.kernel.org/r/20240429-stmmac-no-ethtool-begin-v1-1-04c629c1c142@redhat.com

I'd still like a few more folks to test this to feel more confident that
I'm not breaking anyone, but at least I've gotten one on list feedback
and one off list that its fine for them.
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index 7008219fd88d..220c582904f4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -438,13 +438,6 @@ static void stmmac_ethtool_setmsglevel(struct net_device *dev, u32 level)
 
 }
 
-static int stmmac_check_if_running(struct net_device *dev)
-{
-	if (!netif_running(dev))
-		return -EBUSY;
-	return 0;
-}
-
 static int stmmac_ethtool_get_regs_len(struct net_device *dev)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
@@ -1273,7 +1266,6 @@ static int stmmac_set_tunable(struct net_device *dev,
 static const struct ethtool_ops stmmac_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
 				     ETHTOOL_COALESCE_MAX_FRAMES,
-	.begin = stmmac_check_if_running,
 	.get_drvinfo = stmmac_ethtool_getdrvinfo,
 	.get_msglevel = stmmac_ethtool_getmsglevel,
 	.set_msglevel = stmmac_ethtool_setmsglevel,

---
base-commit: b18bbfc14a38b5234e09c2adcf713e38063a7e6e
change-id: 20240424-stmmac-no-ethtool-begin-f306f2f1f2f4

Best regards,
-- 
Andrew Halaney <ahalaney@redhat.com>

Re: [PATCH net-next v2] net: stmmac: drop the ethtool begin() callback

[Alexander Lobakin]

From: Andrew Halaney <ahalaney@redhat.com>
Date: Thu, 29 Aug 2024 15:48:44 -0500

> This callback doesn't seem to serve much purpose, and prevents things
> like:
> 
>     - systemd.link files from disabling autonegotiation
>     - carrier detection in NetworkManager
>     - any ethtool setting
> 
> prior to userspace bringing the link up.
> 
> The only fear I can think of is accessing unclocked resources due to
> pm_runtime, but ethtool ioctls handle that as of commit
> f32a21376573 ("ethtool: runtime-resume netdev parent before ethtool ioctl ops")
> 
> Reviewed-by: Dmitry Dolenko <d.dolenko@metrotek.ru>
> Tested-by: Dmitry Dolenko <d.dolenko@metrotek.ru>

Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>

> Signed-off-by: Andrew Halaney <ahalaney@redhat.com>
Thanks,
Olek

Re: [PATCH net-next v2] net: stmmac: drop the ethtool begin() callback

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Thu, 29 Aug 2024 15:48:44 -0500 you wrote:
> This callback doesn't seem to serve much purpose, and prevents things
> like:
> 
>     - systemd.link files from disabling autonegotiation
>     - carrier detection in NetworkManager
>     - any ethtool setting
> 
> [...]

Here is the summary with links:
  - [net-next,v2] net: stmmac: drop the ethtool begin() callback
    https://git.kernel.org/netdev/net-next/c/55ddb6c5a3ae

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