Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Message-ID: <020ff11184bb22909287ef68d97c00f7d2c73bd6.camel@redhat.com>
Subject: Re: [PATCH 2/2] net: usb: ax88179_178a: avoid two consecutive
 device resets
From:   Paolo Abeni <pabeni@redhat.com>
To:     Jose Ignacio Tornos Martinez <jtornosm@redhat.com>,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        linux-usb@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     weihao.bj@ieisystem.com
Date:   Thu, 16 Nov 2023 10:42:42 +0100
In-Reply-To: <20231114125111.313229-3-jtornosm@redhat.com>
References: <20231114125111.313229-1-jtornosm@redhat.com>
         <20231114125111.313229-3-jtornosm@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.46.4 (3.46.4-1.fc37) 
MIME-Version: 1.0
Precedence: bulk

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 fro=
m
> usbnet_open.
>=20
> Let only the reset during the device binding to prepare the device as soo=
n
> as possible and not repeat the reset operation (tested with generic ASIX
> Electronics Corp. AX88179 Gigabit Ethernet device).
>=20
> 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(-)
>=20
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178=
a.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 =3D {
>  	.unbind =3D ax88179_unbind,
>  	.status =3D ax88179_status,
>  	.link_reset =3D ax88179_link_reset,
> -	.reset =3D ax88179_reset,
>  	.stop =3D ax88179_stop,
>  	.flags =3D FLAG_ETHER | FLAG_FRAMING_AX,
>  	.rx_fixup =3D 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