Re: [PATCH] usb: gadget: udc-core: move sysfs_notify() to a workqueue

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 18 Jul 2013, Felipe Balbi wrote:

> usb_gadget_set_state() will call sysfs_notify()
> which might sleep. Some users might want to call
> usb_gadget_set_state() from the very IRQ handler
> which actually changes the gadget state.
> 
> Instead of having every UDC driver add their own
> workqueue for such a simple notification, we're
> adding it generically to our struct usb_gadget,
> so the details are hidden from all UDC drivers.
> 
> Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> ---
> 
> Tested on OMAP5 uEVM.
> 
>  drivers/usb/gadget/udc-core.c | 11 ++++++++++-
>  include/linux/usb/gadget.h    |  4 ++++
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
> index ffd8fa5..b0d91b1 100644
> --- a/drivers/usb/gadget/udc-core.c
> +++ b/drivers/usb/gadget/udc-core.c
> @@ -23,6 +23,7 @@
>  #include <linux/list.h>
>  #include <linux/err.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/workqueue.h>
>  
>  #include <linux/usb/ch9.h>
>  #include <linux/usb/gadget.h>
> @@ -101,11 +102,18 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
>  
>  /* ------------------------------------------------------------------------- */
>  
> +static void usb_gadget_state_work(struct work_struct *work)
> +{
> +	struct usb_gadget	*gadget = work_to_gadget(work);
> +
> +	sysfs_notify(&gadget->dev.kobj, NULL, "status");
> +}
> +
>  void usb_gadget_set_state(struct usb_gadget *gadget,
>  		enum usb_device_state state)
>  {
>  	gadget->state = state;
> -	sysfs_notify(&gadget->dev.kobj, NULL, "status");
> +	schedule_work(&gadget->work);
>  }
>  EXPORT_SYMBOL_GPL(usb_gadget_set_state);
>  
> @@ -192,6 +200,7 @@ int usb_add_gadget_udc_release(struct device *parent, struct usb_gadget *gadget,
>  		goto err1;
>  
>  	dev_set_name(&gadget->dev, "gadget");
> +	INIT_WORK(&gadget->work, usb_gadget_state_work);
>  	gadget->dev.parent = parent;
>  
>  	dma_set_coherent_mask(&gadget->dev, parent->coherent_dma_mask);

Deallocation of the gadget structure races with the work routine.  You 
need to wait for any scheduled work to complete when the gadget is 
unregistered.

Also, what happens if two state transitions occur before the work queue 
gets around to executing the work routine?

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux