| Message ID | 20230616021952.1025854-1-xiaolei.wang@windriver.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] usb: cdns3: Put the cdns set active part outside the spin lock | expand |
On 23-06-16 10:19:51, 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> Acked-by: Peter Chen <peter.chen@kernel.org> Peter > --- > v3: > * Fix build error: > cdns3-plat.c:258:18: error: too few arguments to function call, expected 2, have 1 > 258 | cdns_resume(cdns); > * use 'cdns_resume(struct cdns *cdns) { return 0; }' instead of > 'cdns_resume(struct cdns *cdns, u8 set_active) { return 0; }' > and add 'static inline int cdns_set_active(struct cdns *cdns, u8 set_active) > { return 0; }' > v2: > * Fix build error: unused variable 'dev' > * delete unused 'struct device *dev = cdns->dev;' in cdns_resume() > > drivers/usb/cdns3/cdns3-plat.c | 3 ++- > drivers/usb/cdns3/cdnsp-pci.c | 3 ++- > drivers/usb/cdns3/core.c | 15 +++++++++++---- > drivers/usb/cdns3/core.h | 7 +++++-- > 4 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c > index 884e2301237f..1168dbeed2ce 100644 > --- a/drivers/usb/cdns3/cdns3-plat.c > +++ b/drivers/usb/cdns3/cdns3-plat.c > @@ -255,9 +255,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..7b20d2d5c262 100644 > --- a/drivers/usb/cdns3/core.c > +++ b/drivers/usb/cdns3/core.c > @@ -522,9 +522,8 @@ 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; > bool role_changed = false; > int ret = 0; > @@ -556,15 +555,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..4a4dbc2c1561 100644 > --- a/drivers/usb/cdns3/core.h > +++ b/drivers/usb/cdns3/core.h > @@ -125,10 +125,13 @@ 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) > +static inline int cdns_resume(struct cdns *cdns) > +{ return 0; } > +static inline int cdns_set_active(struct cdns *cdns, u8 set_active) > { return 0; } > static inline int cdns_suspend(struct cdns *cdns) > { return 0; } > -- > 2.25.1 >
diff --git a/drivers/usb/cdns3/cdns3-plat.c b/drivers/usb/cdns3/cdns3-plat.c index 884e2301237f..1168dbeed2ce 100644 --- a/drivers/usb/cdns3/cdns3-plat.c +++ b/drivers/usb/cdns3/cdns3-plat.c @@ -255,9 +255,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..7b20d2d5c262 100644 --- a/drivers/usb/cdns3/core.c +++ b/drivers/usb/cdns3/core.c @@ -522,9 +522,8 @@ 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; bool role_changed = false; int ret = 0; @@ -556,15 +555,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..4a4dbc2c1561 100644 --- a/drivers/usb/cdns3/core.h +++ b/drivers/usb/cdns3/core.h @@ -125,10 +125,13 @@ 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) +static inline int cdns_resume(struct cdns *cdns) +{ return 0; } +static inline int cdns_set_active(struct cdns *cdns, u8 set_active) { return 0; } static inline int cdns_suspend(struct cdns *cdns) { 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> --- v3: * Fix build error: cdns3-plat.c:258:18: error: too few arguments to function call, expected 2, have 1 258 | cdns_resume(cdns); * use 'cdns_resume(struct cdns *cdns) { return 0; }' instead of 'cdns_resume(struct cdns *cdns, u8 set_active) { return 0; }' and add 'static inline int cdns_set_active(struct cdns *cdns, u8 set_active) { return 0; }' v2: * Fix build error: unused variable 'dev' * delete unused 'struct device *dev = cdns->dev;' in cdns_resume() drivers/usb/cdns3/cdns3-plat.c | 3 ++- drivers/usb/cdns3/cdnsp-pci.c | 3 ++- drivers/usb/cdns3/core.c | 15 +++++++++++---- drivers/usb/cdns3/core.h | 7 +++++-- 4 files changed, 20 insertions(+), 8 deletions(-)