Hi ThinhI'm trying to update the DWC3 driver to allow the status phase of a transaction to be explicit; meaning the gadget driver has the choice to either Ack or Stall _after_ the data phase so that the contents of the data phase can be validated. I thought that I should be able to achieve this by preventing dwc3_ep0_xfernotready() from calling dwc3_ep0_do_control_status() (relying on an "explicit_status" flag added to the usb_request to decide whether or not to do so) and then calling it manually later once the data phase was validated by the gadget driver (or indeed userspace). A very barebones version of my attempt to do that looks like this:
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 81c486b3941c..c35436f3d3c3 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -788,6 +788,7 @@ enum dwc3_ep0_state {
EP0_SETUP_PHASE,
EP0_DATA_PHASE,
EP0_STATUS_PHASE,
+ EP0_AWAITING_EXPLICIT_STATUS,
};
enum dwc3_link_state {
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 5d642660fd15..692a99debcd9 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -894,10 +894,14 @@ static void dwc3_ep0_complete_data(struct dwc3 *dwc,
dwc->ep0_bounced = false;
}
- if ((epnum & 1) && ur->actual < ur->length)
+ if ((epnum & 1) && ur->actual < ur->length) {
dwc3_ep0_stall_and_restart(dwc);
- else
+ } else {
+ if (r->request.explicit_status)
+ dwc->ep0state = EP0_AWAITING_EXPLICIT_STATUS;
+
dwc3_gadget_giveback(ep0, r, 0);
+ }
}
static void dwc3_ep0_complete_status(struct dwc3 *dwc,
@@ -1111,6 +1115,15 @@ void dwc3_ep0_end_control_data(struct dwc3 *dwc,
struct dwc3_ep *dep)
dep->resource_index = 0;
}
+void dwc3_gadget_ep0_control_ack(struct usb_ep *ep)
+{
+ struct dwc3_ep *dep = to_dwc3_ep(ep);
+ struct dwc3 *dwc = dep->dwc;
+
+ dwc->ep0state = EP0_STATUS_PHASE;
+ __dwc3_ep0_do_control_status(dwc, dep);
+}
+
static void dwc3_ep0_xfernotready(struct dwc3 *dwc,
const struct dwc3_event_depevt *event)
{
@@ -1140,6 +1153,14 @@ static void dwc3_ep0_xfernotready(struct dwc3 *dwc,
if (dwc->ep0_next_event != DWC3_EP0_NRDY_STATUS)
return;
+ /*
+ * If the request asked for an explicit status stage
hold off
+ * on sending the status until userspace asks us to.
+ */
+ if (dwc->ep0state == EP0_AWAITING_EXPLICIT_STATUS &&
+ !event->endpoint_number)
+ return;
+
dwc->ep0state = EP0_STATUS_PHASE;
if (dwc->delayed_status) {
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 0d89dfa6eef5..85044bbbce02 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2228,6 +2228,7 @@ static const struct usb_ep_ops dwc3_gadget_ep0_ops = {
.dequeue = dwc3_gadget_ep_dequeue,
.set_halt = dwc3_gadget_ep0_set_halt,
.set_wedge = dwc3_gadget_ep_set_wedge,
+ .control_ack = dwc3_gadget_ep0_control_ack,
};
static const struct usb_ep_ops dwc3_gadget_ep_ops = {
diff --git a/drivers/usb/dwc3/gadget.h b/drivers/usb/dwc3/gadget.h
index 55a56cf67d73..4fc9737b54ca 100644
--- a/drivers/usb/dwc3/gadget.h
+++ b/drivers/usb/dwc3/gadget.h
@@ -116,6 +116,7 @@ int __dwc3_gadget_ep0_set_halt(struct usb_ep *ep,
int value);
int dwc3_gadget_ep0_set_halt(struct usb_ep *ep, int value); int dwc3_gadget_ep0_queue(struct usb_ep *ep, struct usb_request *request, gfp_t gfp_flags); +void dwc3_gadget_ep0_control_ack(struct usb_ep *ep);int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol);
void dwc3_ep0_send_delayed_status(struct dwc3 *dwc);void dwc3_stop_active_transfer(struct dwc3_ep *dep, bool force, bool interrupt);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 3ad58b7a0824..6ee43966eafb 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -122,6 +122,8 @@ struct usb_request {
int status;
unsigned actual;
+
+ bool explicit_status;
};
/*-------------------------------------------------------------------------*/
@@ -152,6 +154,7 @@ struct usb_ep_ops {
int (*fifo_status) (struct usb_ep *ep);
void (*fifo_flush) (struct usb_ep *ep);
+ void (*control_ack) (struct usb_ep *ep);
};
/**
And then the ack would be triggered by the gadget driver calling
dwc3_gadget_ep0_control_ack() (in this case just through
gadget->ep0->ops->control_ack(gadget->ep0), but probably I would
abstract it out to the gadget layer or just mixed into usb_ep_queue()
for ep0...).
When I do this, we do subsequently get the dwc3_ep0_xfer_complete() interrupt that calls dwc3_ep0_complete_status(), but the dwc3 gets stuck in the loop in dwc3_send_gadget_ep_cmd() immediately afterwards. It seems the "CMDACT" bit is never cleared (I assume that means "command active"...) but I can't see what's supposed to be clearing that so I assume it's internal firmware or something. I can't figure out how to proceed at this point, so I'm hoping you might have some advice about the right way to go about this. I attached a trace of the dwc3 events that shows the problem.
Side note; I realised when looking for your email that I started a thread about errors on the interrupt endpoint on the dwc3 and then abandoned it; sorry about that. Turns out the UVC gadget doesn't have any support for that endpoint anyway so I dropped it as low priority and forgot to reply to the thread.
Thanks Dan
Attachment:
trace.tar.gz
Description: application/gzip
