Re: [PATCH v5] net: stmmac: fix 'ethtool -P' return -EBUSY
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: 2021-07-19 13:57:54
Also in:
lkml
On 19.07.2021 15:08, Hao Chen wrote:
The permanent mac address should be available for query when the device is not up. NetworkManager, the system network daemon, uses 'ethtool -P' to obtain the permanent address after the kernel start. When the network device is not up, it will return the device busy error with 'ethtool -P'. At that time, it is unable to access the Internet through the permanent address by NetworkManager.
At first a few formal aspects: - You don't get a lot of new friends when posting a new version every few hours. Consolidate feedback and then post a new version, typically not more than one version per day. Mileage of maintainers may vary here. - When posting a new version include a change log. - Properly annotate the patch as net vs. net-next. - If you declare something to be a fix, add a Fixes tag.
I think that the '.begin' is not used to check if the device is up, it's just a pre hook for ethtool. We shouldn't check if the device is up there.
Supposedly the check is there for a reason. Your statement leaves the impression that you're not aware of why the check exists. Therefore: First understand this, and then explain why it's safe to remove the check. This drivers uses runtime pm, so device might be in PCI D3 if interface is down (don't know this driver in detail). Therefore registers may not be accessible what may impact certain ethtool ops.
quoted hunk ↗ jump to hunk
Signed-off-by: Hao Chen <redacted> --- 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 d0ce608b81c3..8901dc9f758e 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c@@ -410,13 +410,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);@@ -1073,7 +1066,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,