The idea was to keep only one reset at initialization stage in order to reduce the total delay, or the reset from usbnet_probe or the reset from usbnet_open.
I have seen that restarting from usbnet_probe is necessary to avoid doing too complex things. But when the link is set to down/up (for example to configure a different mac address) the link is not correctly recovered unless a reset is commanded from usbnet_open.
So, detect the initialization stage (first call) to not reset from usbnet_open after the reset from usbnet_probe and after this stage, always reset from usbnet_open too (when the link needs to be rechecked).
Apply to all the possible devices, the behavior now is going to be the same.
cc: stable@vger.kernel.org # 6.6+ Fixes: 56f78615bcb1 ("net: usb: ax88179_178a: avoid writing the mac address before first reading") Reported-by: Isaac Ganoung inventor500@vivaldi.net Reported-by: Yongqin Liu yongqin.liu@linaro.org Signed-off-by: Jose Ignacio Tornos Martinez jtornosm@redhat.com --- drivers/net/usb/ax88179_178a.c | 37 ++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 377be0d9ef14..a0edb410f746 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -174,6 +174,7 @@ struct ax88179_data { u32 wol_supported; u32 wolopts; u8 disconnecting; + u8 initialized; };
struct ax88179_int_data { @@ -1672,6 +1673,18 @@ static int ax88179_reset(struct usbnet *dev) return 0; }
+static int ax88179_net_reset(struct usbnet *dev) +{ + struct ax88179_data *ax179_data = dev->driver_priv; + + if (ax179_data->initialized) + ax88179_reset(dev); + else + ax179_data->initialized = 1; + + return 0; +} + static int ax88179_stop(struct usbnet *dev) { u16 tmp16; @@ -1691,6 +1704,7 @@ static const struct driver_info ax88179_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1703,6 +1717,7 @@ static const struct driver_info ax88178a_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1715,7 +1730,7 @@ static const struct driver_info cypress_GX3_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1728,7 +1743,7 @@ static const struct driver_info dlink_dub1312_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1741,7 +1756,7 @@ static const struct driver_info sitecom_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1754,7 +1769,7 @@ static const struct driver_info samsung_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1767,7 +1782,7 @@ static const struct driver_info lenovo_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1780,7 +1795,7 @@ static const struct driver_info belkin_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1793,7 +1808,7 @@ static const struct driver_info toshiba_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1806,7 +1821,7 @@ static const struct driver_info mct_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1819,7 +1834,7 @@ static const struct driver_info at_umc2000_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1832,7 +1847,7 @@ static const struct driver_info at_umc200_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup, @@ -1845,7 +1860,7 @@ static const struct driver_info at_umc2000sp_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset, - .reset = ax88179_reset, + .reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,
On Fri, May 10, 2024 at 11:08:28AM +0200, Jose Ignacio Tornos Martinez wrote:
The idea was to keep only one reset at initialization stage in order to reduce the total delay, or the reset from usbnet_probe or the reset from usbnet_open.
I have seen that restarting from usbnet_probe is necessary to avoid doing too complex things. But when the link is set to down/up (for example to configure a different mac address) the link is not correctly recovered unless a reset is commanded from usbnet_open.
So, detect the initialization stage (first call) to not reset from usbnet_open after the reset from usbnet_probe and after this stage, always reset from usbnet_open too (when the link needs to be rechecked).
Apply to all the possible devices, the behavior now is going to be the same.
cc: stable@vger.kernel.org # 6.6+ Fixes: 56f78615bcb1 ("net: usb: ax88179_178a: avoid writing the mac address before first reading") Reported-by: Isaac Ganoung inventor500@vivaldi.net Reported-by: Yongqin Liu yongqin.liu@linaro.org Signed-off-by: Jose Ignacio Tornos Martinez jtornosm@redhat.com
Reviewed-by: Simon Horman horms@kernel.org
Hello:
This patch was applied to netdev/net.git (main) by Jakub Kicinski kuba@kernel.org:
On Fri, 10 May 2024 11:08:28 +0200 you wrote:
The idea was to keep only one reset at initialization stage in order to reduce the total delay, or the reset from usbnet_probe or the reset from usbnet_open.
I have seen that restarting from usbnet_probe is necessary to avoid doing too complex things. But when the link is set to down/up (for example to configure a different mac address) the link is not correctly recovered unless a reset is commanded from usbnet_open.
[...]
Here is the summary with links: - net: usb: ax88179_178a: fix link status when link is set to down/up https://git.kernel.org/netdev/net/c/ecf848eb934b
You are awesome, thank you!
Hi, Jose
On Fri, 10 May 2024 at 17:09, Jose Ignacio Tornos Martinez jtornosm@redhat.com wrote:
The idea was to keep only one reset at initialization stage in order to reduce the total delay, or the reset from usbnet_probe or the reset from usbnet_open.
I have seen that restarting from usbnet_probe is necessary to avoid doing too complex things. But when the link is set to down/up (for example to configure a different mac address) the link is not correctly recovered unless a reset is commanded from usbnet_open.
So, detect the initialization stage (first call) to not reset from usbnet_open after the reset from usbnet_probe and after this stage, always reset from usbnet_open too (when the link needs to be rechecked).
Apply to all the possible devices, the behavior now is going to be the same.
cc: stable@vger.kernel.org # 6.6+ Fixes: 56f78615bcb1 ("net: usb: ax88179_178a: avoid writing the mac address before first reading") Reported-by: Isaac Ganoung inventor500@vivaldi.net Reported-by: Yongqin Liu yongqin.liu@linaro.org Signed-off-by: Jose Ignacio Tornos Martinez jtornosm@redhat.com
Sorry, I just have a time to test this patch, and it does not help for the issue I reported here: https://lore.kernel.org/all/CAMSo37UN11V8UeDM4cyD+iXyRR1Us53a00e34wTy+zP6vx9...
Here is the serial console log after I cherry picked this change: https://gist.github.com/liuyq/6255f2ccd98fa98ac0ed296a61f49883
Could you please help to check it again? Please let me know if there is anything I could provide for the investigation.
Thanks, Yongqin Liu
drivers/net/usb/ax88179_178a.c | 37 ++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 377be0d9ef14..a0edb410f746 100644 --- a/drivers/net/usb/ax88179_178a.c +++ b/drivers/net/usb/ax88179_178a.c @@ -174,6 +174,7 @@ struct ax88179_data { u32 wol_supported; u32 wolopts; u8 disconnecting;
u8 initialized;};
struct ax88179_int_data { @@ -1672,6 +1673,18 @@ static int ax88179_reset(struct usbnet *dev) return 0; }
+static int ax88179_net_reset(struct usbnet *dev) +{
struct ax88179_data *ax179_data = dev->driver_priv;if (ax179_data->initialized)ax88179_reset(dev);elseax179_data->initialized = 1;return 0;+}
static int ax88179_stop(struct usbnet *dev) { u16 tmp16; @@ -1691,6 +1704,7 @@ static const struct driver_info ax88179_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1703,6 +1717,7 @@ static const struct driver_info ax88178a_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1715,7 +1730,7 @@ static const struct driver_info cypress_GX3_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1728,7 +1743,7 @@ static const struct driver_info dlink_dub1312_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1741,7 +1756,7 @@ static const struct driver_info sitecom_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1754,7 +1769,7 @@ static const struct driver_info samsung_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1767,7 +1782,7 @@ static const struct driver_info lenovo_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1780,7 +1795,7 @@ static const struct driver_info belkin_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1793,7 +1808,7 @@ static const struct driver_info toshiba_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1806,7 +1821,7 @@ static const struct driver_info mct_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1819,7 +1834,7 @@ static const struct driver_info at_umc2000_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1832,7 +1847,7 @@ static const struct driver_info at_umc200_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,@@ -1845,7 +1860,7 @@ static const struct driver_info at_umc2000sp_info = { .unbind = ax88179_unbind, .status = ax88179_status, .link_reset = ax88179_link_reset,
.reset = ax88179_reset,
.reset = ax88179_net_reset, .stop = ax88179_stop, .flags = FLAG_ETHER | FLAG_FRAMING_AX, .rx_fixup = ax88179_rx_fixup,-- 2.44.0
Hello Yongqin,
Again, not a lot of information from the logs, but perhaps you coud give me more information for your scenario.
Could you try to execute the down interface operation, mac assignment and the up interface operation from command line? That works for me.
Maybe some synchronization issue is happening in your boot operation. Could you provide more information about how/when you are doing the commented operations to try to reproduce?
Best regards José Ignacio
Hi, Jose
On Thu, 23 May 2024 at 14:45, Jose Ignacio Tornos Martinez jtornosm@redhat.com wrote:
Hello Yongqin,
Again, not a lot of information from the logs, but perhaps you coud give me more information for your scenario.
Could you try to execute the down interface operation, mac assignment and the up interface operation from command line? That works for me.
When I tried the down and up operations manually from the command line, it worked. But it only worked after I ran the down and up operations after the boot. It fails to work by default after the boot for both the fresh deployment, and for the later reboot
One thing I noticed is that the following message was printed twice "ax88179_178a 2-3:1.0 eth0: ax88179 - Link status is: 1" after I ran the up operation,
Is that expected?
For details, please check the log here: https://gist.github.com/liuyq/be8f5305d538067a344001f1d35f677b
Maybe some synchronization issue is happening in your boot operation. Could you provide more information about how/when you are doing the commented operations to try to reproduce?
The scripts are simple, here are the two scripts for Android build: https://android.googlesource.com/device/linaro/dragonboard/+/refs/heads/main... https://android.googlesource.com/device/linaro/dragonboard/+/refs/heads/main...
Is the one to run the down/change mac/up operations script.
Not sure why the up in the script does not work, but works when run manually.
-- Best Regards, Yongqin Liu --------------------------------------------------------------- #mailing list linaro-android@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-android
Hello Yongqin,
When I tried the down and up operations manually from the command line, it worked. But it only worked after I ran the down and up operations after the boot. It fails to work by default after the boot for both the fresh deployment, and for the later reboot
Ok, so it works as well for you after the initialization.
One thing I noticed is that the following message was printed twice "ax88179_178a 2-3:1.0 eth0: ax88179 - Link status is: 1" after I ran the up operation,
Is that expected?
For details, please check the log here: https://gist.github.com/liuyq/be8f5305d538067a344001f1d35f677b
That is another thing that I am analyzing, to clean those spurious. But they are appearing in my case too, and I am not modifying anything at boot time.
The scripts are simple, here are the two scripts for Android build: https://android.googlesource.com/device/linaro/dragonboard/+/refs/heads/main... https://android.googlesource.com/device/linaro/dragonboard/+/refs/heads/main...
Is the one to run the down/change mac/up operations script.
Not sure why the up in the script does not work, but works when run manually.
Ok, I am not working with Android but it doesn't seem spscial, the only doubt is when the script is executed, if the driver initialization is complete, ... Anyway, I will try to reproduce here and analyze it.
Best regards José Ignacio
On Tue, 28 May 2024 at 17:18, Jose Ignacio Tornos Martinez jtornosm@redhat.com wrote:
Hello Yongqin,
When I tried the down and up operations manually from the command line, it worked. But it only worked after I ran the down and up operations after the boot. It fails to work by default after the boot for both the fresh deployment, and for the later reboot
Ok, so it works as well for you after the initialization.
One thing I noticed is that the following message was printed twice "ax88179_178a 2-3:1.0 eth0: ax88179 - Link status is: 1" after I ran the up operation,
Is that expected?
For details, please check the log here: https://gist.github.com/liuyq/be8f5305d538067a344001f1d35f677b
That is another thing that I am analyzing, to clean those spurious. But they are appearing in my case too, and I am not modifying anything at boot time.
The scripts are simple, here are the two scripts for Android build: https://android.googlesource.com/device/linaro/dragonboard/+/refs/heads/main... https://android.googlesource.com/device/linaro/dragonboard/+/refs/heads/main...
Is the one to run the down/change mac/up operations script.
Not sure why the up in the script does not work, but works when run manually.
Ok, I am not working with Android but it doesn't seem spscial, the only doubt is when the script is executed, if the driver initialization is complete, ...
is there any message that I could check to make sure if the initialization is finished? or like with adding some printk lines for some kernel functions to hack
Anyway, I will try to reproduce here and analyze it.
Thanks very much! And please feel free to let me know if there is anything I could help with on the Android build.
Hello Yongqin,
is there any message that I could check to make sure if the initialization is finished? or like with adding some printk lines for some kernel functions to hack
Anyway, I will try to reproduce here and analyze it.
Thanks very much! And please feel free to let me know if there is anything I could help with on the Android build.
I have finally managed to reproduce an error similar to the one mentioned during the boot stage. I created a systemd service with a similar configuration script to do the same and it works (I can reconfigure the mac address at boot time, the ip address is configured and the interface works), because the driver is completely initialized. In order to reproduce, I have introduced a big delay in the probe operation to get closer in time to the configuration script and the problem is there. Maybe, the script set_ethaddr.sh could be synchronized with the driver, but I think, if possible, it is better to check in a better way in the driver; I will try it. When I have something I can comment you, if you can test it, to be sure about the solution.
By the way, I have tried with my other fix to avoid the spurious link messages, but it didn't help.
Best regards José Ignacio
Hello Yongqin,
After some research and testing, it seems to happen that if initialization is slower the second reset from open is needed too. So, I have been working with some reproducers and I think I have the solution for detecting when there is a problem. If you can test it in your real environment that would be great.
Here the patch on the latest version of the file: $ git diff drivers/net/usb/ax88179_178a.c diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 51c295e1e823..60357796be99 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 { @@ -327,7 +326,8 @@ 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) diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 51c295e1e823..60357796be99 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 { @@ -327,7 +326,8 @@ 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); } }
@@ -1543,6 +1543,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; @@ -1580,6 +1581,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; }
@@ -1678,12 +1681,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; }
In addition, I have fixed the logs to show the link correclty.
If this is ok, I will submit the patch.
Thanks
Best regards José Ignacio
Hello again,
There was a problem copying the patch, sorry, here the good one:
$ git diff drivers/net/usb/ax88179_178a.c diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 51c295e1e823..60357796be99 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 { @@ -327,7 +326,8 @@ 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); } }
@@ -1543,6 +1543,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; @@ -1580,6 +1581,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; }
@@ -1678,12 +1681,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; }
Best regards José Ignacio
Hi, Jose
On Thu, 13 Jun 2024 at 17:59, Jose Ignacio Tornos Martinez jtornosm@redhat.com wrote:
Hello again,
There was a problem copying the patch, sorry, here the good one:
Thanks very much for the work!
I will test it tomorrow, and let you know the result then.
Best regards, Yongqin Liu
$ git diff drivers/net/usb/ax88179_178a.c diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 51c295e1e823..60357796be99 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 { @@ -327,7 +326,8 @@ 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); }}
@@ -1543,6 +1543,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;@@ -1580,6 +1581,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;}
@@ -1678,12 +1681,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);
elseax179_data->initialized = 1;
} return 0;}
Best regards José Ignacio
Hello, Jose
On Thu, 13 Jun 2024 at 19:46, Yongqin Liu yongqin.liu@linaro.org wrote:
Hi, Jose
On Thu, 13 Jun 2024 at 17:59, Jose Ignacio Tornos Martinez jtornosm@redhat.com wrote:
Hello again,
There was a problem copying the patch, sorry, here the good one:
Thanks very much for the work!
I will test it tomorrow, and let you know the result then.
I tested with the ACK android15-6.6 and the android-mainline branches, which have the issue reported, after applying this patch, the network works again now.
Here is the console output from the mainline branch, in case you want to check: https://gist.github.com/liuyq/bd3fdada41411bc89a0cd4acf9ec11cf
Thanks again for all the help!
Best regards, Yongqin Liu
$ git diff drivers/net/usb/ax88179_178a.c diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c index 51c295e1e823..60357796be99 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 { @@ -327,7 +326,8 @@ 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); }}
@@ -1543,6 +1543,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;@@ -1580,6 +1581,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;}
@@ -1678,12 +1681,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);
elseax179_data->initialized = 1;
} return 0;}
Best regards José Ignacio
-- Best Regards, Yongqin Liu
#mailing list linaro-android@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-android
-- Best Regards, Yongqin Liu --------------------------------------------------------------- #mailing list linaro-android@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-android
linux-stable-mirror@lists.linaro.org
-
Jose Ignacio Tornos Martinez -
patchwork-bot+netdevbpf＠kernel.org -
Simon Horman -
Yongqin Liu