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

[Felipe Balbi <balbi@xxxxxx>]

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.

-- 
balbi

Attachment: signature.asc
Description: Digital signature