Received-SPF: pass (google.com: domain of linux-kernel+bounces-186967-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240510090846.328201-1-jtornosm@redhat.com>
In-Reply-To: <20240510090846.328201-1-jtornosm@redhat.com>
From: Yongqin Liu <yongqin.liu@linaro.org>
Date: Thu, 23 May 2024 12:15:01 +0800
Message-ID: <CAMSo37V5uqJ229iFk-t9DBs-1M5pkWNidM6xZocp4Osi+AOc1g@mail.gmail.com>
Subject: Re: [PATCH] net: usb: ax88179_178a: fix link status when link is set
 to down/up
To: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org, 
	pabeni@redhat.com, linux-usb@vger.kernel.org, netdev@vger.kernel.org, 
	linux-kernel@vger.kernel.org, stable@vger.kernel.org, 
	Isaac Ganoung <inventor500@vivaldi.net>
Content-Type: text/plain; charset="UTF-8"

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+zP6vx935A@mail.gmail.com/

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);
> +       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,
> --
> 2.44.0
>


-- 
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android