Re: [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context

[Peter Chen <peter.chen@nxp.com>]

On 20-03-10 09:57:00, Alan Stern wrote:
> On Tue, 10 Mar 2020, Peter Chen wrote:
> 
> > From: Peter Chen <peter.chen@nxp.com>
> > 
> > The code calls pm_runtime_get_sync with irq disabled, it causes below
> > warning:
> > 
> > BUG: sleeping function called from invalid context at
> > wer/runtime.c:1075
> > in_atomic(): 1, irqs_disabled(): 128, non_block: 0, pid:
> > er/u8:1
> 
> ...
> 
> > Tested-by: Dmitry Osipenko <digetx@gmail.com>
> > Cc: <stable@vger.kernel.org> #v5.5
> > Fixes: 72dc8df7920f ("usb: chipidea: udc: protect usb interrupt enable")
> > Reported-by: Dmitry Osipenko <digetx@gmail.com>
> > Signed-off-by: Peter Chen <peter.chen@nxp.com>
> > ---
> >  drivers/usb/chipidea/udc.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> > index ffaf46f5d062..1fa587ec52fc 100644
> > --- a/drivers/usb/chipidea/udc.c
> > +++ b/drivers/usb/chipidea/udc.c
> > @@ -1539,9 +1539,11 @@ static void ci_hdrc_gadget_connect(struct usb_gadget *_gadget, int is_active)
> >  		if (ci->driver) {
> >  			hw_device_state(ci, ci->ep0out->qh.dma);
> >  			usb_gadget_set_state(_gadget, USB_STATE_POWERED);
> > +			spin_unlock_irqrestore(&ci->lock, flags);
> >  			usb_udc_vbus_handler(_gadget, true);
> > +		} else {
> > +			spin_unlock_irqrestore(&ci->lock, flags);
> >  		}
> > -		spin_unlock_irqrestore(&ci->lock, flags);
> 
> There's something strange about this patch.
> 
> Do you really know that interrupts will be enabled following the 
> spin_unlock_irqrestore()?  In other words, do you know that interrupts 
> were enabled upon entry to this routine?
> 
> If they were, then why use spin_lock_irqsave/spin_unlock_irqrestore?  
> Why not use spin_lock_irq and spin_unlock_irq?
> 

This function is called at process context, so the interrupt is
enabled, I will use spin_lock_irq, thanks, Alan.

-- 

Thanks,
Peter Chen