2020-07-14 08:48:42

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH] usb: dwc2: Fix error path in gadget registration

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 <[email protected]>
---
drivers/usb/dwc2/platform.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

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 */
--
2.17.1


2020-07-14 12:34:10

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH] usb: dwc2: Fix error path in gadget registration

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

2020-07-15 09:14:28

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH] usb: dwc2: Fix error path in gadget registration

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
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2020-07-15 15:10:42

by Minas Harutyunyan

[permalink] [raw]
Subject: Re: [PATCH] usb: dwc2: Fix error path in gadget registration

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

2020-07-16 12:01:19

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH] usb: dwc2: Fix error path in gadget registration

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/[email protected]/T/#u

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2020-07-16 12:04:48

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH] usb: dwc2: Fix error path in gadget registration

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
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland