Re: [PATCHv5 1/4] usb: Provide usb_speed_string() function

[Greg KH]

On Fri, Aug 26, 2011 at 02:26:43PM -0400, Alan Stern wrote:
> On Fri, 26 Aug 2011, Greg KH wrote:
> 
> > > --- a/drivers/usb/Makefile
> > > +++ b/drivers/usb/Makefile
> > > @@ -53,3 +53,5 @@ obj-$(CONFIG_USB_MUSB_HDRC)	+= musb/
> > >  obj-$(CONFIG_USB_RENESAS_USBHS)	+= renesas_usbhs/
> > >  obj-$(CONFIG_USB_OTG_UTILS)	+= otg/
> > >  obj-$(CONFIG_USB_GADGET)	+= gadget/
> > > +
> > > +obj-$(CONFIG_USB_SUPPORT)	+= common.o
> > 
> > You just built this into the kernel, which while ok for some things,
> > might not be what some people want.
> > 
> > Please make this into a separate module if people are building the usb
> > code as modules, usb_common.ko perhaps?
> 
> > > +#ifdef __KERNEL__
> > > +
> > > +/**
> > > + * usb_speed_string() - Returns human readable-name of the speed.
> > > + * @speed: The speed to return human-readable name for.  If it's not
> > > + *   any of the speeds defined in usb_device_speed enum, string for
> > > + *   USB_SPEED_UNKNOWN will be returned.
> > > + */
> > > +extern const char *usb_speed_string(enum usb_device_speed speed);
> > > +
> > > +#endif
> > 
> > No, this should be in include/linux/usb.h, not ch9.h.
> 
> There's a reason for both of these things.  The usb_speed_string 
> routine, like the other stuff in ch9.h, is meant to be used with both 
> the host-side and device-side USB stacks.

Ok, the __KERNEL__ stuff threw me off, those should not be in the patch
as they are not needed, so it's ok to keep it here, sorry.

> This makes deciding where to put it kind of difficult.  The two stacks 
> are pretty much independent; one might be built into the kernel while 
> the other is built as modules.  The easiest solution that will always 
> work is to put common.c into the main kernel.

A module, if the USB Host core is a module, and a module if the USB
gadge core is a module, would be best if at all possible.  I'm a bit
leery of adding stuff to be built into the kernel no matter how the user
decided to configure USB.

Is this possible with the Kbuild system somehow?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/