* musb RPM sleep-while-atomic in 4.9-rc1 @ 2016-10-20 15:37 Johan Hovold 2016-10-21 7:08 ` Tony Lindgren 0 siblings, 1 reply; 24+ messages in thread From: Johan Hovold @ 2016-10-20 15:37 UTC (permalink / raw) To: Tony Lindgren Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA Hi Tony, I'm getting the splat below when booting 4.9-rc1 on a BBB and tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") which added a synchronous RPM get in a timer callback: [ 6.466226] BUG: sleeping function called from invalid context at /home/johan/work/omicron/src/linux/drivers/base/power/runtime.c:955 [ 6.478850] in_atomic(): 1, irqs_disabled(): 0, pid: 59, name: grep [ 6.485434] 1 lock held by grep/59: [ 6.489102] #0: [ 6.490966] ( ((&glue->timer))[ 6.494206] ){+.-...} , at: [ 6.497194] [<c0191084>] call_timer_fn+0x0/0x41c [ 6.502048] Preemption disabled at:[ 6.505540] [<c01015e0>] __do_softirq+0x80/0x594 [ 6.510393] [ 6.511974] CPU: 0 PID: 59 Comm: grep Not tainted 4.9.0-rc1 #46 [ 6.518190] Hardware name: Generic AM33XX (Flattened Device Tree) [ 6.524613] [<c01116e4>] (unwind_backtrace) from [<c010dec0>] (show_stack+0x20/0x24) [ 6.532753] [<c010dec0>] (show_stack) from [<c0364f1c>] (dump_stack+0x24/0x28) [ 6.540351] [<c0364f1c>] (dump_stack) from [<c015ea58>] (___might_sleep+0x1d8/0x2c4) [ 6.548486] [<c015ea58>] (___might_sleep) from [<c015ebb4>] (__might_sleep+0x70/0xa8) [ 6.556720] [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) [ 6.565404] [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) [ 6.573813] [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) [ 6.581675] [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) [ 6.589990] [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) [ 6.598487] [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) [ 6.607079] [<c010168c>] (__do_softirq) from [<c01384cc>] (irq_exit+0xf4/0x130) [ 6.614766] [<c01384cc>] (irq_exit) from [<c017d910>] (__handle_domain_irq+0x84/0xec) [ 6.622991] [<c017d910>] (__handle_domain_irq) from [<c0101500>] (omap_intc_handle_irq+0x44/0xa0) [ 6.632306] [<c0101500>] (omap_intc_handle_irq) from [<c010ee78>] (__irq_usr+0x58/0x80) [ 6.640716] Exception stack(0xcf669fb0 to 0xcf669ff8) [ 6.646028] 9fa0: b6fd5050 0012ebb0 b6e71058 b6e1c088 [ 6.654614] 9fc0: b6e07000 00000001 b6e1cb10 b6fd5000 00000001 00000000 b6fd38e8 bec32a54 [ 6.663198] 9fe0: b6e1c870 bec32990 b6fb0a20 b6fb9758 20030010 ffffffff Setting the irq_safe flag seems to do the trick, but not sure that's what you intended to do. I saw you posted some regression fixes lately, but they did not look related to this at first glance at least. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-10-20 15:37 musb RPM sleep-while-atomic in 4.9-rc1 Johan Hovold @ 2016-10-21 7:08 ` Tony Lindgren [not found] ` <20161021070848.rum7wrlihjayqdbh-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-10-21 7:08 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161020 08:38]: > Hi Tony, > > I'm getting the splat below when booting 4.9-rc1 on a BBB and > tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support for > MUSB DSPS glue layer") which added a synchronous RPM get in a timer > callback: OK, sorry to hear about that. Care to email me your .config and how to reproduce and what do you have connected like a hub? Also do you use built-in gadgets or configure them via configfs? > [ 6.466226] BUG: sleeping function called from invalid context at /home/johan/work/omicron/src/linux/drivers/base/power/runtime.c:955 > [ 6.478850] in_atomic(): 1, irqs_disabled(): 0, pid: 59, name: grep > [ 6.485434] 1 lock held by grep/59: > [ 6.489102] #0: [ 6.490966] ( > ((&glue->timer))[ 6.494206] ){+.-...} > , at: [ 6.497194] [<c0191084>] call_timer_fn+0x0/0x41c > [ 6.502048] Preemption disabled at:[ 6.505540] [<c01015e0>] __do_softirq+0x80/0x594 > [ 6.510393] > [ 6.511974] CPU: 0 PID: 59 Comm: grep Not tainted 4.9.0-rc1 #46 > [ 6.518190] Hardware name: Generic AM33XX (Flattened Device Tree) > [ 6.524613] [<c01116e4>] (unwind_backtrace) from [<c010dec0>] (show_stack+0x20/0x24) > [ 6.532753] [<c010dec0>] (show_stack) from [<c0364f1c>] (dump_stack+0x24/0x28) > [ 6.540351] [<c0364f1c>] (dump_stack) from [<c015ea58>] (___might_sleep+0x1d8/0x2c4) > [ 6.548486] [<c015ea58>] (___might_sleep) from [<c015ebb4>] (__might_sleep+0x70/0xa8) > [ 6.556720] [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > [ 6.565404] [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > [ 6.573813] [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > [ 6.581675] [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > [ 6.589990] [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > [ 6.598487] [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > [ 6.607079] [<c010168c>] (__do_softirq) from [<c01384cc>] (irq_exit+0xf4/0x130) > [ 6.614766] [<c01384cc>] (irq_exit) from [<c017d910>] (__handle_domain_irq+0x84/0xec) > [ 6.622991] [<c017d910>] (__handle_domain_irq) from [<c0101500>] (omap_intc_handle_irq+0x44/0xa0) > [ 6.632306] [<c0101500>] (omap_intc_handle_irq) from [<c010ee78>] (__irq_usr+0x58/0x80) > [ 6.640716] Exception stack(0xcf669fb0 to 0xcf669ff8) > [ 6.646028] 9fa0: b6fd5050 0012ebb0 b6e71058 b6e1c088 > [ 6.654614] 9fc0: b6e07000 00000001 b6e1cb10 b6fd5000 00000001 00000000 b6fd38e8 bec32a54 > [ 6.663198] 9fe0: b6e1c870 bec32990 b6fb0a20 b6fb9758 20030010 ffffffff > > Setting the irq_safe flag seems to do the trick, but not sure that's > what you intended to do. That's what we want to avoid as it keep the parent device permanently enabled. To avoid that we want to just queue things and deal with them from pm_runtime_resume. > I saw you posted some regression fixes lately, but they did not look > related to this at first glance at least. Yeah this seems different. Can you still try v4.9-rc + patches from thread "[PATCH 0/2] Fixes for two more musb regressions"? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161021070848.rum7wrlihjayqdbh-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161021070848.rum7wrlihjayqdbh-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-10-21 9:25 ` Johan Hovold 2016-10-21 9:49 ` Tony Lindgren 0 siblings, 1 reply; 24+ messages in thread From: Johan Hovold @ 2016-10-21 9:25 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA [-- Attachment #1: Type: text/plain, Size: 4011 bytes --] On Fri, Oct 21, 2016 at 12:08:49AM -0700, Tony Lindgren wrote: > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161020 08:38]: > > Hi Tony, > > > > I'm getting the splat below when booting 4.9-rc1 on a BBB and > > tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support for > > MUSB DSPS glue layer") which added a synchronous RPM get in a timer > > callback: > > OK, sorry to hear about that. Care to email me your .config and how > to reproduce and what do you have connected like a hub? Also do > you use built-in gadgets or configure them via configfs? It happens even with no devices connected to the host port, and with no gadgets configured (or the peripheral port simply disabled). Whenever the glue timer fires and calls pm_runtime_get_sync() I get the splat. For some reason the might_sleep() in get_sync() does not trigger the first time, which means that I see this four seconds after probe, and then every other second when the timer fires. Attaching my defconfig. > > [ 6.466226] BUG: sleeping function called from invalid context at /home/johan/work/omicron/src/linux/drivers/base/power/runtime.c:955 > > [ 6.478850] in_atomic(): 1, irqs_disabled(): 0, pid: 59, name: grep > > [ 6.485434] 1 lock held by grep/59: > > [ 6.489102] #0: [ 6.490966] ( > > ((&glue->timer))[ 6.494206] ){+.-...} > > , at: [ 6.497194] [<c0191084>] call_timer_fn+0x0/0x41c > > [ 6.502048] Preemption disabled at:[ 6.505540] [<c01015e0>] __do_softirq+0x80/0x594 > > [ 6.510393] > > [ 6.511974] CPU: 0 PID: 59 Comm: grep Not tainted 4.9.0-rc1 #46 > > [ 6.518190] Hardware name: Generic AM33XX (Flattened Device Tree) > > [ 6.524613] [<c01116e4>] (unwind_backtrace) from [<c010dec0>] (show_stack+0x20/0x24) > > [ 6.532753] [<c010dec0>] (show_stack) from [<c0364f1c>] (dump_stack+0x24/0x28) > > [ 6.540351] [<c0364f1c>] (dump_stack) from [<c015ea58>] (___might_sleep+0x1d8/0x2c4) > > [ 6.548486] [<c015ea58>] (___might_sleep) from [<c015ebb4>] (__might_sleep+0x70/0xa8) > > [ 6.556720] [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > > [ 6.565404] [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > > [ 6.573813] [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > > [ 6.581675] [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > > [ 6.589990] [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > > [ 6.598487] [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > > [ 6.607079] [<c010168c>] (__do_softirq) from [<c01384cc>] (irq_exit+0xf4/0x130) > > [ 6.614766] [<c01384cc>] (irq_exit) from [<c017d910>] (__handle_domain_irq+0x84/0xec) > > [ 6.622991] [<c017d910>] (__handle_domain_irq) from [<c0101500>] (omap_intc_handle_irq+0x44/0xa0) > > [ 6.632306] [<c0101500>] (omap_intc_handle_irq) from [<c010ee78>] (__irq_usr+0x58/0x80) > > [ 6.640716] Exception stack(0xcf669fb0 to 0xcf669ff8) > > [ 6.646028] 9fa0: b6fd5050 0012ebb0 b6e71058 b6e1c088 > > [ 6.654614] 9fc0: b6e07000 00000001 b6e1cb10 b6fd5000 00000001 00000000 b6fd38e8 bec32a54 > > [ 6.663198] 9fe0: b6e1c870 bec32990 b6fb0a20 b6fb9758 20030010 ffffffff > > > > Setting the irq_safe flag seems to do the trick, but not sure that's > > what you intended to do. > > That's what we want to avoid as it keep the parent device permanently > enabled. To avoid that we want to just queue things and deal with them > from pm_runtime_resume. I figured, so then that pm_runtime_get_sync() in the dsps timer callback needs to go. > > I saw you posted some regression fixes lately, but they did not look > > related to this at first glance at least. > > Yeah this seems different. Can you still try v4.9-rc + patches from > thread "[PATCH 0/2] Fixes for two more musb regressions"? As expected, those two fixes makes no difference. Thanks, Johan [-- Attachment #2: defconfig --] [-- Type: text/plain, Size: 8159 bytes --] CONFIG_COMPILE_TEST=y CONFIG_DEFAULT_HOSTNAME="omicron" # CONFIG_SWAP is not set CONFIG_SYSVIPC=y CONFIG_POSIX_MQUEUE=y # CONFIG_CROSS_MEMORY_ATTACH is not set # CONFIG_FHANDLE is not set CONFIG_USELIB=y CONFIG_NO_HZ_IDLE=y CONFIG_HIGH_RES_TIMERS=y CONFIG_BSD_PROCESS_ACCT=y CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_LOG_BUF_SHIFT=16 CONFIG_EXPERT=y # CONFIG_ADVISE_SYSCALLS is not set CONFIG_SLAB=y CONFIG_PROFILING=y CONFIG_MODULES=y CONFIG_MODULE_FORCE_LOAD=y CONFIG_MODULE_UNLOAD=y # CONFIG_LBDAF is not set # CONFIG_BLK_DEV_BSG is not set CONFIG_PARTITION_ADVANCED=y # CONFIG_IOSCHED_DEADLINE is not set CONFIG_OMAP_RESET_CLOCKS=y CONFIG_OMAP_MUX_DEBUG=y CONFIG_ARCH_OMAP3=y CONFIG_SOC_AM33XX=y # CONFIG_SOC_OMAP3430 is not set # CONFIG_SOC_TI81XX is not set # CONFIG_MACH_OMAP3517EVM is not set # CONFIG_MACH_OMAP3_PANDORA is not set CONFIG_ARM_THUMBEE=y # CONFIG_CACHE_L2X0 is not set CONFIG_ARM_ERRATA_720789=y CONFIG_ARM_ERRATA_754322=y CONFIG_ARM_ERRATA_775420=y CONFIG_PCI=y CONFIG_HAVE_ARM_ARCH_TIMER=y CONFIG_PREEMPT=y CONFIG_HZ_1000=y # CONFIG_HIGHPTE is not set # CONFIG_COMPACTION is not set CONFIG_ZBOOT_ROM_TEXT=0x0 CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_ARM_APPENDED_DTB=y CONFIG_CMDLINE="root=/dev/nfs ip=dhcp debug" CONFIG_CMDLINE_EXTEND=y # CONFIG_COREDUMP is not set CONFIG_PM_DEBUG=y CONFIG_PM_ADVANCED_DEBUG=y CONFIG_NET=y CONFIG_PACKET=y CONFIG_UNIX=y CONFIG_INET=y CONFIG_IP_PNP=y CONFIG_IP_PNP_DHCP=y # CONFIG_INET_XFRM_MODE_TRANSPORT is not set # CONFIG_INET_XFRM_MODE_TUNNEL is not set # CONFIG_INET_XFRM_MODE_BEET is not set # CONFIG_INET_DIAG is not set # CONFIG_IPV6 is not set CONFIG_IRDA=m CONFIG_IRCOMM=m # CONFIG_WIRELESS is not set # CONFIG_UEVENT_HELPER is not set CONFIG_DEVTMPFS=y CONFIG_DEVTMPFS_MOUNT=y # CONFIG_STANDALONE is not set CONFIG_FW_LOADER=m CONFIG_MTD=y CONFIG_MTD_CMDLINE_PARTS=y CONFIG_MTD_BLOCK=y CONFIG_MTD_NAND=y CONFIG_MTD_NAND_ECC_BCH=y CONFIG_MTD_NAND_OMAP2=y CONFIG_MTD_UBI=y CONFIG_OF_ALL_DTBS=y CONFIG_OF_OVERLAY=y CONFIG_BLK_DEV_LOOP=y CONFIG_SCSI=y # CONFIG_SCSI_PROC_FS is not set CONFIG_BLK_DEV_SD=y # CONFIG_SCSI_LOWLEVEL is not set CONFIG_NETDEVICES=y CONFIG_NETCONSOLE=m # CONFIG_NET_VENDOR_3COM is not set # CONFIG_NET_VENDOR_ADAPTEC is not set # CONFIG_NET_VENDOR_AGERE is not set # CONFIG_NET_VENDOR_ALTEON is not set # CONFIG_NET_VENDOR_AMAZON is not set # CONFIG_NET_VENDOR_AMD is not set # CONFIG_NET_VENDOR_ARC is not set # CONFIG_NET_VENDOR_ATHEROS is not set # CONFIG_NET_CADENCE is not set # CONFIG_NET_VENDOR_BROADCOM is not set # CONFIG_NET_VENDOR_BROCADE is not set # CONFIG_NET_VENDOR_CAVIUM is not set # CONFIG_NET_VENDOR_CHELSIO is not set # CONFIG_NET_VENDOR_CIRRUS is not set # CONFIG_NET_VENDOR_CISCO is not set # CONFIG_NET_VENDOR_DEC is not set # CONFIG_NET_VENDOR_DLINK is not set # CONFIG_NET_VENDOR_EMULEX is not set # CONFIG_NET_VENDOR_EZCHIP is not set # CONFIG_NET_VENDOR_EXAR is not set # CONFIG_NET_VENDOR_FARADAY is not set # CONFIG_NET_VENDOR_HISILICON is not set # CONFIG_NET_VENDOR_HP is not set # CONFIG_NET_VENDOR_INTEL is not set # CONFIG_NET_VENDOR_MARVELL is not set # CONFIG_NET_VENDOR_MELLANOX is not set # CONFIG_NET_VENDOR_MICREL is not set # CONFIG_NET_VENDOR_MYRI is not set # CONFIG_NET_VENDOR_NATSEMI is not set # CONFIG_NET_VENDOR_NETRONOME is not set # CONFIG_NET_VENDOR_NVIDIA is not set # CONFIG_NET_VENDOR_OKI is not set # CONFIG_NET_PACKET_ENGINE is not set # CONFIG_NET_VENDOR_QLOGIC is not set # CONFIG_NET_VENDOR_QUALCOMM is not set # CONFIG_NET_VENDOR_REALTEK is not set # CONFIG_NET_VENDOR_RENESAS is not set # CONFIG_NET_VENDOR_RDC is not set # CONFIG_NET_VENDOR_ROCKER is not set # CONFIG_NET_VENDOR_SAMSUNG is not set # CONFIG_NET_VENDOR_SEEQ is not set # CONFIG_NET_VENDOR_SILAN is not set # CONFIG_NET_VENDOR_SIS is not set CONFIG_SMSC911X=y # CONFIG_NET_VENDOR_STMICRO is not set # CONFIG_NET_VENDOR_SUN is not set # CONFIG_NET_VENDOR_SYNOPSYS is not set # CONFIG_NET_VENDOR_TEHUTI is not set CONFIG_TI_CPSW=y # CONFIG_NET_VENDOR_VIA is not set # CONFIG_NET_VENDOR_WIZNET is not set CONFIG_SMSC_PHY=y CONFIG_USB_USBNET=y # CONFIG_USB_NET_AX8817X is not set # CONFIG_USB_NET_AX88179_178A is not set CONFIG_USB_NET_CDC_EEM=y # CONFIG_USB_NET_NET1080 is not set CONFIG_USB_NET_RNDIS_HOST=y # CONFIG_USB_NET_CDC_SUBSET is not set # CONFIG_USB_NET_ZAURUS is not set # CONFIG_WLAN is not set # CONFIG_INPUT_LEDS is not set # CONFIG_INPUT_MOUSEDEV is not set CONFIG_INPUT_EVDEV=y CONFIG_KEYBOARD_LM8323=m # CONFIG_INPUT_MOUSE is not set # CONFIG_VT is not set CONFIG_LEGACY_PTY_COUNT=4 CONFIG_SERIAL_OMAP=y CONFIG_SERIAL_OMAP_CONSOLE=y # CONFIG_HW_RANDOM is not set CONFIG_I2C_CHARDEV=y # CONFIG_I2C_BRCMSTB is not set CONFIG_PINCTRL_SINGLE=y # CONFIG_PINCTRL_UNIPHIER is not set CONFIG_DEBUG_GPIO=y CONFIG_GPIO_SYSFS=y CONFIG_GPIO_TEGRA=y CONFIG_GPIO_TWL4030=y CONFIG_POWER_SUPPLY=y # CONFIG_HWMON is not set CONFIG_WATCHDOG=y CONFIG_OMAP_WATCHDOG=y CONFIG_TWL4030_WATCHDOG=y CONFIG_MFD_DA9052_I2C=y CONFIG_MFD_TPS65217=y CONFIG_REGULATOR_DA9052=m CONFIG_REGULATOR_PBIAS=y CONFIG_REGULATOR_TPS65217=y CONFIG_REGULATOR_TWL4030=y # CONFIG_HID is not set # CONFIG_USB_HID is not set CONFIG_USB=y CONFIG_USB_ANNOUNCE_NEW_DEVICES=y CONFIG_USB_MON=y CONFIG_USB_EHCI_HCD=m CONFIG_USB_ACM=m CONFIG_USB_STORAGE=y CONFIG_USB_MUSB_HDRC=y CONFIG_USB_MUSB_DSPS=y CONFIG_USB_TI_CPPI41_DMA=y CONFIG_USB_SERIAL=m CONFIG_USB_SERIAL_GENERIC=y CONFIG_USB_SERIAL_SIMPLE=m CONFIG_USB_SERIAL_AIRCABLE=m CONFIG_USB_SERIAL_ARK3116=m CONFIG_USB_SERIAL_BELKIN=m CONFIG_USB_SERIAL_CH341=m CONFIG_USB_SERIAL_WHITEHEAT=m CONFIG_USB_SERIAL_DIGI_ACCELEPORT=m CONFIG_USB_SERIAL_CP210X=m CONFIG_USB_SERIAL_CYPRESS_M8=m CONFIG_USB_SERIAL_EMPEG=m CONFIG_USB_SERIAL_FTDI_SIO=m CONFIG_USB_SERIAL_VISOR=m CONFIG_USB_SERIAL_IPAQ=m CONFIG_USB_SERIAL_IR=m CONFIG_USB_SERIAL_EDGEPORT=m CONFIG_USB_SERIAL_EDGEPORT_TI=m CONFIG_USB_SERIAL_F81232=m CONFIG_USB_SERIAL_GARMIN=m CONFIG_USB_SERIAL_IPW=m CONFIG_USB_SERIAL_IUU=m CONFIG_USB_SERIAL_KEYSPAN_PDA=m CONFIG_USB_SERIAL_KEYSPAN=m CONFIG_USB_SERIAL_KLSI=m CONFIG_USB_SERIAL_KOBIL_SCT=m CONFIG_USB_SERIAL_MCT_U232=m CONFIG_USB_SERIAL_METRO=m CONFIG_USB_SERIAL_MOS7720=m CONFIG_USB_SERIAL_MOS7840=m CONFIG_USB_SERIAL_MXUPORT=m CONFIG_USB_SERIAL_NAVMAN=m CONFIG_USB_SERIAL_PL2303=m CONFIG_USB_SERIAL_OTI6858=m CONFIG_USB_SERIAL_QCAUX=m CONFIG_USB_SERIAL_QUALCOMM=m CONFIG_USB_SERIAL_SPCP8X5=m CONFIG_USB_SERIAL_SAFE=m CONFIG_USB_SERIAL_SIERRAWIRELESS=m CONFIG_USB_SERIAL_SYMBOL=m CONFIG_USB_SERIAL_TI=m CONFIG_USB_SERIAL_CYBERJACK=m CONFIG_USB_SERIAL_XIRCOM=m CONFIG_USB_SERIAL_OPTION=m CONFIG_USB_SERIAL_OMNINET=m CONFIG_USB_SERIAL_OPTICON=m CONFIG_USB_SERIAL_XSENS_MT=m CONFIG_USB_SERIAL_WISHBONE=m CONFIG_USB_SERIAL_SSU100=m CONFIG_USB_SERIAL_QT2=m CONFIG_USB_SERIAL_DEBUG=m CONFIG_NOP_USB_XCEIV=y CONFIG_AM335X_PHY_USB=y CONFIG_USB_GADGET=y CONFIG_USB_DUMMY_HCD=m CONFIG_USB_G_SERIAL=m CONFIG_MMC=y CONFIG_MMC_OMAP_HS=y CONFIG_NEW_LEDS=y CONFIG_LEDS_CLASS=y CONFIG_LEDS_TRIGGERS=y CONFIG_LEDS_TRIGGER_TIMER=y CONFIG_LEDS_TRIGGER_ONESHOT=y CONFIG_LEDS_TRIGGER_HEARTBEAT=y CONFIG_RTC_CLASS=y CONFIG_RTC_DRV_TWL4030=y # CONFIG_RTC_DRV_SPEAR is not set CONFIG_RTC_DRV_OMAP=y CONFIG_DMADEVICES=y CONFIG_DMA_OMAP=y CONFIG_TI_EDMA=y # CONFIG_SH_DMAE_BASE is not set # CONFIG_COMMON_CLK_XGENE is not set # CONFIG_CLK_BCM_KONA is not set # CONFIG_ARM_ARCH_TIMER_EVTSTREAM is not set # CONFIG_IOMMU_SUPPORT is not set CONFIG_SOC_TI=y # CONFIG_ARM_SCPI_POWER_DOMAIN is not set # CONFIG_MANDATORY_FILE_LOCKING is not set CONFIG_VFAT_FS=y CONFIG_TMPFS=y CONFIG_UBIFS_FS=y CONFIG_NFS_FS=y CONFIG_ROOT_NFS=y CONFIG_NLS_CODEPAGE_437=y CONFIG_NLS_ISO8859_1=y CONFIG_NLS_UTF8=y CONFIG_PRINTK_TIME=y CONFIG_DYNAMIC_DEBUG=y # CONFIG_SECTION_MISMATCH_WARN_ONLY is not set CONFIG_MAGIC_SYSRQ=y CONFIG_DEBUG_SLAB=y CONFIG_DEBUG_SLAB_LEAK=y CONFIG_DEBUG_KMEMLEAK=y # CONFIG_SCHED_DEBUG is not set CONFIG_SCHED_STACK_END_CHECK=y CONFIG_DEBUG_RT_MUTEXES=y CONFIG_PROVE_LOCKING=y CONFIG_DEBUG_ATOMIC_SLEEP=y CONFIG_DEBUG_PI_LIST=y CONFIG_RCU_EQS_DEBUG=y CONFIG_STACK_TRACER=y CONFIG_FUNCTION_PROFILER=y # CONFIG_TRACING_EVENTS_GPIO is not set CONFIG_DEBUG_LL=y CONFIG_EARLY_PRINTK=y # CONFIG_CRYPTO_ECHAINIV is not set # CONFIG_CRYPTO_HW is not set ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-10-21 9:25 ` Johan Hovold @ 2016-10-21 9:49 ` Tony Lindgren [not found] ` <20161021094904.q66kjsl33yzf2kir-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-10-21 9:49 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161021 02:26]: > On Fri, Oct 21, 2016 at 12:08:49AM -0700, Tony Lindgren wrote: > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161020 08:38]: > > > Hi Tony, > > > > > > I'm getting the splat below when booting 4.9-rc1 on a BBB and > > > tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support for > > > MUSB DSPS glue layer") which added a synchronous RPM get in a timer > > > callback: > > > > OK, sorry to hear about that. Care to email me your .config and how > > to reproduce and what do you have connected like a hub? Also do > > you use built-in gadgets or configure them via configfs? > > It happens even with no devices connected to the host port, and with no > gadgets configured (or the peripheral port simply disabled). Whenever > the glue timer fires and calls pm_runtime_get_sync() I get the splat. > For some reason the might_sleep() in get_sync() does not trigger the > first time, which means that I see this four seconds after probe, and > then every other second when the timer fires. OK. I'm totally baffled how come I did not hit this earlier with my tests. I did have some extra patches for using the pmic vbus irq for cable detection, but that's only for the peripheral instance and the host instance is still using timer. > Attaching my defconfig. Not seeing anything special there, musb built in, using dma. And no reason why this should not always happen when polling the cable status. > > > Setting the irq_safe flag seems to do the trick, but not sure that's > > > what you intended to do. > > > > That's what we want to avoid as it keep the parent device permanently > > enabled. To avoid that we want to just queue things and deal with them > > from pm_runtime_resume. > > I figured, so then that pm_runtime_get_sync() in the dsps timer callback > needs to go. Agreed, that is clearly wrong to call from softirq context. I need to figure out what is right fix here but don't have access to my bbb until next week. > > > I saw you posted some regression fixes lately, but they did not look > > > related to this at first glance at least. > > > > Yeah this seems different. Can you still try v4.9-rc + patches from > > thread "[PATCH 0/2] Fixes for two more musb regressions"? > > As expected, those two fixes makes no difference. Yeah thanks for checking. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161021094904.q66kjsl33yzf2kir-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161021094904.q66kjsl33yzf2kir-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-10-21 11:07 ` Johan Hovold 2016-10-21 11:27 ` Tony Lindgren 0 siblings, 1 reply; 24+ messages in thread From: Johan Hovold @ 2016-10-21 11:07 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA On Fri, Oct 21, 2016 at 02:49:05AM -0700, Tony Lindgren wrote: > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161021 02:26]: > > On Fri, Oct 21, 2016 at 12:08:49AM -0700, Tony Lindgren wrote: > > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161020 08:38]: > > > > Hi Tony, > > > > > > > > I'm getting the splat below when booting 4.9-rc1 on a BBB and > > > > tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support for > > > > MUSB DSPS glue layer") which added a synchronous RPM get in a timer > > > > callback: > > > > > > OK, sorry to hear about that. Care to email me your .config and how > > > to reproduce and what do you have connected like a hub? Also do > > > you use built-in gadgets or configure them via configfs? > > > > It happens even with no devices connected to the host port, and with no > > gadgets configured (or the peripheral port simply disabled). Whenever > > the glue timer fires and calls pm_runtime_get_sync() I get the splat. > > For some reason the might_sleep() in get_sync() does not trigger the > > first time, which means that I see this four seconds after probe, and > > then every other second when the timer fires. > > OK. I'm totally baffled how come I did not hit this earlier with > my tests. I did have some extra patches for using the pmic vbus irq > for cable detection, but that's only for the peripheral instance and > the host instance is still using timer. Yeah, I was a bit surprised as well given that other people have also apparently run this code. Guess CONFIG_DEBUG_ATOMIC_SLEEP must have been disabled? > > > > Setting the irq_safe flag seems to do the trick, but not sure that's > > > > what you intended to do. > > > > > > That's what we want to avoid as it keep the parent device permanently > > > enabled. To avoid that we want to just queue things and deal with them > > > from pm_runtime_resume. > > > > I figured, so then that pm_runtime_get_sync() in the dsps timer callback > > needs to go. > > Agreed, that is clearly wrong to call from softirq context. I need to > figure out what is right fix here but don't have access to my bbb > until next week. Ok, I'll make do with irq_safe meanwhile. Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-10-21 11:07 ` Johan Hovold @ 2016-10-21 11:27 ` Tony Lindgren [not found] ` <20161021112745.lancojpgv4h6aqpw-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-10-21 11:27 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161021 04:08]: > On Fri, Oct 21, 2016 at 02:49:05AM -0700, Tony Lindgren wrote: > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161021 02:26]: > > > On Fri, Oct 21, 2016 at 12:08:49AM -0700, Tony Lindgren wrote: > > > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161020 08:38]: > > > > > Hi Tony, > > > > > > > > > > I'm getting the splat below when booting 4.9-rc1 on a BBB and > > > > > tracked it down to 65b3f50ed6fa ("usb: musb: Add PM runtime support for > > > > > MUSB DSPS glue layer") which added a synchronous RPM get in a timer > > > > > callback: > > > > > > > > OK, sorry to hear about that. Care to email me your .config and how > > > > to reproduce and what do you have connected like a hub? Also do > > > > you use built-in gadgets or configure them via configfs? > > > > > > It happens even with no devices connected to the host port, and with no > > > gadgets configured (or the peripheral port simply disabled). Whenever > > > the glue timer fires and calls pm_runtime_get_sync() I get the splat. > > > For some reason the might_sleep() in get_sync() does not trigger the > > > first time, which means that I see this four seconds after probe, and > > > then every other second when the timer fires. > > > > OK. I'm totally baffled how come I did not hit this earlier with > > my tests. I did have some extra patches for using the pmic vbus irq > > for cable detection, but that's only for the peripheral instance and > > the host instance is still using timer. > > Yeah, I was a bit surprised as well given that other people have also > apparently run this code. Guess CONFIG_DEBUG_ATOMIC_SLEEP must have been > disabled? Yeh that's it, I've been just using omap2plus_defconfig where it seems to be disabled. Probably a good idea to enable it there. > > > > > Setting the irq_safe flag seems to do the trick, but not sure that's > > > > > what you intended to do. > > > > > > > > That's what we want to avoid as it keep the parent device permanently > > > > enabled. To avoid that we want to just queue things and deal with them > > > > from pm_runtime_resume. > > > > > > I figured, so then that pm_runtime_get_sync() in the dsps timer callback > > > needs to go. > > > > Agreed, that is clearly wrong to call from softirq context. I need to > > figure out what is right fix here but don't have access to my bbb > > until next week. > > Ok, I'll make do with irq_safe meanwhile. OK Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161021112745.lancojpgv4h6aqpw-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161021112745.lancojpgv4h6aqpw-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-10-24 17:35 ` Tony Lindgren [not found] ` <20161024173538.26xvlitxiwjmh4fx-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-10-24 17:35 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA Hi, * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161021 04:28]: > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161021 04:08]: > > > Agreed, that is clearly wrong to call from softirq context. I need to > > > figure out what is right fix here but don't have access to my bbb > > > until next week. > > > > Ok, I'll make do with irq_safe meanwhile. > > OK Below is a fix against v4.9-rc2 that seems to work for me. Regards, Tony 8< --------------------- >From tony Mon Sep 17 00:00:00 2001 From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> Date: Mon, 24 Oct 2016 09:18:02 -0700 Subject: [PATCH] usb: musb: Fix sleeping function called from invalid context for hdrc glue Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer that runs in softirq context. That causes a "BUG: sleeping function called from invalid context" every time when polling the cable status: [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. Let's fix the issue by adding dsps_check_status() and dsps_runtime_resume() functions. And let's have otg_timer() check for the PM runtime status, and if not already enabled, have dsps_runtime_resume() call dsps_check_status() directly. Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") Reported-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> --- drivers/usb/musb/musb_dsps.c | 46 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -188,9 +188,8 @@ static void dsps_musb_disable(struct musb *musb) musb_writeb(musb->mregs, MUSB_DEVCTL, 0); } -static void otg_timer(unsigned long _musb) +static void dsps_check_status(struct musb *musb) { - struct musb *musb = (void *)_musb; void __iomem *mregs = musb->mregs; struct device *dev = musb->controller; struct dsps_glue *glue = dev_get_drvdata(dev->parent); @@ -198,11 +197,6 @@ static void otg_timer(unsigned long _musb) u8 devctl; unsigned long flags; int skip_session = 0; - int err; - - err = pm_runtime_get_sync(dev); - if (err < 0) - dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); /* * We poll because DSPS IP's won't expose several OTG-critical @@ -246,7 +240,25 @@ static void otg_timer(unsigned long _musb) break; } spin_unlock_irqrestore(&musb->lock, flags); +} + +static void otg_timer(unsigned long _musb) +{ + struct musb *musb = (void *)_musb; + struct device *dev = musb->controller; + int err; + + err = pm_runtime_get(dev); + if (err < 0) + dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); + + /* If not active, dsps_runtime_resume() will call us again */ + if (!pm_runtime_active(dev)) + goto done; + + dsps_check_status(musb); +done: pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); } @@ -847,6 +859,21 @@ static const struct of_device_id musb_dsps_of_match[] = { }; MODULE_DEVICE_TABLE(of, musb_dsps_of_match); +static int dsps_runtime_resume(struct device *dev) +{ + struct dsps_glue *glue = dev_get_drvdata(dev); + struct musb *musb; + + if (!glue->musb) + return 0; + + musb = platform_get_drvdata(glue->musb); + if (musb) + dsps_check_status(musb); + + return 0; +} + #ifdef CONFIG_PM_SLEEP static int dsps_suspend(struct device *dev) { @@ -900,7 +927,10 @@ static int dsps_resume(struct device *dev) } #endif -static SIMPLE_DEV_PM_OPS(dsps_pm_ops, dsps_suspend, dsps_resume); +static const struct dev_pm_ops dsps_pm_ops = { + SET_RUNTIME_PM_OPS(NULL, dsps_runtime_resume, NULL) + SET_SYSTEM_SLEEP_PM_OPS(dsps_suspend, dsps_resume) +}; static struct platform_driver dsps_usbss_driver = { .probe = dsps_probe, -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161024173538.26xvlitxiwjmh4fx-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161024173538.26xvlitxiwjmh4fx-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-10-25 8:32 ` Johan Hovold 2016-10-25 15:11 ` Tony Lindgren 0 siblings, 1 reply; 24+ messages in thread From: Johan Hovold @ 2016-10-25 8:32 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA On Mon, Oct 24, 2016 at 10:35:38AM -0700, Tony Lindgren wrote: > From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > Date: Mon, 24 Oct 2016 09:18:02 -0700 > Subject: [PATCH] usb: musb: Fix sleeping function called from invalid > context for hdrc glue > > Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer > that runs in softirq context. That causes a "BUG: sleeping function called > from invalid context" every time when polling the cable status: > > [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > > I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. > > Let's fix the issue by adding dsps_check_status() and dsps_runtime_resume() > functions. And let's have otg_timer() check for the PM runtime status, and > if not already enabled, have dsps_runtime_resume() call dsps_check_status() > directly. While this makes the sleeping-while-atomic warning go away, it does not seem correct. In case we were suspended, the glue layer will now call dsps_check_status while the controller (child) is still suspended: [ 8.577538] musb-hdrc musb-hdrc.0.auto: otg_timer [ 8.582642] musb-dsps 47401400.usb: dsps_runtime_resume [ 8.600651] musb-hdrc musb-hdrc.0.auto: dsps_check_status [ 8.630617] musb-hdrc musb-hdrc.0.auto: musb_runtime_resume Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-10-25 8:32 ` Johan Hovold @ 2016-10-25 15:11 ` Tony Lindgren [not found] ` <20161025151110.vih52s47a2g2col5-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-10-25 15:11 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161025 01:33]: > On Mon, Oct 24, 2016 at 10:35:38AM -0700, Tony Lindgren wrote: > > > From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > > Date: Mon, 24 Oct 2016 09:18:02 -0700 > > Subject: [PATCH] usb: musb: Fix sleeping function called from invalid > > context for hdrc glue > > > > Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > > glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer > > that runs in softirq context. That causes a "BUG: sleeping function called > > from invalid context" every time when polling the cable status: > > > > [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > > [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > > [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > > [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > > [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > > [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > > > > I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. > > > > Let's fix the issue by adding dsps_check_status() and dsps_runtime_resume() > > functions. And let's have otg_timer() check for the PM runtime status, and > > if not already enabled, have dsps_runtime_resume() call dsps_check_status() > > directly. > > While this makes the sleeping-while-atomic warning go away, it does not > seem correct. In case we were suspended, the glue layer will now call > dsps_check_status while the controller (child) is still suspended: > > [ 8.577538] musb-hdrc musb-hdrc.0.auto: otg_timer > [ 8.582642] musb-dsps 47401400.usb: dsps_runtime_resume > [ 8.600651] musb-hdrc musb-hdrc.0.auto: dsps_check_status > [ 8.630617] musb-hdrc musb-hdrc.0.auto: musb_runtime_resume Hmm that's a good point yeah. If we start doing something generic in musb-core.c musb_runtime_resume, things could go wrong in the future. With this particular hardware musb registers are always accessible though when the parent of the two musb instances musb_am335x is done. We could just reprogam the otg timer if !pm_runtime_active(dev). But the sucky things with that approach is that when idle we have two timers, one to wake-up, then another one to check the status. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161025151110.vih52s47a2g2col5-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161025151110.vih52s47a2g2col5-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-10-26 14:20 ` Johan Hovold 2016-10-26 14:31 ` Tony Lindgren 0 siblings, 1 reply; 24+ messages in thread From: Johan Hovold @ 2016-10-26 14:20 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA On Tue, Oct 25, 2016 at 08:11:10AM -0700, Tony Lindgren wrote: > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161025 01:33]: > > On Mon, Oct 24, 2016 at 10:35:38AM -0700, Tony Lindgren wrote: > > > > > From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > > > Date: Mon, 24 Oct 2016 09:18:02 -0700 > > > Subject: [PATCH] usb: musb: Fix sleeping function called from invalid > > > context for hdrc glue > > > > > > Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > > > glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer > > > that runs in softirq context. That causes a "BUG: sleeping function called > > > from invalid context" every time when polling the cable status: > > > > > > [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > > > [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > > > [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > > > [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > > > [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > > > [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > > > > > > I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. > > > > > > Let's fix the issue by adding dsps_check_status() and dsps_runtime_resume() > > > functions. And let's have otg_timer() check for the PM runtime status, and > > > if not already enabled, have dsps_runtime_resume() call dsps_check_status() > > > directly. > > > > While this makes the sleeping-while-atomic warning go away, it does not > > seem correct. In case we were suspended, the glue layer will now call > > dsps_check_status while the controller (child) is still suspended: > > > > [ 8.577538] musb-hdrc musb-hdrc.0.auto: otg_timer > > [ 8.582642] musb-dsps 47401400.usb: dsps_runtime_resume > > [ 8.600651] musb-hdrc musb-hdrc.0.auto: dsps_check_status > > [ 8.630617] musb-hdrc musb-hdrc.0.auto: musb_runtime_resume > > Hmm that's a good point yeah. If we start doing something generic in > musb-core.c musb_runtime_resume, things could go wrong in the future. > > With this particular hardware musb registers are always accessible > though when the parent of the two musb instances musb_am335x is done. > > We could just reprogam the otg timer if !pm_runtime_active(dev). But > the sucky things with that approach is that when idle we have two > timers, one to wake-up, then another one to check the status. Maybe add another callback from musb_runtime_resume that can be used for such deferred work instead? Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-10-26 14:20 ` Johan Hovold @ 2016-10-26 14:31 ` Tony Lindgren [not found] ` <20161026143100.rg4pse6mjyq32hxm-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-10-26 14:31 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161026 07:21]: > On Tue, Oct 25, 2016 at 08:11:10AM -0700, Tony Lindgren wrote: > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161025 01:33]: > > > On Mon, Oct 24, 2016 at 10:35:38AM -0700, Tony Lindgren wrote: > > > > > > > From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > > > > Date: Mon, 24 Oct 2016 09:18:02 -0700 > > > > Subject: [PATCH] usb: musb: Fix sleeping function called from invalid > > > > context for hdrc glue > > > > > > > > Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > > > > glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer > > > > that runs in softirq context. That causes a "BUG: sleeping function called > > > > from invalid context" every time when polling the cable status: > > > > > > > > [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > > > > [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > > > > [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > > > > [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > > > > [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > > > > [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > > > > > > > > I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. > > > > > > > > Let's fix the issue by adding dsps_check_status() and dsps_runtime_resume() > > > > functions. And let's have otg_timer() check for the PM runtime status, and > > > > if not already enabled, have dsps_runtime_resume() call dsps_check_status() > > > > directly. > > > > > > While this makes the sleeping-while-atomic warning go away, it does not > > > seem correct. In case we were suspended, the glue layer will now call > > > dsps_check_status while the controller (child) is still suspended: > > > > > > [ 8.577538] musb-hdrc musb-hdrc.0.auto: otg_timer > > > [ 8.582642] musb-dsps 47401400.usb: dsps_runtime_resume > > > [ 8.600651] musb-hdrc musb-hdrc.0.auto: dsps_check_status > > > [ 8.630617] musb-hdrc musb-hdrc.0.auto: musb_runtime_resume > > > > Hmm that's a good point yeah. If we start doing something generic in > > musb-core.c musb_runtime_resume, things could go wrong in the future. > > > > With this particular hardware musb registers are always accessible > > though when the parent of the two musb instances musb_am335x is done. > > > > We could just reprogam the otg timer if !pm_runtime_active(dev). But > > the sucky things with that approach is that when idle we have two > > timers, one to wake-up, then another one to check the status. > > Maybe add another callback from musb_runtime_resume that can be used for > such deferred work instead? OK that's a good idea :) Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161026143100.rg4pse6mjyq32hxm-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161026143100.rg4pse6mjyq32hxm-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-10-27 15:14 ` Tony Lindgren [not found] ` <20161027151446.ffj6w2tydf6ymv7c-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-10-27 15:14 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161026 07:32]: > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161026 07:21]: > > On Tue, Oct 25, 2016 at 08:11:10AM -0700, Tony Lindgren wrote: > > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161025 01:33]: > > > > On Mon, Oct 24, 2016 at 10:35:38AM -0700, Tony Lindgren wrote: > > > > > > > > > From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > > > > > Date: Mon, 24 Oct 2016 09:18:02 -0700 > > > > > Subject: [PATCH] usb: musb: Fix sleeping function called from invalid > > > > > context for hdrc glue > > > > > > > > > > Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > > > > > glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer > > > > > that runs in softirq context. That causes a "BUG: sleeping function called > > > > > from invalid context" every time when polling the cable status: > > > > > > > > > > [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > > > > > [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > > > > > [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > > > > > [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > > > > > [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > > > > > [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > > > > > > > > > > I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. > > > > > > > > > > Let's fix the issue by adding dsps_check_status() and dsps_runtime_resume() > > > > > functions. And let's have otg_timer() check for the PM runtime status, and > > > > > if not already enabled, have dsps_runtime_resume() call dsps_check_status() > > > > > directly. > > > > > > > > While this makes the sleeping-while-atomic warning go away, it does not > > > > seem correct. In case we were suspended, the glue layer will now call > > > > dsps_check_status while the controller (child) is still suspended: > > > > > > > > [ 8.577538] musb-hdrc musb-hdrc.0.auto: otg_timer > > > > [ 8.582642] musb-dsps 47401400.usb: dsps_runtime_resume > > > > [ 8.600651] musb-hdrc musb-hdrc.0.auto: dsps_check_status > > > > [ 8.630617] musb-hdrc musb-hdrc.0.auto: musb_runtime_resume > > > > > > Hmm that's a good point yeah. If we start doing something generic in > > > musb-core.c musb_runtime_resume, things could go wrong in the future. > > > > > > With this particular hardware musb registers are always accessible > > > though when the parent of the two musb instances musb_am335x is done. > > > > > > We could just reprogam the otg timer if !pm_runtime_active(dev). But > > > the sucky things with that approach is that when idle we have two > > > timers, one to wake-up, then another one to check the status. > > > > Maybe add another callback from musb_runtime_resume that can be used for > > such deferred work instead? > > OK that's a good idea :) Here's this fix updated for the callback. I also noticed that we have musb_gadget_queue() also doing pm_runtime_get_sync so that's now fixed too. Regards, Tony 8< ------------------------------- >From tony Mon Sep 17 00:00:00 2001 From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> Date: Tue, 25 Oct 2016 08:42:00 -0700 Subject: [PATCH] usb: musb: Fix sleeping function called from invalid context for hdrc glue Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer that runs in softirq context. That causes a "BUG: sleeping function called from invalid context" every time when polling the cable status: [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. Let's fix the issue by adding dsps_check_status() and then register a callback with musb_runtime_resume() so it gets called only when musb core and it's parent devices are awake. Note that we don't want to do this from PM runtime resume in musb_dsps.c as musb core is not awake yet at that point as noted by Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>. Note that musb_gadget_queue() also suffers from a similar issue when connecting the cable and cannot use pm_runtime_get_sync(). Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") Reported-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> --- drivers/usb/musb/musb_core.c | 44 +++++++++++++++++++++++++++++++++++++++++- drivers/usb/musb/musb_core.h | 7 +++++++ drivers/usb/musb/musb_dsps.c | 29 +++++++++++++++++++++------- drivers/usb/musb/musb_gadget.c | 16 +++++++++++++-- 4 files changed, 86 insertions(+), 10 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -122,7 +122,6 @@ MODULE_AUTHOR(DRIVER_AUTHOR); MODULE_LICENSE("GPL"); MODULE_ALIAS("platform:" MUSB_DRIVER_NAME); - /*-------------------------------------------------------------------------*/ static inline struct musb *dev_to_musb(struct device *dev) @@ -1896,6 +1895,46 @@ static void musb_pm_runtime_check_session(struct musb *musb) musb->session = s; } +struct musb_resume_work { + void (*callback)(struct musb *musb, void *data); + void *data; + struct list_head node; +}; + +void musb_queue_on_resume(struct musb *musb, + void (*callback)(struct musb *musb, void *data), + void *data) +{ + struct musb_resume_work *w; + unsigned long flags; + + w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC); + if (!w) + return; + + w->callback = callback; + w->data = data; + spin_lock_irqsave(&musb->list_lock, flags); + list_add_tail(&w->node, &musb->resume_work); + spin_unlock_irqrestore(&musb->list_lock, flags); +} +EXPORT_SYMBOL_GPL(musb_queue_on_resume); + +static void musb_run_pending(struct musb *musb) +{ + struct musb_resume_work *w, *_w; + unsigned long flags; + + spin_lock_irqsave(&musb->list_lock, flags); + list_for_each_entry_safe(w, _w, &musb->resume_work, node) { + if (w->callback) + w->callback(musb, w->data); + list_del(&w->node); + devm_kfree(musb->controller, w); + } + spin_unlock_irqrestore(&musb->list_lock, flags); +} + /* Only used to provide driver mode change events */ static void musb_irq_work(struct work_struct *data) { @@ -1969,6 +2008,7 @@ static struct musb *allocate_instance(struct device *dev, INIT_LIST_HEAD(&musb->control); INIT_LIST_HEAD(&musb->in_bulk); INIT_LIST_HEAD(&musb->out_bulk); + INIT_LIST_HEAD(&musb->resume_work); musb->vbuserr_retry = VBUSERR_RETRY_COUNT; musb->a_wait_bcon = OTG_TIME_A_WAIT_BCON; @@ -2065,6 +2105,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) } spin_lock_init(&musb->lock); + spin_lock_init(&musb->list_lock); musb->board_set_power = plat->set_power; musb->min_power = plat->min_power; musb->ops = plat->platform_ops; @@ -2374,6 +2415,7 @@ static int musb_remove(struct platform_device *pdev) * - Peripheral mode: peripheral is deactivated (or never-activated) * - OTG mode: both roles are deactivated (or never-activated) */ + musb_run_pending(musb); musb_exit_debugfs(musb); cancel_work_sync(&musb->irq_work); diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -303,6 +303,7 @@ struct musb_context_registers { struct musb { /* device lock */ spinlock_t lock; + spinlock_t list_lock; /* resume work list lock */ struct musb_io io; const struct musb_platform_ops *ops; @@ -337,6 +338,7 @@ struct musb { struct list_head control; /* of musb_qh */ struct list_head in_bulk; /* of musb_qh */ struct list_head out_bulk; /* of musb_qh */ + struct list_head resume_work; /* pending work on resume */ struct timer_list otg_timer; struct notifier_block nb; @@ -540,6 +542,11 @@ extern irqreturn_t musb_interrupt(struct musb *); extern void musb_hnp_stop(struct musb *musb); +extern void +musb_queue_on_resume(struct musb *musb, + void (*callback)(struct musb *musb, void *data), + void *data); + static inline void musb_platform_set_vbus(struct musb *musb, int is_on) { if (musb->ops->set_vbus) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -188,9 +188,8 @@ static void dsps_musb_disable(struct musb *musb) musb_writeb(musb->mregs, MUSB_DEVCTL, 0); } -static void otg_timer(unsigned long _musb) +static void dsps_check_status(struct musb *musb) { - struct musb *musb = (void *)_musb; void __iomem *mregs = musb->mregs; struct device *dev = musb->controller; struct dsps_glue *glue = dev_get_drvdata(dev->parent); @@ -198,11 +197,6 @@ static void otg_timer(unsigned long _musb) u8 devctl; unsigned long flags; int skip_session = 0; - int err; - - err = pm_runtime_get_sync(dev); - if (err < 0) - dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); /* * We poll because DSPS IP's won't expose several OTG-critical @@ -246,6 +240,27 @@ static void otg_timer(unsigned long _musb) break; } spin_unlock_irqrestore(&musb->lock, flags); +} + +static void dsps_check_status_resume_work(struct musb *musb, void *unused) +{ + dsps_check_status(musb); +} + +static void otg_timer(unsigned long _musb) +{ + struct musb *musb = (void *)_musb; + struct device *dev = musb->controller; + int err; + + err = pm_runtime_get(dev); + if (err < 0) + dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); + + if (pm_runtime_active(dev)) + dsps_check_status(musb); + else + musb_queue_on_resume(musb, dsps_check_status_resume_work, NULL); pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -1222,6 +1222,13 @@ void musb_ep_restart(struct musb *musb, struct musb_request *req) rxstate(musb, req); } +void musb_ep_restart_resume_work(struct musb *musb, void *data) +{ + struct musb_request *req = data; + + musb_ep_restart(musb, req); +} + static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, gfp_t gfp_flags) { @@ -1271,8 +1278,13 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, list_add_tail(&request->list, &musb_ep->req_list); /* it this is the head of the queue, start i/o ... */ - if (!musb_ep->busy && &request->list == musb_ep->req_list.next) - musb_ep_restart(musb, request); + if (!musb_ep->busy && &request->list == musb_ep->req_list.next) { + if (pm_runtime_active(musb->controller)) + musb_ep_restart(musb, request); + else + musb_queue_on_resume(musb, musb_ep_restart_resume_work, + request); + } unlock: spin_unlock_irqrestore(&musb->lock, lockflags); -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161027151446.ffj6w2tydf6ymv7c-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161027151446.ffj6w2tydf6ymv7c-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-10-27 16:44 ` Johan Hovold 2016-10-27 17:40 ` Tony Lindgren 0 siblings, 1 reply; 24+ messages in thread From: Johan Hovold @ 2016-10-27 16:44 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA On Thu, Oct 27, 2016 at 08:14:46AM -0700, Tony Lindgren wrote: > * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161026 07:32]: > 8< ------------------------------- > From tony Mon Sep 17 00:00:00 2001 > From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > Date: Tue, 25 Oct 2016 08:42:00 -0700 > Subject: [PATCH] usb: musb: Fix sleeping function called from invalid > context for hdrc glue > > Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer > that runs in softirq context. That causes a "BUG: sleeping function called > from invalid context" every time when polling the cable status: > > [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > > I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. > > Let's fix the issue by adding dsps_check_status() and then register a > callback with musb_runtime_resume() so it gets called only when musb core > and it's parent devices are awake. Note that we don't want to do this from > PM runtime resume in musb_dsps.c as musb core is not awake yet at that > point as noted by Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>. It seems some chunks are missing since this patch only runs the deferred work at remove and not at runtime resume, effectively breaking detection. > Note that musb_gadget_queue() also suffers from a similar issue when > connecting the cable and cannot use pm_runtime_get_sync(). You seem to have left that pm_runtime_get_sync() in there though. > Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > glue layer") > Reported-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > --- > drivers/usb/musb/musb_core.c | 44 +++++++++++++++++++++++++++++++++++++++++- > drivers/usb/musb/musb_core.h | 7 +++++++ > drivers/usb/musb/musb_dsps.c | 29 +++++++++++++++++++++------- > drivers/usb/musb/musb_gadget.c | 16 +++++++++++++-- > 4 files changed, 86 insertions(+), 10 deletions(-) > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > --- a/drivers/usb/musb/musb_core.c > +++ b/drivers/usb/musb/musb_core.c > @@ -122,7 +122,6 @@ MODULE_AUTHOR(DRIVER_AUTHOR); > MODULE_LICENSE("GPL"); > MODULE_ALIAS("platform:" MUSB_DRIVER_NAME); > > - > /*-------------------------------------------------------------------------*/ > > static inline struct musb *dev_to_musb(struct device *dev) > @@ -1896,6 +1895,46 @@ static void musb_pm_runtime_check_session(struct musb *musb) > musb->session = s; > } > > +struct musb_resume_work { > + void (*callback)(struct musb *musb, void *data); > + void *data; > + struct list_head node; > +}; > + > +void musb_queue_on_resume(struct musb *musb, > + void (*callback)(struct musb *musb, void *data), > + void *data) > +{ > + struct musb_resume_work *w; > + unsigned long flags; > + > + w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC); > + if (!w) > + return; > + > + w->callback = callback; > + w->data = data; > + spin_lock_irqsave(&musb->list_lock, flags); > + list_add_tail(&w->node, &musb->resume_work); > + spin_unlock_irqrestore(&musb->list_lock, flags); > +} > +EXPORT_SYMBOL_GPL(musb_queue_on_resume); > + > +static void musb_run_pending(struct musb *musb) > +{ > + struct musb_resume_work *w, *_w; > + unsigned long flags; > + > + spin_lock_irqsave(&musb->list_lock, flags); > + list_for_each_entry_safe(w, _w, &musb->resume_work, node) { > + if (w->callback) > + w->callback(musb, w->data); > + list_del(&w->node); > + devm_kfree(musb->controller, w); > + } > + spin_unlock_irqrestore(&musb->list_lock, flags); > +} > + > /* Only used to provide driver mode change events */ > static void musb_irq_work(struct work_struct *data) > { > @@ -1969,6 +2008,7 @@ static struct musb *allocate_instance(struct device *dev, > INIT_LIST_HEAD(&musb->control); > INIT_LIST_HEAD(&musb->in_bulk); > INIT_LIST_HEAD(&musb->out_bulk); > + INIT_LIST_HEAD(&musb->resume_work); > > musb->vbuserr_retry = VBUSERR_RETRY_COUNT; > musb->a_wait_bcon = OTG_TIME_A_WAIT_BCON; > @@ -2065,6 +2105,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) > } > > spin_lock_init(&musb->lock); > + spin_lock_init(&musb->list_lock); > musb->board_set_power = plat->set_power; > musb->min_power = plat->min_power; > musb->ops = plat->platform_ops; > @@ -2374,6 +2415,7 @@ static int musb_remove(struct platform_device *pdev) > * - Peripheral mode: peripheral is deactivated (or never-activated) > * - OTG mode: both roles are deactivated (or never-activated) > */ > + musb_run_pending(musb); > musb_exit_debugfs(musb); > > cancel_work_sync(&musb->irq_work); > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > --- a/drivers/usb/musb/musb_core.h > +++ b/drivers/usb/musb/musb_core.h > @@ -303,6 +303,7 @@ struct musb_context_registers { > struct musb { > /* device lock */ > spinlock_t lock; > + spinlock_t list_lock; /* resume work list lock */ > > struct musb_io io; > const struct musb_platform_ops *ops; > @@ -337,6 +338,7 @@ struct musb { > struct list_head control; /* of musb_qh */ > struct list_head in_bulk; /* of musb_qh */ > struct list_head out_bulk; /* of musb_qh */ > + struct list_head resume_work; /* pending work on resume */ > > struct timer_list otg_timer; > struct notifier_block nb; > @@ -540,6 +542,11 @@ extern irqreturn_t musb_interrupt(struct musb *); > > extern void musb_hnp_stop(struct musb *musb); > > +extern void > +musb_queue_on_resume(struct musb *musb, > + void (*callback)(struct musb *musb, void *data), > + void *data); > + > static inline void musb_platform_set_vbus(struct musb *musb, int is_on) > { > if (musb->ops->set_vbus) > diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c > --- a/drivers/usb/musb/musb_dsps.c > +++ b/drivers/usb/musb/musb_dsps.c > @@ -188,9 +188,8 @@ static void dsps_musb_disable(struct musb *musb) > musb_writeb(musb->mregs, MUSB_DEVCTL, 0); > } > > -static void otg_timer(unsigned long _musb) > +static void dsps_check_status(struct musb *musb) > { > - struct musb *musb = (void *)_musb; > void __iomem *mregs = musb->mregs; > struct device *dev = musb->controller; > struct dsps_glue *glue = dev_get_drvdata(dev->parent); > @@ -198,11 +197,6 @@ static void otg_timer(unsigned long _musb) > u8 devctl; > unsigned long flags; > int skip_session = 0; > - int err; > - > - err = pm_runtime_get_sync(dev); > - if (err < 0) > - dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); > > /* > * We poll because DSPS IP's won't expose several OTG-critical > @@ -246,6 +240,27 @@ static void otg_timer(unsigned long _musb) > break; > } > spin_unlock_irqrestore(&musb->lock, flags); > +} > + > +static void dsps_check_status_resume_work(struct musb *musb, void *unused) > +{ > + dsps_check_status(musb); > +} > + > +static void otg_timer(unsigned long _musb) > +{ > + struct musb *musb = (void *)_musb; > + struct device *dev = musb->controller; > + int err; > + > + err = pm_runtime_get(dev); > + if (err < 0) > + dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); > + > + if (pm_runtime_active(dev)) > + dsps_check_status(musb); > + else > + musb_queue_on_resume(musb, dsps_check_status_resume_work, NULL); > > pm_runtime_mark_last_busy(dev); > pm_runtime_put_autosuspend(dev); > diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c > --- a/drivers/usb/musb/musb_gadget.c > +++ b/drivers/usb/musb/musb_gadget.c > @@ -1222,6 +1222,13 @@ void musb_ep_restart(struct musb *musb, struct musb_request *req) > rxstate(musb, req); > } > > +void musb_ep_restart_resume_work(struct musb *musb, void *data) > +{ > + struct musb_request *req = data; > + > + musb_ep_restart(musb, req); This one is supposed to be called with musb->lock held (according to the function header anyway). > +} > + > static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > gfp_t gfp_flags) > { > @@ -1271,8 +1278,13 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > list_add_tail(&request->list, &musb_ep->req_list); > > /* it this is the head of the queue, start i/o ... */ > - if (!musb_ep->busy && &request->list == musb_ep->req_list.next) > - musb_ep_restart(musb, request); > + if (!musb_ep->busy && &request->list == musb_ep->req_list.next) { > + if (pm_runtime_active(musb->controller)) > + musb_ep_restart(musb, request); > + else > + musb_queue_on_resume(musb, musb_ep_restart_resume_work, > + request); > + } > > unlock: > spin_unlock_irqrestore(&musb->lock, lockflags); And then this looks like it could trigger an ABBA deadlock as musb->lock is held while queue_on_resume() takes musb->list_lock, and musb_run_pending() would take the same locks in the reverse order. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-10-27 16:44 ` Johan Hovold @ 2016-10-27 17:40 ` Tony Lindgren [not found] ` <20161027174016.43twztwekvb3b25t-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-10-27 17:40 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161027 09:45]: > On Thu, Oct 27, 2016 at 08:14:46AM -0700, Tony Lindgren wrote: > > * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161026 07:32]: > > > 8< ------------------------------- > > From tony Mon Sep 17 00:00:00 2001 > > From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > > Date: Tue, 25 Oct 2016 08:42:00 -0700 > > Subject: [PATCH] usb: musb: Fix sleeping function called from invalid > > context for hdrc glue > > > > Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > > glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer > > that runs in softirq context. That causes a "BUG: sleeping function called > > from invalid context" every time when polling the cable status: > > > > [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > > [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > > [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > > [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > > [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > > [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > > > > I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. > > > > Let's fix the issue by adding dsps_check_status() and then register a > > callback with musb_runtime_resume() so it gets called only when musb core > > and it's parent devices are awake. Note that we don't want to do this from > > PM runtime resume in musb_dsps.c as musb core is not awake yet at that > > point as noted by Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>. > > It seems some chunks are missing since this patch only runs the > deferred work at remove and not at runtime resume, effectively breaking > detection. Oops sorry yeah looks like I had that in a separate debug hacks patch.. > > Note that musb_gadget_queue() also suffers from a similar issue when > > connecting the cable and cannot use pm_runtime_get_sync(). > > You seem to have left that pm_runtime_get_sync() in there though. And that one I must have hosed when cleaning up, thanks for noticing these. Updated patch below. Regards, Tony 8< --------------------- >From tony Mon Sep 17 00:00:00 2001 From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> Date: Tue, 25 Oct 2016 08:42:00 -0700 Subject: [PATCH] usb: musb: Fix sleeping function called from invalid context for hdrc glue Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer that runs in softirq context. That causes a "BUG: sleeping function called from invalid context" every time when polling the cable status: [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. Let's fix the issue by adding dsps_check_status() and then register a callback with musb_runtime_resume() so it gets called only when musb core and it's parent devices are awake. Note that we don't want to do this from PM runtime resume in musb_dsps.c as musb core is not awake yet at that point as noted by Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>. Note that musb_gadget_queue() also suffers from a similar issue when connecting the cable and cannot use pm_runtime_get_sync(). Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") Reported-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> --- drivers/usb/musb/musb_core.c | 48 ++++++++++++++++++++++++++++++++++++++++-- drivers/usb/musb/musb_core.h | 7 ++++++ drivers/usb/musb/musb_dsps.c | 29 +++++++++++++++++++------ drivers/usb/musb/musb_gadget.c | 18 +++++++++++++--- 4 files changed, 90 insertions(+), 12 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -122,7 +122,6 @@ MODULE_AUTHOR(DRIVER_AUTHOR); MODULE_LICENSE("GPL"); MODULE_ALIAS("platform:" MUSB_DRIVER_NAME); - /*-------------------------------------------------------------------------*/ static inline struct musb *dev_to_musb(struct device *dev) @@ -1896,6 +1895,46 @@ static void musb_pm_runtime_check_session(struct musb *musb) musb->session = s; } +struct musb_resume_work { + void (*callback)(struct musb *musb, void *data); + void *data; + struct list_head node; +}; + +void musb_queue_on_resume(struct musb *musb, + void (*callback)(struct musb *musb, void *data), + void *data) +{ + struct musb_resume_work *w; + unsigned long flags; + + w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC); + if (!w) + return; + + w->callback = callback; + w->data = data; + spin_lock_irqsave(&musb->list_lock, flags); + list_add_tail(&w->node, &musb->resume_work); + spin_unlock_irqrestore(&musb->list_lock, flags); +} +EXPORT_SYMBOL_GPL(musb_queue_on_resume); + +static void musb_run_pending(struct musb *musb) +{ + struct musb_resume_work *w, *_w; + unsigned long flags; + + spin_lock_irqsave(&musb->list_lock, flags); + list_for_each_entry_safe(w, _w, &musb->resume_work, node) { + if (w->callback) + w->callback(musb, w->data); + list_del(&w->node); + devm_kfree(musb->controller, w); + } + spin_unlock_irqrestore(&musb->list_lock, flags); +} + /* Only used to provide driver mode change events */ static void musb_irq_work(struct work_struct *data) { @@ -1969,6 +2008,7 @@ static struct musb *allocate_instance(struct device *dev, INIT_LIST_HEAD(&musb->control); INIT_LIST_HEAD(&musb->in_bulk); INIT_LIST_HEAD(&musb->out_bulk); + INIT_LIST_HEAD(&musb->resume_work); musb->vbuserr_retry = VBUSERR_RETRY_COUNT; musb->a_wait_bcon = OTG_TIME_A_WAIT_BCON; @@ -2065,6 +2105,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) } spin_lock_init(&musb->lock); + spin_lock_init(&musb->list_lock); musb->board_set_power = plat->set_power; musb->min_power = plat->min_power; musb->ops = plat->platform_ops; @@ -2374,6 +2415,7 @@ static int musb_remove(struct platform_device *pdev) * - Peripheral mode: peripheral is deactivated (or never-activated) * - OTG mode: both roles are deactivated (or never-activated) */ + musb_run_pending(musb); musb_exit_debugfs(musb); cancel_work_sync(&musb->irq_work); @@ -2645,8 +2687,10 @@ static int musb_runtime_resume(struct device *dev) * Also context restore without save does not make * any sense */ - if (!first) + if (!first) { musb_restore_context(musb); + musb_run_pending(musb); + } first = 0; if (musb->need_finish_resume) { diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -303,6 +303,7 @@ struct musb_context_registers { struct musb { /* device lock */ spinlock_t lock; + spinlock_t list_lock; /* resume work list lock */ struct musb_io io; const struct musb_platform_ops *ops; @@ -337,6 +338,7 @@ struct musb { struct list_head control; /* of musb_qh */ struct list_head in_bulk; /* of musb_qh */ struct list_head out_bulk; /* of musb_qh */ + struct list_head resume_work; /* pending work on resume */ struct timer_list otg_timer; struct notifier_block nb; @@ -540,6 +542,11 @@ extern irqreturn_t musb_interrupt(struct musb *); extern void musb_hnp_stop(struct musb *musb); +extern void +musb_queue_on_resume(struct musb *musb, + void (*callback)(struct musb *musb, void *data), + void *data); + static inline void musb_platform_set_vbus(struct musb *musb, int is_on) { if (musb->ops->set_vbus) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -188,9 +188,8 @@ static void dsps_musb_disable(struct musb *musb) musb_writeb(musb->mregs, MUSB_DEVCTL, 0); } -static void otg_timer(unsigned long _musb) +static void dsps_check_status(struct musb *musb) { - struct musb *musb = (void *)_musb; void __iomem *mregs = musb->mregs; struct device *dev = musb->controller; struct dsps_glue *glue = dev_get_drvdata(dev->parent); @@ -198,11 +197,6 @@ static void otg_timer(unsigned long _musb) u8 devctl; unsigned long flags; int skip_session = 0; - int err; - - err = pm_runtime_get_sync(dev); - if (err < 0) - dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); /* * We poll because DSPS IP's won't expose several OTG-critical @@ -246,6 +240,27 @@ static void otg_timer(unsigned long _musb) break; } spin_unlock_irqrestore(&musb->lock, flags); +} + +static void dsps_check_status_resume_work(struct musb *musb, void *unused) +{ + dsps_check_status(musb); +} + +static void otg_timer(unsigned long _musb) +{ + struct musb *musb = (void *)_musb; + struct device *dev = musb->controller; + int err; + + err = pm_runtime_get(dev); + if (err < 0) + dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); + + if (pm_runtime_active(dev)) + dsps_check_status(musb); + else + musb_queue_on_resume(musb, dsps_check_status_resume_work, NULL); pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -1222,6 +1222,13 @@ void musb_ep_restart(struct musb *musb, struct musb_request *req) rxstate(musb, req); } +void musb_ep_restart_resume_work(struct musb *musb, void *data) +{ + struct musb_request *req = data; + + musb_ep_restart(musb, req); +} + static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, gfp_t gfp_flags) { @@ -1255,7 +1262,7 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, map_dma_buffer(request, musb, musb_ep); - pm_runtime_get_sync(musb->controller); + pm_runtime_get(musb->controller); spin_lock_irqsave(&musb->lock, lockflags); /* don't queue if the ep is down */ @@ -1271,8 +1278,13 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, list_add_tail(&request->list, &musb_ep->req_list); /* it this is the head of the queue, start i/o ... */ - if (!musb_ep->busy && &request->list == musb_ep->req_list.next) - musb_ep_restart(musb, request); + if (!musb_ep->busy && &request->list == musb_ep->req_list.next) { + if (pm_runtime_active(musb->controller)) + musb_ep_restart(musb, request); + else + musb_queue_on_resume(musb, musb_ep_restart_resume_work, + request); + } unlock: spin_unlock_irqrestore(&musb->lock, lockflags); -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161027174016.43twztwekvb3b25t-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161027174016.43twztwekvb3b25t-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-10-27 18:45 ` Johan Hovold 2016-10-27 19:15 ` Tony Lindgren 0 siblings, 1 reply; 24+ messages in thread From: Johan Hovold @ 2016-10-27 18:45 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA On Thu, Oct 27, 2016 at 10:40:17AM -0700, Tony Lindgren wrote: > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161027 09:45]: > > On Thu, Oct 27, 2016 at 08:14:46AM -0700, Tony Lindgren wrote: > > > * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [161026 07:32]: > > > > > 8< ------------------------------- > > > From tony Mon Sep 17 00:00:00 2001 > > > From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > > > Date: Tue, 25 Oct 2016 08:42:00 -0700 > > > Subject: [PATCH] usb: musb: Fix sleeping function called from invalid > > > context for hdrc glue > > > > > > Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > > > glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer > > > that runs in softirq context. That causes a "BUG: sleeping function called > > > from invalid context" every time when polling the cable status: > > > > > > [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > > > [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > > > [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > > > [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > > > [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > > > [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > > > > > > I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. > > > > > > Let's fix the issue by adding dsps_check_status() and then register a > > > callback with musb_runtime_resume() so it gets called only when musb core > > > and it's parent devices are awake. Note that we don't want to do this from > > > PM runtime resume in musb_dsps.c as musb core is not awake yet at that > > > point as noted by Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>. > > > > It seems some chunks are missing since this patch only runs the > > deferred work at remove and not at runtime resume, effectively breaking > > detection. > > Oops sorry yeah looks like I had that in a separate debug hacks patch.. > > > > Note that musb_gadget_queue() also suffers from a similar issue when > > > connecting the cable and cannot use pm_runtime_get_sync(). > > > > You seem to have left that pm_runtime_get_sync() in there though. > > And that one I must have hosed when cleaning up, thanks for noticing > these. Updated patch below. I had a couple of inline comments to the previous version about locking in the gadget code as well (hidden after too much context). Looks like there's a lock missing for the deferred work, and something that seems like a possible ABBA deadlock. > diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c > --- a/drivers/usb/musb/musb_gadget.c > +++ b/drivers/usb/musb/musb_gadget.c > @@ -1222,6 +1222,13 @@ void musb_ep_restart(struct musb *musb, struct musb_request *req) > rxstate(musb, req); > } > > +void musb_ep_restart_resume_work(struct musb *musb, void *data) > +{ > + struct musb_request *req = data; > + > + musb_ep_restart(musb, req); This one is supposed to be called with musb->lock held (according to the function header anyway). > +} > + > static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > gfp_t gfp_flags) > { > @@ -1255,7 +1262,7 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > > map_dma_buffer(request, musb, musb_ep); > > - pm_runtime_get_sync(musb->controller); > + pm_runtime_get(musb->controller); > spin_lock_irqsave(&musb->lock, lockflags); > > /* don't queue if the ep is down */ > @@ -1271,8 +1278,13 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > list_add_tail(&request->list, &musb_ep->req_list); > > /* it this is the head of the queue, start i/o ... */ > - if (!musb_ep->busy && &request->list == musb_ep->req_list.next) > - musb_ep_restart(musb, request); > + if (!musb_ep->busy && &request->list == musb_ep->req_list.next) { > + if (pm_runtime_active(musb->controller)) > + musb_ep_restart(musb, request); > + else > + musb_queue_on_resume(musb, musb_ep_restart_resume_work, > + request); > + } But then this looks like it could trigger an ABBA deadlock as musb->lock is held while queue_on_resume() takes musb->list_lock, and musb_run_pending() would take the same locks in the reverse order. > > unlock: > spin_unlock_irqrestore(&musb->lock, lockflags); Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-10-27 18:45 ` Johan Hovold @ 2016-10-27 19:15 ` Tony Lindgren [not found] ` <20161027191552.tuutyslp5qtu2b4f-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-10-27 19:15 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161027 11:46]: > On Thu, Oct 27, 2016 at 10:40:17AM -0700, Tony Lindgren wrote: > > And that one I must have hosed when cleaning up, thanks for noticing > > these. Updated patch below. > > I had a couple of inline comments to the previous version about locking > in the gadget code as well (hidden after too much context). Looks like > there's a lock missing for the deferred work, and something that seems > like a possible ABBA deadlock. Oh sorry, I totally missed those. > > diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c > > --- a/drivers/usb/musb/musb_gadget.c > > +++ b/drivers/usb/musb/musb_gadget.c > > @@ -1222,6 +1222,13 @@ void musb_ep_restart(struct musb *musb, struct musb_request *req) > > rxstate(musb, req); > > } > > > > +void musb_ep_restart_resume_work(struct musb *musb, void *data) > > +{ > > + struct musb_request *req = data; > > + > > + musb_ep_restart(musb, req); > > This one is supposed to be called with musb->lock held (according to the > function header anyway). Good point, yeah that calls the monster functions txstate and rxstate. > > static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > > gfp_t gfp_flags) > > { > > @@ -1255,7 +1262,7 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > > > > map_dma_buffer(request, musb, musb_ep); > > > > - pm_runtime_get_sync(musb->controller); > > + pm_runtime_get(musb->controller); > > spin_lock_irqsave(&musb->lock, lockflags); > > > > /* don't queue if the ep is down */ > > @@ -1271,8 +1278,13 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > > list_add_tail(&request->list, &musb_ep->req_list); > > > > /* it this is the head of the queue, start i/o ... */ > > - if (!musb_ep->busy && &request->list == musb_ep->req_list.next) > > - musb_ep_restart(musb, request); > > + if (!musb_ep->busy && &request->list == musb_ep->req_list.next) { > > + if (pm_runtime_active(musb->controller)) > > + musb_ep_restart(musb, request); > > + else > > + musb_queue_on_resume(musb, musb_ep_restart_resume_work, > > + request); > > + } > > But then this looks like it could trigger an ABBA deadlock as musb->lock > is held while queue_on_resume() takes musb->list_lock, and > musb_run_pending() would take the same locks in the reverse order. It seems we can avoid that by locking only list_add_tail() and list_del(): list_for_each_entry_safe(w, _w, &musb->resume_work, node) { spin_lock_irqsave(&musb->list_lock, flags); list_del(&w->node); spin_unlock_irqrestore(&musb->list_lock, flags); if (w->callback) w->callback(musb, w->data); devm_kfree(musb->controller, w); } Or do you have some better ideas? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161027191552.tuutyslp5qtu2b4f-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161027191552.tuutyslp5qtu2b4f-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-10-28 9:44 ` Johan Hovold 2016-10-28 18:13 ` Tony Lindgren 0 siblings, 1 reply; 24+ messages in thread From: Johan Hovold @ 2016-10-28 9:44 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA On Thu, Oct 27, 2016 at 12:15:52PM -0700, Tony Lindgren wrote: > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161027 11:46]: > > On Thu, Oct 27, 2016 at 10:40:17AM -0700, Tony Lindgren wrote: > > > diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c > > > --- a/drivers/usb/musb/musb_gadget.c > > > +++ b/drivers/usb/musb/musb_gadget.c > > > @@ -1222,6 +1222,13 @@ void musb_ep_restart(struct musb *musb, struct musb_request *req) > > > rxstate(musb, req); > > > } > > > > > > +void musb_ep_restart_resume_work(struct musb *musb, void *data) > > > +{ > > > + struct musb_request *req = data; > > > + > > > + musb_ep_restart(musb, req); > > > > This one is supposed to be called with musb->lock held (according to the > > function header anyway). > > Good point, yeah that calls the monster functions txstate and rxstate. > > > > static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > > > gfp_t gfp_flags) > > > { > > > @@ -1255,7 +1262,7 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > > > > > > map_dma_buffer(request, musb, musb_ep); > > > > > > - pm_runtime_get_sync(musb->controller); > > > + pm_runtime_get(musb->controller); > > > spin_lock_irqsave(&musb->lock, lockflags); > > > > > > /* don't queue if the ep is down */ > > > @@ -1271,8 +1278,13 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, > > > list_add_tail(&request->list, &musb_ep->req_list); > > > > > > /* it this is the head of the queue, start i/o ... */ > > > - if (!musb_ep->busy && &request->list == musb_ep->req_list.next) > > > - musb_ep_restart(musb, request); > > > + if (!musb_ep->busy && &request->list == musb_ep->req_list.next) { > > > + if (pm_runtime_active(musb->controller)) > > > + musb_ep_restart(musb, request); > > > + else > > > + musb_queue_on_resume(musb, musb_ep_restart_resume_work, > > > + request); > > > + } > > > > But then this looks like it could trigger an ABBA deadlock as musb->lock > > is held while queue_on_resume() takes musb->list_lock, and > > musb_run_pending() would take the same locks in the reverse order. > > It seems we can avoid that by locking only list_add_tail() and list_del(): > > list_for_each_entry_safe(w, _w, &musb->resume_work, node) { > spin_lock_irqsave(&musb->list_lock, flags); > list_del(&w->node); > spin_unlock_irqrestore(&musb->list_lock, flags); > if (w->callback) > w->callback(musb, w->data); > devm_kfree(musb->controller, w); > } I think you still need to hold the lock while traversing the list (even if you temporarily release it during the callback). But this is related to another issue; what if a new callback is registered while musb_runtime_resume is running? It seems we could end up queueing work which might never be executed (e.g. if queued just after musb_run_pending() is done or while processing the last callback). > Or do you have some better ideas? Not right now sorry, and I'll be away from keyboard the rest of the day, I'm afraid. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-10-28 9:44 ` Johan Hovold @ 2016-10-28 18:13 ` Tony Lindgren [not found] ` <20161028181318.umwn3rx55pg2cvwd-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-10-28 18:13 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161028 02:45]: > On Thu, Oct 27, 2016 at 12:15:52PM -0700, Tony Lindgren wrote: > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161027 11:46]: > > > But then this looks like it could trigger an ABBA deadlock as musb->lock > > > is held while queue_on_resume() takes musb->list_lock, and > > > musb_run_pending() would take the same locks in the reverse order. > > > > It seems we can avoid that by locking only list_add_tail() and list_del(): > > > > list_for_each_entry_safe(w, _w, &musb->resume_work, node) { > > spin_lock_irqsave(&musb->list_lock, flags); > > list_del(&w->node); > > spin_unlock_irqrestore(&musb->list_lock, flags); > > if (w->callback) > > w->callback(musb, w->data); > > devm_kfree(musb->controller, w); > > } > > I think you still need to hold the lock while traversing the list (even > if you temporarily release it during the callback). Hmm yeah we need iterate through the list again to avoid missing new elements being added. I've updated the patch to use a the common while (!list_empty(&musb->resume_work)) loop. Does that solve the concern you had or did you also had some other concern there? Regards, Tony 8< --------------------------- >From tony Mon Sep 17 00:00:00 2001 From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> Date: Tue, 25 Oct 2016 08:42:00 -0700 Subject: [PATCH] usb: musb: Fix sleeping function called from invalid context for hdrc glue Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer that runs in softirq context. That causes a "BUG: sleeping function called from invalid context" every time when polling the cable status: [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. Let's fix the issue by adding dsps_check_status() and then register a callback with musb_runtime_resume() so it gets called only when musb core and it's parent devices are awake. Note that we don't want to do this from PM runtime resume in musb_dsps.c as musb core is not awake yet at that point as noted by Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>. Note that musb_gadget_queue() also suffers from a similar issue when connecting the cable and cannot use pm_runtime_get_sync(). Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") Reported-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> --- drivers/usb/musb/musb_core.c | 52 +++++++++++++++++++++++++++++++++++++++++- drivers/usb/musb/musb_core.h | 7 ++++++ drivers/usb/musb/musb_dsps.c | 29 +++++++++++++++++------ drivers/usb/musb/musb_gadget.c | 21 ++++++++++++++--- 4 files changed, 98 insertions(+), 11 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1896,6 +1896,51 @@ static void musb_pm_runtime_check_session(struct musb *musb) musb->session = s; } +struct musb_resume_work { + void (*callback)(struct musb *musb, void *data); + void *data; + struct list_head node; +}; + +void musb_queue_on_resume(struct musb *musb, + void (*callback)(struct musb *musb, void *data), + void *data) +{ + struct musb_resume_work *w; + unsigned long flags; + + w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC); + if (!w) + return; + + w->callback = callback; + w->data = data; + spin_lock_irqsave(&musb->list_lock, flags); + list_add_tail(&w->node, &musb->resume_work); + spin_unlock_irqrestore(&musb->list_lock, flags); +} +EXPORT_SYMBOL_GPL(musb_queue_on_resume); + +static void musb_run_pending(struct musb *musb) +{ + struct musb_resume_work *w; + unsigned long flags; + + spin_lock_irqsave(&musb->list_lock, flags); + while (!list_empty(&musb->resume_work)) { + w = list_first_entry(&musb->resume_work, + struct musb_resume_work, + node); + list_del(&w->node); + spin_unlock_irqrestore(&musb->list_lock, flags); + if (w->callback) + w->callback(musb, w->data); + devm_kfree(musb->controller, w); + spin_lock_irqsave(&musb->list_lock, flags); + } + spin_unlock_irqrestore(&musb->list_lock, flags); +} + /* Only used to provide driver mode change events */ static void musb_irq_work(struct work_struct *data) { @@ -1969,6 +2014,7 @@ static struct musb *allocate_instance(struct device *dev, INIT_LIST_HEAD(&musb->control); INIT_LIST_HEAD(&musb->in_bulk); INIT_LIST_HEAD(&musb->out_bulk); + INIT_LIST_HEAD(&musb->resume_work); musb->vbuserr_retry = VBUSERR_RETRY_COUNT; musb->a_wait_bcon = OTG_TIME_A_WAIT_BCON; @@ -2065,6 +2111,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) } spin_lock_init(&musb->lock); + spin_lock_init(&musb->list_lock); musb->board_set_power = plat->set_power; musb->min_power = plat->min_power; musb->ops = plat->platform_ops; @@ -2374,6 +2421,7 @@ static int musb_remove(struct platform_device *pdev) * - Peripheral mode: peripheral is deactivated (or never-activated) * - OTG mode: both roles are deactivated (or never-activated) */ + musb_run_pending(musb); musb_exit_debugfs(musb); cancel_work_sync(&musb->irq_work); @@ -2645,8 +2693,10 @@ static int musb_runtime_resume(struct device *dev) * Also context restore without save does not make * any sense */ - if (!first) + if (!first) { musb_restore_context(musb); + musb_run_pending(musb); + } first = 0; if (musb->need_finish_resume) { diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -303,6 +303,7 @@ struct musb_context_registers { struct musb { /* device lock */ spinlock_t lock; + spinlock_t list_lock; /* resume work list lock */ struct musb_io io; const struct musb_platform_ops *ops; @@ -337,6 +338,7 @@ struct musb { struct list_head control; /* of musb_qh */ struct list_head in_bulk; /* of musb_qh */ struct list_head out_bulk; /* of musb_qh */ + struct list_head resume_work; /* pending work on resume */ struct timer_list otg_timer; struct notifier_block nb; @@ -540,6 +542,11 @@ extern irqreturn_t musb_interrupt(struct musb *); extern void musb_hnp_stop(struct musb *musb); +extern void +musb_queue_on_resume(struct musb *musb, + void (*callback)(struct musb *musb, void *data), + void *data); + static inline void musb_platform_set_vbus(struct musb *musb, int is_on) { if (musb->ops->set_vbus) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -188,9 +188,8 @@ static void dsps_musb_disable(struct musb *musb) musb_writeb(musb->mregs, MUSB_DEVCTL, 0); } -static void otg_timer(unsigned long _musb) +static void dsps_check_status(struct musb *musb) { - struct musb *musb = (void *)_musb; void __iomem *mregs = musb->mregs; struct device *dev = musb->controller; struct dsps_glue *glue = dev_get_drvdata(dev->parent); @@ -198,11 +197,6 @@ static void otg_timer(unsigned long _musb) u8 devctl; unsigned long flags; int skip_session = 0; - int err; - - err = pm_runtime_get_sync(dev); - if (err < 0) - dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); /* * We poll because DSPS IP's won't expose several OTG-critical @@ -246,6 +240,27 @@ static void otg_timer(unsigned long _musb) break; } spin_unlock_irqrestore(&musb->lock, flags); +} + +static void dsps_check_status_resume_work(struct musb *musb, void *unused) +{ + dsps_check_status(musb); +} + +static void otg_timer(unsigned long _musb) +{ + struct musb *musb = (void *)_musb; + struct device *dev = musb->controller; + int err; + + err = pm_runtime_get(dev); + if (err < 0) + dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); + + if (pm_runtime_active(dev)) + dsps_check_status(musb); + else + musb_queue_on_resume(musb, dsps_check_status_resume_work, NULL); pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -1222,6 +1222,16 @@ void musb_ep_restart(struct musb *musb, struct musb_request *req) rxstate(musb, req); } +void musb_ep_restart_resume_work(struct musb *musb, void *data) +{ + struct musb_request *req = data; + unsigned long flags; + + spin_lock_irqsave(&musb->lock, flags); + musb_ep_restart(musb, req); + spin_unlock_irqrestore(&musb->lock, flags); +} + static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, gfp_t gfp_flags) { @@ -1255,7 +1265,7 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, map_dma_buffer(request, musb, musb_ep); - pm_runtime_get_sync(musb->controller); + pm_runtime_get(musb->controller); spin_lock_irqsave(&musb->lock, lockflags); /* don't queue if the ep is down */ @@ -1271,8 +1281,13 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, list_add_tail(&request->list, &musb_ep->req_list); /* it this is the head of the queue, start i/o ... */ - if (!musb_ep->busy && &request->list == musb_ep->req_list.next) - musb_ep_restart(musb, request); + if (!musb_ep->busy && &request->list == musb_ep->req_list.next) { + if (pm_runtime_active(musb->controller)) + musb_ep_restart(musb, request); + else + musb_queue_on_resume(musb, musb_ep_restart_resume_work, + request); + } unlock: spin_unlock_irqrestore(&musb->lock, lockflags); -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161028181318.umwn3rx55pg2cvwd-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161028181318.umwn3rx55pg2cvwd-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-10-31 11:49 ` Johan Hovold 2016-11-03 21:26 ` Tony Lindgren 0 siblings, 1 reply; 24+ messages in thread From: Johan Hovold @ 2016-10-31 11:49 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA On Fri, Oct 28, 2016 at 11:13:19AM -0700, Tony Lindgren wrote: > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161028 02:45]: > > On Thu, Oct 27, 2016 at 12:15:52PM -0700, Tony Lindgren wrote: > > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161027 11:46]: > > > > But then this looks like it could trigger an ABBA deadlock as musb->lock > > > > is held while queue_on_resume() takes musb->list_lock, and > > > > musb_run_pending() would take the same locks in the reverse order. > > > > > > It seems we can avoid that by locking only list_add_tail() and list_del(): > > > > > > list_for_each_entry_safe(w, _w, &musb->resume_work, node) { > > > spin_lock_irqsave(&musb->list_lock, flags); > > > list_del(&w->node); > > > spin_unlock_irqrestore(&musb->list_lock, flags); > > > if (w->callback) > > > w->callback(musb, w->data); > > > devm_kfree(musb->controller, w); > > > } > > > > I think you still need to hold the lock while traversing the list (even > > if you temporarily release it during the callback). > > Hmm yeah we need iterate through the list again to avoid missing new > elements being added. I've updated the patch to use a the common > while (!list_empty(&musb->resume_work)) loop. Does that solve the > concern you had or did you also had some other concern there? Yeah, while that minimises the race window it is still possible that the timer callback checks pm_runtime_active() after the queue has been processed but before the rpm status is updated. How about using a work struct and synchronous get for the deferred case? Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-10-31 11:49 ` Johan Hovold @ 2016-11-03 21:26 ` Tony Lindgren [not found] ` <20161103212635.GC21430-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 24+ messages in thread From: Tony Lindgren @ 2016-11-03 21:26 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, Ladislav Michl, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161031 04:50]: > On Fri, Oct 28, 2016 at 11:13:19AM -0700, Tony Lindgren wrote: > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161028 02:45]: > > > On Thu, Oct 27, 2016 at 12:15:52PM -0700, Tony Lindgren wrote: > > > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161027 11:46]: > > > > > But then this looks like it could trigger an ABBA deadlock as musb->lock > > > > > is held while queue_on_resume() takes musb->list_lock, and > > > > > musb_run_pending() would take the same locks in the reverse order. > > > > > > > > It seems we can avoid that by locking only list_add_tail() and list_del(): > > > > > > > > list_for_each_entry_safe(w, _w, &musb->resume_work, node) { > > > > spin_lock_irqsave(&musb->list_lock, flags); > > > > list_del(&w->node); > > > > spin_unlock_irqrestore(&musb->list_lock, flags); > > > > if (w->callback) > > > > w->callback(musb, w->data); > > > > devm_kfree(musb->controller, w); > > > > } > > > > > > I think you still need to hold the lock while traversing the list (even > > > if you temporarily release it during the callback). > > > > Hmm yeah we need iterate through the list again to avoid missing new > > elements being added. I've updated the patch to use a the common > > while (!list_empty(&musb->resume_work)) loop. Does that solve the > > concern you had or did you also had some other concern there? > > Yeah, while that minimises the race window it is still possible that the > timer callback checks pm_runtime_active() after the queue has been > processed but before the rpm status is updated. OK. Sorry for the delay responding, had my motherboard fail over the weekend.. > How about using a work struct and synchronous get for the deferred case? Here's the patch updated to use the existing finish_resume_work. Is that along the lines you were thinking? Adding also Ladis to Cc too. Regards, Tony 8< ---------------------------- >From tony Mon Sep 17 00:00:00 2001 From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> Date: Wed, 2 Nov 2016 19:59:05 -0700 Subject: [PATCH] usb: musb: Fix sleeping function called from invalid context for hdrc glue Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer that runs in softirq context. That causes a "BUG: sleeping function called from invalid context" every time when polling the cable status: [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. And looks like also musb_gadget_queue() suffers from the same problem. Let's fix the issue by using a list of delayed work then call it on resume. We can use the existing finish_resume_work for that. Note that we want to do this only when both musb core and it's parent devices are awake as noted by Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>. This allows us also to get rid of musb_gadget_work and need_finish_resume flag. Note that we now also need to get rid of static int first as that won't work right on devices with two musb instances like am335x. Note that we don't want to mess with deassert_reset_work as that's more time sensitive and USB spec related instead of PM runtime related. Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS glue layer") Reported-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> --- drivers/usb/musb/musb_core.c | 89 +++++++++++++++++++++++++++++++++-------- drivers/usb/musb/musb_core.h | 9 ++++- drivers/usb/musb/musb_dsps.c | 24 +++++++---- drivers/usb/musb/musb_gadget.c | 31 +++++++++----- drivers/usb/musb/musb_host.h | 6 ++- drivers/usb/musb/musb_virthub.c | 5 +-- 6 files changed, 123 insertions(+), 41 deletions(-) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -578,8 +578,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, | MUSB_PORT_STAT_RESUME; musb->rh_timer = jiffies + msecs_to_jiffies(USB_RESUME_TIMEOUT); - musb->need_finish_resume = 1; - + musb_queue_resume_work(musb, + musb_host_finish_resume, + NULL); musb->xceiv->otg->state = OTG_STATE_A_HOST; musb->is_active = 1; musb_host_resume_root_hub(musb); @@ -1969,6 +1970,7 @@ static struct musb *allocate_instance(struct device *dev, INIT_LIST_HEAD(&musb->control); INIT_LIST_HEAD(&musb->in_bulk); INIT_LIST_HEAD(&musb->out_bulk); + INIT_LIST_HEAD(&musb->pending_list); musb->vbuserr_retry = VBUSERR_RETRY_COUNT; musb->a_wait_bcon = OTG_TIME_A_WAIT_BCON; @@ -2018,6 +2020,64 @@ static void musb_free(struct musb *musb) musb_host_free(musb); } +struct musb_pending_work { + void (*callback)(struct musb *musb, void *data); + void *data; + struct list_head node; +}; + +static void musb_pending_work(struct work_struct *work) +{ + struct musb *musb; + struct musb_pending_work *w; + unsigned long flags; + + musb = container_of(work, struct musb, finish_resume_work.work); + pm_runtime_get_sync(musb->controller); + spin_lock_irqsave(&musb->list_lock, flags); + while (!list_empty(&musb->pending_list)) { + w = list_first_entry(&musb->pending_list, + struct musb_pending_work, + node); + list_del(&w->node); + spin_unlock_irqrestore(&musb->list_lock, flags); + if (w->callback) + w->callback(musb, w->data); + devm_kfree(musb->controller, w); + spin_lock_irqsave(&musb->list_lock, flags); + } + spin_unlock_irqrestore(&musb->list_lock, flags); + pm_runtime_mark_last_busy(musb->controller); + pm_runtime_put_autosuspend(musb->controller); +} + +void musb_queue_resume_work(struct musb *musb, + void (*callback)(struct musb *musb, void *data), + void *data) +{ + struct musb_pending_work *w; + unsigned long flags; + + if (!callback) + return; + + w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC); + if (!w) + return; + + w->callback = callback; + w->data = data; + spin_lock_irqsave(&musb->list_lock, flags); + list_add_tail(&w->node, &musb->pending_list); + spin_unlock_irqrestore(&musb->list_lock, flags); +} +EXPORT_SYMBOL_GPL(musb_queue_resume_work); + +void musb_cancel_resume_work(struct musb *musb) +{ + cancel_delayed_work_sync(&musb->finish_resume_work); +} + static void musb_deassert_reset(struct work_struct *work) { struct musb *musb; @@ -2065,6 +2125,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) } spin_lock_init(&musb->lock); + spin_lock_init(&musb->list_lock); musb->board_set_power = plat->set_power; musb->min_power = plat->min_power; musb->ops = plat->platform_ops; @@ -2215,7 +2276,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) /* Init IRQ workqueue before request_irq */ INIT_WORK(&musb->irq_work, musb_irq_work); INIT_DELAYED_WORK(&musb->deassert_reset_work, musb_deassert_reset); - INIT_DELAYED_WORK(&musb->finish_resume_work, musb_host_finish_resume); + INIT_DELAYED_WORK(&musb->finish_resume_work, musb_pending_work); /* setup musb parts of the core (especially endpoints) */ status = musb_core_init(plat->config->multipoint @@ -2310,7 +2371,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) fail3: cancel_work_sync(&musb->irq_work); - cancel_delayed_work_sync(&musb->finish_resume_work); + musb_cancel_resume_work(musb); cancel_delayed_work_sync(&musb->deassert_reset_work); if (musb->dma_controller) musb_dma_controller_destroy(musb->dma_controller); @@ -2377,7 +2438,7 @@ static int musb_remove(struct platform_device *pdev) musb_exit_debugfs(musb); cancel_work_sync(&musb->irq_work); - cancel_delayed_work_sync(&musb->finish_resume_work); + musb_cancel_resume_work(musb); cancel_delayed_work_sync(&musb->deassert_reset_work); pm_runtime_get_sync(musb->controller); musb_host_cleanup(musb); @@ -2603,11 +2664,9 @@ static int musb_resume(struct device *dev) mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV; if ((devctl & mask) != (musb->context.devctl & mask)) musb->port1_status = 0; - if (musb->need_finish_resume) { - musb->need_finish_resume = 0; - schedule_delayed_work(&musb->finish_resume_work, - msecs_to_jiffies(USB_RESUME_TIMEOUT)); - } + + schedule_delayed_work(&musb->finish_resume_work, + msecs_to_jiffies(USB_RESUME_TIMEOUT)); /* * The USB HUB code expects the device to be in RPM_ACTIVE once it came @@ -2633,8 +2692,8 @@ static int musb_runtime_suspend(struct device *dev) static int musb_runtime_resume(struct device *dev) { - struct musb *musb = dev_to_musb(dev); - static int first = 1; + struct musb *musb = dev_to_musb(dev); + struct delayed_work *d = &musb->finish_resume_work; /* * When pm_runtime_get_sync called for the first time in driver @@ -2645,12 +2704,8 @@ static int musb_runtime_resume(struct device *dev) * Also context restore without save does not make * any sense */ - if (!first) + if (d->work.func) { musb_restore_context(musb); - first = 0; - - if (musb->need_finish_resume) { - musb->need_finish_resume = 0; schedule_delayed_work(&musb->finish_resume_work, msecs_to_jiffies(USB_RESUME_TIMEOUT)); } diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -303,6 +303,7 @@ struct musb_context_registers { struct musb { /* device lock */ spinlock_t lock; + spinlock_t list_lock; /* resume work list lock */ struct musb_io io; const struct musb_platform_ops *ops; @@ -312,7 +313,6 @@ struct musb { struct work_struct irq_work; struct delayed_work deassert_reset_work; struct delayed_work finish_resume_work; - struct delayed_work gadget_work; u16 hwvers; u16 intrrxe; @@ -337,6 +337,7 @@ struct musb { struct list_head control; /* of musb_qh */ struct list_head in_bulk; /* of musb_qh */ struct list_head out_bulk; /* of musb_qh */ + struct list_head pending_list; /* pending work list */ struct timer_list otg_timer; struct notifier_block nb; @@ -404,7 +405,6 @@ struct musb { /* is_suspended means USB B_PERIPHERAL suspend */ unsigned is_suspended:1; - unsigned need_finish_resume :1; /* may_wakeup means remote wakeup is enabled */ unsigned may_wakeup:1; @@ -540,6 +540,11 @@ extern irqreturn_t musb_interrupt(struct musb *); extern void musb_hnp_stop(struct musb *musb); +void musb_queue_resume_work(struct musb *musb, + void (*callback)(struct musb *musb, void *data), + void *data); +void musb_cancel_resume_work(struct musb *musb); + static inline void musb_platform_set_vbus(struct musb *musb, int is_on) { if (musb->ops->set_vbus) diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -188,9 +188,8 @@ static void dsps_musb_disable(struct musb *musb) musb_writeb(musb->mregs, MUSB_DEVCTL, 0); } -static void otg_timer(unsigned long _musb) +static void dsps_check_status(struct musb *musb, void *unused) { - struct musb *musb = (void *)_musb; void __iomem *mregs = musb->mregs; struct device *dev = musb->controller; struct dsps_glue *glue = dev_get_drvdata(dev->parent); @@ -198,11 +197,6 @@ static void otg_timer(unsigned long _musb) u8 devctl; unsigned long flags; int skip_session = 0; - int err; - - err = pm_runtime_get_sync(dev); - if (err < 0) - dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); /* * We poll because DSPS IP's won't expose several OTG-critical @@ -246,6 +240,22 @@ static void otg_timer(unsigned long _musb) break; } spin_unlock_irqrestore(&musb->lock, flags); +} + +static void otg_timer(unsigned long _musb) +{ + struct musb *musb = (void *)_musb; + struct device *dev = musb->controller; + int err; + + err = pm_runtime_get(dev); + if (err < 0) + dev_err(dev, "Poll could not pm_runtime_get: %i\n", err); + + if (pm_runtime_active(dev)) + dsps_check_status(musb, NULL); + else + musb_queue_resume_work(musb, dsps_check_status, NULL); pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c --- a/drivers/usb/musb/musb_gadget.c +++ b/drivers/usb/musb/musb_gadget.c @@ -1222,6 +1222,16 @@ void musb_ep_restart(struct musb *musb, struct musb_request *req) rxstate(musb, req); } +void musb_ep_restart_resume_work(struct musb *musb, void *data) +{ + struct musb_request *req = data; + unsigned long flags; + + spin_lock_irqsave(&musb->lock, flags); + musb_ep_restart(musb, req); + spin_unlock_irqrestore(&musb->lock, flags); +} + static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, gfp_t gfp_flags) { @@ -1255,7 +1265,7 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, map_dma_buffer(request, musb, musb_ep); - pm_runtime_get_sync(musb->controller); + pm_runtime_get(musb->controller); spin_lock_irqsave(&musb->lock, lockflags); /* don't queue if the ep is down */ @@ -1271,8 +1281,14 @@ static int musb_gadget_queue(struct usb_ep *ep, struct usb_request *req, list_add_tail(&request->list, &musb_ep->req_list); /* it this is the head of the queue, start i/o ... */ - if (!musb_ep->busy && &request->list == musb_ep->req_list.next) - musb_ep_restart(musb, request); + if (!musb_ep->busy && &request->list == musb_ep->req_list.next) { + if (pm_runtime_active(musb->controller)) + musb_ep_restart(musb, request); + else + musb_queue_resume_work(musb, + musb_ep_restart_resume_work, + request); + } unlock: spin_unlock_irqrestore(&musb->lock, lockflags); @@ -1650,12 +1666,10 @@ static int musb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA) return usb_phy_set_power(musb->xceiv, mA); } -static void musb_gadget_work(struct work_struct *work) +static void musb_gadget_work(struct musb *musb, void *unused) { - struct musb *musb; unsigned long flags; - musb = container_of(work, struct musb, gadget_work.work); pm_runtime_get_sync(musb->controller); spin_lock_irqsave(&musb->lock, flags); musb_pullup(musb, musb->softconnect); @@ -1677,7 +1691,7 @@ static int musb_gadget_pullup(struct usb_gadget *gadget, int is_on) spin_lock_irqsave(&musb->lock, flags); if (is_on != musb->softconnect) { musb->softconnect = is_on; - schedule_delayed_work(&musb->gadget_work, 0); + musb_queue_resume_work(musb, musb_gadget_work, NULL); } spin_unlock_irqrestore(&musb->lock, flags); @@ -1849,7 +1863,6 @@ int musb_gadget_setup(struct musb *musb) #elif IS_ENABLED(CONFIG_USB_MUSB_GADGET) musb->g.is_otg = 0; #endif - INIT_DELAYED_WORK(&musb->gadget_work, musb_gadget_work); musb_g_init_endpoints(musb); musb->is_active = 0; @@ -1871,7 +1884,7 @@ void musb_gadget_cleanup(struct musb *musb) if (musb->port_mode == MUSB_PORT_MODE_HOST) return; - cancel_delayed_work_sync(&musb->gadget_work); + musb_cancel_resume_work(musb); usb_del_gadget_udc(&musb->g); } diff --git a/drivers/usb/musb/musb_host.h b/drivers/usb/musb/musb_host.h --- a/drivers/usb/musb/musb_host.h +++ b/drivers/usb/musb/musb_host.h @@ -94,7 +94,7 @@ extern void musb_host_resume_root_hub(struct musb *musb); extern void musb_host_poke_root_hub(struct musb *musb); extern void musb_port_suspend(struct musb *musb, bool do_suspend); extern void musb_port_reset(struct musb *musb, bool do_reset); -extern void musb_host_finish_resume(struct work_struct *work); +void musb_host_finish_resume(struct musb *musb, void *data); #else static inline struct musb *hcd_to_musb(struct usb_hcd *hcd) { @@ -126,7 +126,9 @@ static inline void musb_host_poll_rh_status(struct musb *musb) {} static inline void musb_host_poke_root_hub(struct musb *musb) {} static inline void musb_port_suspend(struct musb *musb, bool do_suspend) {} static inline void musb_port_reset(struct musb *musb, bool do_reset) {} -static inline void musb_host_finish_resume(struct work_struct *work) {} +static inline void musb_host_finish_resume(struct musb *musb, void *data) +{ +} #endif struct usb_hcd; diff --git a/drivers/usb/musb/musb_virthub.c b/drivers/usb/musb/musb_virthub.c --- a/drivers/usb/musb/musb_virthub.c +++ b/drivers/usb/musb/musb_virthub.c @@ -43,14 +43,11 @@ #include "musb_core.h" -void musb_host_finish_resume(struct work_struct *work) +void musb_host_finish_resume(struct musb *musb, void *unused) { - struct musb *musb; unsigned long flags; u8 power; - musb = container_of(work, struct musb, finish_resume_work.work); ^ permalink raw reply [flat|nested] 24+ messages in thread
[parent not found: <20161103212635.GC21430-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161103212635.GC21430-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2016-11-03 22:01 ` Ladislav Michl 2016-11-04 14:16 ` Johan Hovold 1 sibling, 0 replies; 24+ messages in thread From: Ladislav Michl @ 2016-11-03 22:01 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, linux-omap-u79uwXL29TY76Z2rM5mHXA On Thu, Nov 03, 2016 at 02:26:35PM -0700, Tony Lindgren wrote: [snip] > Here's the patch updated to use the existing finish_resume_work. > Is that along the lines you were thinking? > > Adding also Ladis to Cc too. Just gave it a try. Sure it wasn't supposed to fix "musb with hub" issue, but at least it doesn not make things worse :-) ladis -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 [not found] ` <20161103212635.GC21430-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-11-03 22:01 ` Ladislav Michl @ 2016-11-04 14:16 ` Johan Hovold 2016-11-04 15:13 ` Tony Lindgren 2016-11-07 18:28 ` Tony Lindgren 1 sibling, 2 replies; 24+ messages in thread From: Johan Hovold @ 2016-11-04 14:16 UTC (permalink / raw) To: Tony Lindgren Cc: Johan Hovold, Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, Ladislav Michl, linux-omap-u79uwXL29TY76Z2rM5mHXA On Thu, Nov 03, 2016 at 02:26:35PM -0700, Tony Lindgren wrote: > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161031 04:50]: > > On Fri, Oct 28, 2016 at 11:13:19AM -0700, Tony Lindgren wrote: > > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161028 02:45]: > > > > On Thu, Oct 27, 2016 at 12:15:52PM -0700, Tony Lindgren wrote: > > > > > * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161027 11:46]: > > > > > > But then this looks like it could trigger an ABBA deadlock as musb->lock > > > > > > is held while queue_on_resume() takes musb->list_lock, and > > > > > > musb_run_pending() would take the same locks in the reverse order. > > > > > > > > > > It seems we can avoid that by locking only list_add_tail() and list_del(): > > > > > > > > > > list_for_each_entry_safe(w, _w, &musb->resume_work, node) { > > > > > spin_lock_irqsave(&musb->list_lock, flags); > > > > > list_del(&w->node); > > > > > spin_unlock_irqrestore(&musb->list_lock, flags); > > > > > if (w->callback) > > > > > w->callback(musb, w->data); > > > > > devm_kfree(musb->controller, w); > > > > > } > > > > > > > > I think you still need to hold the lock while traversing the list (even > > > > if you temporarily release it during the callback). > > > > > > Hmm yeah we need iterate through the list again to avoid missing new > > > elements being added. I've updated the patch to use a the common > > > while (!list_empty(&musb->resume_work)) loop. Does that solve the > > > concern you had or did you also had some other concern there? > > > > Yeah, while that minimises the race window it is still possible that the > > timer callback checks pm_runtime_active() after the queue has been > > processed but before the rpm status is updated. > > OK. Sorry for the delay responding, had my motherboard fail > over the weekend.. Ouch. Hope the recovery process wasn't too painful. > > How about using a work struct and synchronous get for the deferred case? > > Here's the patch updated to use the existing finish_resume_work. > Is that along the lines you were thinking? Along those lines, yes, but I'm not sure about reusing finish_resume_work currently used to deassert resume signalling only. If work is already queued it seems you could end up deasserting resume prematurely for example. It currently also add unnecessary latency to other deferred work. You also forgot to queue musb_host_finish_resume() from musb_port_suspend() which looks like it would break port resume. > 8< ---------------------------- > From tony Mon Sep 17 00:00:00 2001 > From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > Date: Wed, 2 Nov 2016 19:59:05 -0700 > Subject: [PATCH] usb: musb: Fix sleeping function called from invalid > context for hdrc glue > > Commit 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > glue layer") wrongly added a call for pm_runtime_get_sync to otg_timer > that runs in softirq context. That causes a "BUG: sleeping function called > from invalid context" every time when polling the cable status: > > [<c015ebb4>] (__might_sleep) from [<c0413d60>] (__pm_runtime_resume+0x9c/0xa0) > [<c0413d60>] (__pm_runtime_resume) from [<c04d0bc4>] (otg_timer+0x3c/0x254) > [<c04d0bc4>] (otg_timer) from [<c0191180>] (call_timer_fn+0xfc/0x41c) > [<c0191180>] (call_timer_fn) from [<c01915c0>] (expire_timers+0x120/0x210) > [<c01915c0>] (expire_timers) from [<c0191acc>] (run_timer_softirq+0xa4/0xdc) > [<c0191acc>] (run_timer_softirq) from [<c010168c>] (__do_softirq+0x12c/0x594) > > I did not notice that as I did not have CONFIG_DEBUG_ATOMIC_SLEEP enabled. > And looks like also musb_gadget_queue() suffers from the same problem. > > Let's fix the issue by using a list of delayed work then call it on > resume. We can use the existing finish_resume_work for that. Note that > we want to do this only when both musb core and it's parent devices are > awake as noted by Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>. This allows us also > to get rid of musb_gadget_work and need_finish_resume flag. > > Note that we now also need to get rid of static int first as that > won't work right on devices with two musb instances like am335x. > > Note that we don't want to mess with deassert_reset_work as that's > more time sensitive and USB spec related instead of PM runtime related. > > Fixes: 65b3f50ed6fa ("usb: musb: Add PM runtime support for MUSB DSPS > glue layer") > Reported-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> > --- > drivers/usb/musb/musb_core.c | 89 +++++++++++++++++++++++++++++++++-------- > drivers/usb/musb/musb_core.h | 9 ++++- > drivers/usb/musb/musb_dsps.c | 24 +++++++---- > drivers/usb/musb/musb_gadget.c | 31 +++++++++----- > drivers/usb/musb/musb_host.h | 6 ++- > drivers/usb/musb/musb_virthub.c | 5 +-- > 6 files changed, 123 insertions(+), 41 deletions(-) > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > --- a/drivers/usb/musb/musb_core.c > +++ b/drivers/usb/musb/musb_core.c > @@ -578,8 +578,9 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb, > | MUSB_PORT_STAT_RESUME; > musb->rh_timer = jiffies > + msecs_to_jiffies(USB_RESUME_TIMEOUT); > - musb->need_finish_resume = 1; > - > + musb_queue_resume_work(musb, > + musb_host_finish_resume, > + NULL); > musb->xceiv->otg->state = OTG_STATE_A_HOST; > musb->is_active = 1; > musb_host_resume_root_hub(musb); > @@ -1969,6 +1970,7 @@ static struct musb *allocate_instance(struct device *dev, > INIT_LIST_HEAD(&musb->control); > INIT_LIST_HEAD(&musb->in_bulk); > INIT_LIST_HEAD(&musb->out_bulk); > + INIT_LIST_HEAD(&musb->pending_list); > > musb->vbuserr_retry = VBUSERR_RETRY_COUNT; > musb->a_wait_bcon = OTG_TIME_A_WAIT_BCON; > @@ -2018,6 +2020,64 @@ static void musb_free(struct musb *musb) > musb_host_free(musb); > } > > +struct musb_pending_work { > + void (*callback)(struct musb *musb, void *data); > + void *data; > + struct list_head node; > +}; > + > +static void musb_pending_work(struct work_struct *work) > +{ > + struct musb *musb; > + struct musb_pending_work *w; > + unsigned long flags; > + > + musb = container_of(work, struct musb, finish_resume_work.work); > + pm_runtime_get_sync(musb->controller); Should still check for errors here. > + spin_lock_irqsave(&musb->list_lock, flags); > + while (!list_empty(&musb->pending_list)) { > + w = list_first_entry(&musb->pending_list, > + struct musb_pending_work, > + node); > + list_del(&w->node); > + spin_unlock_irqrestore(&musb->list_lock, flags); > + if (w->callback) > + w->callback(musb, w->data); > + devm_kfree(musb->controller, w); > + spin_lock_irqsave(&musb->list_lock, flags); > + } > + spin_unlock_irqrestore(&musb->list_lock, flags); > + pm_runtime_mark_last_busy(musb->controller); > + pm_runtime_put_autosuspend(musb->controller); > +} > + > +void musb_queue_resume_work(struct musb *musb, > + void (*callback)(struct musb *musb, void *data), > + void *data) > +{ > + struct musb_pending_work *w; > + unsigned long flags; > + > + if (!callback) > + return; WARN_ON (e.g. in case someone switches the arguments)? > + > + w = devm_kzalloc(musb->controller, sizeof(*w), GFP_ATOMIC); > + if (!w) > + return; > + > + w->callback = callback; > + w->data = data; > + spin_lock_irqsave(&musb->list_lock, flags); > + list_add_tail(&w->node, &musb->pending_list); > + spin_unlock_irqrestore(&musb->list_lock, flags); > +} > +EXPORT_SYMBOL_GPL(musb_queue_resume_work); > + > +void musb_cancel_resume_work(struct musb *musb) > +{ > + cancel_delayed_work_sync(&musb->finish_resume_work); > +} > + > static void musb_deassert_reset(struct work_struct *work) > { > struct musb *musb; > @@ -2065,6 +2125,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) > } > > spin_lock_init(&musb->lock); > + spin_lock_init(&musb->list_lock); > musb->board_set_power = plat->set_power; > musb->min_power = plat->min_power; > musb->ops = plat->platform_ops; > @@ -2215,7 +2276,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) > /* Init IRQ workqueue before request_irq */ > INIT_WORK(&musb->irq_work, musb_irq_work); > INIT_DELAYED_WORK(&musb->deassert_reset_work, musb_deassert_reset); > - INIT_DELAYED_WORK(&musb->finish_resume_work, musb_host_finish_resume); > + INIT_DELAYED_WORK(&musb->finish_resume_work, musb_pending_work); > > /* setup musb parts of the core (especially endpoints) */ > status = musb_core_init(plat->config->multipoint > @@ -2310,7 +2371,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) > > fail3: > cancel_work_sync(&musb->irq_work); > - cancel_delayed_work_sync(&musb->finish_resume_work); > + musb_cancel_resume_work(musb); > cancel_delayed_work_sync(&musb->deassert_reset_work); > if (musb->dma_controller) > musb_dma_controller_destroy(musb->dma_controller); > @@ -2377,7 +2438,7 @@ static int musb_remove(struct platform_device *pdev) > musb_exit_debugfs(musb); > > cancel_work_sync(&musb->irq_work); > - cancel_delayed_work_sync(&musb->finish_resume_work); > + musb_cancel_resume_work(musb); > cancel_delayed_work_sync(&musb->deassert_reset_work); > pm_runtime_get_sync(musb->controller); > musb_host_cleanup(musb); > @@ -2603,11 +2664,9 @@ static int musb_resume(struct device *dev) > mask = MUSB_DEVCTL_BDEVICE | MUSB_DEVCTL_FSDEV | MUSB_DEVCTL_LSDEV; > if ((devctl & mask) != (musb->context.devctl & mask)) > musb->port1_status = 0; > - if (musb->need_finish_resume) { > - musb->need_finish_resume = 0; > - schedule_delayed_work(&musb->finish_resume_work, > - msecs_to_jiffies(USB_RESUME_TIMEOUT)); > - } > + > + schedule_delayed_work(&musb->finish_resume_work, > + msecs_to_jiffies(USB_RESUME_TIMEOUT)); > > /* > * The USB HUB code expects the device to be in RPM_ACTIVE once it came > @@ -2633,8 +2692,8 @@ static int musb_runtime_suspend(struct device *dev) > > static int musb_runtime_resume(struct device *dev) > { > - struct musb *musb = dev_to_musb(dev); > - static int first = 1; > + struct musb *musb = dev_to_musb(dev); > + struct delayed_work *d = &musb->finish_resume_work; > > /* > * When pm_runtime_get_sync called for the first time in driver > @@ -2645,12 +2704,8 @@ static int musb_runtime_resume(struct device *dev) > * Also context restore without save does not make > * any sense > */ Perhaps using a dedicated initialised flag in struct musb would be more self-explanatory (and less error prone) than overloading finish_resume_work->work.func. Could be done in a preparatory patch. Otherwise the above comment should probably be updated and mention this use of d->work.func. > - if (!first) > + if (d->work.func) { > musb_restore_context(musb); > - first = 0; > - > - if (musb->need_finish_resume) { > - musb->need_finish_resume = 0; > schedule_delayed_work(&musb->finish_resume_work, Use d as first argument here? > msecs_to_jiffies(USB_RESUME_TIMEOUT)); Perhaps sleep in musb_host_finish_resume() instead of always adding this delay which is only needed for deasserting resume signalling and not for generic (runtime) resume work (if you decide to use a common work queue). But why schedule from resume at all? It seems you could just schedule the deferred-work processing directly after having registered a callback. > -static void musb_gadget_work(struct work_struct *work) > +static void musb_gadget_work(struct musb *musb, void *unused) > { > - struct musb *musb; > unsigned long flags; > > - musb = container_of(work, struct musb, gadget_work.work); > pm_runtime_get_sync(musb->controller); The get would already have been taken care of by musb_pending_work(). > spin_lock_irqsave(&musb->lock, flags); > musb_pullup(musb, musb->softconnect); > @@ -1677,7 +1691,7 @@ static int musb_gadget_pullup(struct usb_gadget *gadget, int is_on) > spin_lock_irqsave(&musb->lock, flags); > if (is_on != musb->softconnect) { > musb->softconnect = is_on; > - schedule_delayed_work(&musb->gadget_work, 0); > + musb_queue_resume_work(musb, musb_gadget_work, NULL); You also need a pm_runtime_get() here to actually trigger the resume. But if already resumed, the gadget work would not run until next suspend-resume cycle. So if you want to reuse the resume work for this, then you need to check the active status here and do the work directly if already resumed as for musb_gadget_queue(). Or just always schedule deferred work as mentioned above. > } > spin_unlock_irqrestore(&musb->lock, flags); > > @@ -1849,7 +1863,6 @@ int musb_gadget_setup(struct musb *musb) > #elif IS_ENABLED(CONFIG_USB_MUSB_GADGET) > musb->g.is_otg = 0; > #endif > - INIT_DELAYED_WORK(&musb->gadget_work, musb_gadget_work); > musb_g_init_endpoints(musb); > > musb->is_active = 0; > @@ -1871,7 +1884,7 @@ void musb_gadget_cleanup(struct musb *musb) > if (musb->port_mode == MUSB_PORT_MODE_HOST) > return; > > - cancel_delayed_work_sync(&musb->gadget_work); > + musb_cancel_resume_work(musb); Doing this here as well, looks a bit weird but I guess it is needed for the probe error path where musb_gadget_cleanup() is called before cancelling the pending work (remove() would not need it, though). > usb_del_gadget_udc(&musb->g); > } Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-11-04 14:16 ` Johan Hovold @ 2016-11-04 15:13 ` Tony Lindgren 2016-11-07 18:28 ` Tony Lindgren 1 sibling, 0 replies; 24+ messages in thread From: Tony Lindgren @ 2016-11-04 15:13 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, Ladislav Michl, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161104 07:16]: > On Thu, Nov 03, 2016 at 02:26:35PM -0700, Tony Lindgren wrote: > > OK. Sorry for the delay responding, had my motherboard fail > > over the weekend.. > > Ouch. Hope the recovery process wasn't too painful. Only the usual scratches on my knuckles :) > > > How about using a work struct and synchronous get for the deferred case? > > > > Here's the patch updated to use the existing finish_resume_work. > > Is that along the lines you were thinking? > > Along those lines, yes, but I'm not sure about reusing > finish_resume_work currently used to deassert resume signalling only. OK I'll drop the changes for the other ones. Those can be then combined later as needed once we're done with fixes. > If work is already queued it seems you could end up deasserting resume > prematurely for example. It currently also add unnecessary latency to > other deferred work. Yeah good point so best to keep this one strictly limited to resume work. > You also forgot to queue musb_host_finish_resume() from > musb_port_suspend() which looks like it would break port resume. Oh OK will take a look. Might be best to leave that one out that change for now. > > + struct musb_pending_work *w; > > + unsigned long flags; > > + > > + if (!callback) > > + return; > > WARN_ON (e.g. in case someone switches the arguments)? Sure. Will fix also your other comments and make it into a more minimal patch for fixes. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: musb RPM sleep-while-atomic in 4.9-rc1 2016-11-04 14:16 ` Johan Hovold 2016-11-04 15:13 ` Tony Lindgren @ 2016-11-07 18:28 ` Tony Lindgren 1 sibling, 0 replies; 24+ messages in thread From: Tony Lindgren @ 2016-11-07 18:28 UTC (permalink / raw) To: Johan Hovold Cc: Bin Liu, linux-usb-u79uwXL29TY76Z2rM5mHXA, Ladislav Michl, linux-omap-u79uwXL29TY76Z2rM5mHXA * Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [161104 07:16]: > Perhaps sleep in musb_host_finish_resume() instead of always adding this > delay which is only needed for deasserting resume signalling and not for > generic (runtime) resume work (if you decide to use a common work > queue). Looking at your comments again, this is a good idea for future use. > But why schedule from resume at all? It seems you could just schedule > the deferred-work processing directly after having registered a > callback. But with this one we need to schedule the deferred work only from resume as waking up things can take a while potentially. Anyways, I'll post all the pending fixes as a new thread shortly. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-11-07 18:28 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-20 15:37 musb RPM sleep-while-atomic in 4.9-rc1 Johan Hovold 2016-10-21 7:08 ` Tony Lindgren [not found] ` <20161021070848.rum7wrlihjayqdbh-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-10-21 9:25 ` Johan Hovold 2016-10-21 9:49 ` Tony Lindgren [not found] ` <20161021094904.q66kjsl33yzf2kir-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-10-21 11:07 ` Johan Hovold 2016-10-21 11:27 ` Tony Lindgren [not found] ` <20161021112745.lancojpgv4h6aqpw-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-10-24 17:35 ` Tony Lindgren [not found] ` <20161024173538.26xvlitxiwjmh4fx-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-10-25 8:32 ` Johan Hovold 2016-10-25 15:11 ` Tony Lindgren [not found] ` <20161025151110.vih52s47a2g2col5-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-10-26 14:20 ` Johan Hovold 2016-10-26 14:31 ` Tony Lindgren [not found] ` <20161026143100.rg4pse6mjyq32hxm-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-10-27 15:14 ` Tony Lindgren [not found] ` <20161027151446.ffj6w2tydf6ymv7c-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-10-27 16:44 ` Johan Hovold 2016-10-27 17:40 ` Tony Lindgren [not found] ` <20161027174016.43twztwekvb3b25t-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-10-27 18:45 ` Johan Hovold 2016-10-27 19:15 ` Tony Lindgren [not found] ` <20161027191552.tuutyslp5qtu2b4f-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-10-28 9:44 ` Johan Hovold 2016-10-28 18:13 ` Tony Lindgren [not found] ` <20161028181318.umwn3rx55pg2cvwd-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-10-31 11:49 ` Johan Hovold 2016-11-03 21:26 ` Tony Lindgren [not found] ` <20161103212635.GC21430-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2016-11-03 22:01 ` Ladislav Michl 2016-11-04 14:16 ` Johan Hovold 2016-11-04 15:13 ` Tony Lindgren 2016-11-07 18:28 ` Tony Lindgren
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.