diff mbox series

[v2] net: usb: ax88179_178a: improve link status logs

Message ID 20240618115054.101577-1-jtornosm@redhat.com
State Superseded
Headers show
Series [v2] net: usb: ax88179_178a: improve link status logs | expand

Commit Message

Jose Ignacio Tornos Martinez June 18, 2024, 11:50 a.m. UTC
Avoid spurious link status logs that may ultimately be wrong; for example,
if the link is set to down with the cable plugged, then the cable is
unplugged and after this the link is set to up, the last new log that is
appearing is incorrectly telling that the link is up.

In order to avoid errors, show link status logs after link_reset
processing, and in order to avoid spurious as much as possible, only show
the link loss when some link status change is detected.

cc: stable@vger.kernel.org
Fixes: e2ca90c276e1 ("ax88179_178a: ASIX AX88179_178A USB 3.0/2.0 to gigabit ethernet adapter driver")
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
v2:
  - Fix the nits
v1: https://lore.kernel.org/netdev/20240617103405.654567-1-jtornosm@redhat.com/

 drivers/net/usb/ax88179_178a.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Simon Horman June 19, 2024, 4:45 p.m. UTC | #1
On Tue, Jun 18, 2024 at 01:50:40PM +0200, Jose Ignacio Tornos Martinez wrote:
> Avoid spurious link status logs that may ultimately be wrong; for example,
> if the link is set to down with the cable plugged, then the cable is
> unplugged and after this the link is set to up, the last new log that is
> appearing is incorrectly telling that the link is up.
> 
> In order to avoid errors, show link status logs after link_reset
> processing, and in order to avoid spurious as much as possible, only show
> the link loss when some link status change is detected.
> 
> cc: stable@vger.kernel.org
> Fixes: e2ca90c276e1 ("ax88179_178a: ASIX AX88179_178A USB 3.0/2.0 to gigabit ethernet adapter driver")
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
> ---
> v2:
>   - Fix the nits
> v1: https://lore.kernel.org/netdev/20240617103405.654567-1-jtornosm@redhat.com/

Hi Jose,

Thanks for the updates.
This version looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

...
Paolo Abeni June 20, 2024, 11:43 a.m. UTC | #2
On Tue, 2024-06-18 at 13:50 +0200, Jose Ignacio Tornos Martinez wrote:
> Avoid spurious link status logs that may ultimately be wrong; for example,
> if the link is set to down with the cable plugged, then the cable is
> unplugged and after this the link is set to up, the last new log that is
> appearing is incorrectly telling that the link is up.
> 
> In order to avoid errors, show link status logs after link_reset
> processing, and in order to avoid spurious as much as possible, only show
> the link loss when some link status change is detected.
> 
> cc: stable@vger.kernel.org
> Fixes: e2ca90c276e1 ("ax88179_178a: ASIX AX88179_178A USB 3.0/2.0 to gigabit ethernet adapter driver")
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>

Please always include the target tree in the subj prefix - in this case
it should have been 'net'.

> ---
> v2:
>   - Fix the nits
> v1: https://lore.kernel.org/netdev/20240617103405.654567-1-jtornosm@redhat.com/
> 
>  drivers/net/usb/ax88179_178a.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> index c2fb736f78b2..d90ceab282ff 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -326,7 +326,9 @@ static void ax88179_status(struct usbnet *dev, struct urb *urb)
>  
>  	if (netif_carrier_ok(dev->net) != link) {
>  		usbnet_link_change(dev, link, 1);
> -		netdev_info(dev->net, "ax88179 - Link status is: %d\n", link);
> +		if (!link)
> +			netdev_info(dev->net, "ax88179 - Link status is: %d\n",
> +				    link);

Here                                ^^^^ link value is always 0, so you
should using a constant string.


Thanks,

Paolo
diff mbox series

Patch

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index c2fb736f78b2..d90ceab282ff 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -326,7 +326,9 @@  static void ax88179_status(struct usbnet *dev, struct urb *urb)
 
 	if (netif_carrier_ok(dev->net) != link) {
 		usbnet_link_change(dev, link, 1);
-		netdev_info(dev->net, "ax88179 - Link status is: %d\n", link);
+		if (!link)
+			netdev_info(dev->net, "ax88179 - Link status is: %d\n",
+				    link);
 	}
 }
 
@@ -1542,6 +1544,7 @@  static int ax88179_link_reset(struct usbnet *dev)
 			 GMII_PHY_PHYSR, 2, &tmp16);
 
 	if (!(tmp16 & GMII_PHY_PHYSR_LINK)) {
+		netdev_info(dev->net, "ax88179 - Link status is: 0\n");
 		return 0;
 	} else if (GMII_PHY_PHYSR_GIGA == (tmp16 & GMII_PHY_PHYSR_SMASK)) {
 		mode |= AX_MEDIUM_GIGAMODE | AX_MEDIUM_EN_125MHZ;
@@ -1579,6 +1582,8 @@  static int ax88179_link_reset(struct usbnet *dev)
 
 	netif_carrier_on(dev->net);
 
+	netdev_info(dev->net, "ax88179 - Link status is: 1\n");
+
 	return 0;
 }