On 08/24/2012 10:31 AM, Felipe Balbi wrote: > 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> Thanks, I'll rephrase the descipton a bit. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
Attachment:
signature.asc
Description: OpenPGP digital signature
