Re: [PATCH] usb: max-3421: Use driver data instead of maintaining a list of bound devices

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Oct 18, 2021 at 02:00:55PM +0200, Uwe Kleine-König wrote:
> Instead of maintaining a single-linked list of devices that must be
> searched linearly in .remove() just use spi_set_drvdata() to remember the
> link between the spi device and the driver struct. Then the global list
> and the next member can be dropped.
> 
> This simplifies the driver, reduces the memory footprint and the time to
> search the list. Also it makes obvious that there is always a corresponding
> driver struct for a given device in .remove(), so the error path for
> !max3421_hcd can be dropped, too.
> 
> As a side effect this fixes a data inconsistency when .probe() races with
> itself for a second max3421 device in manipulating max3421_hcd_list. A
> similar race is fixed in .remove(), too.
> 
> Fixes: 2d53139f3162 ("Add support for using a MAX3421E chip as a host driver.")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/usb/host/max3421-hcd.c | 26 +++++---------------------
>  1 file changed, 5 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
> index 59cc1bc7f12f..3e39f62904af 100644
> --- a/drivers/usb/host/max3421-hcd.c
> +++ b/drivers/usb/host/max3421-hcd.c
> @@ -125,8 +125,6 @@ struct max3421_hcd {
>  
>  	struct task_struct *spi_thread;
>  
> -	struct max3421_hcd *next;
> -
>  	enum max3421_rh_state rh_state;
>  	/* lower 16 bits contain port status, upper 16 bits the change mask: */
>  	u32 port_status;
> @@ -174,8 +172,6 @@ struct max3421_ep {
>  	u8 retransmit;			/* packet needs retransmission */
>  };
>  
> -static struct max3421_hcd *max3421_hcd_list;
> -
>  #define MAX3421_FIFO_SIZE	64
>  
>  #define MAX3421_SPI_DIR_RD	0	/* read register from MAX3421 */
> @@ -1881,10 +1877,8 @@ max3421_probe(struct spi_device *spi)
>  		goto error;
>  	}
>  	set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
> -	max3421_hcd = hcd_to_max3421(hcd);

I don't think you should have deleted this line :(

Did you test this?

thanks,

greg k-h