1. Patchew
  2. linux
  3. [PATCH 0/2] net: usb: ax88179_178a: fix and improve reset procedure
  4. View patch

[PATCH 2/2] net: usb: ax88179_178a: avoid two consecutive device resets

Jose Ignacio Tornos Martinez posted 2 patches 2 years, 1 month ago
[PATCH 2/2] net: usb: ax88179_178a: avoid two consecutive device resets
Posted by Jose Ignacio Tornos Martinez 2 years, 1 month ago 
The device is always reset two consecutive times (ax88179_reset is called
twice), one from usbnet_probe during the device binding and the other from
usbnet_open.

Let only the reset during the device binding to prepare the device as soon
as possible and not repeat the reset operation (tested with generic ASIX
Electronics Corp. AX88179 Gigabit Ethernet device).

Reported-by: Herb Wei <weihao.bj@ieisystem.com>
Tested-by: Herb Wei <weihao.bj@ieisystem.com>
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 drivers/net/usb/ax88179_178a.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 4ea0e155bb0d..864c6fc2db33 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1678,7 +1678,6 @@ static const struct driver_info ax88179_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset = ax88179_reset,
 	.stop = ax88179_stop,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1691,7 +1690,6 @@ static const struct driver_info ax88178a_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset = ax88179_reset,
 	.stop = ax88179_stop,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1704,7 +1702,6 @@ static const struct driver_info cypress_GX3_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset = ax88179_reset,
 	.stop = ax88179_stop,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1717,7 +1714,6 @@ static const struct driver_info dlink_dub1312_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset = ax88179_reset,
 	.stop = ax88179_stop,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1730,7 +1726,6 @@ static const struct driver_info sitecom_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset = ax88179_reset,
 	.stop = ax88179_stop,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1743,7 +1738,6 @@ static const struct driver_info samsung_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset = ax88179_reset,
 	.stop = ax88179_stop,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1756,7 +1750,6 @@ static const struct driver_info lenovo_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset = ax88179_reset,
 	.stop = ax88179_stop,
 	.flags = FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1769,7 +1762,6 @@ static const struct driver_info belkin_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset	= ax88179_reset,
 	.stop	= ax88179_stop,
 	.flags	= FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1782,7 +1774,6 @@ static const struct driver_info toshiba_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset	= ax88179_reset,
 	.stop = ax88179_stop,
 	.flags	= FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1795,7 +1786,6 @@ static const struct driver_info mct_info = {
 	.unbind	= ax88179_unbind,
 	.status	= ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset	= ax88179_reset,
 	.stop	= ax88179_stop,
 	.flags	= FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1808,7 +1798,6 @@ static const struct driver_info at_umc2000_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset  = ax88179_reset,
 	.stop   = ax88179_stop,
 	.flags  = FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1821,7 +1810,6 @@ static const struct driver_info at_umc200_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset  = ax88179_reset,
 	.stop   = ax88179_stop,
 	.flags  = FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
@@ -1834,7 +1822,6 @@ static const struct driver_info at_umc2000sp_info = {
 	.unbind = ax88179_unbind,
 	.status = ax88179_status,
 	.link_reset = ax88179_link_reset,
-	.reset  = ax88179_reset,
 	.stop   = ax88179_stop,
 	.flags  = FLAG_ETHER | FLAG_FRAMING_AX,
 	.rx_fixup = ax88179_rx_fixup,
-- 
2.41.0
Re: [PATCH 2/2] net: usb: ax88179_178a: avoid two consecutive device resets
Posted by Paolo Abeni 2 years, 1 month ago 
On Tue, 2023-11-14 at 13:50 +0100, Jose Ignacio Tornos Martinez wrote:
> The device is always reset two consecutive times (ax88179_reset is called
> twice), one from usbnet_probe during the device binding and the other from
> usbnet_open.
> 
> Let only the reset during the device binding to prepare the device as soon
> as possible and not repeat the reset operation (tested with generic ASIX
> Electronics Corp. AX88179 Gigabit Ethernet device).
> 
> Reported-by: Herb Wei <weihao.bj@ieisystem.com>
> Tested-by: Herb Wei <weihao.bj@ieisystem.com>
> Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>

We need a suitable Fixes tag even here ;)

> ---
>  drivers/net/usb/ax88179_178a.c | 13 -------------
>  1 file changed, 13 deletions(-)
> 
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> index 4ea0e155bb0d..864c6fc2db33 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -1678,7 +1678,6 @@ static const struct driver_info ax88179_info = {
>  	.unbind = ax88179_unbind,
>  	.status = ax88179_status,
>  	.link_reset = ax88179_link_reset,
> -	.reset = ax88179_reset,
>  	.stop = ax88179_stop,
>  	.flags = FLAG_ETHER | FLAG_FRAMING_AX,
>  	.rx_fixup = ax88179_rx_fixup,

This looks potentially dangerous, as the device will not get a reset in
down/up cycles; *possibly* dropping the reset call from ax88179_bind()
would be safer.

In both cases touching so many H/W variant with testing available on a
single one sounds dangerous. Is the unneeded 2nd reset causing any
specific issue?

Thanks,

Paolo
Re: [PATCH 2/2] net: usb: ax88179_178a: avoid two consecutive device resets
Posted by Jose Ignacio Tornos Martinez 2 years, 1 month ago 
On Thu, Nov 16, 2023 at 10:42 AM Paolo Abeni <pabeni@redhat.com> wrote:
> We need a suitable Fixes tag even here ;)
Ok, I will add it in my next version.

> > ---
> >  drivers/net/usb/ax88179_178a.c | 13 -------------
> >  1 file changed, 13 deletions(-)
> >
> > diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> > index 4ea0e155bb0d..864c6fc2db33 100644
> > --- a/drivers/net/usb/ax88179_178a.c
> > +++ b/drivers/net/usb/ax88179_178a.c
> > @@ -1678,7 +1678,6 @@ static const struct driver_info ax88179_info = {
> >       .unbind = ax88179_unbind,
> >       .status = ax88179_status,
> >       .link_reset = ax88179_link_reset,
> > -     .reset = ax88179_reset,
> >       .stop = ax88179_stop,
> >       .flags = FLAG_ETHER | FLAG_FRAMING_AX,
> >       .rx_fixup = ax88179_rx_fixup,
>
> This looks potentially dangerous, as the device will not get a reset in
> down/up cycles; *possibly* dropping the reset call from ax88179_bind()
> would be safer.
Ok, I had the doubt about which reset would be the best, because it seemed 
to me that reset would be better as soon as possible.
I will try what you say to avoid down/up cycles.

> In both cases touching so many H/W variant with testing available on a
> single one sounds dangerous. Is the unneeded 2nd reset causing any
> specific issue?
Actually, this double reboot somewhat masked the first problem, because the
probability of getting a successful initialization, if there is a previous
problem seems to be higher. So, it is not strictly needed but I think it is 
better to avoid a second unnecessary reset.
Ok, if I modify the call from ax88179_bind() I will be respecting the reset
operation of all devices.

Thanks

Best regards
José Ignacio
[PATCH v2 1/2] net: usb: ax88179_178a: fix failed operations during ax88179_reset
Posted by Jose Ignacio Tornos Martinez 2 years, 1 month ago 
Using generic ASIX Electronics Corp. AX88179 Gigabit Ethernet device,
the following test cycle has been implemented:
    - power on
    - check logs
    - shutdown
    - after detecting the system shutdown, disconnect power
    - after approximately 60 seconds of sleep, power is restored
Running some cycles, sometimes error logs like this appear:
    kernel: ax88179_178a 2-9:1.0 (unnamed net_device) (uninitialized): Failed to write reg index 0x0001: -19
    kernel: ax88179_178a 2-9:1.0 (unnamed net_device) (uninitialized): Failed to read reg index 0x0001: -19
    ...
These failed operation are happening during ax88179_reset execution, so
the initialization could not be correct.

In order to avoid this, we need to increase the delay after reset and
clock initial operations. By using these larger values, many cycles
have been run and no failed operations appear.

It would be better to check some status register to verify when the
operation has finished, but I do not have found any available information
(neither in the public datasheets nor in the manufacturer's driver). The
only available information for the necessary delays is the maufacturer's
driver (original values) but the proposed values are not enough for the
tested devices.

Fixes: e2ca90c276e1f ("ax88179_178a: ASIX AX88179_178A USB 3.0/2.0 to gigabit ethernet adapter driver")
Reported-by: Herb Wei <weihao.bj@ieisystem.com>
Tested-by: Herb Wei <weihao.bj@ieisystem.com>
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
V1 -> V2:
- Add Fixes tag.
- Comments about the available information and manufacturer's driver
  reference to complete why the new values are needed.

 drivers/net/usb/ax88179_178a.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index aff39bf3161d..4ea0e155bb0d 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1583,11 +1583,11 @@ static int ax88179_reset(struct usbnet *dev)
 
 	*tmp16 = AX_PHYPWR_RSTCTL_IPRL;
 	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_PHYPWR_RSTCTL, 2, 2, tmp16);
-	msleep(200);
+	msleep(500);
 
 	*tmp = AX_CLK_SELECT_ACS | AX_CLK_SELECT_BCS;
 	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_CLK_SELECT, 1, 1, tmp);
-	msleep(100);
+	msleep(200);
 
 	/* Ethernet PHY Auto Detach*/
 	ax88179_auto_detach(dev);
-- 
2.42.0
Re: [PATCH v2 1/2] net: usb: ax88179_178a: fix failed operations during ax88179_reset
Posted by patchwork-bot+netdevbpf@kernel.org 2 years, 1 month ago 
Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 20 Nov 2023 13:06:29 +0100 you wrote:
> Using generic ASIX Electronics Corp. AX88179 Gigabit Ethernet device,
> the following test cycle has been implemented:
>     - power on
>     - check logs
>     - shutdown
>     - after detecting the system shutdown, disconnect power
>     - after approximately 60 seconds of sleep, power is restored
> Running some cycles, sometimes error logs like this appear:
>     kernel: ax88179_178a 2-9:1.0 (unnamed net_device) (uninitialized): Failed to write reg index 0x0001: -19
>     kernel: ax88179_178a 2-9:1.0 (unnamed net_device) (uninitialized): Failed to read reg index 0x0001: -19
>     ...
> These failed operation are happening during ax88179_reset execution, so
> the initialization could not be correct.
> 
> [...]

Here is the summary with links:
  - [v2,1/2] net: usb: ax88179_178a: fix failed operations during ax88179_reset
    https://git.kernel.org/netdev/net/c/0739af07d1d9
  - [v2,2/2] net: usb: ax88179_178a: avoid two consecutive device resets
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH v2 1/2] net: usb: ax88179_178a: fix failed operations during ax88179_reset
Posted by patchwork-bot+netdevbpf@kernel.org 2 years, 1 month ago 
Hello:

