On Thu, Jan 24, 2013 at 04:40:47PM +0100, Frans Klaver wrote:
> Signed-off-by: Frans Klaver <[email protected]>

Very nice, but one question:

> +static int xsens_mt_attach(struct usb_serial *serial)

(note, the function can be moved up in the file to get rid of the
declaration earlier, shortening the code by a few lines.)

> +{
> +     if ((serial->num_bulk_in == 0) ||
> +         (serial->num_bulk_out == 0)) {
> +             dev_err(&serial->dev->dev,
> +                     "%s - missing endpoint - "
> +                     "bulk in: %d, bulk out: %d\n",

These two lines should be on the same line, don't split strings, it's ok
to go over 80 columns with them, and in fact, you will not even get
there with this string.

> +                     KBUILD_MODNAME,
> +                     serial->num_bulk_in,
> +                     serial->num_bulk_out);
> +             return -EINVAL;

My question is why have this function at all?  What happens if a user
ends up with a "broken" device like this?  How can that happen?  What
are they supposed to do now?

Or is this left over from debugging some other hardware?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to