Re: [PATCH] USB: remove dbg() usage in USB networking drivers

On Wed, Sep 19, 2012 at 10:53:32AM -0700, Joe Perches wrote:

On Wed, 2012-09-19 at 18:13 +0100, Greg Kroah-Hartman wrote:

quoted

The dbg() USB macro is so old, it predates me.  The USB networking drivers are
the last hold-out using this macro, and we want to get rid of it, so replace
the usage of it with the proper netdev_dbg() or dev_dbg() (depending on the
context) calls.

There's a missing newline.

Oops, good catch.

There are several dev_err(&intf-dev, ...) to dev_err(dev, ...)
conversions.

That is because there is now a local variable for this, so I fixed the
whole function up to use it at the same time.

Either those should be in a separate patch or you should mention
those changes in the commit message.

Ok, let me resend...

quoted

diff --git a/drivers/net/usb/catc.c b/drivers/net/usb/catc.c
index 26c5beb..6bf5672 100644
--- a/drivers/net/usb/catc.c
+++ b/drivers/net/usb/catc.c

@@ -236,7 +236,8 @@ static void catc_rx_done(struct urb *urb)
 	}
 
 	if (status) {
-		dbg("rx_done, status %d, length %d", status, urb->actual_length);
+		dev_dbg(&urb->dev->dev, "rx_done, status %d, length %d",
+			status, urb->actual_length);

newline

quoted

@@ -774,7 +783,7 @@ static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id
 
 	if (usb_set_interface(usbdev,
 			intf->altsetting->desc.bInterfaceNumber, 1)) {
-                dev_err(&intf->dev, "Can't set altsetting 1.\n");
+		dev_err(dev, "Can't set altsetting 1.\n");

?  Odd conversion.

We now have the local variable, so use it for all dev_* calls.

I've been doing that with the 30+ other dbg() cleanup patches in the usb
tree recently.

quoted

diff --git a/drivers/net/usb/kaweth.c b/drivers/net/usb/kaweth.c

[]

quoted

@@ -787,7 +787,8 @@ static void kaweth_usb_transmit_complete(struct urb *urb)
 
 	if (unlikely(status != 0))
 		if (status != -ENOENT)
-			dbg("%s: TX status %d.", kaweth->net->name, status);
+			dev_dbg(&urb->dev->dev, "%s: TX status %d.\n",
+				kaweth->net->name, status);

probably
			netdev_dbg(kaweth->net, "TX status %d\n", status);

[] 

It's the status of the urb that we want here, it is what failed, not the
netdev.  That's the way the rest of the kernel handles urb status debug
messages, so I'm trying to be consistant here.

quoted

@@ -1003,36 +1005,37 @@ static int kaweth_probe(
 		const struct usb_device_id *id      /* from id_table */
 	)
 {
-	struct usb_device *dev = interface_to_usbdev(intf);
+	struct device *dev = &intf->dev;
+	struct usb_device *udev = interface_to_usbdev(intf);

Miscellaneous renaming.

Consistant renaming :)

quoted

diff --git a/drivers/net/usb/net1080.c b/drivers/net/usb/net1080.c
index 28c4d51..aad7abe 100644
--- a/drivers/net/usb/net1080.c
+++ b/drivers/net/usb/net1080.c

@@ -156,11 +156,11 @@ static void nc_dump_registers(struct usbnet *dev)
 	u16	*vp = kmalloc(sizeof (u16));
 
 	if (!vp) {
-		dbg("no memory?");
+		netdev_dbg(dev->net, "no memory?\n");

could delete altogether.

Yeah, will do.

Thanks for the review, let me respin this...

greg k-h