Skip to contentSkip to conversation

USB: remove dbg() usage in USB networking drivers

PATCH Superseded Greg Kroah-Hartman 3 participants 9 messages Sep 19, 2012 – Sep 21, 2012

Subsystems (4)
Networking Network drivers USB networking drivers USB RTL8150 driver
lore
Share & tools
lore

Greg Kroah-Hartman

author
#0 lore raw

PATCH 1/1 USB: remove dbg() usage in USB networking drivers

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

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.

Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/net/usb/asix_devices.c |   36 ++++++-----
 drivers/net/usb/catc.c         |   55 +++++++++-------
 drivers/net/usb/gl620a.c       |   10 ++-
 drivers/net/usb/kaweth.c       |  134 ++++++++++++++++++++---------------------
 drivers/net/usb/net1080.c      |   46 +++++++-------
 drivers/net/usb/rtl8150.c      |    6 -
 6 files changed, 152 insertions(+), 135 deletions(-)

Dave, if I can take this through my usb-next tree, I can finally get rid of the dbg() macro for good. Or you can take it, and I can wait to drop dbg() until after 3.7-rc1, which is also fine with me, which ever is easiest for you.

drivers/net/usb/asix_devices.c +20 -16
index 4fd48df..8d5fdf1 100644
@@ -221,7 +221,8 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
 	/* Get the MAC address */
 	ret = asix_read_cmd(dev, AX88172_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
 	if (ret < 0) {
-		dbg("read AX_CMD_READ_NODE_ID failed: %d", ret);
+		netdev_dbg(dev->net, "read AX_CMD_READ_NODE_ID failed: %d\n",
+			   ret);
 		goto out;
 	}
 	memcpy(dev->net->dev_addr, buf, ETH_ALEN);

@@ -303,7 +304,7 @@ static int ax88772_reset(struct usbnet *dev)
 
 	ret = asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, embd_phy, 0, 0, NULL);
 	if (ret < 0) {
-		dbg("Select PHY #1 failed: %d", ret);
+		netdev_dbg(dev->net, "Select PHY #1 failed: %d\n", ret);
 		goto out;
 	}
 

@@ -331,13 +332,13 @@ static int ax88772_reset(struct usbnet *dev)
 
 	msleep(150);
 	rx_ctl = asix_read_rx_ctl(dev);
-	dbg("RX_CTL is 0x%04x after software reset", rx_ctl);
+	netdev_dbg(dev->net, "RX_CTL is 0x%04x after software reset\n", rx_ctl);
 	ret = asix_write_rx_ctl(dev, 0x0000);
 	if (ret < 0)
 		goto out;
 
 	rx_ctl = asix_read_rx_ctl(dev);
-	dbg("RX_CTL is 0x%04x setting to 0x0000", rx_ctl);
+	netdev_dbg(dev->net, "RX_CTL is 0x%04x setting to 0x0000\n", rx_ctl);
 
 	ret = asix_sw_reset(dev, AX_SWRESET_PRL);
 	if (ret < 0)

@@ -364,7 +365,7 @@ static int ax88772_reset(struct usbnet *dev)
 				AX88772_IPG0_DEFAULT | AX88772_IPG1_DEFAULT,
 				AX88772_IPG2_DEFAULT, 0, NULL);
 	if (ret < 0) {
-		dbg("Write IPG,IPG1,IPG2 failed: %d", ret);
+		netdev_dbg(dev->net, "Write IPG,IPG1,IPG2 failed: %d\n", ret);
 		goto out;
 	}
 

@@ -381,10 +382,13 @@ static int ax88772_reset(struct usbnet *dev)
 		goto out;
 
 	rx_ctl = asix_read_rx_ctl(dev);
-	dbg("RX_CTL is 0x%04x after all initializations", rx_ctl);
+	netdev_dbg(dev->net, "RX_CTL is 0x%04x after all initializations\n",
+		   rx_ctl);
 
 	rx_ctl = asix_read_medium_status(dev);
