* [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the spin lock
@ 2023-05-04 13:16 Xiaolei Wang
[not found] ` <2023050503-handful-shore-ae11@gregkh>
2023-06-14 6:14 ` Pawel Laszczak
0 siblings, 2 replies; 5+ messages in thread
From: Xiaolei Wang @ 2023-05-04 13:16 UTC (permalink / raw)
To: peter.chen, pawell, rogerq, a-govindraju, gregkh; +Cc: linux-usb, linux-kernel
The device may be scheduled during the resume process,
so this cannot appear in atomic operations. Since
pm_runtime_set_active will resume suppliers, put set
active outside the spin lock, which is only used to
protect the struct cdns data structure, otherwise the
kernel will report the following warning:
BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1163
in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 651, name: sh
preempt_count: 1, expected: 0
RCU nest depth: 0, expected: 0
CPU: 0 PID: 651 Comm: sh Tainted: G WC 6.1.20 #1
Hardware name: Freescale i.MX8QM MEK (DT)
Call trace:
dump_backtrace.part.0+0xe0/0xf0
show_stack+0x18/0x30
dump_stack_lvl+0x64/0x80
dump_stack+0x1c/0x38
__might_resched+0x1fc/0x240
__might_sleep+0x68/0xc0
__pm_runtime_resume+0x9c/0xe0
rpm_get_suppliers+0x68/0x1b0
__pm_runtime_set_status+0x298/0x560
cdns_resume+0xb0/0x1c0
cdns3_controller_resume.isra.0+0x1e0/0x250
cdns3_plat_resume+0x28/0x40
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
drivers/usb/cdns3/cdns3-plat.c | 3 ++-
drivers/usb/cdns3/cdnsp-pci.c | 3 ++-
drivers/usb/cdns3/core.c | 14 +++++++++++---
drivers/usb/cdns3/core.h | 3 ++-
4 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
index 2bc5d094548b..726b2e4f67e4 100644
--- a/drivers/usb/cdns3/cdns3-plat.c
+++ b/drivers/usb/cdns3/cdns3-plat.c
@@ -256,9 +256,10 @@ static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
cdns3_set_platform_suspend(cdns->dev, false, false);
spin_lock_irqsave(&cdns->lock, flags);
- cdns_resume(cdns, !PMSG_IS_AUTO(msg));
+ cdns_resume(cdns);
cdns->in_lpm = false;
spin_unlock_irqrestore(&cdns->lock, flags);
+ cdns_set_active(cdns, !PMSG_IS_AUTO(msg));
if (cdns->wakeup_pending) {
cdns->wakeup_pending = false;
enable_irq(cdns->wakeup_irq);
diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
index 7b151f5af3cc..0725668ffea4 100644
--- a/drivers/usb/cdns3/cdnsp-pci.c
+++ b/drivers/usb/cdns3/cdnsp-pci.c
@@ -208,8 +208,9 @@ static int __maybe_unused cdnsp_pci_resume(struct device *dev)
int ret;
spin_lock_irqsave(&cdns->lock, flags);
- ret = cdns_resume(cdns, 1);
+ ret = cdns_resume(cdns);
spin_unlock_irqrestore(&cdns->lock, flags);
+ cdns_set_active(cdns, 1);
return ret;
}
diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
index dbcdf3b24b47..661d5597fb68 100644
--- a/drivers/usb/cdns3/core.c
+++ b/drivers/usb/cdns3/core.c
@@ -522,7 +522,7 @@ int cdns_suspend(struct cdns *cdns)
}
EXPORT_SYMBOL_GPL(cdns_suspend);
-int cdns_resume(struct cdns *cdns, u8 set_active)
+int cdns_resume(struct cdns *cdns)
{
struct device *dev = cdns->dev;
enum usb_role real_role;
@@ -556,15 +556,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
if (cdns->roles[cdns->role]->resume)
cdns->roles[cdns->role]->resume(cdns, cdns_power_is_lost(cdns));
+ return 0;
+}
+EXPORT_SYMBOL_GPL(cdns_resume);
+
+void cdns_set_active(struct cdns *cdns, u8 set_active)
+{
+ struct device *dev = cdns->dev;
+
if (set_active) {
pm_runtime_disable(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
}
- return 0;
+ return;
}
-EXPORT_SYMBOL_GPL(cdns_resume);
+EXPORT_SYMBOL_GPL(cdns_set_active);
#endif /* CONFIG_PM_SLEEP */
MODULE_AUTHOR("Peter Chen <peter.chen@nxp.com>");
diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
index 2d332a788871..0f429042f997 100644
--- a/drivers/usb/cdns3/core.h
+++ b/drivers/usb/cdns3/core.h
@@ -125,8 +125,9 @@ int cdns_init(struct cdns *cdns);
int cdns_remove(struct cdns *cdns);
#ifdef CONFIG_PM_SLEEP
-int cdns_resume(struct cdns *cdns, u8 set_active);
+int cdns_resume(struct cdns *cdns);
int cdns_suspend(struct cdns *cdns);
+void cdns_set_active(struct cdns *cdns, u8 set_active);
#else /* CONFIG_PM_SLEEP */
static inline int cdns_resume(struct cdns *cdns, u8 set_active)
{ return 0; }
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the spin lock
[not found] ` <2023050503-handful-shore-ae11@gregkh>
@ 2023-06-14 3:16 ` wangxiaolei
2023-06-15 9:49 ` Greg KH
0 siblings, 1 reply; 5+ messages in thread
From: wangxiaolei @ 2023-06-14 3:16 UTC (permalink / raw)
To: peter.chen, pawell, rogerq, a-govindraju, gregkh, linux-usb,
linux-kernel
On 5/5/23 6:33 PM, Greg KH wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Thu, May 04, 2023 at 09:16:02PM +0800, Xiaolei Wang wrote:
>> The device may be scheduled during the resume process,
>> so this cannot appear in atomic operations. Since
>> pm_runtime_set_active will resume suppliers, put set
>> active outside the spin lock, which is only used to
>> protect the struct cdns data structure, otherwise the
>> kernel will report the following warning:
>>
>> BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1163
>> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 651, name: sh
>> preempt_count: 1, expected: 0
>> RCU nest depth: 0, expected: 0
>> CPU: 0 PID: 651 Comm: sh Tainted: G WC 6.1.20 #1
>> Hardware name: Freescale i.MX8QM MEK (DT)
>> Call trace:
>> dump_backtrace.part.0+0xe0/0xf0
>> show_stack+0x18/0x30
>> dump_stack_lvl+0x64/0x80
>> dump_stack+0x1c/0x38
>> __might_resched+0x1fc/0x240
>> __might_sleep+0x68/0xc0
>> __pm_runtime_resume+0x9c/0xe0
>> rpm_get_suppliers+0x68/0x1b0
>> __pm_runtime_set_status+0x298/0x560
>> cdns_resume+0xb0/0x1c0
>> cdns3_controller_resume.isra.0+0x1e0/0x250
>> cdns3_plat_resume+0x28/0x40
>>
>> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
>> ---
>> drivers/usb/cdns3/cdns3-plat.c | 3 ++-
>> drivers/usb/cdns3/cdnsp-pci.c | 3 ++-
>> drivers/usb/cdns3/core.c | 14 +++++++++++---
>> drivers/usb/cdns3/core.h | 3 ++-
>> 4 files changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
>> index 2bc5d094548b..726b2e4f67e4 100644
>> --- a/drivers/usb/cdns3/cdns3-plat.c
>> +++ b/drivers/usb/cdns3/cdns3-plat.c
>> @@ -256,9 +256,10 @@ static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
>> cdns3_set_platform_suspend(cdns->dev, false, false);
>>
>> spin_lock_irqsave(&cdns->lock, flags);
>> - cdns_resume(cdns, !PMSG_IS_AUTO(msg));
>> + cdns_resume(cdns);
>> cdns->in_lpm = false;
>> spin_unlock_irqrestore(&cdns->lock, flags);
>> + cdns_set_active(cdns, !PMSG_IS_AUTO(msg));
>> if (cdns->wakeup_pending) {
>> cdns->wakeup_pending = false;
>> enable_irq(cdns->wakeup_irq);
>> diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
>> index 7b151f5af3cc..0725668ffea4 100644
>> --- a/drivers/usb/cdns3/cdnsp-pci.c
>> +++ b/drivers/usb/cdns3/cdnsp-pci.c
>> @@ -208,8 +208,9 @@ static int __maybe_unused cdnsp_pci_resume(struct device *dev)
>> int ret;
>>
>> spin_lock_irqsave(&cdns->lock, flags);
>> - ret = cdns_resume(cdns, 1);
>> + ret = cdns_resume(cdns);
>> spin_unlock_irqrestore(&cdns->lock, flags);
>> + cdns_set_active(cdns, 1);
>>
>> return ret;
>> }
>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> index dbcdf3b24b47..661d5597fb68 100644
>> --- a/drivers/usb/cdns3/core.c
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -522,7 +522,7 @@ int cdns_suspend(struct cdns *cdns)
>> }
>> EXPORT_SYMBOL_GPL(cdns_suspend);
>>
>> -int cdns_resume(struct cdns *cdns, u8 set_active)
>> +int cdns_resume(struct cdns *cdns)
>> {
>> struct device *dev = cdns->dev;
>> enum usb_role real_role;
>> @@ -556,15 +556,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
>> if (cdns->roles[cdns->role]->resume)
>> cdns->roles[cdns->role]->resume(cdns, cdns_power_is_lost(cdns));
>>
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(cdns_resume);
>> +
>> +void cdns_set_active(struct cdns *cdns, u8 set_active)
>> +{
>> + struct device *dev = cdns->dev;
>> +
>> if (set_active) {
>> pm_runtime_disable(dev);
>> pm_runtime_set_active(dev);
>> pm_runtime_enable(dev);
>> }
>>
>> - return 0;
>> + return;
>> }
>> -EXPORT_SYMBOL_GPL(cdns_resume);
>> +EXPORT_SYMBOL_GPL(cdns_set_active);
>> #endif /* CONFIG_PM_SLEEP */
>>
>> MODULE_AUTHOR("Peter Chen <peter.chen@nxp.com>");
>> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>> index 2d332a788871..0f429042f997 100644
>> --- a/drivers/usb/cdns3/core.h
>> +++ b/drivers/usb/cdns3/core.h
>> @@ -125,8 +125,9 @@ int cdns_init(struct cdns *cdns);
>> int cdns_remove(struct cdns *cdns);
>>
>> #ifdef CONFIG_PM_SLEEP
>> -int cdns_resume(struct cdns *cdns, u8 set_active);
>> +int cdns_resume(struct cdns *cdns);
>> int cdns_suspend(struct cdns *cdns);
>> +void cdns_set_active(struct cdns *cdns, u8 set_active);
>> #else /* CONFIG_PM_SLEEP */
>> static inline int cdns_resume(struct cdns *cdns, u8 set_active)
>> { return 0; }
>> --
>> 2.25.1
>>
>
> Hi,
>
> This is the friendly semi-automated patch-bot of Greg Kroah-Hartman.
> You have sent him a patch that has triggered this response.
>
> Right now, the development tree you have sent a patch for is "closed"
> due to the timing of the merge window. Don't worry, the patch(es) you
> have sent are not lost, and will be looked at after the merge window is
> over (after the -rc1 kernel is released by Linus).
>
> So thank you for your patience and your patches will be reviewed at this
> later time, you do not have to do anything further, this is just a short
> note to let you know the patch status and so you don't worry they didn't
> make it through.
Can someone help review this patch?
thanks
xiaolei
>
> thanks,
>
> greg k-h's patch email bot
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the spin lock
2023-05-04 13:16 [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the spin lock Xiaolei Wang
[not found] ` <2023050503-handful-shore-ae11@gregkh>
@ 2023-06-14 6:14 ` Pawel Laszczak
1 sibling, 0 replies; 5+ messages in thread
From: Pawel Laszczak @ 2023-06-14 6:14 UTC (permalink / raw)
To: Xiaolei Wang, peter.chen, rogerq, a-govindraju, gregkh
Cc: linux-usb, linux-kernel
>The device may be scheduled during the resume process, so this cannot
>appear in atomic operations. Since pm_runtime_set_active will resume
>suppliers, put set active outside the spin lock, which is only used to protect the
>struct cdns data structure, otherwise the kernel will report the following
>warning:
>
> BUG: sleeping function called from invalid context at
>drivers/base/power/runtime.c:1163
> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 651, name: sh
> preempt_count: 1, expected: 0
> RCU nest depth: 0, expected: 0
> CPU: 0 PID: 651 Comm: sh Tainted: G WC 6.1.20 #1
> Hardware name: Freescale i.MX8QM MEK (DT)
> Call trace:
> dump_backtrace.part.0+0xe0/0xf0
> show_stack+0x18/0x30
> dump_stack_lvl+0x64/0x80
> dump_stack+0x1c/0x38
> __might_resched+0x1fc/0x240
> __might_sleep+0x68/0xc0
> __pm_runtime_resume+0x9c/0xe0
> rpm_get_suppliers+0x68/0x1b0
> __pm_runtime_set_status+0x298/0x560
> cdns_resume+0xb0/0x1c0
> cdns3_controller_resume.isra.0+0x1e0/0x250
> cdns3_plat_resume+0x28/0x40
>
>Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
Reviewed-by: Pawel Laszczak <pawell@cadence.com>
>---
> drivers/usb/cdns3/cdns3-plat.c | 3 ++- drivers/usb/cdns3/cdnsp-pci.c | 3
>++-
> drivers/usb/cdns3/core.c | 14 +++++++++++---
> drivers/usb/cdns3/core.h | 3 ++-
> 4 files changed, 17 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
>index 2bc5d094548b..726b2e4f67e4 100644
>--- a/drivers/usb/cdns3/cdns3-plat.c
>+++ b/drivers/usb/cdns3/cdns3-plat.c
>@@ -256,9 +256,10 @@ static int cdns3_controller_resume(struct device
>*dev, pm_message_t msg)
> cdns3_set_platform_suspend(cdns->dev, false, false);
>
> spin_lock_irqsave(&cdns->lock, flags);
>- cdns_resume(cdns, !PMSG_IS_AUTO(msg));
>+ cdns_resume(cdns);
> cdns->in_lpm = false;
> spin_unlock_irqrestore(&cdns->lock, flags);
>+ cdns_set_active(cdns, !PMSG_IS_AUTO(msg));
> if (cdns->wakeup_pending) {
> cdns->wakeup_pending = false;
> enable_irq(cdns->wakeup_irq);
>diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
>index 7b151f5af3cc..0725668ffea4 100644
>--- a/drivers/usb/cdns3/cdnsp-pci.c
>+++ b/drivers/usb/cdns3/cdnsp-pci.c
>@@ -208,8 +208,9 @@ static int __maybe_unused cdnsp_pci_resume(struct
>device *dev)
> int ret;
>
> spin_lock_irqsave(&cdns->lock, flags);
>- ret = cdns_resume(cdns, 1);
>+ ret = cdns_resume(cdns);
> spin_unlock_irqrestore(&cdns->lock, flags);
>+ cdns_set_active(cdns, 1);
>
> return ret;
> }
>diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c index
>dbcdf3b24b47..661d5597fb68 100644
>--- a/drivers/usb/cdns3/core.c
>+++ b/drivers/usb/cdns3/core.c
>@@ -522,7 +522,7 @@ int cdns_suspend(struct cdns *cdns) }
>EXPORT_SYMBOL_GPL(cdns_suspend);
>
>-int cdns_resume(struct cdns *cdns, u8 set_active)
>+int cdns_resume(struct cdns *cdns)
> {
> struct device *dev = cdns->dev;
> enum usb_role real_role;
>@@ -556,15 +556,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
> if (cdns->roles[cdns->role]->resume)
> cdns->roles[cdns->role]->resume(cdns,
>cdns_power_is_lost(cdns));
>
>+ return 0;
>+}
>+EXPORT_SYMBOL_GPL(cdns_resume);
>+
>+void cdns_set_active(struct cdns *cdns, u8 set_active) {
>+ struct device *dev = cdns->dev;
>+
> if (set_active) {
> pm_runtime_disable(dev);
> pm_runtime_set_active(dev);
> pm_runtime_enable(dev);
> }
>
>- return 0;
>+ return;
> }
>-EXPORT_SYMBOL_GPL(cdns_resume);
>+EXPORT_SYMBOL_GPL(cdns_set_active);
> #endif /* CONFIG_PM_SLEEP */
>
> MODULE_AUTHOR("Peter Chen <peter.chen@nxp.com>"); diff --git
>a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h index
>2d332a788871..0f429042f997 100644
>--- a/drivers/usb/cdns3/core.h
>+++ b/drivers/usb/cdns3/core.h
>@@ -125,8 +125,9 @@ int cdns_init(struct cdns *cdns); int
>cdns_remove(struct cdns *cdns);
>
> #ifdef CONFIG_PM_SLEEP
>-int cdns_resume(struct cdns *cdns, u8 set_active);
>+int cdns_resume(struct cdns *cdns);
> int cdns_suspend(struct cdns *cdns);
>+void cdns_set_active(struct cdns *cdns, u8 set_active);
> #else /* CONFIG_PM_SLEEP */
> static inline int cdns_resume(struct cdns *cdns, u8 set_active) { return 0; }
>--
>2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the spin lock
2023-06-14 3:16 ` wangxiaolei
@ 2023-06-15 9:49 ` Greg KH
2023-06-15 10:14 ` wang xiaolei
0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2023-06-15 9:49 UTC (permalink / raw)
To: wangxiaolei
Cc: peter.chen, pawell, rogerq, a-govindraju, linux-usb, linux-kernel
On Wed, Jun 14, 2023 at 11:16:05AM +0800, wangxiaolei wrote:
>
> On 5/5/23 6:33 PM, Greg KH wrote:
> > CAUTION: This email comes from a non Wind River email account!
> > Do not click links or open attachments unless you recognize the sender and know the content is safe.
> >
> > On Thu, May 04, 2023 at 09:16:02PM +0800, Xiaolei Wang wrote:
> > > The device may be scheduled during the resume process,
> > > so this cannot appear in atomic operations. Since
> > > pm_runtime_set_active will resume suppliers, put set
> > > active outside the spin lock, which is only used to
> > > protect the struct cdns data structure, otherwise the
> > > kernel will report the following warning:
> > >
> > > BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1163
> > > in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 651, name: sh
> > > preempt_count: 1, expected: 0
> > > RCU nest depth: 0, expected: 0
> > > CPU: 0 PID: 651 Comm: sh Tainted: G WC 6.1.20 #1
> > > Hardware name: Freescale i.MX8QM MEK (DT)
> > > Call trace:
> > > dump_backtrace.part.0+0xe0/0xf0
> > > show_stack+0x18/0x30
> > > dump_stack_lvl+0x64/0x80
> > > dump_stack+0x1c/0x38
> > > __might_resched+0x1fc/0x240
> > > __might_sleep+0x68/0xc0
> > > __pm_runtime_resume+0x9c/0xe0
> > > rpm_get_suppliers+0x68/0x1b0
> > > __pm_runtime_set_status+0x298/0x560
> > > cdns_resume+0xb0/0x1c0
> > > cdns3_controller_resume.isra.0+0x1e0/0x250
> > > cdns3_plat_resume+0x28/0x40
> > >
> > > Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> > > ---
> > > drivers/usb/cdns3/cdns3-plat.c | 3 ++-
> > > drivers/usb/cdns3/cdnsp-pci.c | 3 ++-
> > > drivers/usb/cdns3/core.c | 14 +++++++++++---
> > > drivers/usb/cdns3/core.h | 3 ++-
> > > 4 files changed, 17 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
> > > index 2bc5d094548b..726b2e4f67e4 100644
> > > --- a/drivers/usb/cdns3/cdns3-plat.c
> > > +++ b/drivers/usb/cdns3/cdns3-plat.c
> > > @@ -256,9 +256,10 @@ static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
> > > cdns3_set_platform_suspend(cdns->dev, false, false);
> > >
> > > spin_lock_irqsave(&cdns->lock, flags);
> > > - cdns_resume(cdns, !PMSG_IS_AUTO(msg));
> > > + cdns_resume(cdns);
> > > cdns->in_lpm = false;
> > > spin_unlock_irqrestore(&cdns->lock, flags);
> > > + cdns_set_active(cdns, !PMSG_IS_AUTO(msg));
> > > if (cdns->wakeup_pending) {
> > > cdns->wakeup_pending = false;
> > > enable_irq(cdns->wakeup_irq);
> > > diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
> > > index 7b151f5af3cc..0725668ffea4 100644
> > > --- a/drivers/usb/cdns3/cdnsp-pci.c
> > > +++ b/drivers/usb/cdns3/cdnsp-pci.c
> > > @@ -208,8 +208,9 @@ static int __maybe_unused cdnsp_pci_resume(struct device *dev)
> > > int ret;
> > >
> > > spin_lock_irqsave(&cdns->lock, flags);
> > > - ret = cdns_resume(cdns, 1);
> > > + ret = cdns_resume(cdns);
> > > spin_unlock_irqrestore(&cdns->lock, flags);
> > > + cdns_set_active(cdns, 1);
> > >
> > > return ret;
> > > }
> > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> > > index dbcdf3b24b47..661d5597fb68 100644
> > > --- a/drivers/usb/cdns3/core.c
> > > +++ b/drivers/usb/cdns3/core.c
> > > @@ -522,7 +522,7 @@ int cdns_suspend(struct cdns *cdns)
> > > }
> > > EXPORT_SYMBOL_GPL(cdns_suspend);
> > >
> > > -int cdns_resume(struct cdns *cdns, u8 set_active)
> > > +int cdns_resume(struct cdns *cdns)
> > > {
> > > struct device *dev = cdns->dev;
> > > enum usb_role real_role;
> > > @@ -556,15 +556,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
> > > if (cdns->roles[cdns->role]->resume)
> > > cdns->roles[cdns->role]->resume(cdns, cdns_power_is_lost(cdns));
> > >
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(cdns_resume);
> > > +
> > > +void cdns_set_active(struct cdns *cdns, u8 set_active)
> > > +{
> > > + struct device *dev = cdns->dev;
> > > +
> > > if (set_active) {
> > > pm_runtime_disable(dev);
> > > pm_runtime_set_active(dev);
> > > pm_runtime_enable(dev);
> > > }
> > >
> > > - return 0;
> > > + return;
> > > }
> > > -EXPORT_SYMBOL_GPL(cdns_resume);
> > > +EXPORT_SYMBOL_GPL(cdns_set_active);
> > > #endif /* CONFIG_PM_SLEEP */
> > >
> > > MODULE_AUTHOR("Peter Chen <peter.chen@nxp.com>");
> > > diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
> > > index 2d332a788871..0f429042f997 100644
> > > --- a/drivers/usb/cdns3/core.h
> > > +++ b/drivers/usb/cdns3/core.h
> > > @@ -125,8 +125,9 @@ int cdns_init(struct cdns *cdns);
> > > int cdns_remove(struct cdns *cdns);
> > >
> > > #ifdef CONFIG_PM_SLEEP
> > > -int cdns_resume(struct cdns *cdns, u8 set_active);
> > > +int cdns_resume(struct cdns *cdns);
> > > int cdns_suspend(struct cdns *cdns);
> > > +void cdns_set_active(struct cdns *cdns, u8 set_active);
> > > #else /* CONFIG_PM_SLEEP */
> > > static inline int cdns_resume(struct cdns *cdns, u8 set_active)
> > > { return 0; }
> > > --
> > > 2.25.1
> > >
> >
> > Hi,
> >
> > This is the friendly semi-automated patch-bot of Greg Kroah-Hartman.
> > You have sent him a patch that has triggered this response.
> >
> > Right now, the development tree you have sent a patch for is "closed"
> > due to the timing of the merge window. Don't worry, the patch(es) you
> > have sent are not lost, and will be looked at after the merge window is
> > over (after the -rc1 kernel is released by Linus).
> >
> > So thank you for your patience and your patches will be reviewed at this
> > later time, you do not have to do anything further, this is just a short
> > note to let you know the patch status and so you don't worry they didn't
> > make it through.
>
> Can someone help review this patch?
It breaks the build:
drivers/usb/cdns3/core.c: In function ‘cdns_resume’:
drivers/usb/cdns3/core.c:527:24: error: unused variable ‘dev’ [-Werror=unused-variable]
527 | struct device *dev = cdns->dev;
| ^~~
How did you test it?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the spin lock
2023-06-15 9:49 ` Greg KH
@ 2023-06-15 10:14 ` wang xiaolei
0 siblings, 0 replies; 5+ messages in thread
From: wang xiaolei @ 2023-06-15 10:14 UTC (permalink / raw)
To: Greg KH; +Cc: peter.chen, pawell, rogerq, a-govindraju, linux-usb, linux-kernel
On 6/15/23 8:49 PM, Greg KH wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Wed, Jun 14, 2023 at 11:16:05AM +0800, wangxiaolei wrote:
>> On 5/5/23 6:33 PM, Greg KH wrote:
>>> CAUTION: This email comes from a non Wind River email account!
>>> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>>>
>>> On Thu, May 04, 2023 at 09:16:02PM +0800, Xiaolei Wang wrote:
>>>> The device may be scheduled during the resume process,
>>>> so this cannot appear in atomic operations. Since
>>>> pm_runtime_set_active will resume suppliers, put set
>>>> active outside the spin lock, which is only used to
>>>> protect the struct cdns data structure, otherwise the
>>>> kernel will report the following warning:
>>>>
>>>> BUG: sleeping function called from invalid context at drivers/base/power/runtime.c:1163
>>>> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 651, name: sh
>>>> preempt_count: 1, expected: 0
>>>> RCU nest depth: 0, expected: 0
>>>> CPU: 0 PID: 651 Comm: sh Tainted: G WC 6.1.20 #1
>>>> Hardware name: Freescale i.MX8QM MEK (DT)
>>>> Call trace:
>>>> dump_backtrace.part.0+0xe0/0xf0
>>>> show_stack+0x18/0x30
>>>> dump_stack_lvl+0x64/0x80
>>>> dump_stack+0x1c/0x38
>>>> __might_resched+0x1fc/0x240
>>>> __might_sleep+0x68/0xc0
>>>> __pm_runtime_resume+0x9c/0xe0
>>>> rpm_get_suppliers+0x68/0x1b0
>>>> __pm_runtime_set_status+0x298/0x560
>>>> cdns_resume+0xb0/0x1c0
>>>> cdns3_controller_resume.isra.0+0x1e0/0x250
>>>> cdns3_plat_resume+0x28/0x40
>>>>
>>>> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
>>>> ---
>>>> drivers/usb/cdns3/cdns3-plat.c | 3 ++-
>>>> drivers/usb/cdns3/cdnsp-pci.c | 3 ++-
>>>> drivers/usb/cdns3/core.c | 14 +++++++++++---
>>>> drivers/usb/cdns3/core.h | 3 ++-
>>>> 4 files changed, 17 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c
>>>> index 2bc5d094548b..726b2e4f67e4 100644
>>>> --- a/drivers/usb/cdns3/cdns3-plat.c
>>>> +++ b/drivers/usb/cdns3/cdns3-plat.c
>>>> @@ -256,9 +256,10 @@ static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
>>>> cdns3_set_platform_suspend(cdns->dev, false, false);
>>>>
>>>> spin_lock_irqsave(&cdns->lock, flags);
>>>> - cdns_resume(cdns, !PMSG_IS_AUTO(msg));
>>>> + cdns_resume(cdns);
>>>> cdns->in_lpm = false;
>>>> spin_unlock_irqrestore(&cdns->lock, flags);
>>>> + cdns_set_active(cdns, !PMSG_IS_AUTO(msg));
>>>> if (cdns->wakeup_pending) {
>>>> cdns->wakeup_pending = false;
>>>> enable_irq(cdns->wakeup_irq);
>>>> diff --git a/drivers/usb/cdns3/cdnsp-pci.c b/drivers/usb/cdns3/cdnsp-pci.c
>>>> index 7b151f5af3cc..0725668ffea4 100644
>>>> --- a/drivers/usb/cdns3/cdnsp-pci.c
>>>> +++ b/drivers/usb/cdns3/cdnsp-pci.c
>>>> @@ -208,8 +208,9 @@ static int __maybe_unused cdnsp_pci_resume(struct device *dev)
>>>> int ret;
>>>>
>>>> spin_lock_irqsave(&cdns->lock, flags);
>>>> - ret = cdns_resume(cdns, 1);
>>>> + ret = cdns_resume(cdns);
>>>> spin_unlock_irqrestore(&cdns->lock, flags);
>>>> + cdns_set_active(cdns, 1);
>>>>
>>>> return ret;
>>>> }
>>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>>> index dbcdf3b24b47..661d5597fb68 100644
>>>> --- a/drivers/usb/cdns3/core.c
>>>> +++ b/drivers/usb/cdns3/core.c
>>>> @@ -522,7 +522,7 @@ int cdns_suspend(struct cdns *cdns)
>>>> }
>>>> EXPORT_SYMBOL_GPL(cdns_suspend);
>>>>
>>>> -int cdns_resume(struct cdns *cdns, u8 set_active)
>>>> +int cdns_resume(struct cdns *cdns)
>>>> {
>>>> struct device *dev = cdns->dev;
>>>> enum usb_role real_role;
>>>> @@ -556,15 +556,23 @@ int cdns_resume(struct cdns *cdns, u8 set_active)
>>>> if (cdns->roles[cdns->role]->resume)
>>>> cdns->roles[cdns->role]->resume(cdns, cdns_power_is_lost(cdns));
>>>>
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(cdns_resume);
>>>> +
>>>> +void cdns_set_active(struct cdns *cdns, u8 set_active)
>>>> +{
>>>> + struct device *dev = cdns->dev;
>>>> +
>>>> if (set_active) {
>>>> pm_runtime_disable(dev);
>>>> pm_runtime_set_active(dev);
>>>> pm_runtime_enable(dev);
>>>> }
>>>>
>>>> - return 0;
>>>> + return;
>>>> }
>>>> -EXPORT_SYMBOL_GPL(cdns_resume);
>>>> +EXPORT_SYMBOL_GPL(cdns_set_active);
>>>> #endif /* CONFIG_PM_SLEEP */
>>>>
>>>> MODULE_AUTHOR("Peter Chen <peter.chen@nxp.com>");
>>>> diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>>>> index 2d332a788871..0f429042f997 100644
>>>> --- a/drivers/usb/cdns3/core.h
>>>> +++ b/drivers/usb/cdns3/core.h
>>>> @@ -125,8 +125,9 @@ int cdns_init(struct cdns *cdns);
>>>> int cdns_remove(struct cdns *cdns);
>>>>
>>>> #ifdef CONFIG_PM_SLEEP
>>>> -int cdns_resume(struct cdns *cdns, u8 set_active);
>>>> +int cdns_resume(struct cdns *cdns);
>>>> int cdns_suspend(struct cdns *cdns);
>>>> +void cdns_set_active(struct cdns *cdns, u8 set_active);
>>>> #else /* CONFIG_PM_SLEEP */
>>>> static inline int cdns_resume(struct cdns *cdns, u8 set_active)
>>>> { return 0; }
>>>> --
>>>> 2.25.1
>>>>
>>> Hi,
>>>
>>> This is the friendly semi-automated patch-bot of Greg Kroah-Hartman.
>>> You have sent him a patch that has triggered this response.
>>>
>>> Right now, the development tree you have sent a patch for is "closed"
>>> due to the timing of the merge window. Don't worry, the patch(es) you
>>> have sent are not lost, and will be looked at after the merge window is
>>> over (after the -rc1 kernel is released by Linus).
>>>
>>> So thank you for your patience and your patches will be reviewed at this
>>> later time, you do not have to do anything further, this is just a short
>>> note to let you know the patch status and so you don't worry they didn't
>>> make it through.
>> Can someone help review this patch?
> It breaks the build:
>
> drivers/usb/cdns3/core.c: In function ‘cdns_resume’:
> drivers/usb/cdns3/core.c:527:24: error: unused variable ‘dev’ [-Werror=unused-variable]
> 527 | struct device *dev = cdns->dev;
> | ^~~
>
>
> How did you test it?
Oh, sorry, this may be because I did not set the compilation parameters,
which caused me to ignore this warning. I will delete the unused code
and send the v2 version
thanks
xiaolei
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-15 10:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-04 13:16 [PATCH 1/1] usb: cdns3: Put the cdns set active part outside the spin lock Xiaolei Wang
[not found] ` <2023050503-handful-shore-ae11@gregkh>
2023-06-14 3:16 ` wangxiaolei
2023-06-15 9:49 ` Greg KH
2023-06-15 10:14 ` wang xiaolei
2023-06-14 6:14 ` Pawel Laszczak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).