Skip to contentSkip to conversation

USB debugging rework

RFC In review Greg KH 5 participants 12 messages Aug 9, 2008 – Aug 10, 2008 USB

lore
Share & tools
lore

Greg KH

author
#0 lore raw

cover USB debugging rework

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

author
#1 lore raw

PATCH 1/3 USB: add CONFIG_USB_DEBUG_MESSAGES and usb_dbg()

From: Greg Kroah-Hartman <[email protected]>

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 <[email protected]>
---
 drivers/usb/core/Kconfig |   13 +++++++++++++
 drivers/usb/core/usb.c   |   12 ++++++++++--
 drivers/usb/core/usb.h   |   22 ++++++++++++++++++++++
 3 files changed, 45 insertions(+), 2 deletions(-)
drivers/usb/core/Kconfig +13 -0
@@ -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
drivers/usb/core/usb.c +10 -2
@@ -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 *match_device(s
 	struct usb_device *ret_dev = NULL;
 	int child;
 
-	dev_dbg(&dev->dev, "check for vendor %04x, product %04x ...\n",
+	usb_dbg(&dev->dev, "check for vendor %04x, product %04x ...\n",
 	    le16_to_cpu(dev->descriptor.idVendor),
 	    le16_to_cpu(dev->descriptor.idProduct));
 
 	/* see if this device matches */
 	if ((vendor_id == le16_to_cpu(dev->descriptor.idVendor)) &&
 	    (product_id == le16_to_cpu(dev->descriptor.idProduct))) {
-		dev_dbg(&dev->dev, "matched this device!\n");
+		usb_dbg(&dev->dev, "matched this device!\n");
 		ret_dev = usb_get_dev(dev);
 		goto exit;
 	}
drivers/usb/core/usb.h +22 -0
@@ -132,6 +132,28 @@ static inline int is_active(const struct
 /* for labeling diagnostics */
 extern const char *usbcore_name;
 
+/* USB debugging */
+/* if debug is on or not for the USB core */
+extern int usb_debug;
+
+#if defined(CONFIG_USB_DEBUG_MESSAGES)
+/* macro for debugging */
+#define usb_dbg(dev, format, arg...)					\
+	({								\
+		if (usb_debug)						\
+			dev_printk(KERN_DEBUG , dev , format , ## arg); \
+		0;							\
+	})
+#else
+#define usb_dbg(dev, format, arg...)					\
+	({								\
+		if (0)							\
+			dev_printk(KERN_DEBUG , dev , format , ## arg);	\
+		0;							\
+	})
+#endif
+
+
 /* sysfs stuff */
 extern struct attribute_group *usb_device_groups[];
 extern struct attribute_group *usb_interface_groups[];
author
#2 lore raw

PATCH 2/3 USB: convert the USB core code to use usb_dbg()

From: Greg Kroah-Hartman <[email protected]>

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 <[email protected]>
---
 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(-)
drivers/usb/core/config.c +3 -3
@@ -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;
drivers/usb/core/devio.c +1 -1
@@ -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");
 			usb_driver_release_interface(driver, intf);
 		} else
 			retval = -ENODATA;
drivers/usb/core/driver.c +15 -10
@@ -157,7 +157,7 @@ static int usb_probe_device(struct devic
 	struct usb_device *udev;
 	int error = -ENODEV;
 
-	dev_dbg(dev, "%s\n", __func__);
+	usb_dbg(dev, "%s\n", __func__);
 
 	if (!is_usb_device(dev))	/* Sanity check */
 		return error;

@@ -194,7 +194,7 @@ static int usb_probe_interface(struct de
 	const struct usb_device_id *id;
 	int error = -ENODEV;
 
-	dev_dbg(dev, "%s\n", __func__);
+	usb_dbg(dev, "%s\n", __func__);
 
 	if (is_usb_device(dev))		/* Sanity check */
 		return error;

@@ -212,7 +212,7 @@ static int usb_probe_interface(struct de
 	if (!id)
 		id = usb_match_dynamic_id(intf, driver);
 	if (id) {
-		dev_dbg(dev, "%s - got id\n", __func__);
+		usb_dbg(dev, "%s - got id\n", __func__);
 
 		error = usb_autoresume_device(udev);
 		if (error)

@@ -588,8 +588,9 @@ static int usb_uevent(struct device *dev
 {
 	struct usb_device *usb_dev;
 
-	/* driver is often null here; dev_dbg() would oops */
-	pr_debug("usb %s: uevent\n", dev_name(dev));
+	/* driver is often null here; usb_dbg() would oops */
+	if (usb_debug)
+		printk(KERN_DEBUG "usb %s: uevent\n", dev_name(dev));
 
 	if (is_usb_device(dev))
 		usb_dev = to_usb_device(dev);

@@ -599,11 +600,15 @@ static int usb_uevent(struct device *dev
 	}
 
 	if (usb_dev->devnum < 0) {
-		pr_debug("usb %s: already deleted?\n", dev_name(dev));
+		if (usb_debug)
+			printk(KERN_DEBUG "usb %s: already deleted?\n",
+			       dev_name(dev));
 		return -ENODEV;
 	}
 	if (!usb_dev->bus) {
-		pr_debug("usb %s: bus removed?\n", dev_name(dev));
+		if (usb_debug)
+			printk(KERN_DEBUG "usb %s: bus removed?\n",
+			       dev_name(dev));
 		return -ENODEV;
 	}
 

@@ -785,7 +790,7 @@ void usb_forced_unbind_intf(struct usb_i
 {
 	struct usb_driver *driver = to_usb_driver(intf->dev.driver);
 
-	dev_dbg(&intf->dev, "forced unbind\n");
+	usb_dbg(&intf->dev, "forced unbind\n");
 	usb_driver_release_interface(driver, intf);
 
 	/* Mark the interface for later rebinding */

@@ -809,7 +814,7 @@ void usb_rebind_intf(struct usb_interfac
 		struct usb_driver *driver =
 				to_usb_driver(intf->dev.driver);
 
-		dev_dbg(&intf->dev, "forced unbind\n");
+		usb_dbg(&intf->dev, "forced unbind\n");
 		usb_driver_release_interface(driver, intf);
 	}
 

@@ -1044,7 +1049,7 @@ static int autosuspend_check(struct usb_
 				return -EBUSY;
 			if (intf->needs_remote_wakeup &&
 					!udev->do_remote_wakeup) {
-				dev_dbg(&udev->dev, "remote wakeup needed "
+				usb_dbg(&udev->dev, "remote wakeup needed "
 						"for autosuspend\n");
 				return -EOPNOTSUPP;
 			}
drivers/usb/core/hcd.c +18 -16
@@ -466,7 +466,7 @@ static int rh_call_control (struct usb_h
 		break;
 	case DeviceOutRequest | USB_REQ_SET_ADDRESS:
 		// wValue == urb->dev->devaddr
-		dev_dbg (hcd->self.controller, "root hub device address %d\n",
+		usb_dbg (hcd->self.controller, "root hub device address %d\n",
 			wValue);
 		break;
 

@@ -482,7 +482,7 @@ static int rh_call_control (struct usb_h
 			/* FALLTHROUGH */
 	case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
 	case EndpointOutRequest | USB_REQ_SET_FEATURE:
-		dev_dbg (hcd->self.controller, "no endpoint features yet\n");
+		usb_dbg (hcd->self.controller, "no endpoint features yet\n");
 		break;
 
 	/* CLASS REQUESTS (and errors) */

@@ -510,7 +510,7 @@ error:
 	if (status) {
 		len = 0;
 		if (status != -EPIPE) {
-			dev_dbg (hcd->self.controller,
+			usb_dbg (hcd->self.controller,
 				"CTRL: TypeReq=0x%x val=0x%x "
 				"idx=0x%x len=%d ==> %d\n",
 				typeReq, wValue, wIndex,

@@ -626,7 +626,7 @@ static int rh_queue_status (struct usb_h
 
 	spin_lock_irqsave (&hcd_root_hub_lock, flags);
 	if (hcd->status_urb || urb->transfer_buffer_length < len) {
-		dev_dbg (hcd->self.controller, "not queuing rh status urb\n");
+		usb_dbg (hcd->self.controller, "not queuing rh status urb\n");
 		retval = -EINVAL;
 		goto done;
 	}

@@ -898,7 +898,7 @@ static int register_root_hub(struct usb_
 	retval = usb_get_device_descriptor(usb_dev, USB_DT_DEVICE_SIZE);
 	if (retval != sizeof usb_dev->descriptor) {
 		mutex_unlock(&usb_bus_list_lock);
-		dev_dbg (parent_dev, "can't read %s device descriptor %d\n",
+		usb_dbg (parent_dev, "can't read %s device descriptor %d\n",
 				dev_name(&usb_dev->dev), retval);
 		return (retval < 0) ? retval : -EMSGSIZE;
 	}

@@ -975,7 +975,9 @@ long usb_calc_bus_time (int speed, int i
 			tmp = HS_NSECS (bytecount);
 		return tmp;
 	default:
-		pr_debug ("%s: bogus device speed!\n", usbcore_name);
+		if (usb_debug)
+			printk(KERN_DEBUG "%s: bogus device speed!\n",
+			       usbcore_name);
 		return -1;
 	}
 }

@@ -1389,7 +1391,7 @@ int usb_hcd_unlink_urb (struct urb *urb,
 	if (retval == 0)
 		retval = -EINPROGRESS;
 	else if (retval != -EIDRM && retval != -EBUSY)
-		dev_dbg(&urb->dev->dev, "hcd_unlink_urb %p fail %d\n",
+		usb_dbg(&urb->dev->dev, "hcd_unlink_urb %p fail %d\n",
 				urb, retval);
 	return retval;
 }

@@ -1468,7 +1470,7 @@ rescan:
 
 		/* kick hcd */
 		unlink1(hcd, urb, -ESHUTDOWN);
-		dev_dbg (hcd->self.controller,
+		usb_dbg (hcd->self.controller,
 			"shutdown urb %p ep%d%s%s\n",
 			urb, usb_endpoint_num(&ep->desc),
 			is_in ? "in" : "out",

@@ -1555,7 +1557,7 @@ int hcd_bus_suspend(struct usb_device *r
 	int		status;
 	int		old_state = hcd->state;
 
-	dev_dbg(&rhdev->dev, "bus %s%s\n",
+	usb_dbg(&rhdev->dev, "bus %s%s\n",
 			rhdev->auto_pm ? "auto-" : "", "suspend");
 	if (!hcd->driver->bus_suspend) {
 		status = -ENOENT;

@@ -1568,7 +1570,7 @@ int hcd_bus_suspend(struct usb_device *r
 		hcd->state = HC_STATE_SUSPENDED;
 	} else {
 		hcd->state = old_state;
-		dev_dbg(&rhdev->dev, "bus %s fail, err %d\n",
+		usb_dbg(&rhdev->dev, "bus %s fail, err %d\n",
 				"suspend", status);
 	}
 	return status;

@@ -1580,7 +1582,7 @@ int hcd_bus_resume(struct usb_device *rh
 	int		status;
 	int		old_state = hcd->state;
 
-	dev_dbg(&rhdev->dev, "usb %s%s\n",
+	usb_dbg(&rhdev->dev, "usb %s%s\n",
 			rhdev->auto_pm ? "auto-" : "", "resume");
 	if (!hcd->driver->bus_resume)
 		return -ENOENT;

@@ -1598,7 +1600,7 @@ int hcd_bus_resume(struct usb_device *rh
 		hcd->state = HC_STATE_RUNNING;
 	} else {
 		hcd->state = old_state;
-		dev_dbg(&rhdev->dev, "bus %s fail, err %d\n",
+		usb_dbg(&rhdev->dev, "bus %s fail, err %d\n",
 				"resume", status);
 		if (status != -ESHUTDOWN)
 			usb_hc_died(hcd);

@@ -1769,7 +1771,7 @@ struct usb_hcd *usb_create_hcd (const st
 
 	hcd = kzalloc(sizeof(*hcd) + driver->hcd_priv_size, GFP_KERNEL);
 	if (!hcd) {
-		dev_dbg (dev, "hcd alloc failed\n");
+		usb_dbg (dev, "hcd alloc failed\n");
 		return NULL;
 	}
 	dev_set_drvdata(dev, hcd);

@@ -1842,7 +1844,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
 	 * starts talking to them.  (Note, bus id is assigned early too.)
 	 */
 	if ((retval = hcd_buffer_create(hcd)) != 0) {
-		dev_dbg(hcd->self.controller, "pool alloc failed\n");
+		usb_dbg(hcd->self.controller, "pool alloc failed\n");
 		return retval;
 	}
 

@@ -1875,7 +1877,7 @@ int usb_add_hcd(struct usb_hcd *hcd,
 	/* NOTE: root hub and controller capabilities may not be the same */
 	if (device_can_wakeup(hcd->self.controller)
 			&& device_can_wakeup(&hcd->self.root_hub->dev))
-		dev_dbg(hcd->self.controller, "supports USB remote wakeup\n");
+		usb_dbg(hcd->self.controller, "supports USB remote wakeup\n");
 
 	/* enable irqs just before we start the controller */
 	if (hcd->driver->irq) {

@@ -1964,7 +1966,7 @@ void usb_remove_hcd(struct usb_hcd *hcd)
 	if (HC_IS_RUNNING (hcd->state))
 		hcd->state = HC_STATE_QUIESCING;
 
-	dev_dbg(hcd->self.controller, "roothub graceful disconnect\n");
+	usb_dbg(hcd->self.controller, "roothub graceful disconnect\n");
 	spin_lock_irq (&hcd_root_hub_lock);
 	hcd->rh_registered = 0;
 	spin_unlock_irq (&hcd_root_hub_lock);
drivers/usb/core/hcd-pci.c +12 -12
@@ -94,13 +94,13 @@ int usb_hcd_pci_probe(struct pci_dev *de
 		hcd->rsrc_len = pci_resource_len(dev, 0);
 		if (!request_mem_region(hcd->rsrc_start, hcd->rsrc_len,
 				driver->description)) {
-			dev_dbg(&dev->dev, "controller already in use\n");
+			usb_dbg(&dev->dev, "controller already in use\n");
 			retval = -EBUSY;
 			goto err2;
 		}
 		hcd->regs = ioremap_nocache(hcd->rsrc_start, hcd->rsrc_len);
 		if (hcd->regs == NULL) {
-			dev_dbg(&dev->dev, "error mapping memory\n");
+			usb_dbg(&dev->dev, "error mapping memory\n");
 			retval = -EFAULT;
 			goto err3;
 		}

@@ -121,7 +121,7 @@ int usb_hcd_pci_probe(struct pci_dev *de
 				break;
 		}
 		if (region == PCI_ROM_RESOURCE) {
-			dev_dbg(&dev->dev, "no i/o regions available\n");
+			usb_dbg(&dev->dev, "no i/o regions available\n");
 			retval = -EBUSY;
 			goto err1;
 		}

@@ -256,12 +256,12 @@ int usb_hcd_pci_suspend(struct pci_dev *
 
 		if (message.event == PM_EVENT_FREEZE ||
 				message.event == PM_EVENT_PRETHAW) {
-			dev_dbg(hcd->self.controller, "--> no state change\n");
+			usb_dbg(hcd->self.controller, "--> no state change\n");
 			goto done;
 		}
 
 		if (!has_pci_pm) {
-			dev_dbg(hcd->self.controller, "--> PCI D0/legacy\n");
+			usb_dbg(hcd->self.controller, "--> PCI D0/legacy\n");
 			goto done;
 		}
 

@@ -277,7 +277,7 @@ int usb_hcd_pci_suspend(struct pci_dev *
 
 			wake = wake && device_may_wakeup(hcd->self.controller);
 
-			dev_dbg(hcd->self.controller, "--> PCI D3%s\n",
+			usb_dbg(hcd->self.controller, "--> PCI D3%s\n",
 					wake ? "/wakeup" : "");
 
 			/* Ignore these return values.  We rely on pci code to

@@ -287,13 +287,13 @@ int usb_hcd_pci_suspend(struct pci_dev *
 			(void) pci_enable_wake(dev, PCI_D3hot, wake);
 			(void) pci_enable_wake(dev, PCI_D3cold, wake);
 		} else {
-			dev_dbg(&dev->dev, "PCI D3 suspend fail, %d\n",
+			usb_dbg(&dev->dev, "PCI D3 suspend fail, %d\n",
 					retval);
 			(void) usb_hcd_pci_resume(dev);
 		}
 
 	} else if (hcd->state != HC_STATE_HALT) {
-		dev_dbg(hcd->self.controller, "hcd state %d; not suspended\n",
+		usb_dbg(hcd->self.controller, "hcd state %d; not suspended\n",
 			hcd->state);
 		WARN_ON(1);
 		retval = -EINVAL;

@@ -331,7 +331,7 @@ int usb_hcd_pci_resume(struct pci_dev *d
 
 	hcd = pci_get_drvdata(dev);
 	if (hcd->state != HC_STATE_SUSPENDED) {
-		dev_dbg(hcd->self.controller,
+		usb_dbg(hcd->self.controller,
 				"can't resume, not suspended!\n");
 		return 0;
 	}

@@ -364,7 +364,7 @@ int usb_hcd_pci_resume(struct pci_dev *d
 			/* Clean case:  power to USB and to HC registers was
 			 * maintained; remote wakeup is easy.
 			 */
-			dev_dbg(hcd->self.controller, "resume from PCI D%d\n",
+			usb_dbg(hcd->self.controller, "resume from PCI D%d\n",
 					pmcr);
 		} else {
 			/* Clean:  HC lost Vcc power, D0 uninitialized

@@ -376,7 +376,7 @@ int usb_hcd_pci_resume(struct pci_dev *d
 			 *   + after BIOS init
 			 *   + after Linux init (HCD statically linked)
 			 */
-			dev_dbg(hcd->self.controller,
+			usb_dbg(hcd->self.controller,
 				"PCI D0, from previous PCI D%d\n",
 				dev->current_state);
 		}

@@ -386,7 +386,7 @@ int usb_hcd_pci_resume(struct pci_dev *d
 		(void) pci_enable_wake(dev, PCI_D3cold, 0);
 	} else {
 		/* Same basic cases: clean (powered/not), dirty */
-		dev_dbg(hcd->self.controller, "PCI legacy resume\n");
+		usb_dbg(hcd->self.controller, "PCI legacy resume\n");
 	}
 
 	/* NOTE:  the PCI API itself is asymmetric here.  We don't need to
drivers/usb/core/hub.c +81 -79

584 lines not on this page · raw message

drivers/usb/core/message.c +9 -9
@@ -56,7 +56,7 @@ static int usb_start_wait_urb(struct urb
 		usb_kill_urb(urb);
 		retval = (ctx.status == -ENOENT ? -ETIMEDOUT : ctx.status);
 
-		dev_dbg(&urb->dev->dev,
+		usb_dbg(&urb->dev->dev,
 			"%s timed out on ep%d%s len=%d/%d\n",
 			current->comm,
 			usb_endpoint_num(&urb->ep->desc),

@@ -551,7 +551,7 @@ void usb_sg_wait(struct usb_sg_request *
 		default:
 			io->urbs[i]->dev = NULL;
 			io->urbs[i]->status = retval;
-			dev_dbg(&io->dev->dev, "%s, submit --> %d\n",
+			usb_dbg(&io->dev->dev, "%s, submit --> %d\n",
 				__func__, retval);
 			usb_sg_cancel(io);
 		}

@@ -806,7 +806,7 @@ int usb_string(struct usb_device *dev, i
 			dev->have_langid = 1;
 			dev->string_langid = tbuf[2] | (tbuf[3] << 8);
 			/* always use the first langid listed */
-			dev_dbg(&dev->dev, "default language 0x%04x\n",
+			usb_dbg(&dev->dev, "default language 0x%04x\n",
 				dev->string_langid);
 		}
 	}

@@ -828,7 +828,7 @@ int usb_string(struct usb_device *dev, i
 	err = idx;
 
 	if (tbuf[1] != USB_DT_STRING)
-		dev_dbg(&dev->dev,
+		usb_dbg(&dev->dev,
 			"wrong descriptor type %02x for string %d (\"%s\")\n",
 			tbuf[1], index, buf);
 

@@ -1070,7 +1070,7 @@ void usb_disable_device(struct usb_devic
 {
 	int i;
 
-	dev_dbg(&dev->dev, "%s nuking %s URBs\n", __func__,
+	usb_dbg(&dev->dev, "%s nuking %s URBs\n", __func__,
 		skip_ep0 ? "non-ep0" : "all");
 	for (i = skip_ep0; i < 16; ++i) {
 		usb_disable_endpoint(dev, i);

@@ -1089,7 +1089,7 @@ void usb_disable_device(struct usb_devic
 			interface = dev->actconfig->interface[i];
 			if (!device_is_registered(&interface->dev))
 				continue;
-			dev_dbg(&dev->dev, "unregistering interface %s\n",
+			usb_dbg(&dev->dev, "unregistering interface %s\n",
 				dev_name(&interface->dev));
 			usb_remove_sysfs_intf_files(interface);
 			device_del(&interface->dev);

@@ -1197,7 +1197,7 @@ int usb_set_interface(struct usb_device 
 
 	iface = usb_ifnum_to_if(dev, interface);
 	if (!iface) {
-		dev_dbg(&dev->dev, "selecting invalid interface %d\n",
+		usb_dbg(&dev->dev, "selecting invalid interface %d\n",
 			interface);
 		return -EINVAL;
 	}

@@ -1219,7 +1219,7 @@ int usb_set_interface(struct usb_device 
 	 * request if the interface only has one alternate setting.
 	 */
 	if (ret == -EPIPE && iface->num_altsetting == 1) {
-		dev_dbg(&dev->dev,
+		usb_dbg(&dev->dev,
 			"manual set_interface for iface %d, alt %d\n",
 			interface, alternate);
 		manual = 1;

@@ -1629,7 +1629,7 @@ free_interfaces:
 	for (i = 0; i < nintf; ++i) {
 		struct usb_interface *intf = cp->interface[i];
 
-		dev_dbg(&dev->dev,
+		usb_dbg(&dev->dev,
 			"adding %s (config #%d, interface %d)\n",
 			dev_name(&intf->dev), configuration,
 			intf->cur_altsetting->desc.bInterfaceNumber);
drivers/usb/core/quirks.c +1 -1
@@ -96,7 +96,7 @@ void usb_detect_quirks(struct usb_device
 	if (id)
 		udev->quirks = (u32)(id->driver_info);
 	if (udev->quirks)
-		dev_dbg(&udev->dev, "USB quirks for this device: %x\n",
+		usb_dbg(&udev->dev, "USB quirks for this device: %x\n",
 				udev->quirks);
 
 	/* By default, disable autosuspend for all non-hubs */
drivers/usb/core/urb.c +2 -1
@@ -6,6 +6,7 @@
 #include <linux/log2.h>
 #include <linux/usb.h>
 #include <linux/wait.h>
+#include "usb.h"
 #include "hcd.h"
 
 #define to_urb(d) container_of(d, struct urb, kref)

@@ -340,7 +341,7 @@ int usb_submit_urb(struct urb *urb, gfp_
 
 	max = le16_to_cpu(ep->desc.wMaxPacketSize);
 	if (max <= 0) {
-		dev_dbg(&dev->dev,
+		usb_dbg(&dev->dev,
 			"bogus endpoint ep%d%s in %s (bad maxpacket %d)\n",
 			usb_endpoint_num(&ep->desc), is_out ? "out" : "in",
 			__func__, max);
author
#3 lore raw

PATCH 3/3 USB: remove CONFIG_USB_DEBUG

From: Greg Kroah-Hartman <[email protected]>

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

Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/usb/core/Kconfig  |    8 --------
 drivers/usb/core/Makefile |    4 ----
 2 files changed, 12 deletions(-)
drivers/usb/core/Kconfig +0 -8
@@ -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
drivers/usb/core/Makefile +0 -4
@@ -15,7 +15,3 @@ ifeq ($(CONFIG_USB_DEVICEFS),y)
 endif
 
 obj-$(CONFIG_USB)	+= usbcore.o
-
-ifeq ($(CONFIG_USB_DEBUG),y)
-EXTRA_CFLAGS += -DDEBUG
-endif
#4 lore raw

Re: [01/03] USB: add CONFIG_USB_DEBUG_MESSAGES and usb_dbg()

In reply to Greg KH
Quoted 5 lines · Greg KH
drivers/usb/core/usb.h +0 -0
@@ -132,6 +132,28 @@ static inline int is_active(const struct
 /* for labeling diagnostics */
 extern const char *usbcore_name;

drivers/usb/core/usb.h static inline int is_active(const struct · line 143

Quoted 10 lines
drivers/usb/core/usb.h +9 -0
 
+/* USB debugging */
+/* if debug is on or not for the USB core */
+extern int usb_debug;
+
+#if defined(CONFIG_USB_DEBUG_MESSAGES)
+/* macro for debugging */
+#define usb_dbg(dev, format, arg...)					\
+	({								\
+		if (usb_debug)						\

maybe unlikely here?

Quoted 16 lines
drivers/usb/core/usb.h +13 -0
+			dev_printk(KERN_DEBUG , dev , format , ## arg); \
+		0;							\
+	})
+#else
+#define usb_dbg(dev, format, arg...)					\
+	({								\
+		if (0)							\
+			dev_printk(KERN_DEBUG , dev , format , ## arg);	\
+		0;							\
+	})
+#endif
+
+
 /* sysfs stuff */
 extern struct attribute_group *usb_device_groups[];
 extern struct attribute_group *usb_interface_groups[];
author
#5 lore raw

Re: [01/03] USB: add CONFIG_USB_DEBUG_MESSAGES and usb_dbg()

Quoted 7 lines · Marcin Slusarz
Quoted 6 lines · Greg KH
drivers/usb/core/usb.h +0 -0
@@ -132,6 +132,28 @@ static inline int is_active(const struct
 /* for labeling diagnostics */
 extern const char *usbcore_name;
Quoted 11 lines

drivers/usb/core/usb.h static inline int is_active(const struct · line 143

Quoted 9 lines
drivers/usb/core/usb.h +9 -0
+/* USB debugging */
+/* if debug is on or not for the USB core */
+extern int usb_debug;
+
+#if defined(CONFIG_USB_DEBUG_MESSAGES)
+/* macro for debugging */
+#define usb_dbg(dev, format, arg...)					\
+	({								\
+		if (usb_debug)						\

maybe unlikely here?

Does it really generate better code?

I really dislike using unlikely except for fast paths.

thanks,

greg k-h

#6 lore raw
In reply to Greg KH
Quoted 3 lines · Greg KH

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.

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

author
#7 lore raw
Quoted 11 lines · Pete Zaitcev
Quoted 3 lines · Greg KH

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.

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.

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

#8 lore raw
In reply to Greg KH
Quoted 3 lines · Greg KH

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.

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

#9 lore raw
Quoted 9 lines · Frans Pop
Quoted 3 lines · Greg KH

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.

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?

Quoted 5 lines

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.

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.

Signature · 1 line
balbi
#10 lore raw
Quoted 5 lines · Felipe Balbi

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.

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 :-)

Signature · 1 line
balbi
author
#11 lore raw
In reply to Frans Pop
Quoted 6 lines · Frans Pop
Quoted 3 lines · Greg KH

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.

Will your patches also allow to turn off the usb debugging at some point,

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