-	dbg("Medium Status is 0x%04x after all initializations", rx_ctl);
+	netdev_dbg(dev->net,
+		   "Medium Status is 0x%04x after all initializations\n",
+		   rx_ctl);
 
 	return 0;
 

@@ -416,7 +420,7 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 	/* Get the MAC address */
 	ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
 	if (ret < 0) {
-		dbg("Failed to read MAC address: %d", ret);
+		netdev_dbg(dev->net, "Failed to read MAC address: %d\n", ret);
 		return ret;
 	}
 	memcpy(dev->net->dev_addr, buf, ETH_ALEN);

@@ -439,7 +443,7 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 	/* Reset the PHY to normal operation mode */
 	ret = asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, embd_phy, 0, 0, NULL);
 	if (ret < 0) {
-		dbg("Select PHY #1 failed: %d", ret);
+		netdev_dbg(dev->net, "Select PHY #1 failed: %d\n", ret);
 		return ret;
 	}
 

@@ -459,7 +463,7 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	/* Read PHYID register *AFTER* the PHY was reset properly */
 	phyid = asix_get_phyid(dev);
-	dbg("PHYID=0x%08x", phyid);
+	netdev_dbg(dev->net, "PHYID=0x%08x\n", phyid);
 
 	/* Asix framing packs multiple eth frames into a 2K usb bulk transfer */
 	if (dev->driver_info->flags & FLAG_FRAMING_AX) {

@@ -575,13 +579,13 @@ static int ax88178_reset(struct usbnet *dev)
 	u32 phyid;
 
 	asix_read_cmd(dev, AX_CMD_READ_GPIOS, 0, 0, 1, &status);
-	dbg("GPIO Status: 0x%04x", status);
+	netdev_dbg(dev->net, "GPIO Status: 0x%04x\n", status);
 
 	asix_write_cmd(dev, AX_CMD_WRITE_ENABLE, 0, 0, 0, NULL);
 	asix_read_cmd(dev, AX_CMD_READ_EEPROM, 0x0017, 0, 2, &eeprom);
 	asix_write_cmd(dev, AX_CMD_WRITE_DISABLE, 0, 0, 0, NULL);
 
-	dbg("EEPROM index 0x17 is 0x%04x", eeprom);
+	netdev_dbg(dev->net, "EEPROM index 0x17 is 0x%04x\n", eeprom);
 
 	if (eeprom == cpu_to_le16(0xffff)) {
 		data->phymode = PHY_MODE_MARVELL;

@@ -592,7 +596,7 @@ static int ax88178_reset(struct usbnet *dev)
 		data->ledmode = le16_to_cpu(eeprom) >> 8;
 		gpio0 = (le16_to_cpu(eeprom) & 0x80) ? 0 : 1;
 	}
-	dbg("GPIO0: %d, PhyMode: %d", gpio0, data->phymode);
+	netdev_dbg(dev->net, "GPIO0: %d, PhyMode: %d\n", gpio0, data->phymode);
 
 	/* Power up external GigaPHY through AX88178 GPIO pin */
 	asix_write_gpio(dev, AX_GPIO_RSE | AX_GPIO_GPO_1 | AX_GPIO_GPO1EN, 40);

@@ -601,14 +605,14 @@ static int ax88178_reset(struct usbnet *dev)
 		asix_write_gpio(dev, 0x001c, 300);
 		asix_write_gpio(dev, 0x003c, 30);
 	} else {
-		dbg("gpio phymode == 1 path");
+		netdev_dbg(dev->net, "gpio phymode == 1 path\n");
 		asix_write_gpio(dev, AX_GPIO_GPO1EN, 30);
 		asix_write_gpio(dev, AX_GPIO_GPO1EN | AX_GPIO_GPO_1, 30);
 	}
 
 	/* Read PHYID register *AFTER* powering up PHY */
 	phyid = asix_get_phyid(dev);
