Hi Krzysztof,
Thanks for the patch.
On 12/01/2016 10:02 AM, Krzysztof Opasiak wrote:
> Current implementation of init_vudc_hw() adds ep0 to ep_list
> and then after looping through all endpoints removes it from
> that list.
>
> As this may be misleading let's refactor this function
> and avoid adding and removing ep0 to eplist and place it
> immediately in correct place.
>
> Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>
> ---
> drivers/usb/usbip/vudc_dev.c | 38 +++++++++++++++++++++-----------------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c
> index 7091848..a5ca29b 100644
> --- a/drivers/usb/usbip/vudc_dev.c
> +++ b/drivers/usb/usbip/vudc_dev.c
> @@ -549,30 +549,37 @@ static int init_vudc_hw(struct vudc *udc)
> sprintf(ep->name, "ep%d%s", num,
> i ? (is_out ? "out" : "in") : "");
> ep->ep.name = ep->name;
> +
> + ep->ep.ops = &vep_ops;
> +
> + ep->halted = ep->wedged = ep->already_seen =
> + ep->setup_stage = 0;
Do you need to clear these explicitly. kcalloc() should do it for you.
> + usb_ep_set_maxpacket_limit(&ep->ep, ~0);
> + ep->ep.max_streams = 16;
> + ep->gadget = &udc->gadget;
> + ep->desc = NULL;
> + INIT_LIST_HEAD(&ep->req_queue);
> +
> if (i == 0) {
> + /* ep0 */
> ep->ep.caps.type_control = true;
> ep->ep.caps.dir_out = true;
> ep->ep.caps.dir_in = true;
> +
> + udc->gadget.ep0 = &ep->ep;
> } else {
> + /* All other eps */
> ep->ep.caps.type_iso = true;
> ep->ep.caps.type_int = true;
> ep->ep.caps.type_bulk = true;
> - }
>
> - if (is_out)
> - ep->ep.caps.dir_out = true;
> - else
> - ep->ep.caps.dir_in = true;
> + if (is_out)
> + ep->ep.caps.dir_out = true;
> + else
> + ep->ep.caps.dir_in = true;
You are moving the is_out handling which was common for all eps
including ep0 under non-eop0 - is that right?
>
> - ep->ep.ops = &vep_ops;
> - list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list);
> - ep->halted = ep->wedged = ep->already_seen =
> - ep->setup_stage = 0;
> - usb_ep_set_maxpacket_limit(&ep->ep, ~0);
> - ep->ep.max_streams = 16;
> - ep->gadget = &udc->gadget;
> - ep->desc = NULL;
> - INIT_LIST_HEAD(&ep->req_queue);
> + list_add_tail(&ep->ep.ep_list, &udc->gadget.ep_list);
> + }
> }
>
> spin_lock_init(&udc->lock);
> @@ -589,9 +596,6 @@ static int init_vudc_hw(struct vudc *udc)
> ud->eh_ops.reset = vudc_device_reset;
> ud->eh_ops.unusable = vudc_device_unusable;
>
> - udc->gadget.ep0 = &udc->ep[0].ep;
> - list_del_init(&udc->ep[0].ep.ep_list);
> -
> v_init_timer(udc);
> return 0;
>
>
thanks,
-- Shuah
--
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
