Re: [PATCH] usb: dwc3: gadget: synchronize_irq dwc irq in suspend

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

 



Hi All,

On 2019-01-30 09:19, Wang, Yu1 wrote:
>
>> "Wang, Yu1" <yu1.wang@xxxxxxxxx> 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.
> Is there any conflict if dwc3_thread_interrupt and below three functions
> execute by parallel? 
>
> 	dwc3_gadget_run_stop(dwc, false, false);
> 	dwc3_disconnect_gadget(dwc);
> 	__dwc3_gadget_stop(dwc);
>
> That is means the dwc3_thread_interrupt is trying to handle pending
> events but another core is trying to do dwc3 stop/suspend.

Isn't that serialized by dwc->lock?

It quite late in release cycle and the issue is still not fixed. Maybe
it would make sense to revert that change and prepare a proper fix for
the next release?

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland




[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux for Synopsys ARC Processors]    
  • [Linux on Unisoc (RDA Micro) SoCs]     [Linux Actions SoC]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  •   Powered by Linux