Hi all,

I've been annoyed for a long time that we make users go off and rebuild
their kernels just to enable usb debug messages.  So finally, here's the
start of fixing that issue.

(note, the recent rework of pr_debug() and dev_dbg() patches that have
been posted to lkml will probably obsolete these changes, but I don't
know when they will make it into mainline.  When they do, I'll be glad
to move these changes back to dev_dbg() to work with the rest of the
kernel.)

It seems that other parts of the kernel becides drivers/usb/core/ depend
on CONFIG_USB_DEBUG so I'll not submit these patches until I also fix up
them as well (drivers/net/usb is one big offender...)

Comments?

thanks,

greg k-h
--

[ message continues ]

From: Greg Kroah-Hartman <gregkh@suse.de>

This adds the core function usb_dbg() and a new config option to enable
this.

It can only be disabled if CONFIG_EMBEDDED is set.

It allows the usb core debugging messages to be enabled/disabled on the
fly with the usbcore "debug" module parameter.

Note, other sections of the kernel rely on CONFIG_USB_DEBUG, they will
also need to be converted.

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/usb/core/Kconfig |   13 +++++++++++++
 drivers/usb/core/usb.c   |   12 ++++++++++--
 drivers/usb/core/usb.h   |   22 ++++++++++++++++++++++
 3 files changed, 45 insertions(+), 2 deletions(-)

--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -9,6 +9,19 @@ config USB_DEBUG
 	  of debug messages to the system log. Select this if you are having a
 	  problem with USB support and want to see more of what is going on.
 
