On Thu, Jan 24, 2013 at 04:40:47PM +0100, Frans Klaver wrote: > Signed-off-by: Frans Klaver <frans.klaver@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html