Zitat von Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>:
On Mon, 4 Feb 2019, Guido Kiener wrote:
From: Guido Kiener <guido.kiener@xxxxxxxxxxxxxxxxx>
A reset e.g. calling ep_reset_338x() can happen while endpoints
are enabled.
How can this happen? That routine is called from only two places.
One is in net2280_disable(), so after the endpoint has already been
disabled. The other is in usb_reinit_338x() via usb_reinit() via
stop_activity(), which disconnects the gadget driver and thereby
disables all the endpoints.
Yes, the routine is called when I stop my application. See callstack:
Call Trace:
net2280_disable: Invalid ep=00000000341c7996 or ep->desc
usb_ep_disable+0x24/0xa0 [udc_core]
disableEndpoints+0x1c/0x80 [rsusbtmc]
tmc_func_disable+0x29/0xa0 [rsusbtmc]
reset_config+0x3e/0xa0 [libcomposite]
composite_disconnect+0x36/0x60 [libcomposite]
net2280_pullup+0x9e/0xf0 [net2280]
usb_gadget_disconnect+0x35/0xc0 [udc_core]
usb_gadget_remove_driver+0x29/0xa0 [udc_core]
usb_gadget_unregister_driver+0xc1/0xf0 [udc_core]
usb_composite_unregister+0x12/0x20 [libcomposite]
dev_release+0x3f/0x80 [rsusbtmc]
__fput+0x102/0x230
____fput+0xe/0x10
task_work_run+0x90/0xc0
exit_to_usermode_loop+0xf2/0x100
do_syscall_64+0xfb/0x120
entry_SYSCALL_64_after_hwframe+0x44/0xa9
When I restart my application then the endpoints remains disabled.
A workaround is to reload our (legacy) gadget driver, libcomposite
and net2280.
The ep_reset_338x() sets ep->desc = NULL to mark
endpoint being invalid. A subsequent call of net2280_disable will
fail and return -EINVAL to parent function usb_ep_disable(),
which will fail, too, and do not set the member ep->enabled = false.
So maybe a better fix (assuming there really is a problem) is to make
usb_ep_disable() clear ep->enabled always. After all, the only
reasonable way for usb_ep_disable() to fail is if the endpoint isn't
enabled in the first place.
I had the same idea. However the root cause is the net2280 driver.
We have not seen the problem with other function drivers.
The usb_ep_disable() implementation looks reasonable to me. When the
ret = ep->ops->disable(ep); function fails for any other fictitious
reason (e.g. conflicts, order of endpoints..) then it might be better to
set ep->enabled = false when the function driver really succeeds.
Regards,
Guido
