Re: [PATCH] spi subsystem: destroy the spi_bitbang workqueue only after the spi master is unregistered

[David Brownell]

On Wednesday 07 March 2007 3:48 pm, Chris Lesiak wrote:
> From: Chris Lesiak <chris.lesiak@xxxxxxxxx>
> 
> This patch fixes a bug in the cleanup of an spi_bitbang bus. 

It's nearly right, but see below.


> @@ -505,28 +499,10 @@ EXPORT_SYMBOL_GPL(spi_bitbang_start);
>   */
>  int spi_bitbang_stop(struct spi_bitbang *bitbang)
>  {
> -	unsigned	limit = 500;
> -
> -	spin_lock_irq(&bitbang->lock);
> -	bitbang->shutdown = 0;
> -	while (!list_empty(&bitbang->queue) && limit--) {
> -		spin_unlock_irq(&bitbang->lock);
> -
> -		dev_dbg(bitbang->master->cdev.dev, "wait for queue\n");
> -		msleep(10);
> -
> -		spin_lock_irq(&bitbang->lock);
> -	}

You completely removed an odious busy-wait, which is good ...

> -	spin_unlock_irq(&bitbang->lock);
> -	if (!list_empty(&bitbang->queue)) {
> -		dev_err(bitbang->master->cdev.dev, "queue didn't empty\n");
> -		return -EBUSY;
> -	}
> +	spi_unregister_master(bitbang->master);

... but right here there should be a WARN_ON(!list_empty(...)) to
flag the corresponding bogosity:  that somehow a request never
completed.

>  
>  	destroy_workqueue(bitbang->workqueue);
>  
> -	spi_unregister_master(bitbang->master);
> -
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(spi_bitbang_stop);
> 
> 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/