This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 20 Nov 2023 13:06:29 +0100 you wrote:
> Using generic ASIX Electronics Corp. AX88179 Gigabit Ethernet device,
> the following test cycle has been implemented:
>     - power on
>     - check logs
>     - shutdown
>     - after detecting the system shutdown, disconnect power
>     - after approximately 60 seconds of sleep, power is restored
> Running some cycles, sometimes error logs like this appear:
>     kernel: ax88179_178a 2-9:1.0 (unnamed net_device) (uninitialized): Failed to write reg index 0x0001: -19
>     kernel: ax88179_178a 2-9:1.0 (unnamed net_device) (uninitialized): Failed to read reg index 0x0001: -19
>     ...
> These failed operation are happening during ax88179_reset execution, so
> the initialization could not be correct.
> 
> [...]

Here is the summary with links:
  - [v2,1/2] net: usb: ax88179_178a: fix failed operations during ax88179_reset
    (no matching commit)
  - [v2,2/2] net: usb: ax88179_178a: avoid two consecutive device resets
    https://git.kernel.org/netdev/net-next/c/d2689b6a86b9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
[PATCH v2 2/2] net: usb: ax88179_178a: avoid two consecutive device resets
Posted by Jose Ignacio Tornos Martinez 2 years, 1 month ago 
The device is always reset two consecutive times (ax88179_reset is called
twice), one from usbnet_probe during the device binding and the other from
usbnet_open.

Remove the non-necessary reset during the device binding and let the reset
operation from open to keep the normal behavior (tested with generic ASIX
Electronics Corp. AX88179 Gigabit Ethernet device).

Fixes: e2ca90c276e1f ("ax88179_178a: ASIX AX88179_178A USB 3.0/2.0 to gigabit ethernet adapter driver")
Reported-by: Herb Wei <weihao.bj@ieisystem.com>
Tested-by: Herb Wei <weihao.bj@ieisystem.com>
Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
V1 -> V2:
- Add Fixes tag.
- Follow Paolo Abeni's suggestion and remove the binding reset, not the
  reset operation to keep the normal behavior.

 drivers/net/usb/ax88179_178a.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
index 4ea0e155bb0d..8d835fbc4316 100644
--- a/drivers/net/usb/ax88179_178a.c
+++ b/drivers/net/usb/ax88179_178a.c
@@ -1298,8 +1298,6 @@ static int ax88179_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	netif_set_tso_max_size(dev->net, 16384);
 
-	ax88179_reset(dev);
-
 	return 0;
 }
 
-- 
2.42.0

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.