On Fri, Aug 24, 2012 at 10:30:54AM +0200, Marc Kleine-Budde wrote: > On 08/24/2012 09:36 AM, Felipe Balbi wrote: > > Hi, > > > > On Thu, Aug 23, 2012 at 07:30:45PM +0200, Marc Kleine-Budde wrote: > >> From: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx> > >> > >> Non control endpoints have an undefined maxpacket size on udc start. Some > >> gadget drivers check for the maxpacket size before they enable the endpoint, > >> which leads to a wrong state. > >> > >> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx> > >> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> > > > > which gadget drivers ? This doesn't look right to me. On dwc3 we > > initialize endpoints as early as module probe (before gadget->start() is > > even close to being called) and we have no issues at all setting ep0's > > maxpacket to 512. Or were you setting maxpacket for all endpoints ?? > > This patch preserves the setting of ep0 to CTRL_PAYLOAD_MAX, but changes > all other endpoint to "(unsigned short)~0". > > Other drivers have a similar ~0: > > [mkl@dude:usb (upstream/chipidea-fixes)]$ git grep "(unsigned short) ~0" > gadget/fsl_qe_udc.c: ep->ep.maxpacket = (unsigned short) ~0; > gadget/fsl_udc_core.c: ep->ep.maxpacket = (unsigned short) ~0; > gadget/mv_u3d_core.c: ep->ep.maxpacket = (unsigned short) ~0; > gadget/mv_udc_core.c: ep->ep.maxpacket = (unsigned short) ~0; > > > If that was the case, then this is a fix which should even go to stable > > ;-) > > Michael has discovered the issue, he'll return to the office soon. > > >> drivers/usb/chipidea/udc.c | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c > >> index e4db7af..75a96ec 100644 > >> --- a/drivers/usb/chipidea/udc.c > >> +++ b/drivers/usb/chipidea/udc.c > >> @@ -1455,7 +1455,12 @@ static int init_eps(struct ci13xxx *ci) > >> > >> mEp->ep.name = mEp->name; > >> mEp->ep.ops = &usb_ep_ops; > >> - mEp->ep.maxpacket = CTRL_PAYLOAD_MAX; > >> + /* > >> + * for ep0: maxP defined in desc, for other > >> + * eps, maxP is set by epautoconfig() called > >> + * by gadget layer > >> + */ > >> + mEp->ep.maxpacket = (unsigned short)~0; > > These are endpoint != 0. Ok, then the change makes a lot of sense and needs to be sent to stable too. Acked-by: Felipe Balbi <balbi@xxxxxx> -- balbi
Attachment:
signature.asc
Description: Digital signature
