Hi,
On Mon, Aug 06, 2012 at 07:22:52PM +0530, Pratyush Anand wrote:
> >commit 4b345c9a3c7452340fb477868d8db475f05978b1
> >Author: Felipe Balbi <ba...@ti.com>
> >Date: Mon Jul 16 14:08:16 2012 +0300
> >
> > usb: dwc3: gadget: set Ignore Sequence Number bit from ConnectDone Event
> >
> > Databook says we should set Ignore Sequence Number bit
> > from ConnectDone Event, so let's do so.
> >
> > Signed-off-by: Felipe Balbi <ba...@ti.com>
> >
> >diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> >index 58fdfad..283c0cb 100644
> >--- a/drivers/usb/dwc3/gadget.c
> >+++ b/drivers/usb/dwc3/gadget.c
> >@@ -431,7 +431,8 @@ static int dwc3_gadget_start_config(struct dwc3 *dwc,
> >struct dwc3_ep *dep)
> >
> > static int dwc3_gadget_set_ep_config(struct dwc3 *dwc, struct dwc3_ep *dep,
> > const struct usb_endpoint_descriptor *desc,
> >- const struct usb_ss_ep_comp_descriptor *comp_desc)
> >+ const struct usb_ss_ep_comp_descriptor *comp_desc,
> >+ bool ignore)
> > {
> > struct dwc3_gadget_ep_cmd_params params;
> >
> >@@ -441,6 +442,9 @@ static int dwc3_gadget_set_ep_config(struct dwc3 *dwc,
> >struct dwc3_ep *dep,
> > | DWC3_DEPCFG_MAX_PACKET_SIZE(usb_endpoint_maxp(desc))
> > | DWC3_DEPCFG_BURST_SIZE(dep->endpoint.maxburst - 1);
> >
> >+ if (ignore)
> >+ params.param0 |= DWC3_DEPCFG_IGN_SEQ_NUM;
> >+
> > params.param1 = DWC3_DEPCFG_XFER_COMPLETE_EN
> > | DWC3_DEPCFG_XFER_NOT_READY_EN;
> >
> >@@ -498,7 +502,8 @@ static int dwc3_gadget_set_xfer_resource(struct dwc3
> >*dwc, struct dwc3_ep *dep)
> > */
> > static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> > const struct usb_endpoint_descriptor *desc,
> >- const struct usb_ss_ep_comp_descriptor *comp_desc)
> >+ const struct usb_ss_ep_comp_descriptor *comp_desc,
> >+ bool ignore)
> > {
> > struct dwc3 *dwc = dep->dwc;
> > u32 reg;
> >@@ -510,7 +515,7 @@ static int __dwc3_gadget_ep_enable(struct dwc3_ep *dep,
> > return ret;
> > }
> >
> >- ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc);
> >+ ret = dwc3_gadget_set_ep_config(dwc, dep, desc, comp_desc, ignore);
> > if (ret)
> > return ret;
> >
> >@@ -683,7 +688,7 @@ static int dwc3_gadget_ep_enable(struct usb_ep *ep,
> > dev_vdbg(dwc->dev, "Enabling %s\n", dep->name);
> >
> > spin_lock_irqsave(&dwc->lock, flags);
> >- ret = __dwc3_gadget_ep_enable(dep, desc, ep->comp_desc);
> >+ ret = __dwc3_gadget_ep_enable(dep, desc, ep->comp_desc, false);
>
> I was not sure about it.
> Databook specifically talks about to use "modify" on connection done.
> But, in description of "Config Action: Modify", it says that this
> action is used when modifying an existing endpoint configuration.
>
> So, does it not mean that when we issue DEPCFG for the first time for
> any endpoint, then only we need to set "Config Action :Initialize",
> else "Config Action: Modify".My understanding is that this is only needed when we want to change any configuration of any endpoint without disabling it first. And the only case where this happens is with our physical endpoints 0 and 1. I guess it tells us to use Connect Done event for ep0/1 because at that time we already know the speed we're running, so we can reconfigure ep0/1 wMaxPacketSize properly. For all other endpoints, we only change configuration when the endpoint is disabled. See how the framework is designed today: when we change an interface, our endpoints will be disabled by the gadget driver and re-enabled with different descriptors. This means that, except for ep0/1, no endpoint has an existing configuration ;-) -- balbi
signature.asc
Description: Digital signature