-	dbg("PHYID=0x%08x", phyid);
+	netdev_dbg(dev->net, "PHYID=0x%08x\n", phyid);
 
 	/* Set AX88178 to enable MII/GMII/RGMII interface for external PHY */
 	asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, 0, 0, 0, NULL);

@@ -770,7 +774,7 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
 	/* Get the MAC address */
 	ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
 	if (ret < 0) {
-		dbg("Failed to read MAC address: %d", ret);
+		netdev_dbg(dev->net, "Failed to read MAC address: %d\n", ret);
 		return ret;
 	}
 	memcpy(dev->net->dev_addr, buf, ETH_ALEN);
drivers/net/usb/catc.c +32 -23

178 lines not on this page · raw message

drivers/net/usb/gl620a.c +7 -3
index db3c802..a7e3f4e 100644
@@ -91,7 +91,9 @@ static int genelink_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	// get the packet count of the received skb
 	count = le32_to_cpu(header->packet_count);
 	if (count > GL_MAX_TRANSMIT_PACKETS) {
-		dbg("genelink: invalid received packet count %u", count);
+		netdev_dbg(dev->net,
+			   "genelink: invalid received packet count %u\n",
+			   count);
 		return 0;
 	}
 

@@ -107,7 +109,8 @@ static int genelink_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
 		// this may be a broken packet
 		if (size > GL_MAX_PACKET_LEN) {
-			dbg("genelink: invalid rx length %d", size);
+			netdev_dbg(dev->net, "genelink: invalid rx length %d\n",
+				   size);
 			return 0;
 		}
 

@@ -133,7 +136,8 @@ static int genelink_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	skb_pull(skb, 4);
 
 	if (skb->len > GL_MAX_PACKET_LEN) {
-		dbg("genelink: invalid rx length %d", skb->len);
+		netdev_dbg(dev->net, "genelink: invalid rx length %d\n",
+			   skb->len);
 		return 0;
 	}
 	return 1;
drivers/net/usb/kaweth.c +67 -67

372 lines not on this page · raw message

drivers/net/usb/net1080.c +23 -23

129 lines not on this page · raw message

drivers/net/usb/rtl8150.c +3 -3
index 0e2c92e..5f39a3b 100644
@@ -275,7 +275,7 @@ static int rtl8150_set_mac_address(struct net_device *netdev, void *p)
 		return -EBUSY;
 
 	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
-	dbg("%s: Setting MAC address to %pM\n", netdev->name, netdev->dev_addr);
+	netdev_dbg(netdev, "Setting MAC address to %pM\n", netdev->dev_addr);
 	/* Set the IDR registers. */
 	set_registers(dev, IDR, netdev->addr_len, netdev->dev_addr);
 #ifdef EEPROM_WRITE

@@ -503,12 +503,12 @@ static void intr_callback(struct urb *urb)
 	if ((d[INT_MSR] & MSR_LINK) == 0) {
 		if (netif_carrier_ok(dev->netdev)) {
 			netif_carrier_off(dev->netdev);
-			dbg("%s: LINK LOST\n", __func__);
+			netdev_dbg(dev->netdev, "%s: LINK LOST\n", __func__);
 		}
 	} else {
 		if (!netif_carrier_ok(dev->netdev)) {
 			netif_carrier_on(dev->netdev);
-			dbg("%s: LINK CAME BACK\n", __func__);
+			netdev_dbg(dev->netdev, "%s: LINK CAME BACK\n", __func__);
 		}
 	}

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#1 lore raw
Quoted 4 lines · Greg Kroah-Hartman

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.

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

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

drivers/net/usb/catc.c static void catc_rx_done(struct urb *urb) · line 240

Quoted 11 lines
drivers/net/usb/catc.c +2 -1
index 26c5beb..6bf5672 100644
@@ -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

drivers/net/usb/catc.c static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id · line 786

