Re: [PATCH] can: kvaser_usb: fix usb endpoints detection

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1814 bytes --]

On 10/27/2013 10:07 PM, Olivier Sobrie wrote:
> Some devices, like the Kvaser Memorator Professional, have several
> bulk in endpoints. Only the first one found should be used by the
> driver. The same holds for the bulk out endpoint.

Can you give reference where this information comes from? e.g.
documentation or another driver?

> Signed-off-by: Olivier Sobrie <olivier@sobrie.be>
> ---
>  drivers/net/can/usb/kvaser_usb.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> index 3b95465..bdbf5bf 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -1554,13 +1554,14 @@ static void kvaser_usb_get_endpoints(const struct usb_interface *intf,
>  
>  	iface_desc = &intf->altsetting[0];
>  
> -	for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
> +	for (i = 0; (i < iface_desc->desc.bNumEndpoints) && (!*in || !*out);
> +	     ++i) {
>  		endpoint = &iface_desc->endpoint[i].desc;
>  
> -		if (usb_endpoint_is_bulk_in(endpoint))
> +		if (!*in && usb_endpoint_is_bulk_in(endpoint))
>  			*in = endpoint;
>  
> -		if (usb_endpoint_is_bulk_out(endpoint))
> +		if (!*out && usb_endpoint_is_bulk_out(endpoint))
>  			*out = endpoint;

I personally would add a break or return here, together with a short
comment:

	/* only use first endpoint for in and out */
	if (*in && *out)
		return;

>  	}
>  }
> 

Thanks for taking care of the driver,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]