RE: [PATCHv3] usb: dwc3: use a burst size only ss-mode

[Chanho Park <chanho61.park@xxxxxxxxxxx>]

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@xxxxxx]
> Sent: Friday, August 31, 2012 3:59 PM
> To: Chanho Park
> Cc: balbi@xxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx;
> Kyungmin Park
> Subject: Re: [PATCHv3] usb: dwc3: use a burst size only ss-mode
> 
> Hi,
> 
> On Fri, Aug 31, 2012 at 01:42:23PM +0900, Chanho Park wrote:
> > When connection is established non-ss mode, a maxburst is set to 0.
> > Therefore, the value of burst size has wrong value. We must use the
> > burst size only in ss-mode.
> >
> > Signed-off-by: Chanho Park <chanho61.park@xxxxxxxxxxx>
> > Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> > ---
> >  drivers/usb/dwc3/gadget.c |    6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 58fdfad..440a86b 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -434,12 +434,16 @@ static int dwc3_gadget_set_ep_config(struct
> dwc3 *dwc, struct dwc3_ep *dep,
> >  		const struct usb_ss_ep_comp_descriptor *comp_desc)  {
> >  	struct dwc3_gadget_ep_cmd_params params;
> > +	u32 burst_size;
> >
> >  	memset(&params, 0x00, sizeof(params));
> >
> > +	/* We only use a burst size in ss-mode */
> > +	burst_size = (com_desc) ? (dep->endpoint.maxburst - 1) : 0;
> 
> wont compile. Please, never send patches without testing as that just wastes
> everybody's time.
> 
> >  	params.param0 =
> DWC3_DEPCFG_EP_TYPE(usb_endpoint_type(desc))
> >  		|
> DWC3_DEPCFG_MAX_PACKET_SIZE(usb_endpoint_maxp(desc))
> > -		| DWC3_DEPCFG_BURST_SIZE(dep->endpoint.maxburst - 1);
> > +		| DWC3_DEPCFG_BURST_SIZE(burst_size);
> 
> Also, readability can be improved with something like below:
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index
> e09a7c4..e2dd764 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -442,8 +442,14 @@ static int dwc3_gadget_set_ep_config(struct dwc3
> *dwc, struct dwc3_ep *dep,
>  	memset(&params, 0x00, sizeof(params));
> 
>  	params.param0 =
> DWC3_DEPCFG_EP_TYPE(usb_endpoint_type(desc))
> -		|
> DWC3_DEPCFG_MAX_PACKET_SIZE(usb_endpoint_maxp(desc))
> -		| DWC3_DEPCFG_BURST_SIZE(dep->endpoint.maxburst - 1);
> +		|
> DWC3_DEPCFG_MAX_PACKET_SIZE(usb_endpoint_maxp(desc));
> +
> +	/* bursts are only needed in SuperSpeed mode */
> +	if (dwc->gadget.speed == USB_SPEED_SUPER) {
> +		unsigned int burst = dep->endpoint.maxburst - 1;
> +
> +		params.param0 |= DWC3_DEPCFG_BURST_SIZE(burst);
> +	}
> 
>  	if (ignore)
>  		params.param0 |= DWC3_DEPCFG_IGN_SEQ_NUM;
> 
> this is clearer for anyone reading the source code and it also compiles and
> works.

I'm really sorry for you and everybody's inconvenience.
I'll reposting it after testing patch.

Best regards,
Chanho Park 

> 
> --
> balbi

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html