diff mbox series

[v2] net: stmmac: fix 'ethtool -P' return -EBUSY

Message ID 20210719074410.6787-1-chenhaoa@uniontech.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [v2] net: stmmac: fix 'ethtool -P' return -EBUSY | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cc_maintainers warning 1 maintainers not CCed: linux-arm-kernel@lists.infradead.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 0 this patch: 9
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 12 lines checked
netdev/build_allmodconfig_warn fail Errors and warnings before: 0 this patch: 9
netdev/header_inline success Link

Commit Message

Hao Chen July 19, 2021, 7:44 a.m. UTC
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.
I think that the '.begin' is not used to check if the device is up.

Signed-off-by: Hao Chen <chenhaoa@uniontech.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Joakim Zhang July 19, 2021, 8:52 a.m. UTC | #1
> -----Original Message-----
> From: Hao Chen <chenhaoa@uniontech.com>
> Sent: 2021年7月19日 15:44
> To: peppe.cavallaro@st.com
> Cc: alexandre.torgue@foss.st.com; joabreu@synopsys.com;
> davem@davemloft.net; kuba@kernel.org; mcoquelin.stm32@gmail.com;
> linux@armlinux.org.uk; netdev@vger.kernel.org;
> linux-stm32@st-md-mailman.stormreply.com; linux-kernel@vger.kernel.org;
> Hao Chen <chenhaoa@uniontech.com>
> Subject: [PATCH v2] net: stmmac: fix 'ethtool -P' return -EBUSY
> 
> 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.
> I think that the '.begin' is not used to check if the device is up.
> 
> Signed-off-by: Hao Chen <chenhaoa@uniontech.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> index d0ce608b81c3..7ccb0d738a1c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
> @@ -412,8 +412,10 @@ 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;
> +	struct stmmac_priv *priv = netdev_priv(dev);
> +
> +	pm_runtime_get_sync(priv->device);
> +
>  	return 0;
>  }

After this code change, clocks always enabled even the NIC is not up, it's not good.
Should implement .complete() to disable clocks after the info you got?

Best Regards,
Joakim Zhang
> 
> --
> 2.20.1
> 
>
diff mbox series

Patch

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index d0ce608b81c3..7ccb0d738a1c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -412,8 +412,10 @@  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;
+	struct stmmac_priv *priv = netdev_priv(dev);
+
+	pm_runtime_get_sync(priv->device);
+
 	return 0;
 }