| Message ID | 20240429-stmmac-no-ethtool-begin-v1-1-04c629c1c142@redhat.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [RFC/RFT,net-next] net: stmmac: drop the ethtool begin() callback | expand |
Are there any updates on this topic? We are faced with the fact that we can not read or change settings of interface while it is down, and came up with the same solution for this problem. I do not know if Reviewed-by and Tested-by are suitable for patch marked as RFC but I will post mine in case it is acceptable here. Reviewed-by: Dmitry Dolenko <d.dolenko@metrotek.ru> Tested-by: Dmitry Dolenko <d.dolenko@metrotek.ru>
On Wed, Aug 28, 2024 at 05:35:41PM GMT, Dmitry Dolenko wrote: > Are there any updates on this topic? > > We are faced with the fact that we can not read or change settings of > interface while it is down, and came up with the same solution for this > problem. > > I do not know if Reviewed-by and Tested-by are suitable for patch marked as > RFC but I will post mine in case it is acceptable here. > > Reviewed-by: Dmitry Dolenko <d.dolenko@metrotek.ru> > Tested-by: Dmitry Dolenko <d.dolenko@metrotek.ru> > In my opinion the tags are welcomed. I had sort of forgotten about this until your reply, the use case I had was only to try and force out another bug, so it slipped my mind. Since both of us were bitten by this, and nobody has indicated it's a bad idea otherwise, I'll rebase and send v2 with your tags to try and get this merged. Thanks, Andrew
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c index 542e2633a6f5..c2e2723f7c6a 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,
This callback doesn't seem to serve much purpose, and prevents things like: - systemd.link files from disabling autonegotiation - carrier detection in NetworkManager 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") Signed-off-by: Andrew Halaney <ahalaney@redhat.com> --- drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 8 -------- 1 file changed, 8 deletions(-) --- base-commit: a59668a9397e7245b26e9be85d23f242ff757ae8 change-id: 20240424-stmmac-no-ethtool-begin-f306f2f1f2f4 Best regards,