All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] usb: chipidea: fix for usb-linus
@ 2020-03-16  3:10 Peter Chen
  2020-03-16  3:10 ` [PATCH 1/1] usb: chipidea: udc: fix sleeping function called from invalid context Peter Chen
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Chen @ 2020-03-16  3:10 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, Peter Chen

From: Peter Chen <peter.chen@nxp.com>

Peter Chen (1):
  usb: chipidea: udc: fix sleeping function called from invalid context

 drivers/usb/chipidea/udc.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ 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; 7+ 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] 7+ 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; 7+ messages in thread
From: Peter Chen @ 2020-03-11  6:57 UTC (permalink / raw)
  To: Alan Stern; +Cc: Peter Chen, gregkh, linux-usb, stable

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] 7+ 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; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread

* Re: [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, 0 replies; 7+ 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] 7+ messages in thread

* [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; 7+ 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] 7+ messages in thread

end of thread, other threads:[~2020-03-16  3:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
  -- 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-05  1:55 Peter Chen
2020-03-05 20:53 ` Dmitry Osipenko

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.