Hi, On Fri, Nov 14, 2014 at 07:43:23PM +0000, Paul Zimmerman wrote: > > >>>> @@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev) > > >>>> s3c_hsotg_core_connect(hsotg); > > >>>> spin_unlock_irqrestore(&hsotg->lock, flags); > > >>>> > > >>>> + mutex_unlock(&hsotg->init_mutex); > > >>>> + > > >>>> return ret; > > >>>> } > > >>>> > > >>> Hmm. I can't find any other UDC driver that uses a mutex in its > > >>> suspend/resume functions. Can you explain why this is needed only > > >>> for dwc2? > > >> I've posted this version because I thought you were not convinced that > > >> the patch > > >> "usb: dwc2/gadget: rework suspend/resume code to correctly restore > > >> gadget state" > > >> can add code for initialization and deinitialization in suspend/resume > > >> paths. > > > My problem with that patch was that you were checking the ->enabled > > > flag outside of the spinlock. To address that, you only need to move > > > the check inside of the spinlock. I don't see why a mutex is needed. > > > > It is not that simple. I can add spin_lock() before checking enabled, > > but then > > I would need to spin_unlock() to call regulator_bulk_enable() and > > phy_enable(), > > because both cannot be called from atomic context. This means that the > > spinlock > > in such case will not protect anything and is simply useless. > > Ah, OK. So you're using the mutex instead of the ->enabled flag that you > proposed in the "rework suspend/resume code" patch. So this patch is a > replacement for that one. Somehow I was thinking this patch was on top > of that one. > > So I guess this is OK, but I would like to get Felipe's opinion about > it before we apply this. > > Felipe? I can't think of a better way, I'm afraid :-( -- balbi
Attachment:
signature.asc
Description: Digital signature
