diff mbox series

net: usb: ax88179_178a: improve reset check

Message ID 20240617102839.654316-1-jtornosm@redhat.com (mailing list archive)
State Accepted
Commit 7be4cb7189f747b4e5b6977d0e4387bde3204e62
Delegated to: Netdev Maintainers
Headers show
Series net: usb: ax88179_178a: improve reset check | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 845 this patch: 845
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 849 this patch: 849
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 849 this patch: 849
netdev/checkpatch warning WARNING: 'bahavior' may be misspelled - perhaps 'behavior'? WARNING: line length of 84 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-06-18--12-00 (tests: 654)

Commit Message

Jose Ignacio Tornos Martinez June 17, 2024, 10:28 a.m. UTC
After ecf848eb934b ("net: usb: ax88179_178a: fix link status when link is
set to down/up") to not reset from usbnet_open after the reset from
usbnet_probe at initialization stage to speed up this, some issues have
been reported.

It seems to happen that if the initialization is slower, and some time
passes between the probe operation and the open operation, the second reset
from open is necessary too to have the device working. The reason is that
if there is no activity with the phy, this is "disconnected".

In order to improve this, the solution is to detect when the phy is
"disconnected", and we can use the phy status register for this. So we will
only reset the device from reset operation in this situation, that is, only
if necessary.

The same bahavior is happening when the device is stopped (link set to
down) and later is restarted (link set to up), so if the phy keeps working
we only need to enable the mac again, but if enough time passes between the
device stop and restart, reset is necessary, and we can detect the
situation checking the phy status register too.

cc: stable@vger.kernel.org # 6.6+
Fixes: ecf848eb934b ("net: usb: ax88179_178a: fix link status when link is set to down/up")
Reported-by: Yongqin Liu <yongqin.liu@linaro.org>
Reported-by: Antje Miederhöfer <a.miederhoefer@gmx.de>
Reported-by: Arne Fitzenreiter <arne_f@ipfire.org>
Tested-by: Yongqin Liu <yongqin.liu@linaro.org>
Tested-by: Antje Miederhöfer <a.miederhoefer@gmx.de>
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 drivers/net/usb/ax88179_178a.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

Comments

patchwork-bot+netdevbpf@kernel.org June 19, 2024, 9:50 a.m. UTC | #1
Hello:

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

On Mon, 17 Jun 2024 12:28:21 +0200 you wrote:
> After ecf848eb934b ("net: usb: ax88179_178a: fix link status when link is
> set to down/up") to not reset from usbnet_open after the reset from
> usbnet_probe at initialization stage to speed up this, some issues have
> been reported.
> 
> It seems to happen that if the initialization is slower, and some time
> passes between the probe operation and the open operation, the second reset
> from open is necessary too to have the device working. The reason is that
> if there is no activity with the phy, this is "disconnected".
> 
> [...]

Here is the summary with links:
  - net: usb: ax88179_178a: improve reset check
    https://git.kernel.org/netdev/net/c/7be4cb7189f7

You are awesome, thank you!
Arne Fitzenreiter June 19, 2024, 10:37 a.m. UTC | #2
Hello Jose Ignacio,

this patch fix my issues.

Best regards
Arne


Am 2024-06-17 12:28, schrieb Jose Ignacio Tornos Martinez:
> After ecf848eb934b ("net: usb: ax88179_178a: fix link status when link 
> is
> set to down/up") to not reset from usbnet_open after the reset from
> usbnet_probe at initialization stage to speed up this, some issues have
> been reported.
> 
> It seems to happen that if the initialization is slower, and some time
> passes between the probe operation and the open operation, the second 
> reset
> from open is necessary too to have the device working. The reason is 
> that
> if there is no activity with the phy, this is "disconnected".
> 
> In order to improve this, the solution is to detect when the phy is
> "disconnected", and we can use the phy status register for this. So we 
> will
> only reset the device from reset operation in this situation, that is, 
> only
> if necessary.
> 
> The same bahavior is happening when the device is stopped (link set to
> down) and later is restarted (link set to up), so if the phy keeps 
> working
> we only need to enable the mac again, but if enough time passes between 
> the
> device stop and restart, reset is necessary, and we can detect the
> situation checking the phy status register too.
> 
> cc: stable@vger.kernel.org # 6.6+
> Fixes: ecf848eb934b ("net: usb: ax88179_178a: fix link status when link 
> is set to down/up")
> Reported-by: Yongqin Liu <yongqin.liu@linaro.org>
> Reported-by: Antje Miederhöfer <a.miederhoefer@gmx.de>
> Reported-by: Arne Fitzenreiter <arne_f@ipfire.org>
> Tested-by: Yongqin Liu <yongqin.liu@linaro.org>
> Tested-by: Antje Miederhöfer <a.miederhoefer@gmx.de>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
>  drivers/net/usb/ax88179_178a.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/usb/ax88179_178a.c 
> b/drivers/net/usb/ax88179_178a.c
> index 51c295e1e823..c2fb736f78b2 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -174,7 +174,6 @@ struct ax88179_data {
>  	u32 wol_supported;
>  	u32 wolopts;
>  	u8 disconnecting;
> -	u8 initialized;
>  };
> 
>  struct ax88179_int_data {
> @@ -1678,12 +1677,21 @@ static int ax88179_reset(struct usbnet *dev)
> 
>  static int ax88179_net_reset(struct usbnet *dev)
>  {
> -	struct ax88179_data *ax179_data = dev->driver_priv;
> +	u16 tmp16;
> 
> -	if (ax179_data->initialized)
> +	ax88179_read_cmd(dev, AX_ACCESS_PHY, AX88179_PHY_ID, GMII_PHY_PHYSR,
> +			 2, &tmp16);
> +	if (tmp16) {
> +		ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE,
> +				 2, 2, &tmp16);
> +		if (!(tmp16 & AX_MEDIUM_RECEIVE_EN)) {
> +			tmp16 |= AX_MEDIUM_RECEIVE_EN;
> +			ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE,
> +					  2, 2, &tmp16);
> +		}
> +	} else {
>  		ax88179_reset(dev);
> -	else
> -		ax179_data->initialized = 1;
> +	}
> 
>  	return 0;
>  }
diff mbox series

Patch

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 51c295e1e823..c2fb736f78b2 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -174,7 +174,6 @@  struct ax88179_data {
 	u32 wol_supported;
 	u32 wolopts;
 	u8 disconnecting;
-	u8 initialized;
 };
 
 struct ax88179_int_data {
@@ -1678,12 +1677,21 @@  static int ax88179_reset(struct usbnet *dev)
 
 static int ax88179_net_reset(struct usbnet *dev)
 {
-	struct ax88179_data *ax179_data = dev->driver_priv;
+	u16 tmp16;
 
-	if (ax179_data->initialized)
+	ax88179_read_cmd(dev, AX_ACCESS_PHY, AX88179_PHY_ID, GMII_PHY_PHYSR,
+			 2, &tmp16);
+	if (tmp16) {
+		ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE,
+				 2, 2, &tmp16);
+		if (!(tmp16 & AX_MEDIUM_RECEIVE_EN)) {
+			tmp16 |= AX_MEDIUM_RECEIVE_EN;
+			ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE,
+					  2, 2, &tmp16);
+		}
+	} else {
 		ax88179_reset(dev);
-	else
-		ax179_data->initialized = 1;
+	}
 
 	return 0;
 }