+config USB_DEBUG_MESSAGES
+	default y
+	bool "USB debugging messages" if EMBEDDED
+	depends on USB
+	help
+	  Say Y here if you want the USB core & hub drivers to be able to
+	  produce debugging messages if needed.  The messages can be
+	  enabled or disabled with the usbcore.debug module parameter.
+	  This option just allows you to save a bit of space if you do not
+	  want them to even be built into the kernel.
+
+	  If you have any doubts about this, say Y here.
+
 config USB_ANNOUNCE_NEW_DEVICES
 	bool "USB announce new devices"
 	depends on USB
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -61,6 +61,14 @@ MODULE_PARM_DESC(autosuspend, "default a
 #define usb_autosuspend_delay		0
 #endif
 
+/* if debug is on or not */
+int usb_debug;
+
+/* USB debugging */
+#if defined(CONFIG_USB_DEBUG_MESSAGES)
+module_param_named(debug, usb_debug, bool, 0644);
+MODULE_PARM_DESC(debug, "USB debugging enabled or not");
+#endif
 
 /**
  * usb_ifnum_to_if - get the interface object with a given interface number
@@ -502,14 +510,14 @@ static struct usb_device ...
[ message continues ]

From: Greg Kroah-Hartman <gregkh@suse.de>

It's a plug-in replacement for dev_dbg() and it lets you turn it on or
off based on the usbcore debug parameter.

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/usb/core/config.c  |    6 -
 drivers/usb/core/devio.c   |    2 
 drivers/usb/core/driver.c  |   25 ++++---
 drivers/usb/core/hcd-pci.c |   24 +++---
 drivers/usb/core/hcd.c     |   34 +++++----
 drivers/usb/core/hub.c     |  160 ++++++++++++++++++++++-----------------------
 drivers/usb/core/message.c |   18 ++---
 drivers/usb/core/quirks.c  |    2 
 drivers/usb/core/urb.c     |    3 
 9 files changed, 142 insertions(+), 132 deletions(-)

--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -169,7 +169,7 @@ static int usb_parse_endpoint(struct dev
 	    USB_DT_INTERFACE, &n);
 	endpoint->extralen = i;
 	if (n > 0)
-		dev_dbg(ddev, "skipped %d descriptor%s after %s\n",
+		usb_dbg(ddev, "skipped %d descriptor%s after %s\n",
 		    n, plural(n), "endpoint");
 	return buffer - buffer0 + i;
 
@@ -248,7 +248,7 @@ static int usb_parse_interface(struct de
 	    USB_DT_INTERFACE, &n);
 	alt->extralen = i;
 	if (n > 0)
-		dev_dbg(ddev, "skipped %d descriptor%s after %s\n",
+		usb_dbg(ddev, "skipped %d descriptor%s after %s\n",
 		    n, plural(n), "interface");
 	buffer += i;
 	size -= i;
@@ -459,7 +459,7 @@ static int usb_parse_configuration(struc
 	    USB_DT_INTERFACE, &n);
 	config->extralen = i;
 	if (n > 0)
-		dev_dbg(ddev, "skipped %d descriptor%s after %s\n",
+		usb_dbg(ddev, "skipped %d descriptor%s after %s\n",
 		    n, plural(n), "configuration");
 	buffer += i;
 	size -= i;
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1462,7 +1462,7 @@ static int proc_ioctl(struct dev_state *
 	case USBDEVFS_DISCONNECT:
 		if (intf->dev.driver) {
 			driver = to_usb_driver(intf->dev.driver);
-			dev_dbg(&intf->dev, "disconnect by usbfs\n");
+			usb_dbg(&intf->dev, "disconnect by usbfs\n");
 ...
[ message continues ]

From: Greg Kroah-Hartman <gregkh@suse.de>

It's no longer needed, so remove it from the system.

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/usb/core/Kconfig  |    8 --------
 drivers/usb/core/Makefile |    4 ----
 2 files changed, 12 deletions(-)

--- a/drivers/usb/core/Kconfig
+++ b/drivers/usb/core/Kconfig
@@ -1,14 +1,6 @@
 #
 # USB Core configuration
 #
-config USB_DEBUG
-	bool "USB verbose debug messages"
-	depends on USB
-	help
-	  Say Y here if you want the USB core & hub drivers to produce a bunch
-	  of debug messages to the system log. Select this if you are having a
-	  problem with USB support and want to see more of what is going on.
-
 config USB_DEBUG_MESSAGES
 	default y
 	bool "USB debugging messages" if EMBEDDED
--- a/drivers/usb/core/Makefile
+++ b/drivers/usb/core/Makefile
@@ -15,7 +15,3 @@ ifeq ($(CONFIG_USB_DEVICEFS),y)
 endif
 
 obj-$(CONFIG_USB)	+= usbcore.o
-
-ifeq ($(CONFIG_USB_DEBUG),y)
-EXTRA_CFLAGS += -DDEBUG
-endif
--

[ message continues ]

--

[ message continues ]

Does it really generate better code?

I really dislike using unlikely except for fast paths.

thanks,

greg k-h
--

[ message continues ]

In many cases usbmon does the job, and it's intended to be "always-on",
as to allow building it even on embedded kernels. Sure, it will not print
nice values of quirks and such, but that's a trade-off. I sort of hoped
that the switcheable explicit messages would generally die off. Look
at ub, it only has messages which indicate error conditions.

I'm not against your patch as such, it's a net improvement.

-- Pete
--

[ message continues ]

And that's great, but for some problems we seem to keep needing to
enable debugging for either the core or the host controllers.  This
change should help with that.

thanks,

greg k-h
--

[ message continues ]

Will your patches also allow to turn off the usb debugging at some point, 
for example after system boot has been completed if the debugging was 
only needed to analyze a boot issue?

I noticed earlier this week that USB debugging will really flood the 
syslog and had to rebuild my -rc2 kernel without debugging and reboot to 
get rid of it. AFAICT with your patches only rebooting would be needed, 
but it would be even nicer if it also could be turned off (and on?) using 
a sysfs parameter.

Cheers,
FJP
--

[ message continues ]

no rebooting. Just rmmod and modprobe again with the parameter set to 0,
although a sysfs file would be nice, or maybe allowing the
/sys/module/*/parameters/* to be writable :-)

I think that should be done in driver core.

-- 
balbi
--

[ message continues ]

Hmm... how clever, nothing should be changed in driver core to make it
work, it already uses the same mode we set in the driver

It should work the way it's done :-)

-- 
balbi
--

[ message continues ]

Yes, you can do that right now with them, just change the value at
/sys/module/usbcore/paramaters/debug and debugging is disabled on the
fly.

thanks,

greg k-h
--

[ message continues ]