Re: [PATCH 6/8] usb: cdns3: gadget: need to handle sg case for WA2 case

[Peter Chen <peter.chen@nxp.com>]

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".

-- 

Thanks,
Peter Chen