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

[Felipe Balbi]

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.p...@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
> ---
>  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

signature.asc
Description: Digital signature