LinuxLists.cc - [PATCH v2] usb: cdns3: Put the cdns set active part outside the spin lock

2023-06-15 11:16:18

Subject: [PATCH v2] usb: cdns3: Put the cdns set active part outside the spin lock

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 <[email protected]>
---
Changes since v1:
- Fix build error: unused variable 'dev'

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 | 3 ++-
4 files changed, 17 insertions(+), 7 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..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 <[email protected]>");
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

2023-06-15 16:21:04

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v2] usb: cdns3: Put the cdns set active part outside the spin lock

2023-06-15 22:10:04

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v2] usb: cdns3: Put the cdns set active part outside the spin lock

Hi Xiaolei,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.4-rc6 next-20230615]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Xiaolei-Wang/usb-cdns3-Put-the-cdns-set-active-part-outside-the-spin-lock/20230615-190721
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230615110424.4007675-1-xiaolei.wang%40windriver.com
patch subject: [PATCH v2] usb: cdns3: Put the cdns set active part outside the spin lock
config: riscv-randconfig-r042-20230615 (https://download.01.org/0day-ci/archive/20230616/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
git remote add usb https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
git fetch usb usb-testing
git checkout usb/usb-testing
b4 shazam https://lore.kernel.org/r/[email protected]
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/usb/cdns3/

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

In file included from drivers/usb/cdns3/cdns3-plat.c:16:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/usb/cdns3/cdns3-plat.c:16:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/usb/cdns3/cdns3-plat.c:16:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:743:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
743 | insb(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:104:53: note: expanded from macro 'insb'
104 | #define insb(addr, buffer, count) __insb(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/usb/cdns3/cdns3-plat.c:16:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:751:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
751 | insw(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:105:53: note: expanded from macro 'insw'
105 | #define insw(addr, buffer, count) __insw(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/usb/cdns3/cdns3-plat.c:16:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:759:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
759 | insl(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:106:53: note: expanded from macro 'insl'
106 | #define insl(addr, buffer, count) __insl(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/usb/cdns3/cdns3-plat.c:16:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:768:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
768 | outsb(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:118:55: note: expanded from macro 'outsb'
118 | #define outsb(addr, buffer, count) __outsb(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/usb/cdns3/cdns3-plat.c:16:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:777:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
777 | outsw(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:119:55: note: expanded from macro 'outsw'
119 | #define outsw(addr, buffer, count) __outsw(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/usb/cdns3/cdns3-plat.c:16:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:786:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
786 | outsl(addr, buffer, count);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:120:55: note: expanded from macro 'outsl'
120 | #define outsl(addr, buffer, count) __outsl(PCI_IOBASE + (addr), buffer, count)
| ~~~~~~~~~~ ^
In file included from drivers/usb/cdns3/cdns3-plat.c:16:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:1134:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
1134 | return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
| ~~~~~~~~~~ ^
>> drivers/usb/cdns3/cdns3-plat.c:258:18: error: too few arguments to function call, expected 2, have 1
258 | cdns_resume(cdns);
| ~~~~~~~~~~~ ^
drivers/usb/cdns3/core.h:132:19: note: 'cdns_resume' declared here
132 | static inline int cdns_resume(struct cdns *cdns, u8 set_active)
| ^
>> drivers/usb/cdns3/cdns3-plat.c:261:2: error: call to undeclared function 'cdns_set_active'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
261 | cdns_set_active(cdns, !PMSG_IS_AUTO(msg));
| ^
13 warnings and 2 errors generated.

vim +258 drivers/usb/cdns3/cdns3-plat.c

229
230 static int cdns3_controller_resume(struct device *dev, pm_message_t msg)
231 {
232 struct cdns *cdns = dev_get_drvdata(dev);
233 int ret;
234 unsigned long flags;
235
236 if (!cdns->in_lpm)
237 return 0;
238
239 if (cdns_power_is_lost(cdns)) {
240 phy_exit(cdns->usb2_phy);
241 ret = phy_init(cdns->usb2_phy);
242 if (ret)
243 return ret;
244
245 phy_exit(cdns->usb3_phy);
246 ret = phy_init(cdns->usb3_phy);
247 if (ret)
248 return ret;
249 }
250
251 ret = set_phy_power_on(cdns);
252 if (ret)
253 return ret;
254
255 cdns3_set_platform_suspend(cdns->dev, false, false);
256
257 spin_lock_irqsave(&cdns->lock, flags);
> 258 cdns_resume(cdns);
259 cdns->in_lpm = false;
260 spin_unlock_irqrestore(&cdns->lock, flags);
> 261 cdns_set_active(cdns, !PMSG_IS_AUTO(msg));
262 if (cdns->wakeup_pending) {
263 cdns->wakeup_pending = false;
264 enable_irq(cdns->wakeup_irq);
265 }
266 dev_dbg(cdns->dev, "%s ends\n", __func__);
267
268 return ret;
269 }
270

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki