linux-kernel - Re: [PATCHv5 1/4] usb: Provide usb_speed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110826183450.GA32142@kroah.com>
Date:	Fri, 26 Aug 2011 11:34:50 -0700
From:	Greg KH <greg@...ah.com>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Michal Nazarewicz <mnazarewicz@...gle.com>,
	Felipe Balbi <balbi@...com>,
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
	Yang Rui Rui <ruirui.r.yang@...to.com>,
	Dave Young <hidave.darkstar@...il.com>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv5 1/4] usb: Provide usb_speed_string() function

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/