[PATCH] usb: dwc3: be more verbose on ERRATIC

Hi,

Post by David Cohen
ERRATIC_ERROR interrupt is an event that needs more attention from
developers than currently implemented, since this indicates a serious
stability issue. The only way to get warned about it is by selecting the
maximum driver's verbosity.
This patch increases a bit the error's verbosity.
---
drivers/usb/dwc3/gadget.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3818b26bfc05..132e761d62e4 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2484,7 +2484,8 @@ static void dwc3_gadget_interrupt(struct dwc3 *dwc,
dev_vdbg(dwc->dev, "Start of Periodic Frame\n");
break;
- dev_vdbg(dwc->dev, "Erratic Error\n");
+ WARN_ON_ONCE(1);
+ dev_dbg(dwc->dev, "Erratic Error\n");

how about:
WARN_ONCE(true, "Erratic Error\n");

instead ?

Post by David Cohen
break;
dev_vdbg(dwc->dev, "Command Complete\n");
--
2.1.0

--
balbi

David Cohen

2014-10-15 16:57:18 UTC

WARN_ONCE(true, "Erratic Error\n");
instead ?

When erratic error event happens, in my experience it is usually
followed but many of them in a row. It may end up too verbose.

Br, David

Post by David Cohen
break;
dev_vdbg(dwc->dev, "Command Complete\n");
--
2.1.0

--
balbi

David Cohen

2014-10-15 17:01:45 UTC

WARN_ONCE(true, "Erratic Error\n");
instead ?

When erratic error event happens, in my experience it is usually
followed but many of them in a row. It may end up too verbose.

Oops, sorry for my confusion. I read WARN(), not WARN_ONCE() when I read
for the first time :)

I let the dev_dbg() because it may be useful to get all the occurrences
in case debug is enabled. But WARN_ONCE() is fine too if you prefer.

Br, David

Post by David Cohen
Br, David

Post by David Cohen
break;
dev_vdbg(dwc->dev, "Command Complete\n");
--
2.1.0

--
balbi

Felipe Balbi

2014-10-15 17:06:10 UTC

WARN_ONCE(true, "Erratic Error\n");
instead ?

When erratic error event happens, in my experience it is usually
followed but many of them in a row. It may end up too verbose.

Oops, sorry for my confusion. I read WARN(), not WARN_ONCE() when I read
for the first time :)
I let the dev_dbg() because it may be useful to get all the occurrences
in case debug is enabled. But WARN_ONCE() is fine too if you prefer.

We might WARN_ONCE() and forcibly disconnect from host as the platform
worn't work anymore after Erratic Error happens. Then just make that
clear with "Erratic Error: disconnecting from host\n" or something like
that.

The only way to fix erratic error (by fixing code - probably PHY driver)
will require rebooting the platform anyway. At a minimum, you'd have to
unload and reload dwc3.ko, just make sure (if you can reproduce erratic
error easily) modprobe -r dwc3 && modprobe dwc3 still causes dwc3 to
reconnect to host.

cheers

--
balbi

David Cohen

2014-10-15 18:36:33 UTC

WARN_ONCE(true, "Erratic Error\n");
instead ?

When erratic error event happens, in my experience it is usually
followed but many of them in a row. It may end up too verbose.

Oops, sorry for my confusion. I read WARN(), not WARN_ONCE() when I read
for the first time :)
I let the dev_dbg() because it may be useful to get all the occurrences
in case debug is enabled. But WARN_ONCE() is fine too if you prefer.

That works.
I assume calling dwc3_gadget_disconnect_interrupt() from
DWC3_DEVICE_EVENT_ERRATIC_ERROR should be enough to disconnect the
gadget and propagate the status to userspace.

Post by Felipe Balbi
The only way to fix erratic error (by fixing code - probably PHY driver)
will require rebooting the platform anyway. At a minimum, you'd have to
unload and reload dwc3.ko, just make sure (if you can reproduce erratic
error easily) modprobe -r dwc3 && modprobe dwc3 still causes dwc3 to
reconnect to host.

It does. Forcing soft reset on whole IP is how databook describes to
solve it.

Br, David

Post by Felipe Balbi
cheers
--
balbi

Felipe Balbi

2014-10-15 18:55:39 UTC