Hi, Baolin Wang <[email protected]> writes: >>>>>>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c >>>>>>> index 1783406..ca2ae5b 100644 >>>>>>> --- a/drivers/usb/dwc3/gadget.c >>>>>>> +++ b/drivers/usb/dwc3/gadget.c >>>>>>> @@ -241,6 +241,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, >>>>>>> unsigned cmd, >>>>>>> int susphy = false; >>>>>>> int ret = -EINVAL; >>>>>>> >>>>>>> + if (!dwc->pullups_connected) >>>>>>> + return -ESHUTDOWN; >>>>>>> + >>>> >>>> you skip trace_dwc3_gadget_ep_cmd() >>> >>> Yes, we did not need trace here since we did not send out the command. >>> >> What in such case: enumeration will not work and this will be because >> this ESHUTDOWN or wrong pullups_connected usage. Without a trace you >> will not know where the problem is. >> In my opinion this trace could be useful. > > We have returned the '-ESHUTDOWN' error number for user to know what > happened.
No, this is actually not good and Janusz has a very valid point
here. When we're debugging something in dwc3, we want to rely on
tracepoints to tell us what's going on. I don't want to have to add more
debugging messages to print out that ESHUTDOWN error just so I can
figure out what's going on. You should be patching this in a way that
the tracepoint is still printed out properly.
Keep in mind that you should be setting ret to -ESHUTDOWN and make sure
the trace is printed. Same patch should, then, patch trace.h to handle
-ESHUTDOWN as an error case, i.e. following hunk should be added:
diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
index d93780e84f07..70b783f0507d 100644
--- a/drivers/usb/dwc3/debug.h
+++ b/drivers/usb/dwc3/debug.h
@@ -319,6 +319,8 @@ static inline const char *dwc3_ep_cmd_status_string(int
status)
switch (status) {
case -ETIMEDOUT:
return "Timed Out";
+ case -ESHUTDOWN:
+ return "Shut Down";
case 0:
return "Successful";
case DEPEVT_TRANSFER_NO_RESOURCE:
--
balbi
signature.asc
Description: PGP signature
