Re: [PATCH v3] net: usb: ax88179_178a: avoid failed operations when device is disconnected

[Oliver Neukum]

On 01.12.23 14:26, Jose Ignacio Tornos Martinez wrote:
Hi,

this is much better.
  
> @@ -1661,14 +1668,19 @@ static int ax88179_reset(struct usbnet *dev)
>   
>   static int ax88179_stop(struct usbnet *dev)
>   {
> +	struct ax88179_data *ax179_data = dev->driver_priv;
>   	u16 tmp16;
>   
> +	ax179_data->stopping_unbinding = 1;

This is problematic. ndo_stop() is not limited to disconnection.
It is also used whenever an interface transitions from up to down.

> +
>   	ax88179_read_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE,
>   			 2, 2, &tmp16);
>   	tmp16 &= ~AX_MEDIUM_RECEIVE_EN;
>   	ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_MEDIUM_STATUS_MODE,
>   			  2, 2, &tmp16);
>   
> +	ax179_data->stopping_unbinding = 0;
> +
>   	return 0;
>   }
>   

On a general note, you are going for a belt and suspenders approach.
It seems to me that you have two options.

1. Do as Alan suggested and ignore ENODEV. You'd be acknowledging that
these devices are hotpluggable and therefore -ENODEV is not an error
2. Use only a flag. But if you do that, you are setting it in the wrong
place. It should be set in usbnet_disconnect()

O and, well, this is a very mior issue, but you've introduced a memory
ordering issue. You ought to use smp_wmb() after setting the flag and
smp_rmb() before reading it.

	Regards
		Oliver