Quoted 6 lines
drivers/net/usb/catc.c +1 -1
@@ -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.

drivers/net/usb/kaweth.c

Quoted 1 line
drivers/net/usb/kaweth.c +0 -0

[]

drivers/net/usb/kaweth.c static void kaweth_usb_transmit_complete(struct urb *urb) · line 791

Quoted 7 lines
drivers/net/usb/kaweth.c +2 -1
@@ -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);

[]

drivers/net/usb/kaweth.c static int kaweth_probe( · line 1009

Quoted 7 lines
drivers/net/usb/kaweth.c +2 -1
@@ -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.

[]

drivers/net/usb/net1080.c static void nc_dump_registers(struct usbnet *dev) · line 159

Quoted 10 lines
drivers/net/usb/net1080.c +1 -1
index 28c4d51..aad7abe 100644
@@ -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.

author
#2 lore raw
Quoted 7 lines · Joe Perches
Quoted 4 lines · Greg Kroah-Hartman

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.

Quoted 2 lines

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.

Quoted 2 lines

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

Ok, let me resend...

Quoted 14 lines

drivers/net/usb/catc.c static void catc_rx_done(struct urb *urb) · line 240

Quoted 11 lines
drivers/net/usb/catc.c +2 -1
index 26c5beb..6bf5672 100644
@@ -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 8 lines

drivers/net/usb/catc.c static int catc_probe(struct usb_interface *intf, const struct usb_device_id *id · line 786

Quoted 6 lines
drivers/net/usb/catc.c +1 -1
@@ -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 11 lines

drivers/net/usb/kaweth.c

Quoted 1 line
drivers/net/usb/kaweth.c +0 -0

[]

drivers/net/usb/kaweth.c static void kaweth_usb_transmit_complete(struct urb *urb) · line 791

Quoted 7 lines
drivers/net/usb/kaweth.c +2 -1
@@ -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

Quoted 3 lines
			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 10 lines

drivers/net/usb/kaweth.c static int kaweth_probe( · line 1009

Quoted 8 lines
drivers/net/usb/kaweth.c +2 -1
@@ -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 7 lines
Quoted 7 lines
drivers/net/usb/net1080.c +0 -0
index 28c4d51..aad7abe 100644
@@ -156,11 +156,11 @@ static void nc_dump_registers(struct usbnet *dev)
 	u16	*vp = kmalloc(sizeof (u16));
Quoted 5 lines

drivers/net/usb/net1080.c static void nc_dump_registers(struct usbnet *dev) · line 159

Quoted 3 lines
drivers/net/usb/net1080.c +1 -1
 	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

author
#3 lore raw

PATCH v2 1/1 USB: remove dbg() usage in USB networking drivers

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

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.

Some places we end up using a local variable for the debug call, so also convert the other existing dev_* calls to use it as well, to save tiny amounts of code space.

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
v2: Addressed review comments from Joe Perches

Again, I am glad to take this in the usb-next tree if wanted, whatever is easiest.

 drivers/net/usb/asix_devices.c |  36 ++++++++++++++------------
 drivers/net/usb/catc.c         |  55 ++++++++++++++++++++++-----------------
 drivers/net/usb/gl620a.c       |  10 +++++---
 drivers/net/usb/kaweth.c       | 134 ++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------
 drivers/net/usb/net1080.c      |  48 +++++++++++++++++------------------
 drivers/net/usb/rtl8150.c      |   6 ++---
 6 files changed, 152 insertions(+), 137 deletions(-)
drivers/net/usb/asix_devices.c +20 -16
index 4fd48df..8d5fdf1 100644
@@ -221,7 +221,8 @@ static int ax88172_bind(struct usbnet *dev, struct usb_interface *intf)
 	/* Get the MAC address */
 	ret = asix_read_cmd(dev, AX88172_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
 	if (ret < 0) {
-		dbg("read AX_CMD_READ_NODE_ID failed: %d", ret);
+		netdev_dbg(dev->net, "read AX_CMD_READ_NODE_ID failed: %d\n",
+			   ret);
 		goto out;
 	}
 	memcpy(dev->net->dev_addr, buf, ETH_ALEN);

@@ -303,7 +304,7 @@ static int ax88772_reset(struct usbnet *dev)
 
 	ret = asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, embd_phy, 0, 0, NULL);
 	if (ret < 0) {
-		dbg("Select PHY #1 failed: %d", ret);
+		netdev_dbg(dev->net, "Select PHY #1 failed: %d\n", ret);
 		goto out;
 	}
 

@@ -331,13 +332,13 @@ static int ax88772_reset(struct usbnet *dev)
 
 	msleep(150);
 	rx_ctl = asix_read_rx_ctl(dev);
-	dbg("RX_CTL is 0x%04x after software reset", rx_ctl);
+	netdev_dbg(dev->net, "RX_CTL is 0x%04x after software reset\n", rx_ctl);
 	ret = asix_write_rx_ctl(dev, 0x0000);
 	if (ret < 0)
 		goto out;
 
 	rx_ctl = asix_read_rx_ctl(dev);
-	dbg("RX_CTL is 0x%04x setting to 0x0000", rx_ctl);
+	netdev_dbg(dev->net, "RX_CTL is 0x%04x setting to 0x0000\n", rx_ctl);
 
 	ret = asix_sw_reset(dev, AX_SWRESET_PRL);
 	if (ret < 0)

@@ -364,7 +365,7 @@ static int ax88772_reset(struct usbnet *dev)
 				AX88772_IPG0_DEFAULT | AX88772_IPG1_DEFAULT,
 				AX88772_IPG2_DEFAULT, 0, NULL);
 	if (ret < 0) {
-		dbg("Write IPG,IPG1,IPG2 failed: %d", ret);
+		netdev_dbg(dev->net, "Write IPG,IPG1,IPG2 failed: %d\n", ret);
 		goto out;
 	}
 

@@ -381,10 +382,13 @@ static int ax88772_reset(struct usbnet *dev)
 		goto out;
 
 	rx_ctl = asix_read_rx_ctl(dev);
-	dbg("RX_CTL is 0x%04x after all initializations", rx_ctl);
+	netdev_dbg(dev->net, "RX_CTL is 0x%04x after all initializations\n",
+		   rx_ctl);
 
 	rx_ctl = asix_read_medium_status(dev);
-	dbg("Medium Status is 0x%04x after all initializations", rx_ctl);
+	netdev_dbg(dev->net,
+		   "Medium Status is 0x%04x after all initializations\n",
+		   rx_ctl);
 
 	return 0;
 

@@ -416,7 +420,7 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 	/* Get the MAC address */
 	ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
 	if (ret < 0) {
-		dbg("Failed to read MAC address: %d", ret);
+		netdev_dbg(dev->net, "Failed to read MAC address: %d\n", ret);
 		return ret;
 	}
 	memcpy(dev->net->dev_addr, buf, ETH_ALEN);

@@ -439,7 +443,7 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 	/* Reset the PHY to normal operation mode */
 	ret = asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, embd_phy, 0, 0, NULL);
 	if (ret < 0) {
-		dbg("Select PHY #1 failed: %d", ret);
+		netdev_dbg(dev->net, "Select PHY #1 failed: %d\n", ret);
 		return ret;
 	}
 

@@ -459,7 +463,7 @@ static int ax88772_bind(struct usbnet *dev, struct usb_interface *intf)
 
 	/* Read PHYID register *AFTER* the PHY was reset properly */
 	phyid = asix_get_phyid(dev);
-	dbg("PHYID=0x%08x", phyid);
+	netdev_dbg(dev->net, "PHYID=0x%08x\n", phyid);
 
 	/* Asix framing packs multiple eth frames into a 2K usb bulk transfer */
 	if (dev->driver_info->flags & FLAG_FRAMING_AX) {

@@ -575,13 +579,13 @@ static int ax88178_reset(struct usbnet *dev)
 	u32 phyid;
 
 	asix_read_cmd(dev, AX_CMD_READ_GPIOS, 0, 0, 1, &status);
-	dbg("GPIO Status: 0x%04x", status);
+	netdev_dbg(dev->net, "GPIO Status: 0x%04x\n", status);
 
 	asix_write_cmd(dev, AX_CMD_WRITE_ENABLE, 0, 0, 0, NULL);
 	asix_read_cmd(dev, AX_CMD_READ_EEPROM, 0x0017, 0, 2, &eeprom);
 	asix_write_cmd(dev, AX_CMD_WRITE_DISABLE, 0, 0, 0, NULL);
 
-	dbg("EEPROM index 0x17 is 0x%04x", eeprom);
+	netdev_dbg(dev->net, "EEPROM index 0x17 is 0x%04x\n", eeprom);
 
 	if (eeprom == cpu_to_le16(0xffff)) {
 		data->phymode = PHY_MODE_MARVELL;

@@ -592,7 +596,7 @@ static int ax88178_reset(struct usbnet *dev)
 		data->ledmode = le16_to_cpu(eeprom) >> 8;
 		gpio0 = (le16_to_cpu(eeprom) & 0x80) ? 0 : 1;
 	}
-	dbg("GPIO0: %d, PhyMode: %d", gpio0, data->phymode);
+	netdev_dbg(dev->net, "GPIO0: %d, PhyMode: %d\n", gpio0, data->phymode);
 
 	/* Power up external GigaPHY through AX88178 GPIO pin */
 	asix_write_gpio(dev, AX_GPIO_RSE | AX_GPIO_GPO_1 | AX_GPIO_GPO1EN, 40);

@@ -601,14 +605,14 @@ static int ax88178_reset(struct usbnet *dev)
 		asix_write_gpio(dev, 0x001c, 300);
 		asix_write_gpio(dev, 0x003c, 30);
 	} else {
-		dbg("gpio phymode == 1 path");
+		netdev_dbg(dev->net, "gpio phymode == 1 path\n");
 		asix_write_gpio(dev, AX_GPIO_GPO1EN, 30);
 		asix_write_gpio(dev, AX_GPIO_GPO1EN | AX_GPIO_GPO_1, 30);
 	}
 
 	/* Read PHYID register *AFTER* powering up PHY */
 	phyid = asix_get_phyid(dev);
-	dbg("PHYID=0x%08x", phyid);
+	netdev_dbg(dev->net, "PHYID=0x%08x\n", phyid);
 
 	/* Set AX88178 to enable MII/GMII/RGMII interface for external PHY */
 	asix_write_cmd(dev, AX_CMD_SW_PHY_SELECT, 0, 0, 0, NULL);

@@ -770,7 +774,7 @@ static int ax88178_bind(struct usbnet *dev, struct usb_interface *intf)
 	/* Get the MAC address */
 	ret = asix_read_cmd(dev, AX_CMD_READ_NODE_ID, 0, 0, ETH_ALEN, buf);
 	if (ret < 0) {
-		dbg("Failed to read MAC address: %d", ret);
+		netdev_dbg(dev->net, "Failed to read MAC address: %d\n", ret);
 		return ret;
 	}
 	memcpy(dev->net->dev_addr, buf, ETH_ALEN);
drivers/net/usb/catc.c +32 -23

178 lines not on this page · raw message

drivers/net/usb/gl620a.c +7 -3
index db3c802..a7e3f4e 100644
@@ -91,7 +91,9 @@ static int genelink_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	// get the packet count of the received skb
 	count = le32_to_cpu(header->packet_count);
 	if (count > GL_MAX_TRANSMIT_PACKETS) {
-		dbg("genelink: invalid received packet count %u", count);
+		netdev_dbg(dev->net,
+			   "genelink: invalid received packet count %u\n",
+			   count);
 		return 0;
 	}
 

@@ -107,7 +109,8 @@ static int genelink_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 
 		// this may be a broken packet
 		if (size > GL_MAX_PACKET_LEN) {
-			dbg("genelink: invalid rx length %d", size);
+			netdev_dbg(dev->net, "genelink: invalid rx length %d\n",
+				   size);
 			return 0;
 		}
 

@@ -133,7 +136,8 @@ static int genelink_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	skb_pull(skb, 4);
 
 	if (skb->len > GL_MAX_PACKET_LEN) {
-		dbg("genelink: invalid rx length %d", skb->len);
+		netdev_dbg(dev->net, "genelink: invalid rx length %d\n",
+			   skb->len);
 		return 0;
 	}
 	return 1;
drivers/net/usb/kaweth.c +67 -67

372 lines not on this page · raw message

drivers/net/usb/net1080.c +23 -25

130 lines not on this page · raw message

drivers/net/usb/rtl8150.c +3 -3
index 0e2c92e..5f39a3b 100644
@@ -275,7 +275,7 @@ static int rtl8150_set_mac_address(struct net_device *netdev, void *p)
 		return -EBUSY;
 
 	memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
-	dbg("%s: Setting MAC address to %pM\n", netdev->name, netdev->dev_addr);
+	netdev_dbg(netdev, "Setting MAC address to %pM\n", netdev->dev_addr);
 	/* Set the IDR registers. */
 	set_registers(dev, IDR, netdev->addr_len, netdev->dev_addr);
 #ifdef EEPROM_WRITE

@@ -503,12 +503,12 @@ static void intr_callback(struct urb *urb)
 	if ((d[INT_MSR] & MSR_LINK) == 0) {
 		if (netif_carrier_ok(dev->netdev)) {
 			netif_carrier_off(dev->netdev);
-			dbg("%s: LINK LOST\n", __func__);
+			netdev_dbg(dev->netdev, "%s: LINK LOST\n", __func__);
 		}
 	} else {
 		if (!netif_carrier_ok(dev->netdev)) {
 			netif_carrier_on(dev->netdev);
-			dbg("%s: LINK CAME BACK\n", __func__);
+			netdev_dbg(dev->netdev, "%s: LINK CAME BACK\n", __func__);
 		}
 	}
#4 lore raw
Quoted 4 lines · Greg Kroah-Hartman

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.

OK, one more bit of trivia

drivers/net/usb/net1080.c

Quoted 1 line
drivers/net/usb/net1080.c +0 -0

[]

drivers/net/usb/net1080.c static int net1080_rx_fixup(struct usbnet *dev, struct sk_buff *skb) · line 425

Quoted 10 lines
drivers/net/usb/net1080.c +3 -2
@@ -422,8 +419,9 @@ static int net1080_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	if (!(skb->len & 0x01)) {
 #ifdef DEBUG
 		struct net_device	*net = dev->net;
-		dbg("rx framesize %d range %d..%d mtu %d", skb->len,
-			net->hard_header_len, dev->hard_mtu, net->mtu);
+		netdev_dbg(dev->net, "rx framesize %d range %d..%d mtu %d\n",
+			   skb->len, net->hard_header_len, dev->hard_mtu,
+			   net->mtu);
 #endif

maybe

		netdev_dbg(net, ...

or remove the odd #ifdef DEBUG surround used to guard the otherwise unused net variable and use:

		netdev_dbg(dev->net, "rx framesize %d range %d..%d mtu %d\n",
			   skb->len, dev->net->hard_header_len, dev->hard_mtu,
			   dev->net->mtu);
author
#5 lore raw
Quoted 22 lines · Joe Perches
Quoted 4 lines · Greg Kroah-Hartman

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.

OK, one more bit of trivia

drivers/net/usb/net1080.c

Quoted 1 line
drivers/net/usb/net1080.c +0 -0

[]

drivers/net/usb/net1080.c static int net1080_rx_fixup(struct usbnet *dev, struct sk_buff *skb) · line 425

Quoted 10 lines
drivers/net/usb/net1080.c +3 -2
@@ -422,8 +419,9 @@ static int net1080_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	if (!(skb->len & 0x01)) {
 #ifdef DEBUG
 		struct net_device	*net = dev->net;
-		dbg("rx framesize %d range %d..%d mtu %d", skb->len,
-			net->hard_header_len, dev->hard_mtu, net->mtu);
+		netdev_dbg(dev->net, "rx framesize %d range %d..%d mtu %d\n",
+			   skb->len, net->hard_header_len, dev->hard_mtu,
+			   net->mtu);
 #endif

maybe

Quoted 9 lines
		netdev_dbg(net, ...

or remove the odd #ifdef DEBUG surround used to guard the otherwise unused net variable and use:

		netdev_dbg(dev->net, "rx framesize %d range %d..%d mtu %d\n",
			   skb->len, dev->net->hard_header_len, dev->hard_mtu,
			   dev->net->mtu);

Yeah, that would be better.

Even better would be just to delete all of this debug crud from these drivers. Almost all of the messages are there from when the developer originally wrote the driver, trying to figure out what was going on.

Quoted 1 line

From what I have seen, in doing all of these cleanups, is that the need

for maybe a few debug lines that can be used if users have issues, but the majority are just useless.

But, as I'm not the author or maintainer of these drivers, I'll be nice and just leave them in, all I want to do is get rid of the old, foolish, macros for debugging and use the proper dynamic debug code that works so much better.

So I'll leave this change alone, and if someone wants to do the cleanup better, the 3 liner above is fine with me to add later.

thanks,

greg k-h

maintainer
#6 lore raw
In reply to Greg Kroah-Hartman

From: Greg Kroah-Hartman <[email protected]>
Date: Wed, 19 Sep 2012 20:46:14 +0100

Quoted 11 lines

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

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.

Some places we end up using a local variable for the debug call, so also convert the other existing dev_* calls to use it as well, to save tiny amounts of code space.

Quoted 6 lines
Signed-off-by Greg Kroah-Hartman <[email protected]>

---
v2: Addressed review comments from Joe Perches

Again, I am glad to take this in the usb-next tree if wanted, whatever is easiest.

I'll take this into net-next, applied, thanks Greg.

#7 lore raw

PATCH 1/1 net1080: Neaten netdev_dbg use

Remove unnecessary temporary variable and #ifdef DEBUG block.

Signed-off-by Joe Perches <[email protected]>
---
 drivers/net/usb/net1080.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)
drivers/net/usb/net1080.c +2 -5
index 29e06e1..c062a3e 100644
@@ -417,12 +417,9 @@ static int net1080_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 	u16			hdr_len, packet_len;
 
 	if (!(skb->len & 0x01)) {
-#ifdef DEBUG
-		struct net_device	*net = dev->net;
 		netdev_dbg(dev->net, "rx framesize %d range %d..%d mtu %d\n",
-			   skb->len, net->hard_header_len, dev->hard_mtu,
-			   net->mtu);
-#endif
+			   skb->len, dev->net->hard_header_len, dev->hard_mtu,
+			   dev->net->mtu);
 		dev->net->stats.rx_frame_errors++;
 		nc_ensure_sync(dev);
 		return 0;
maintainer
#8 lore raw

Re: [PATCH net-next] net1080: Neaten netdev_dbg use

From: Joe Perches <[email protected]>
Date: Thu, 20 Sep 2012 17:10:23 -0700

Quoted 3 lines

Remove unnecessary temporary variable and #ifdef DEBUG block.

Signed-off-by Joe Perches <[email protected]>

Applied, thanks Joe.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html