| Message ID | 20230504131602.3273835-1-xiaolei.wang@windriver.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] usb: cdns3: Put the cdns set active part outside the spin lock | expand |
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
>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
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
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
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; }
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(-)