| Message ID | 20200714084800.11340-1-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc2: Fix error path in gadget registration | expand |
Hi Marek, On 7/14/2020 12:48 PM, Marek Szyprowski wrote: > When gadget registration fails, one should not call usb_del_gadget_udc(). > Ensure this by setting gadget->udc to NULL. Also in case of a failure I was able to reproduce issue. I'm Ok with this fix. > there is no need to disable low-level hardware, so return immiedetly > instead of jumping to error_init label. > Why do you think that disable low-level hardware not required which was enabled before? Also for some platforms required to call regulator_disable() which was enabled earlier in probe function. So, I suggest to keep jump to error_init label. > This fixes the following kernel NULL ptr dereference on gadget failure > (can be easily triggered with g_mass_storage without any module > parameters): > Thanks, Minas
Hi Minas, On 14.07.2020 14:32, Minas Harutyunyan wrote: > On 7/14/2020 12:48 PM, Marek Szyprowski wrote: >> When gadget registration fails, one should not call usb_del_gadget_udc(). >> Ensure this by setting gadget->udc to NULL. Also in case of a failure > I was able to reproduce issue. I'm Ok with this fix. > >> there is no need to disable low-level hardware, so return immiedetly >> instead of jumping to error_init label. >> > Why do you think that disable low-level hardware not required which was > enabled before? Also for some platforms required to call > regulator_disable() which was enabled earlier in probe function. > So, I suggest to keep jump to error_init label. If I keep the jump to error_init label, then there is unbalanced call to dwc2_lowlevel_hw_disable(). usb_add_gadget_udc() can fail in 2 places: on gadget->bind() or during udc_start(). In the first case, the HW was not yet enabled, so there is no need to disable it. In the latter one, the error might be returned only from the dwc2_lowlevel_hw_enable(), so again there is no need to call dwc2_lowlevel_hw_disable(). If I keep the "goto error_init;" line, I get the following errors: dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter besl=1 dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter g_np_tx_fifo_size=1024 dwc2 12480000.hsotg: EPs: 16, dedicated fifos, 7808 entries in SPRAM Mass Storage Function, version: 2009/09/11 LUN: removable file: (no medium) no file given for LUN0 g_mass_storage 12480000.hsotg: failed to start g_mass_storage: -22 ------------[ cut here ]------------ WARNING: CPU: 3 PID: 49 at drivers/clk/clk.c:958 clk_core_disable+0x1e4/0x314 usb_device already disabled Modules linked in: CPU: 3 PID: 49 Comm: kworker/3:1 Not tainted 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 Hardware name: Samsung Exynos (Flattened Device Tree) Workqueue: events deferred_probe_work_func [<c011184c>] (unwind_backtrace) from [<c010d250>] (show_stack+0x10/0x14) [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) [<c0126f64>] (warn_slowpath_fmt) from [<c056c338>] (clk_core_disable+0x1e4/0x314) [<c056c338>] (clk_core_disable) from [<c056c480>] (clk_core_disable_lock+0x18/0x24) [<c056c480>] (clk_core_disable_lock) from [<c073f9d4>] (__dwc2_lowlevel_hw_disable+0x3c/0xa0) [<c073f9d4>] (__dwc2_lowlevel_hw_disable) from [<c0740188>] (dwc2_driver_probe+0x2d4/0x6ac) [<c0740188>] (dwc2_driver_probe) from [<c0653640>] (platform_drv_probe+0x6c/0xa4) [<c0653640>] (platform_drv_probe) from [<c0650bdc>] (really_probe+0x200/0x4fc) [<c0650bdc>] (really_probe) from [<c0651040>] (driver_probe_device+0x78/0x1fc) [<c0651040>] (driver_probe_device) from [<c064ea78>] (bus_for_each_drv+0x74/0xb8) [<c064ea78>] (bus_for_each_drv) from [<c065093c>] (__device_attach+0xd4/0x16c) [<c065093c>] (__device_attach) from [<c064fa3c>] (bus_probe_device+0x88/0x90) [<c064fa3c>] (bus_probe_device) from [<c064ff54>] (deferred_probe_work_func+0x3c/0xd0) [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] (process_one_work+0x234/0x7dc) [<c0148a5c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) Exception stack(0xee923fb0 to 0xee923ff8) ... irq event stamp: 36966 hardirqs last enabled at (36965): [<c02b92d8>] kfree+0x1a4/0x3f0 hardirqs last disabled at (36966): [<c0569edc>] clk_enable_lock+0x14/0x134 softirqs last enabled at (36814): [<c010174c>] __do_softirq+0x50c/0x608 softirqs last disabled at (36803): [<c0130218>] irq_exit+0x168/0x16c ---[ end trace f55f4b28f3080c12 ]--- ------------[ cut here ]------------ WARNING: CPU: 3 PID: 49 at drivers/clk/clk.c:817 clk_core_unprepare+0x33c/0x470 usb_device already unprepared Modules linked in: CPU: 3 PID: 49 Comm: kworker/3:1 Tainted: G W 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 Hardware name: Samsung Exynos (Flattened Device Tree) Workqueue: events deferred_probe_work_func [<c011184c>] (unwind_backtrace) from [<c010d250>] (show_stack+0x10/0x14) [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) [<c0126f64>] (warn_slowpath_fmt) from [<c056dcb8>] (clk_core_unprepare+0x33c/0x470) [<c056dcb8>] (clk_core_unprepare) from [<c056de10>] (clk_unprepare+0x24/0x2c) [<c056de10>] (clk_unprepare) from [<c073f9dc>] (__dwc2_lowlevel_hw_disable+0x44/0xa0) [<c073f9dc>] (__dwc2_lowlevel_hw_disable) from [<c0740188>] (dwc2_driver_probe+0x2d4/0x6ac) [<c0740188>] (dwc2_driver_probe) from [<c0653640>] (platform_drv_probe+0x6c/0xa4) [<c0653640>] (platform_drv_probe) from [<c0650bdc>] (really_probe+0x200/0x4fc) [<c0650bdc>] (really_probe) from [<c0651040>] (driver_probe_device+0x78/0x1fc) [<c0651040>] (driver_probe_device) from [<c064ea78>] (bus_for_each_drv+0x74/0xb8) [<c064ea78>] (bus_for_each_drv) from [<c065093c>] (__device_attach+0xd4/0x16c) [<c065093c>] (__device_attach) from [<c064fa3c>] (bus_probe_device+0x88/0x90) [<c064fa3c>] (bus_probe_device) from [<c064ff54>] (deferred_probe_work_func+0x3c/0xd0) [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] (process_one_work+0x234/0x7dc) [<c0148a5c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) Exception stack(0xee923fb0 to 0xee923ff8) ... irq event stamp: 37073 hardirqs last enabled at (37081): [<c019d624>] console_unlock+0x430/0x6cc hardirqs last disabled at (37108): [<c019d348>] console_unlock+0x154/0x6cc softirqs last enabled at (37124): [<c010174c>] __do_softirq+0x50c/0x608 softirqs last disabled at (37135): [<c0130218>] irq_exit+0x168/0x16c ---[ end trace f55f4b28f3080c13 ]--- ------------[ cut here ]------------ WARNING: CPU: 3 PID: 49 at drivers/regulator/core.c:2603 _regulator_disable+0x180/0x1d8 unbalanced disables for VUOTG_3.0V Modules linked in: CPU: 3 PID: 49 Comm: kworker/3:1 Tainted: G W 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 Hardware name: Samsung Exynos (Flattened Device Tree) Workqueue: events deferred_probe_work_func [<c011184c>] (unwind_backtrace) from [<c010d250>] (show_stack+0x10/0x14) [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) [<c0126f64>] (warn_slowpath_fmt) from [<c0585e78>] (_regulator_disable+0x180/0x1d8) [<c0585e78>] (_regulator_disable) from [<c0585f04>] (regulator_disable+0x34/0xe0) [<c0585f04>] (regulator_disable) from [<c0587ef8>] (regulator_bulk_disable+0x28/0xb0) [<c0587ef8>] (regulator_bulk_disable) from [<c0740188>] (dwc2_driver_probe+0x2d4/0x6ac) [<c0740188>] (dwc2_driver_probe) from [<c0653640>] (platform_drv_probe+0x6c/0xa4) [<c0653640>] (platform_drv_probe) from [<c0650bdc>] (really_probe+0x200/0x4fc) [<c0650bdc>] (really_probe) from [<c0651040>] (driver_probe_device+0x78/0x1fc) [<c0651040>] (driver_probe_device) from [<c064ea78>] (bus_for_each_drv+0x74/0xb8) [<c064ea78>] (bus_for_each_drv) from [<c065093c>] (__device_attach+0xd4/0x16c) [<c065093c>] (__device_attach) from [<c064fa3c>] (bus_probe_device+0x88/0x90) [<c064fa3c>] (bus_probe_device) from [<c064ff54>] (deferred_probe_work_func+0x3c/0xd0) [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] (process_one_work+0x234/0x7dc) [<c0148a5c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) Exception stack(0xee923fb0 to 0xee923ff8) ... irq event stamp: 37243 hardirqs last enabled at (37251): [<c019d624>] console_unlock+0x430/0x6cc hardirqs last disabled at (37274): [<c0ac0790>] __schedule+0xd8/0x860 softirqs last enabled at (37270): [<c010174c>] __do_softirq+0x50c/0x608 softirqs last disabled at (37291): [<c0130218>] irq_exit+0x168/0x16c ---[ end trace f55f4b28f3080c14 ]--- Failed to disable vusb_a: -5 dwc2: probe of 12480000.hsotg failed with error -22 8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 00000004 pgd = (ptrval) [00000004] *pgd=00000000 Internal error: Oops: 5 [#1] PREEMPT SMP ARM Modules linked in: CPU: 3 PID: 87 Comm: kworker/3:2 Tainted: G W 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 Hardware name: Samsung Exynos (Flattened Device Tree) Workqueue: 0x0 (rcu_gp) PC is at process_one_work+0x44/0x7dc LR is at 0xedc226d0 ... Process kworker/3:2 (pid: 87, stack limit = 0x(ptrval)) Stack: (0xeda03f00 to 0xeda04000) ... [<c014886c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) Exception stack(0xeda03fb0 to 0xeda03ff8) ... ---[ end trace f55f4b28f3080c15 ]--- note: kworker/3:2[87] exited with preempt_count 1 Best regards
Hi Marek, On 7/15/2020 12:42 PM, Marek Szyprowski wrote: > Hi Minas, > > On 14.07.2020 14:32, Minas Harutyunyan wrote: >> On 7/14/2020 12:48 PM, Marek Szyprowski wrote: >>> When gadget registration fails, one should not call usb_del_gadget_udc(). >>> Ensure this by setting gadget->udc to NULL. Also in case of a failure >> I was able to reproduce issue. I'm Ok with this fix. >> >>> there is no need to disable low-level hardware, so return immiedetly >>> instead of jumping to error_init label. >>> >> Why do you think that disable low-level hardware not required which was >> enabled before? Also for some platforms required to call >> regulator_disable() which was enabled earlier in probe function. >> So, I suggest to keep jump to error_init label. > > If I keep the jump to error_init label, then there is unbalanced call to > dwc2_lowlevel_hw_disable(). usb_add_gadget_udc() can fail in 2 places: > on gadget->bind() or during udc_start(). In the first case, the HW was > not yet enabled, so there is no need to disable it. In the latter one, > the error might be returned only from the dwc2_lowlevel_hw_enable(), so > again there is no need to call dwc2_lowlevel_hw_disable(). > > If I keep the "goto error_init;" line, I get the following errors: > > dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter besl=1 > dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter > g_np_tx_fifo_size=1024 > dwc2 12480000.hsotg: EPs: 16, dedicated fifos, 7808 entries in SPRAM > Mass Storage Function, version: 2009/09/11 > LUN: removable file: (no medium) > no file given for LUN0 > g_mass_storage 12480000.hsotg: failed to start g_mass_storage: -22 > ------------[ cut here ]------------ > WARNING: CPU: 3 PID: 49 at drivers/clk/clk.c:958 > clk_core_disable+0x1e4/0x314 > usb_device already disabled > Modules linked in: > CPU: 3 PID: 49 Comm: kworker/3:1 Not tainted > 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 > Hardware name: Samsung Exynos (Flattened Device Tree) > Workqueue: events deferred_probe_work_func > [<c011184c>] (unwind_backtrace) from [<c010d250>] (show_stack+0x10/0x14) > [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) > [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) > [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) > [<c0126f64>] (warn_slowpath_fmt) from [<c056c338>] > (clk_core_disable+0x1e4/0x314) > [<c056c338>] (clk_core_disable) from [<c056c480>] > (clk_core_disable_lock+0x18/0x24) > [<c056c480>] (clk_core_disable_lock) from [<c073f9d4>] > (__dwc2_lowlevel_hw_disable+0x3c/0xa0) > [<c073f9d4>] (__dwc2_lowlevel_hw_disable) from [<c0740188>] > (dwc2_driver_probe+0x2d4/0x6ac) > [<c0740188>] (dwc2_driver_probe) from [<c0653640>] > (platform_drv_probe+0x6c/0xa4) > [<c0653640>] (platform_drv_probe) from [<c0650bdc>] > (really_probe+0x200/0x4fc) > [<c0650bdc>] (really_probe) from [<c0651040>] > (driver_probe_device+0x78/0x1fc) > [<c0651040>] (driver_probe_device) from [<c064ea78>] > (bus_for_each_drv+0x74/0xb8) > [<c064ea78>] (bus_for_each_drv) from [<c065093c>] > (__device_attach+0xd4/0x16c) > [<c065093c>] (__device_attach) from [<c064fa3c>] > (bus_probe_device+0x88/0x90) > [<c064fa3c>] (bus_probe_device) from [<c064ff54>] > (deferred_probe_work_func+0x3c/0xd0) > [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] > (process_one_work+0x234/0x7dc) > [<c0148a5c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) > [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) > [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) > Exception stack(0xee923fb0 to 0xee923ff8) > ... > irq event stamp: 36966 > hardirqs last enabled at (36965): [<c02b92d8>] kfree+0x1a4/0x3f0 > hardirqs last disabled at (36966): [<c0569edc>] clk_enable_lock+0x14/0x134 > softirqs last enabled at (36814): [<c010174c>] __do_softirq+0x50c/0x608 > softirqs last disabled at (36803): [<c0130218>] irq_exit+0x168/0x16c > ---[ end trace f55f4b28f3080c12 ]--- > ------------[ cut here ]------------ > WARNING: CPU: 3 PID: 49 at drivers/clk/clk.c:817 > clk_core_unprepare+0x33c/0x470 > usb_device already unprepared > Modules linked in: > CPU: 3 PID: 49 Comm: kworker/3:1 Tainted: G W > 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 > Hardware name: Samsung Exynos (Flattened Device Tree) > Workqueue: events deferred_probe_work_func > [<c011184c>] (unwind_backtrace) from [<c010d250>] (show_stack+0x10/0x14) > [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) > [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) > [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) > [<c0126f64>] (warn_slowpath_fmt) from [<c056dcb8>] > (clk_core_unprepare+0x33c/0x470) > [<c056dcb8>] (clk_core_unprepare) from [<c056de10>] > (clk_unprepare+0x24/0x2c) > [<c056de10>] (clk_unprepare) from [<c073f9dc>] > (__dwc2_lowlevel_hw_disable+0x44/0xa0) > [<c073f9dc>] (__dwc2_lowlevel_hw_disable) from [<c0740188>] > (dwc2_driver_probe+0x2d4/0x6ac) > [<c0740188>] (dwc2_driver_probe) from [<c0653640>] > (platform_drv_probe+0x6c/0xa4) > [<c0653640>] (platform_drv_probe) from [<c0650bdc>] > (really_probe+0x200/0x4fc) > [<c0650bdc>] (really_probe) from [<c0651040>] > (driver_probe_device+0x78/0x1fc) > [<c0651040>] (driver_probe_device) from [<c064ea78>] > (bus_for_each_drv+0x74/0xb8) > [<c064ea78>] (bus_for_each_drv) from [<c065093c>] > (__device_attach+0xd4/0x16c) > [<c065093c>] (__device_attach) from [<c064fa3c>] > (bus_probe_device+0x88/0x90) > [<c064fa3c>] (bus_probe_device) from [<c064ff54>] > (deferred_probe_work_func+0x3c/0xd0) > [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] > (process_one_work+0x234/0x7dc) > [<c0148a5c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) > [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) > [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) > Exception stack(0xee923fb0 to 0xee923ff8) > ... > irq event stamp: 37073 > hardirqs last enabled at (37081): [<c019d624>] console_unlock+0x430/0x6cc > hardirqs last disabled at (37108): [<c019d348>] console_unlock+0x154/0x6cc > softirqs last enabled at (37124): [<c010174c>] __do_softirq+0x50c/0x608 > softirqs last disabled at (37135): [<c0130218>] irq_exit+0x168/0x16c > ---[ end trace f55f4b28f3080c13 ]--- > ------------[ cut here ]------------ > WARNING: CPU: 3 PID: 49 at drivers/regulator/core.c:2603 > _regulator_disable+0x180/0x1d8 > unbalanced disables for VUOTG_3.0V > Modules linked in: > CPU: 3 PID: 49 Comm: kworker/3:1 Tainted: G W > 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 > Hardware name: Samsung Exynos (Flattened Device Tree) > Workqueue: events deferred_probe_work_func > [<c011184c>] (unwind_backtrace) from [<c010d250>] (show_stack+0x10/0x14) > [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) > [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) > [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) > [<c0126f64>] (warn_slowpath_fmt) from [<c0585e78>] > (_regulator_disable+0x180/0x1d8) > [<c0585e78>] (_regulator_disable) from [<c0585f04>] > (regulator_disable+0x34/0xe0) > [<c0585f04>] (regulator_disable) from [<c0587ef8>] > (regulator_bulk_disable+0x28/0xb0) > [<c0587ef8>] (regulator_bulk_disable) from [<c0740188>] > (dwc2_driver_probe+0x2d4/0x6ac) > [<c0740188>] (dwc2_driver_probe) from [<c0653640>] > (platform_drv_probe+0x6c/0xa4) > [<c0653640>] (platform_drv_probe) from [<c0650bdc>] > (really_probe+0x200/0x4fc) > [<c0650bdc>] (really_probe) from [<c0651040>] > (driver_probe_device+0x78/0x1fc) > [<c0651040>] (driver_probe_device) from [<c064ea78>] > (bus_for_each_drv+0x74/0xb8) > [<c064ea78>] (bus_for_each_drv) from [<c065093c>] > (__device_attach+0xd4/0x16c) > [<c065093c>] (__device_attach) from [<c064fa3c>] > (bus_probe_device+0x88/0x90) > [<c064fa3c>] (bus_probe_device) from [<c064ff54>] > (deferred_probe_work_func+0x3c/0xd0) > [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] > (process_one_work+0x234/0x7dc) > [<c0148a5c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) > [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) > [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) > Exception stack(0xee923fb0 to 0xee923ff8) > ... > irq event stamp: 37243 > hardirqs last enabled at (37251): [<c019d624>] console_unlock+0x430/0x6cc > hardirqs last disabled at (37274): [<c0ac0790>] __schedule+0xd8/0x860 > softirqs last enabled at (37270): [<c010174c>] __do_softirq+0x50c/0x608 > softirqs last disabled at (37291): [<c0130218>] irq_exit+0x168/0x16c > ---[ end trace f55f4b28f3080c14 ]--- > Failed to disable vusb_a: -5 > dwc2: probe of 12480000.hsotg failed with error -22 > 8<--- cut here --- > Unable to handle kernel NULL pointer dereference at virtual address 00000004 > pgd = (ptrval) > [00000004] *pgd=00000000 > Internal error: Oops: 5 [#1] PREEMPT SMP ARM > Modules linked in: > CPU: 3 PID: 87 Comm: kworker/3:2 Tainted: G W > 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 > Hardware name: Samsung Exynos (Flattened Device Tree) > Workqueue: 0x0 (rcu_gp) > PC is at process_one_work+0x44/0x7dc > LR is at 0xedc226d0 > ... > Process kworker/3:2 (pid: 87, stack limit = 0x(ptrval)) > Stack: (0xeda03f00 to 0xeda04000) > ... > [<c014886c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) > [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) > [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) > Exception stack(0xeda03fb0 to 0xeda03ff8) > ... > ---[ end trace f55f4b28f3080c15 ]--- > note: kworker/3:2[87] exited with preempt_count 1 > > Best regards > I guess you build driver in (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) and this is why call of dwc2_lowlevel_hw_disable() at 'error:' label create issue. Could you please test on your setup follow implementation: ... error: if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) dwc2_lowlevel_hw_disable(hsotg); return retval; } ... Thanks, Minas
Hi Minas, On 15.07.2020 15:54, Minas Harutyunyan wrote: > On 7/15/2020 12:42 PM, Marek Szyprowski wrote: >> On 14.07.2020 14:32, Minas Harutyunyan wrote: >>> On 7/14/2020 12:48 PM, Marek Szyprowski wrote: >>>> When gadget registration fails, one should not call usb_del_gadget_udc(). >>>> Ensure this by setting gadget->udc to NULL. Also in case of a failure >>> I was able to reproduce issue. I'm Ok with this fix. >>> >>>> there is no need to disable low-level hardware, so return immiedetly >>>> instead of jumping to error_init label. >>>> >>> Why do you think that disable low-level hardware not required which was >>> enabled before? Also for some platforms required to call >>> regulator_disable() which was enabled earlier in probe function. >>> So, I suggest to keep jump to error_init label. >> If I keep the jump to error_init label, then there is unbalanced call to >> dwc2_lowlevel_hw_disable(). usb_add_gadget_udc() can fail in 2 places: >> on gadget->bind() or during udc_start(). In the first case, the HW was >> not yet enabled, so there is no need to disable it. In the latter one, >> the error might be returned only from the dwc2_lowlevel_hw_enable(), so >> again there is no need to call dwc2_lowlevel_hw_disable(). >> >> If I keep the "goto error_init;" line, I get the following errors: >> >> dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter besl=1 >> dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter >> g_np_tx_fifo_size=1024 >> dwc2 12480000.hsotg: EPs: 16, dedicated fifos, 7808 entries in SPRAM >> Mass Storage Function, version: 2009/09/11 >> LUN: removable file: (no medium) >> no file given for LUN0 >> g_mass_storage 12480000.hsotg: failed to start g_mass_storage: -22 >> ------------[ cut here ]------------ >> WARNING: CPU: 3 PID: 49 at drivers/clk/clk.c:958 >> clk_core_disable+0x1e4/0x314 >> usb_device already disabled >> Modules linked in: >> CPU: 3 PID: 49 Comm: kworker/3:1 Not tainted >> 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 >> Hardware name: Samsung Exynos (Flattened Device Tree) >> Workqueue: events deferred_probe_work_func >> [<c011184c>] (unwind_backtrace) from [<c010d250>] (show_stack+0x10/0x14) >> [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) >> [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) >> [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) >> [<c0126f64>] (warn_slowpath_fmt) from [<c056c338>] >> (clk_core_disable+0x1e4/0x314) >> [<c056c338>] (clk_core_disable) from [<c056c480>] >> (clk_core_disable_lock+0x18/0x24) >> [<c056c480>] (clk_core_disable_lock) from [<c073f9d4>] >> (__dwc2_lowlevel_hw_disable+0x3c/0xa0) >> [<c073f9d4>] (__dwc2_lowlevel_hw_disable) from [<c0740188>] >> (dwc2_driver_probe+0x2d4/0x6ac) >> [<c0740188>] (dwc2_driver_probe) from [<c0653640>] >> (platform_drv_probe+0x6c/0xa4) >> [<c0653640>] (platform_drv_probe) from [<c0650bdc>] >> (really_probe+0x200/0x4fc) >> [<c0650bdc>] (really_probe) from [<c0651040>] >> (driver_probe_device+0x78/0x1fc) >> [<c0651040>] (driver_probe_device) from [<c064ea78>] >> (bus_for_each_drv+0x74/0xb8) >> [<c064ea78>] (bus_for_each_drv) from [<c065093c>] >> (__device_attach+0xd4/0x16c) >> [<c065093c>] (__device_attach) from [<c064fa3c>] >> (bus_probe_device+0x88/0x90) >> [<c064fa3c>] (bus_probe_device) from [<c064ff54>] >> (deferred_probe_work_func+0x3c/0xd0) >> [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] >> (process_one_work+0x234/0x7dc) >> [<c0148a5c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) >> [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) >> [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) >> Exception stack(0xee923fb0 to 0xee923ff8) >> ... >> irq event stamp: 36966 >> hardirqs last enabled at (36965): [<c02b92d8>] kfree+0x1a4/0x3f0 >> hardirqs last disabled at (36966): [<c0569edc>] clk_enable_lock+0x14/0x134 >> softirqs last enabled at (36814): [<c010174c>] __do_softirq+0x50c/0x608 >> softirqs last disabled at (36803): [<c0130218>] irq_exit+0x168/0x16c >> ---[ end trace f55f4b28f3080c12 ]--- >> ------------[ cut here ]------------ >> WARNING: CPU: 3 PID: 49 at drivers/clk/clk.c:817 >> clk_core_unprepare+0x33c/0x470 >> usb_device already unprepared >> Modules linked in: >> CPU: 3 PID: 49 Comm: kworker/3:1 Tainted: G W >> 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 >> Hardware name: Samsung Exynos (Flattened Device Tree) >> Workqueue: events deferred_probe_work_func >> [<c011184c>] (unwind_backtrace) from [<c010d250>] (show_stack+0x10/0x14) >> [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) >> [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) >> [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) >> [<c0126f64>] (warn_slowpath_fmt) from [<c056dcb8>] >> (clk_core_unprepare+0x33c/0x470) >> [<c056dcb8>] (clk_core_unprepare) from [<c056de10>] >> (clk_unprepare+0x24/0x2c) >> [<c056de10>] (clk_unprepare) from [<c073f9dc>] >> (__dwc2_lowlevel_hw_disable+0x44/0xa0) >> [<c073f9dc>] (__dwc2_lowlevel_hw_disable) from [<c0740188>] >> (dwc2_driver_probe+0x2d4/0x6ac) >> [<c0740188>] (dwc2_driver_probe) from [<c0653640>] >> (platform_drv_probe+0x6c/0xa4) >> [<c0653640>] (platform_drv_probe) from [<c0650bdc>] >> (really_probe+0x200/0x4fc) >> [<c0650bdc>] (really_probe) from [<c0651040>] >> (driver_probe_device+0x78/0x1fc) >> [<c0651040>] (driver_probe_device) from [<c064ea78>] >> (bus_for_each_drv+0x74/0xb8) >> [<c064ea78>] (bus_for_each_drv) from [<c065093c>] >> (__device_attach+0xd4/0x16c) >> [<c065093c>] (__device_attach) from [<c064fa3c>] >> (bus_probe_device+0x88/0x90) >> [<c064fa3c>] (bus_probe_device) from [<c064ff54>] >> (deferred_probe_work_func+0x3c/0xd0) >> [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] >> (process_one_work+0x234/0x7dc) >> [<c0148a5c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) >> [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) >> [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) >> Exception stack(0xee923fb0 to 0xee923ff8) >> ... >> irq event stamp: 37073 >> hardirqs last enabled at (37081): [<c019d624>] console_unlock+0x430/0x6cc >> hardirqs last disabled at (37108): [<c019d348>] console_unlock+0x154/0x6cc >> softirqs last enabled at (37124): [<c010174c>] __do_softirq+0x50c/0x608 >> softirqs last disabled at (37135): [<c0130218>] irq_exit+0x168/0x16c >> ---[ end trace f55f4b28f3080c13 ]--- >> ------------[ cut here ]------------ >> WARNING: CPU: 3 PID: 49 at drivers/regulator/core.c:2603 >> _regulator_disable+0x180/0x1d8 >> unbalanced disables for VUOTG_3.0V >> Modules linked in: >> CPU: 3 PID: 49 Comm: kworker/3:1 Tainted: G W >> 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 >> Hardware name: Samsung Exynos (Flattened Device Tree) >> Workqueue: events deferred_probe_work_func >> [<c011184c>] (unwind_backtrace) from [<c010d250>] (show_stack+0x10/0x14) >> [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) >> [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) >> [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) >> [<c0126f64>] (warn_slowpath_fmt) from [<c0585e78>] >> (_regulator_disable+0x180/0x1d8) >> [<c0585e78>] (_regulator_disable) from [<c0585f04>] >> (regulator_disable+0x34/0xe0) >> [<c0585f04>] (regulator_disable) from [<c0587ef8>] >> (regulator_bulk_disable+0x28/0xb0) >> [<c0587ef8>] (regulator_bulk_disable) from [<c0740188>] >> (dwc2_driver_probe+0x2d4/0x6ac) >> [<c0740188>] (dwc2_driver_probe) from [<c0653640>] >> (platform_drv_probe+0x6c/0xa4) >> [<c0653640>] (platform_drv_probe) from [<c0650bdc>] >> (really_probe+0x200/0x4fc) >> [<c0650bdc>] (really_probe) from [<c0651040>] >> (driver_probe_device+0x78/0x1fc) >> [<c0651040>] (driver_probe_device) from [<c064ea78>] >> (bus_for_each_drv+0x74/0xb8) >> [<c064ea78>] (bus_for_each_drv) from [<c065093c>] >> (__device_attach+0xd4/0x16c) >> [<c065093c>] (__device_attach) from [<c064fa3c>] >> (bus_probe_device+0x88/0x90) >> [<c064fa3c>] (bus_probe_device) from [<c064ff54>] >> (deferred_probe_work_func+0x3c/0xd0) >> [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] >> (process_one_work+0x234/0x7dc) >> [<c0148a5c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) >> [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) >> [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) >> Exception stack(0xee923fb0 to 0xee923ff8) >> ... >> irq event stamp: 37243 >> hardirqs last enabled at (37251): [<c019d624>] console_unlock+0x430/0x6cc >> hardirqs last disabled at (37274): [<c0ac0790>] __schedule+0xd8/0x860 >> softirqs last enabled at (37270): [<c010174c>] __do_softirq+0x50c/0x608 >> softirqs last disabled at (37291): [<c0130218>] irq_exit+0x168/0x16c >> ---[ end trace f55f4b28f3080c14 ]--- >> Failed to disable vusb_a: -5 >> dwc2: probe of 12480000.hsotg failed with error -22 >> 8<--- cut here --- >> Unable to handle kernel NULL pointer dereference at virtual address 00000004 >> pgd = (ptrval) >> [00000004] *pgd=00000000 >> Internal error: Oops: 5 [#1] PREEMPT SMP ARM >> Modules linked in: >> CPU: 3 PID: 87 Comm: kworker/3:2 Tainted: G W >> 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 >> Hardware name: Samsung Exynos (Flattened Device Tree) >> Workqueue: 0x0 (rcu_gp) >> PC is at process_one_work+0x44/0x7dc >> LR is at 0xedc226d0 >> ... >> Process kworker/3:2 (pid: 87, stack limit = 0x(ptrval)) >> Stack: (0xeda03f00 to 0xeda04000) >> ... >> [<c014886c>] (process_one_work) from [<c0149048>] (worker_thread+0x44/0x51c) >> [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) >> [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) >> Exception stack(0xeda03fb0 to 0xeda03ff8) >> ... >> ---[ end trace f55f4b28f3080c15 ]--- >> note: kworker/3:2[87] exited with preempt_count 1 >> >> Best regards >> > I guess you build driver in (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) > and this is why call of dwc2_lowlevel_hw_disable() at 'error:' label > create issue. Could you please test on your setup follow implementation: > ... > error: > if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) > dwc2_lowlevel_hw_disable(hsotg); > return retval; > } > ... Right, this will work. However, I would also change the order of operations - disabling usb33d regulator should be done after disabling lowlevel hardware to properly pair with the enable operations. While playing with this I've also found an issue in UDC core: https://lore.kernel.org/linux-usb/20200716115525.27519-1-m.szyprowski@samsung.com/T/#u Best regards
On 16.07.2020 13:58, Marek Szyprowski wrote: > On 15.07.2020 15:54, Minas Harutyunyan wrote: >> On 7/15/2020 12:42 PM, Marek Szyprowski wrote: >>> On 14.07.2020 14:32, Minas Harutyunyan wrote: >>>> On 7/14/2020 12:48 PM, Marek Szyprowski wrote: >>>>> When gadget registration fails, one should not call >>>>> usb_del_gadget_udc(). >>>>> Ensure this by setting gadget->udc to NULL. Also in case of a failure >>>> I was able to reproduce issue. I'm Ok with this fix. >>>> >>>>> there is no need to disable low-level hardware, so return immiedetly >>>>> instead of jumping to error_init label. >>>>> >>>> Why do you think that disable low-level hardware not required which >>>> was >>>> enabled before? Also for some platforms required to call >>>> regulator_disable() which was enabled earlier in probe function. >>>> So, I suggest to keep jump to error_init label. >>> If I keep the jump to error_init label, then there is unbalanced >>> call to >>> dwc2_lowlevel_hw_disable(). usb_add_gadget_udc() can fail in 2 places: >>> on gadget->bind() or during udc_start(). In the first case, the HW was >>> not yet enabled, so there is no need to disable it. In the latter one, >>> the error might be returned only from the dwc2_lowlevel_hw_enable(), so >>> again there is no need to call dwc2_lowlevel_hw_disable(). >>> >>> If I keep the "goto error_init;" line, I get the following errors: >>> >>> dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter besl=1 >>> dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter >>> g_np_tx_fifo_size=1024 >>> dwc2 12480000.hsotg: EPs: 16, dedicated fifos, 7808 entries in SPRAM >>> Mass Storage Function, version: 2009/09/11 >>> LUN: removable file: (no medium) >>> no file given for LUN0 >>> g_mass_storage 12480000.hsotg: failed to start g_mass_storage: -22 >>> ------------[ cut here ]------------ >>> WARNING: CPU: 3 PID: 49 at drivers/clk/clk.c:958 >>> clk_core_disable+0x1e4/0x314 >>> usb_device already disabled >>> Modules linked in: >>> CPU: 3 PID: 49 Comm: kworker/3:1 Not tainted >>> 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 >>> Hardware name: Samsung Exynos (Flattened Device Tree) >>> Workqueue: events deferred_probe_work_func >>> [<c011184c>] (unwind_backtrace) from [<c010d250>] >>> (show_stack+0x10/0x14) >>> [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) >>> [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) >>> [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) >>> [<c0126f64>] (warn_slowpath_fmt) from [<c056c338>] >>> (clk_core_disable+0x1e4/0x314) >>> [<c056c338>] (clk_core_disable) from [<c056c480>] >>> (clk_core_disable_lock+0x18/0x24) >>> [<c056c480>] (clk_core_disable_lock) from [<c073f9d4>] >>> (__dwc2_lowlevel_hw_disable+0x3c/0xa0) >>> [<c073f9d4>] (__dwc2_lowlevel_hw_disable) from [<c0740188>] >>> (dwc2_driver_probe+0x2d4/0x6ac) >>> [<c0740188>] (dwc2_driver_probe) from [<c0653640>] >>> (platform_drv_probe+0x6c/0xa4) >>> [<c0653640>] (platform_drv_probe) from [<c0650bdc>] >>> (really_probe+0x200/0x4fc) >>> [<c0650bdc>] (really_probe) from [<c0651040>] >>> (driver_probe_device+0x78/0x1fc) >>> [<c0651040>] (driver_probe_device) from [<c064ea78>] >>> (bus_for_each_drv+0x74/0xb8) >>> [<c064ea78>] (bus_for_each_drv) from [<c065093c>] >>> (__device_attach+0xd4/0x16c) >>> [<c065093c>] (__device_attach) from [<c064fa3c>] >>> (bus_probe_device+0x88/0x90) >>> [<c064fa3c>] (bus_probe_device) from [<c064ff54>] >>> (deferred_probe_work_func+0x3c/0xd0) >>> [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] >>> (process_one_work+0x234/0x7dc) >>> [<c0148a5c>] (process_one_work) from [<c0149048>] >>> (worker_thread+0x44/0x51c) >>> [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) >>> [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) >>> Exception stack(0xee923fb0 to 0xee923ff8) >>> ... >>> irq event stamp: 36966 >>> hardirqs last enabled at (36965): [<c02b92d8>] kfree+0x1a4/0x3f0 >>> hardirqs last disabled at (36966): [<c0569edc>] >>> clk_enable_lock+0x14/0x134 >>> softirqs last enabled at (36814): [<c010174c>] >>> __do_softirq+0x50c/0x608 >>> softirqs last disabled at (36803): [<c0130218>] irq_exit+0x168/0x16c >>> ---[ end trace f55f4b28f3080c12 ]--- >>> ------------[ cut here ]------------ >>> WARNING: CPU: 3 PID: 49 at drivers/clk/clk.c:817 >>> clk_core_unprepare+0x33c/0x470 >>> usb_device already unprepared >>> Modules linked in: >>> CPU: 3 PID: 49 Comm: kworker/3:1 Tainted: G W >>> 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 >>> Hardware name: Samsung Exynos (Flattened Device Tree) >>> Workqueue: events deferred_probe_work_func >>> [<c011184c>] (unwind_backtrace) from [<c010d250>] >>> (show_stack+0x10/0x14) >>> [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) >>> [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) >>> [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) >>> [<c0126f64>] (warn_slowpath_fmt) from [<c056dcb8>] >>> (clk_core_unprepare+0x33c/0x470) >>> [<c056dcb8>] (clk_core_unprepare) from [<c056de10>] >>> (clk_unprepare+0x24/0x2c) >>> [<c056de10>] (clk_unprepare) from [<c073f9dc>] >>> (__dwc2_lowlevel_hw_disable+0x44/0xa0) >>> [<c073f9dc>] (__dwc2_lowlevel_hw_disable) from [<c0740188>] >>> (dwc2_driver_probe+0x2d4/0x6ac) >>> [<c0740188>] (dwc2_driver_probe) from [<c0653640>] >>> (platform_drv_probe+0x6c/0xa4) >>> [<c0653640>] (platform_drv_probe) from [<c0650bdc>] >>> (really_probe+0x200/0x4fc) >>> [<c0650bdc>] (really_probe) from [<c0651040>] >>> (driver_probe_device+0x78/0x1fc) >>> [<c0651040>] (driver_probe_device) from [<c064ea78>] >>> (bus_for_each_drv+0x74/0xb8) >>> [<c064ea78>] (bus_for_each_drv) from [<c065093c>] >>> (__device_attach+0xd4/0x16c) >>> [<c065093c>] (__device_attach) from [<c064fa3c>] >>> (bus_probe_device+0x88/0x90) >>> [<c064fa3c>] (bus_probe_device) from [<c064ff54>] >>> (deferred_probe_work_func+0x3c/0xd0) >>> [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] >>> (process_one_work+0x234/0x7dc) >>> [<c0148a5c>] (process_one_work) from [<c0149048>] >>> (worker_thread+0x44/0x51c) >>> [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) >>> [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) >>> Exception stack(0xee923fb0 to 0xee923ff8) >>> ... >>> irq event stamp: 37073 >>> hardirqs last enabled at (37081): [<c019d624>] >>> console_unlock+0x430/0x6cc >>> hardirqs last disabled at (37108): [<c019d348>] >>> console_unlock+0x154/0x6cc >>> softirqs last enabled at (37124): [<c010174c>] >>> __do_softirq+0x50c/0x608 >>> softirqs last disabled at (37135): [<c0130218>] irq_exit+0x168/0x16c >>> ---[ end trace f55f4b28f3080c13 ]--- >>> ------------[ cut here ]------------ >>> WARNING: CPU: 3 PID: 49 at drivers/regulator/core.c:2603 >>> _regulator_disable+0x180/0x1d8 >>> unbalanced disables for VUOTG_3.0V >>> Modules linked in: >>> CPU: 3 PID: 49 Comm: kworker/3:1 Tainted: G W >>> 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 >>> Hardware name: Samsung Exynos (Flattened Device Tree) >>> Workqueue: events deferred_probe_work_func >>> [<c011184c>] (unwind_backtrace) from [<c010d250>] >>> (show_stack+0x10/0x14) >>> [<c010d250>] (show_stack) from [<c051b8fc>] (dump_stack+0xbc/0xe8) >>> [<c051b8fc>] (dump_stack) from [<c0126ed8>] (__warn+0xf0/0x108) >>> [<c0126ed8>] (__warn) from [<c0126f64>] (warn_slowpath_fmt+0x74/0xb8) >>> [<c0126f64>] (warn_slowpath_fmt) from [<c0585e78>] >>> (_regulator_disable+0x180/0x1d8) >>> [<c0585e78>] (_regulator_disable) from [<c0585f04>] >>> (regulator_disable+0x34/0xe0) >>> [<c0585f04>] (regulator_disable) from [<c0587ef8>] >>> (regulator_bulk_disable+0x28/0xb0) >>> [<c0587ef8>] (regulator_bulk_disable) from [<c0740188>] >>> (dwc2_driver_probe+0x2d4/0x6ac) >>> [<c0740188>] (dwc2_driver_probe) from [<c0653640>] >>> (platform_drv_probe+0x6c/0xa4) >>> [<c0653640>] (platform_drv_probe) from [<c0650bdc>] >>> (really_probe+0x200/0x4fc) >>> [<c0650bdc>] (really_probe) from [<c0651040>] >>> (driver_probe_device+0x78/0x1fc) >>> [<c0651040>] (driver_probe_device) from [<c064ea78>] >>> (bus_for_each_drv+0x74/0xb8) >>> [<c064ea78>] (bus_for_each_drv) from [<c065093c>] >>> (__device_attach+0xd4/0x16c) >>> [<c065093c>] (__device_attach) from [<c064fa3c>] >>> (bus_probe_device+0x88/0x90) >>> [<c064fa3c>] (bus_probe_device) from [<c064ff54>] >>> (deferred_probe_work_func+0x3c/0xd0) >>> [<c064ff54>] (deferred_probe_work_func) from [<c0148a5c>] >>> (process_one_work+0x234/0x7dc) >>> [<c0148a5c>] (process_one_work) from [<c0149048>] >>> (worker_thread+0x44/0x51c) >>> [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) >>> [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) >>> Exception stack(0xee923fb0 to 0xee923ff8) >>> ... >>> irq event stamp: 37243 >>> hardirqs last enabled at (37251): [<c019d624>] >>> console_unlock+0x430/0x6cc >>> hardirqs last disabled at (37274): [<c0ac0790>] __schedule+0xd8/0x860 >>> softirqs last enabled at (37270): [<c010174c>] >>> __do_softirq+0x50c/0x608 >>> softirqs last disabled at (37291): [<c0130218>] irq_exit+0x168/0x16c >>> ---[ end trace f55f4b28f3080c14 ]--- >>> Failed to disable vusb_a: -5 >>> dwc2: probe of 12480000.hsotg failed with error -22 >>> 8<--- cut here --- >>> Unable to handle kernel NULL pointer dereference at virtual address >>> 00000004 >>> pgd = (ptrval) >>> [00000004] *pgd=00000000 >>> Internal error: Oops: 5 [#1] PREEMPT SMP ARM >>> Modules linked in: >>> CPU: 3 PID: 87 Comm: kworker/3:2 Tainted: G W >>> 5.8.0-rc5-next-20200714-00003-g105f360ba595-dirty #8758 >>> Hardware name: Samsung Exynos (Flattened Device Tree) >>> Workqueue: 0x0 (rcu_gp) >>> PC is at process_one_work+0x44/0x7dc >>> LR is at 0xedc226d0 >>> ... >>> Process kworker/3:2 (pid: 87, stack limit = 0x(ptrval)) >>> Stack: (0xeda03f00 to 0xeda04000) >>> ... >>> [<c014886c>] (process_one_work) from [<c0149048>] >>> (worker_thread+0x44/0x51c) >>> [<c0149048>] (worker_thread) from [<c015039c>] (kthread+0x158/0x1a0) >>> [<c015039c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) >>> Exception stack(0xeda03fb0 to 0xeda03ff8) >>> ... >>> ---[ end trace f55f4b28f3080c15 ]--- >>> note: kworker/3:2[87] exited with preempt_count 1 >>> >>> Best regards >>> >> I guess you build driver in (hsotg->dr_mode == USB_DR_MODE_PERIPHERAL) >> and this is why call of dwc2_lowlevel_hw_disable() at 'error:' label >> create issue. Could you please test on your setup follow implementation: >> ... >> error: >> if (hsotg->dr_mode != USB_DR_MODE_PERIPHERAL) >> dwc2_lowlevel_hw_disable(hsotg); >> return retval; >> } >> ... > > Right, this will work. However, I would also change the order of > operations - disabling usb33d regulator should be done after disabling > lowlevel hardware to properly pair with the enable operations. > My fault, I've mixed vbus and usb33d regulators. The order is correct. Best regards
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c index c347d93eae64..a6022dc4766d 100644 --- a/drivers/usb/dwc2/platform.c +++ b/drivers/usb/dwc2/platform.c @@ -581,8 +581,9 @@ static int dwc2_driver_probe(struct platform_device *dev) if (hsotg->gadget_enabled) { retval = usb_add_gadget_udc(hsotg->dev, &hsotg->gadget); if (retval) { + hsotg->gadget.udc = NULL; dwc2_hsotg_remove(hsotg); - goto error_init; + return retval; } } #endif /* CONFIG_USB_DWC2_PERIPHERAL || CONFIG_USB_DWC2_DUAL_ROLE */
When gadget registration fails, one should not call usb_del_gadget_udc(). Ensure this by setting gadget->udc to NULL. Also in case of a failure there is no need to disable low-level hardware, so return immiedetly instead of jumping to error_init label. This fixes the following kernel NULL ptr dereference on gadget failure (can be easily triggered with g_mass_storage without any module parameters): dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter besl=1 dwc2 12480000.hsotg: dwc2_check_params: Invalid parameter g_np_tx_fifo_size=1024 dwc2 12480000.hsotg: EPs: 16, dedicated fifos, 7808 entries in SPRAM Mass Storage Function, version: 2009/09/11 LUN: removable file: (no medium) no file given for LUN0 g_mass_storage 12480000.hsotg: failed to start g_mass_storage: -22 8<--- cut here --- Unable to handle kernel NULL pointer dereference at virtual address 00000104 pgd = (ptrval) [00000104] *pgd=00000000 Internal error: Oops: 805 [#1] PREEMPT SMP ARM Modules linked in: CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.8.0-rc5 #3133 Hardware name: Samsung Exynos (Flattened Device Tree) Workqueue: events deferred_probe_work_func PC is at usb_del_gadget_udc+0x38/0xc4 LR is at __mutex_lock+0x31c/0xb18 ... Process kworker/0:1 (pid: 12, stack limit = 0x(ptrval)) Stack: (0xef121db0 to 0xef122000) ... [<c076bf3c>] (usb_del_gadget_udc) from [<c0726bec>] (dwc2_hsotg_remove+0x10/0x20) [<c0726bec>] (dwc2_hsotg_remove) from [<c0711208>] (dwc2_driver_probe+0x57c/0x69c) [<c0711208>] (dwc2_driver_probe) from [<c06247c0>] (platform_drv_probe+0x6c/0xa4) [<c06247c0>] (platform_drv_probe) from [<c0621df4>] (really_probe+0x200/0x48c) [<c0621df4>] (really_probe) from [<c06221e8>] (driver_probe_device+0x78/0x1fc) [<c06221e8>] (driver_probe_device) from [<c061fcd4>] (bus_for_each_drv+0x74/0xb8) [<c061fcd4>] (bus_for_each_drv) from [<c0621b54>] (__device_attach+0xd4/0x16c) [<c0621b54>] (__device_attach) from [<c0620c98>] (bus_probe_device+0x88/0x90) [<c0620c98>] (bus_probe_device) from [<c06211b0>] (deferred_probe_work_func+0x3c/0xd0) [<c06211b0>] (deferred_probe_work_func) from [<c0149280>] (process_one_work+0x234/0x7dc) [<c0149280>] (process_one_work) from [<c014986c>] (worker_thread+0x44/0x51c) [<c014986c>] (worker_thread) from [<c0150b1c>] (kthread+0x158/0x1a0) [<c0150b1c>] (kthread) from [<c0100114>] (ret_from_fork+0x14/0x20) Exception stack(0xef121fb0 to 0xef121ff8) ... ---[ end trace 9724c2fc7cc9c982 ]--- Fixes: 207324a321a8 ("usb: dwc2: Postponed gadget registration to the udc class driver") Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/usb/dwc2/platform.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)