[1/3] udc: net2280: Fix net2280_disable

Zitat von Alan Stern [off-list ref]:

On Mon, 4 Feb 2019, Guido Kiener wrote:

quoted

From: Guido Kiener <redacted>

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.

quoted

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