| Message ID | 20200901084454.28649-7-peter.chen@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: cdns3: improve the sg use case | expand |
Hi, Peter Chen <peter.chen@nxp.com> writes: > Add sg support for WA2 case. what's WA2? Care to spell it out? > Signed-off-by: Peter Chen <peter.chen@nxp.com> > --- > drivers/usb/cdns3/gadget.c | 44 +++++++++++++++++++++++++++----------- > 1 file changed, 31 insertions(+), 13 deletions(-) > > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c > index 6cb44c354f40..1fd36bc5c6db 100644 > --- a/drivers/usb/cdns3/gadget.c > +++ b/drivers/usb/cdns3/gadget.c > @@ -462,6 +462,36 @@ static int cdns3_start_all_request(struct cdns3_device *priv_dev, > (reg) |= EP_STS_EN_DESCMISEN; \ > } } while (0) > > +static void __cdns3_descmiss_copy_data(struct usb_request *request, > + struct usb_request *descmiss_req) > +{ > + int length = request->actual + descmiss_req->actual; > + struct scatterlist *s = request->sg; > + > + if (!s) { > + if (length <= request->length) { > + memcpy(&((u8 *)request->buf)[request->actual], memcpy(request->buf + request->actual, ... ? > + descmiss_req->buf, > + descmiss_req->actual); > + request->actual = length; > + } else { > + /* It should never occures */ ^^^^^^^ occur ps: famous last words :-) > + request->status = -ENOMEM; this is not documented as a valid status for usb request completion. Who's treating this -ENOMEM case? Which gadgets have you tested with this change?
On 20-09-08 09:29:55, Felipe Balbi wrote: > > Hi, > > Peter Chen <peter.chen@nxp.com> writes: > > Add sg support for WA2 case. > > what's WA2? Care to spell it out? It is the workaround 2, there is a description for it at the beginning of this file. If you search the file, you may find there are some functions has "wa2" prefix. I will improve the commit log for it. > > > Signed-off-by: Peter Chen <peter.chen@nxp.com> > > --- > > drivers/usb/cdns3/gadget.c | 44 +++++++++++++++++++++++++++----------- > > 1 file changed, 31 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c > > index 6cb44c354f40..1fd36bc5c6db 100644 > > --- a/drivers/usb/cdns3/gadget.c > > +++ b/drivers/usb/cdns3/gadget.c > > @@ -462,6 +462,36 @@ static int cdns3_start_all_request(struct cdns3_device *priv_dev, > > (reg) |= EP_STS_EN_DESCMISEN; \ > > } } while (0) > > > > +static void __cdns3_descmiss_copy_data(struct usb_request *request, > > + struct usb_request *descmiss_req) > > +{ > > + int length = request->actual + descmiss_req->actual; > > + struct scatterlist *s = request->sg; > > + > > + if (!s) { > > + if (length <= request->length) { > > + memcpy(&((u8 *)request->buf)[request->actual], > > memcpy(request->buf + request->actual, ... ? > > > + descmiss_req->buf, > > + descmiss_req->actual); > > + request->actual = length; > > + } else { > > + /* It should never occures */ > ^^^^^^^ > occur Will fix. > > ps: famous last words :-) > > > + request->status = -ENOMEM; > > this is not documented as a valid status for usb request > completion. Who's treating this -ENOMEM case? Which gadgets have you > tested with this change? > I just add sg use case for this WA2, do not touch the current design. The legacy chip has issue that it gets OUT data at FIFO and send ACK to host even the endpoints are not enabled, it assumes the length of this data is not longer than the data length the gadget request later. We use acm + ncm to reproduce it, the pre-load data at FIFO is several bytes, no more than 1024 bytes. I have not found the valid status described at gadget.h, it only describes the usage for "-ESHUTDOWN".
diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c index 6cb44c354f40..1fd36bc5c6db 100644 --- a/drivers/usb/cdns3/gadget.c +++ b/drivers/usb/cdns3/gadget.c @@ -462,6 +462,36 @@ static int cdns3_start_all_request(struct cdns3_device *priv_dev, (reg) |= EP_STS_EN_DESCMISEN; \ } } while (0) +static void __cdns3_descmiss_copy_data(struct usb_request *request, + struct usb_request *descmiss_req) +{ + int length = request->actual + descmiss_req->actual; + struct scatterlist *s = request->sg; + + if (!s) { + if (length <= request->length) { + memcpy(&((u8 *)request->buf)[request->actual], + descmiss_req->buf, + descmiss_req->actual); + request->actual = length; + } else { + /* It should never occures */ + request->status = -ENOMEM; + } + } else { + if (length <= sg_dma_len(s)) { + void *p = phys_to_virt(sg_dma_address(s)); + + memcpy(&((u8 *)p)[request->actual], + descmiss_req->buf, + descmiss_req->actual); + request->actual = length; + } else { + request->status = -ENOMEM; + } + } +} + /** * cdns3_wa2_descmiss_copy_data copy data from internal requests to * request queued by class driver. @@ -488,21 +518,9 @@ static void cdns3_wa2_descmiss_copy_data(struct cdns3_endpoint *priv_ep, chunk_end = descmiss_priv_req->flags & REQUEST_INTERNAL_CH; length = request->actual + descmiss_req->actual; - request->status = descmiss_req->status; - - if (length <= request->length) { - memcpy(&((u8 *)request->buf)[request->actual], - descmiss_req->buf, - descmiss_req->actual); - request->actual = length; - } else { - /* It should never occures */ - request->status = -ENOMEM; - } - + __cdns3_descmiss_copy_data(request, descmiss_req); list_del_init(&descmiss_priv_req->list); - kfree(descmiss_req->buf); cdns3_gadget_ep_free_request(&priv_ep->endpoint, descmiss_req); --priv_ep->wa2_counter;
Add sg support for WA2 case. Signed-off-by: Peter Chen <peter.chen@nxp.com> --- drivers/usb/cdns3/gadget.c | 44 +++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 13 deletions(-)