On Fri, Jan 25, 2013 at 08:50:46PM +0100, Frans Klaver wrote:
> On Fri, Jan 25, 2013 at 5:51 PM, Greg KH <[email protected]> wrote:
> > On Fri, Jan 25, 2013 at 05:05:44PM +0100, Frans Klaver wrote:
> >> 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 thought as much. There are some other drivers that filter out
> interfaces using attach, though.
> 
> 
> >> I'd remove the bNumEndpoints if I could.
> >
> > What do you mean by that?
> 
> checkpatch was warning me about camel casing.

checkpatch is stupid at times, thanks for ignoring it in this case :)

> >> +static int has_required_endpoints(const struct usb_host_interface 
> >> *interface)
> >> +{
> >> +     __u8 i;
> 
> <snip>
> 
> >> +}
> >
> > 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.
> 
> I wasn't aware of that. Feel free to remove the leading underscores,
> if that's acceptable.

It's not a big deal at all, I've left it alone, and applied it as-is.

thanks for the clean driver submission,

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