Hi All,
On 2019-01-30 07:53, Felipe Balbi wrote:
> "Wang, Yu1" <yu1.w...@intel.com> writes:
>>>> so it's better move the synchronize_irq() after the
>>>> spin_unlock_irqrestore().
>>>> static int dwc3_suspend_common(struct dwc3 *dwc)
>>>> {
>>>> unsigned long flags;
>>>>
>>>> switch (dwc->dr_mode) {
>>>> case USB_DR_MODE_PERIPHERAL:
>>>> case USB_DR_MODE_OTG:
>>>> spin_lock_irqsave(&dwc->lock, flags);
>>>> dwc3_gadget_suspend(dwc);
>>>> spin_unlock_irqrestore(&dwc->lock, flags);
>>>> synchronize_irq()
>>> indeed, I missed that when I first reviewed the patch. Care to send a fix?
>> With this new change, the dwc3 irq thread will be sync after dwc3
>> stopped, is this irq thread can be handled correctly for this case?
> how about this?
>
> modified drivers/usb/dwc3/gadget.c
> @@ -3373,6 +3373,8 @@ void dwc3_gadget_exit(struct dwc3 *dwc)
> }
>
> int dwc3_gadget_suspend(struct dwc3 *dwc)
> + __releases(dwc->lock)
> + __acquires(dwc->lock)
> {
> if (!dwc->gadget_driver)
> return 0;
> @@ -3381,7 +3383,9 @@ int dwc3_gadget_suspend(struct dwc3 *dwc)
> dwc3_disconnect_gadget(dwc);
> __dwc3_gadget_stop(dwc);
>
> + spin_unlock_irq(dwc->lock);
> synchronize_irq(dwc->irq_gadget);
> + spin_lock_irq(dwc->lock);
>
> return 0;
> }
>
> Not the most elegant solution. I'm open to other suggestions.
Frankly this the same as the previous proposal. spinlock is being
released just after dwc3_gadget_suspend() function, so I see no point in
playing with spinlock at the end of dwc3_gadget_suspend().
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
