* [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context
@ 2020-03-05 1:55 Peter Chen
2020-03-05 20:53 ` Dmitry Osipenko
0 siblings, 1 reply; 6+ messages in thread
From: Peter Chen @ 2020-03-05 1:55 UTC (permalink / raw)
To: linux-usb; +Cc: linux-imx, jun.li, Peter Chen, stable
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
CPU: 1 PID: 37 Comm: kworker/u8:1 Not tainted
20200304-00181-gbebfd2a5be98 #1588
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
Workqueue: ci_otg ci_otg_work
[<c010e8bd>] (unwind_backtrace) from [<c010a315>]
1/0x14)
[<c010a315>] (show_stack) from [<c0987d29>]
5/0x94)
[<c0987d29>] (dump_stack) from [<c013e77f>]
+0xeb/0x118)
[<c013e77f>] (___might_sleep) from [<c052fa1d>]
esume+0x75/0x78)
[<c052fa1d>] (__pm_runtime_resume) from [<c0627a33>]
0x23/0x74)
[<c0627a33>] (ci_udc_pullup) from [<c062fb93>]
nect+0x2b/0xcc)
[<c062fb93>] (usb_gadget_connect) from [<c062769d>]
_connect+0x59/0x104)
[<c062769d>] (ci_hdrc_gadget_connect) from [<c062778b>]
ssion+0x43/0x48)
[<c062778b>] (ci_udc_vbus_session) from [<c062f997>]
s_connect+0x17/0x9c)
[<c062f997>] (usb_gadget_vbus_connect) from [<c062634d>]
bd/0x128)
[<c062634d>] (ci_otg_work) from [<c0134719>]
rk+0x149/0x404)
[<c0134719>] (process_one_work) from [<c0134acb>]
0xf7/0x3bc)
[<c0134acb>] (worker_thread) from [<c0139433>]
x118)
[<c0139433>] (kthread) from [<c01010bd>]
(ret_from_fork+0x11/0x34)
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 94feaecc6059..9d74fe856ce8 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1601,9 +1601,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);
} else {
usb_udc_vbus_handler(_gadget, false);
if (ci->driver)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context
2020-03-05 1:55 [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context Peter Chen
@ 2020-03-05 20:53 ` Dmitry Osipenko
0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Osipenko @ 2020-03-05 20:53 UTC (permalink / raw)
To: Peter Chen, linux-usb; +Cc: linux-imx, jun.li, Peter Chen, stable
05.03.2020 04:55, Peter Chen пишет:
> 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
> CPU: 1 PID: 37 Comm: kworker/u8:1 Not tainted
> 20200304-00181-gbebfd2a5be98 #1588
> Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
> Workqueue: ci_otg ci_otg_work
> [<c010e8bd>] (unwind_backtrace) from [<c010a315>]
> 1/0x14)
> [<c010a315>] (show_stack) from [<c0987d29>]
> 5/0x94)
> [<c0987d29>] (dump_stack) from [<c013e77f>]
> +0xeb/0x118)
> [<c013e77f>] (___might_sleep) from [<c052fa1d>]
> esume+0x75/0x78)
> [<c052fa1d>] (__pm_runtime_resume) from [<c0627a33>]
> 0x23/0x74)
> [<c0627a33>] (ci_udc_pullup) from [<c062fb93>]
> nect+0x2b/0xcc)
> [<c062fb93>] (usb_gadget_connect) from [<c062769d>]
> _connect+0x59/0x104)
> [<c062769d>] (ci_hdrc_gadget_connect) from [<c062778b>]
> ssion+0x43/0x48)
> [<c062778b>] (ci_udc_vbus_session) from [<c062f997>]
> s_connect+0x17/0x9c)
> [<c062f997>] (usb_gadget_vbus_connect) from [<c062634d>]
> bd/0x128)
> [<c062634d>] (ci_otg_work) from [<c0134719>]
> rk+0x149/0x404)
> [<c0134719>] (process_one_work) from [<c0134acb>]
> 0xf7/0x3bc)
> [<c0134acb>] (worker_thread) from [<c0139433>]
> x118)
> [<c0139433>] (kthread) from [<c01010bd>]
> (ret_from_fork+0x11/0x34)
>
> 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 94feaecc6059..9d74fe856ce8 100644
> --- a/drivers/usb/chipidea/udc.c
> +++ b/drivers/usb/chipidea/udc.c
> @@ -1601,9 +1601,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);
> } else {
> usb_udc_vbus_handler(_gadget, false);
> if (ci->driver)
>
Thank you very much!
Tested-by: Dmitry Osipenko <digetx@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context
2020-03-10 6:59 [PATCH 0/1] usb: chipidea: fixes for usb-linus Peter Chen
@ 2020-03-10 6:59 ` Peter Chen
2020-03-10 13:57 ` Alan Stern
0 siblings, 1 reply; 6+ messages in thread
From: Peter Chen @ 2020-03-10 6:59 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Peter Chen, stable
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
CPU: 1 PID: 37 Comm: kworker/u8:1 Not tainted
20200304-00181-gbebfd2a5be98 #1588
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
Workqueue: ci_otg ci_otg_work
[<c010e8bd>] (unwind_backtrace) from [<c010a315>]
1/0x14)
[<c010a315>] (show_stack) from [<c0987d29>]
5/0x94)
[<c0987d29>] (dump_stack) from [<c013e77f>]
+0xeb/0x118)
[<c013e77f>] (___might_sleep) from [<c052fa1d>]
esume+0x75/0x78)
[<c052fa1d>] (__pm_runtime_resume) from [<c0627a33>]
0x23/0x74)
[<c0627a33>] (ci_udc_pullup) from [<c062fb93>]
nect+0x2b/0xcc)
[<c062fb93>] (usb_gadget_connect) from [<c062769d>]
_connect+0x59/0x104)
[<c062769d>] (ci_hdrc_gadget_connect) from [<c062778b>]
ssion+0x43/0x48)
[<c062778b>] (ci_udc_vbus_session) from [<c062f997>]
s_connect+0x17/0x9c)
[<c062f997>] (usb_gadget_vbus_connect) from [<c062634d>]
bd/0x128)
[<c062634d>] (ci_otg_work) from [<c0134719>]
rk+0x149/0x404)
[<c0134719>] (process_one_work) from [<c0134acb>]
0xf7/0x3bc)
[<c0134acb>] (worker_thread) from [<c0139433>]
x118)
[<c0139433>] (kthread) from [<c01010bd>]
(ret_from_fork+0x11/0x34)
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);
} else {
usb_udc_vbus_handler(_gadget, false);
if (ci->driver)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context
2020-03-10 6:59 ` [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context Peter Chen
@ 2020-03-10 13:57 ` Alan Stern
2020-03-11 6:57 ` Peter Chen
0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2020-03-10 13:57 UTC (permalink / raw)
To: Peter Chen; +Cc: gregkh, linux-usb, Peter Chen, stable
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?
Alan Stern
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context
2020-03-10 13:57 ` Alan Stern
@ 2020-03-11 6:57 ` Peter Chen
0 siblings, 0 replies; 6+ messages in thread
From: Peter Chen @ 2020-03-11 6:57 UTC (permalink / raw)
To: Alan Stern
Cc: Peter Chen, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
stable@vger.kernel.org
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context
2020-03-16 3:10 [PATCH 0/1] usb: chipidea: fix for usb-linus Peter Chen
@ 2020-03-16 3:10 ` Peter Chen
0 siblings, 0 replies; 6+ messages in thread
From: Peter Chen @ 2020-03-16 3:10 UTC (permalink / raw)
To: gregkh; +Cc: linux-usb, Peter Chen, stable
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
CPU: 1 PID: 37 Comm: kworker/u8:1 Not tainted
20200304-00181-gbebfd2a5be98 #1588
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
Workqueue: ci_otg ci_otg_work
[<c010e8bd>] (unwind_backtrace) from [<c010a315>]
1/0x14)
[<c010a315>] (show_stack) from [<c0987d29>]
5/0x94)
[<c0987d29>] (dump_stack) from [<c013e77f>]
+0xeb/0x118)
[<c013e77f>] (___might_sleep) from [<c052fa1d>]
esume+0x75/0x78)
[<c052fa1d>] (__pm_runtime_resume) from [<c0627a33>]
0x23/0x74)
[<c0627a33>] (ci_udc_pullup) from [<c062fb93>]
nect+0x2b/0xcc)
[<c062fb93>] (usb_gadget_connect) from [<c062769d>]
_connect+0x59/0x104)
[<c062769d>] (ci_hdrc_gadget_connect) from [<c062778b>]
ssion+0x43/0x48)
[<c062778b>] (ci_udc_vbus_session) from [<c062f997>]
s_connect+0x17/0x9c)
[<c062f997>] (usb_gadget_vbus_connect) from [<c062634d>]
bd/0x128)
[<c062634d>] (ci_otg_work) from [<c0134719>]
rk+0x149/0x404)
[<c0134719>] (process_one_work) from [<c0134acb>]
0xf7/0x3bc)
[<c0134acb>] (worker_thread) from [<c0139433>]
x118)
[<c0139433>] (kthread) from [<c01010bd>]
(ret_from_fork+0x11/0x34)
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 | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index ffaf46f5d062..4c4ac30db498 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1530,18 +1530,19 @@ static const struct usb_ep_ops usb_ep_ops = {
static void ci_hdrc_gadget_connect(struct usb_gadget *_gadget, int is_active)
{
struct ci_hdrc *ci = container_of(_gadget, struct ci_hdrc, gadget);
- unsigned long flags;
if (is_active) {
pm_runtime_get_sync(&_gadget->dev);
hw_device_reset(ci);
- spin_lock_irqsave(&ci->lock, flags);
+ spin_lock_irq(&ci->lock);
if (ci->driver) {
hw_device_state(ci, ci->ep0out->qh.dma);
usb_gadget_set_state(_gadget, USB_STATE_POWERED);
+ spin_unlock_irq(&ci->lock);
usb_udc_vbus_handler(_gadget, true);
+ } else {
+ spin_unlock_irq(&ci->lock);
}
- spin_unlock_irqrestore(&ci->lock, flags);
} else {
usb_udc_vbus_handler(_gadget, false);
if (ci->driver)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-16 3:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-05 1:55 [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context Peter Chen
2020-03-05 20:53 ` Dmitry Osipenko
-- strict thread matches above, loose matches on Subject: below --
2020-03-10 6:59 [PATCH 0/1] usb: chipidea: fixes for usb-linus Peter Chen
2020-03-10 6:59 ` [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context Peter Chen
2020-03-10 13:57 ` Alan Stern
2020-03-11 6:57 ` Peter Chen
2020-03-16 3:10 [PATCH 0/1] usb: chipidea: fix for usb-linus Peter Chen
2020-03-16 3:10 ` [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context Peter Chen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.