> -----Original Message-----
> From: David Brownell [mailto:[EMAIL PROTECTED]
> Sent: Saturday, March 17, 2007 6:38 AM
> To: [email protected]
> Cc: Li Yang-r58472; [EMAIL PROTECTED]
> Subject: Re: [linux-usb-devel] [PATCH] Fix gadget serial response on
> USB_CDC_REQ_SET_LINE_CODING
>
> On Wednesday 14 March 2007 5:18 am, Li Yang wrote:
> > Apparently this gadget doesn't support setting usb_cdc_line_coding.
>
> It does support it ... but that support is buggy. It should be
issuing
> a read request before doing the memcpy(); instead, it wrongly assumes
> that something else already issued the read.
Memcpy() the line_coding from host to the private port_line_coding
structure doesn't mean the port is really changed. Port_line_coding
structure isn't used anywhere in the driver accept it fools around the
next USB_CDC_REQ_GET_LINE_CODING request from host.
>
> Plus, it doesn't correctly test for some error cases: port is null,
> there's not enough incoming data.
>
>
> > But the code pretends that it supports, and queues a zero length
> > request for the DATA phase. That will cause overflow for udc in
> > processing the DATA transaction. The patch make it return
-EOPNOTSUPP
> > and stall cleanly.
>
> So, a workaround for the bug. Did you notice how the other SET
> request has the same bug?
>
> If you're not going to really fix this, please update the patch
> to address the same bug in the other path; and its description
> to say that you're replacing buggy code with an explicit failure.
>
> Best of course would be to fix the underlying bugs, since you
> clearly have a test setup and the fixes should be trivial, more or
> less
>
> if (wLength == right length) {
> update req->complete
> ret = right length
> }
>
> and providing new completion handlers which copy the data.
I knew it was not a complete fix. But I didn't have the time and
knowledge to add support to this function. Moreover, this is an
optional function for CDC class, so it is all right to leave it
unsupported.
>
> - Dave
>
>
> > Signed-off-by: Li Yang <[EMAIL PROTECTED]>
> > ---
> >
> > diff --git a/drivers/usb/gadget/serial.c
b/drivers/usb/gadget/serial.c
> > index e552668..0911bc4 100644
> > --- a/drivers/usb/gadget/serial.c
> > +++ b/drivers/usb/gadget/serial.c
> > @@ -1692,14 +1692,7 @@ static int gs_setup_class(struct usb_gadget
*gadget,
> >
> > switch (ctrl->bRequest) {
> > case USB_CDC_REQ_SET_LINE_CODING:
> > - ret = min(wLength,
> > - (u16)sizeof(struct usb_cdc_line_coding));
> > - if (port) {
> > - spin_lock(&port->port_lock);
> > - memcpy(&port->port_line_coding, req->buf, ret);
> > - spin_unlock(&port->port_lock);
> > - }
> > - ret = 0;
> > + /* FIXME: We don't support set line coding */
> > break;
> >
> > case USB_CDC_REQ_GET_LINE_CODING:
> >
> >
> >
------------------------------------------------------------------------
-
> > Take Surveys. Earn Cash. Influence the Future of IT
> > Join SourceForge.net's Techsay panel and you'll get the chance to
share your
> > opinions on IT & business topics through brief surveys-and earn cash
> >
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDE
V
> > _______________________________________________
> > [email protected]
> > To unsubscribe, use the last form field at:
> > https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
> >
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
