On 20-03-10 09:57:00, Alan Stern wrote:
> On Tue, 10 Mar 2020, Peter Chen wrote:
>
> > From: Peter Chen <peter.chen@xxxxxxx>
> >
> > 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@xxxxxxxxx>
> > Cc: <stable@xxxxxxxxxxxxxxx> #v5.5
> > Fixes: 72dc8df7920f ("usb: chipidea: udc: protect usb interrupt enable")
> > Reported-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> > Signed-off-by: Peter Chen <peter.chen@xxxxxxx>
> > ---
> > 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
