On Fri, Jan 25, 2013 at 05:05:44PM +0100, Frans Klaver wrote:
> Signed-off-by: Frans Klaver <frans.klaver@xxxxxxxxx>
>
> ---
>
> This is a revision of the xsens_mt module. Since previous one, I've
> reduced the allocated buffers, cause there seem to be no problems with
> the default size right now.
That's good. Bigger buffers can make things go faster, but only up to a
point, it's nice to hear that our default sizes work well.
> Filtering the correct interface has been moved to the probe function,
> which seems more appropriate. Filtering in attach is slightly easier,
> but returning any error will result in EIO.
That's the proper place for it (probe).
> I'd remove the bNumEndpoints if I could.
What do you mean by that? You are iterating over the endpoints
correctly in the code:
> +static int has_required_endpoints(const struct usb_host_interface *interface)
> +{
> + __u8 i;
> + int has_bulk_in = 0;
> + int has_bulk_out = 0;
> +
> + for (i = 0; i < interface->desc.bNumEndpoints; ++i) {
> + if (usb_endpoint_is_bulk_in(&interface->endpoint[i].desc))
> + has_bulk_in = 1;
> + else if (usb_endpoint_is_bulk_out(&interface->endpoint[i].desc))
> + has_bulk_out = 1;
> + }
> +
> + return has_bulk_in && has_bulk_out;
> +}
Well, only one _very_ tiny issue. And it's nothing to stop me from
taking this patch, but only for you to know about.
Variable types that start with "__" are there as they cross the
user/kernel boundry. On the kernel side, they can always be referenced
by using the non-"__" type, as it is the same. So you almost never see
local variables with the "__" type.
So you really should just make the first line of this function be:
u8 i;
But it's not a big deal at all, I'll go queue this patch up now.
Nice job with it,
greg k-h
--
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
