Patchwork [PATCHv2,RFC] smsc95xx: enable dynamic autosuspend

login
register
mail settings
Submitter Joe Perches
Date Dec. 11, 2012, 4:27 p.m.
Message ID <1355243276.24219.3.camel@joe-AO722>
Download mbox | patch
Permalink /patch/205270/
State RFC
Delegated to: David Miller
Headers show

Comments

Joe Perches - Dec. 11, 2012, 4:27 p.m.
On Tue, 2012-12-11 at 15:26 +0000, Steve Glendinning wrote:
> This patch enables USB dynamic autosuspend for LAN9500A.  This
> saves very little power in itself, but it allows power saving
> in upstream hubs/hosts.

[]> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
[]
> +	ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val);
> +	if (ret < 0) {
> +		netdev_warn(dev->net, "Error reading RX_FIFO_INF");

Please remember terminating newlines.

If you are always going to warn bad reads or writes,
it'd be good to change smsc95xx_read_reg_nopm
and write to emit the message.

It saves ~3% of code, 1K
$ size drivers/net/usb/smsc95xx.o*
   text	   data	    bss	    dec	    hex	filename
  27064	   2724	   6072	  35860	   8c14	drivers/net/usb/smsc95xx.o.new
  27921	   2724	   6256	  36901	   9025	drivers/net/usb/smsc95xx.o.old

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/usb/smsc95xx.c |  126 +++++++++++++++-----------------------------
 1 files changed, 42 insertions(+), 84 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve Glendinning - Dec. 14, 2012, 1:38 p.m.
On 11 December 2012 16:27, Joe Perches <joe@perches.com> wrote:
> On Tue, 2012-12-11 at 15:26 +0000, Steve Glendinning wrote:
> []> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> []
>> +     ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val);
>> +     if (ret < 0) {
>> +             netdev_warn(dev->net, "Error reading RX_FIFO_INF");
>
> Please remember terminating newlines.

Oops, I've fixed this now in my tree.

> If you are always going to warn bad reads or writes,
> it'd be good to change smsc95xx_read_reg_nopm
> and write to emit the message.
>
> It saves ~3% of code, 1K
> $ size drivers/net/usb/smsc95xx.o*
>    text    data     bss     dec     hex filename
>   27064    2724    6072   35860    8c14 drivers/net/usb/smsc95xx.o.new
>   27921    2724    6256   36901    9025 drivers/net/usb/smsc95xx.o.old
>
> Signed-off-by: Joe Perches <joe@perches.com>

Thanks for this Joe, actually as the register access functions already
print a warning with the register number I don't think we need to log
this at the calling site at all.  I'll rip out all the warnings that
don't add any significant new information (i.e. *why* the call may
have failed, or what its failure means).

--
Steve Glendinning
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 9b73670..e86eca7 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -125,13 +125,28 @@  static int __must_check __smsc95xx_write_reg(struct usbnet *dev, u32 index,
 static int __must_check smsc95xx_read_reg_nopm(struct usbnet *dev, u32 index,
 					       u32 *data)
 {
-	return __smsc95xx_read_reg(dev, index, data, 1);
+	int rtn = __smsc95xx_read_reg(dev, index, data, 1);
+	if (rtn < 0)
+		netdev_warn(dev->net, "Error reading %#x:%s\n",
+			    index,
+			    index == PM_CTRL ? "PM_CTRL" :
+			    index == WUCSR ? "WUCSR" :
+			    "unknown");
+	return rtn;
 }
 
 static int __must_check smsc95xx_write_reg_nopm(struct usbnet *dev, u32 index,
 						u32 data)
 {
-	return __smsc95xx_write_reg(dev, index, data, 1);
+	int rtn = __smsc95xx_write_reg(dev, index, data, 1);
+	if (rtn < 0)
+		netdev_warn(dev->net, "Error writing %#x:%s\n",
+			    index,
+			    index == PM_CTRL ? "PM_CTRL" :
+			    index == WUCSR ? "WUCSR" :
+			    index == WUFF ? "WUFF" :
+			    "unknown");
+	return rtn;
 }
 
 static int __must_check smsc95xx_read_reg(struct usbnet *dev, u32 index,
@@ -1308,19 +1323,15 @@  static int smsc95xx_enter_suspend0(struct usbnet *dev)
 	int ret;
 
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Error reading PM_CTRL\n");
+	if (ret < 0)
 		return ret;
-	}
 
 	val &= (~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_));
 	val |= PM_CTL_SUS_MODE_0;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Error writing PM_CTRL\n");
+	if (ret < 0)
 		return ret;
-	}
 
 	/* clear wol status */
 	val &= ~PM_CTL_WUPS_;
@@ -1331,15 +1342,11 @@  static int smsc95xx_enter_suspend0(struct usbnet *dev)
 		val |= PM_CTL_WUPS_ED_;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Error writing PM_CTRL\n");
+	if (ret < 0)
 		return ret;
-	}
 
 	/* read back PM_CTRL */
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	if (ret < 0)
-		netdev_warn(dev->net, "Error reading PM_CTRL\n");
 
 	return ret;
 }
@@ -1371,27 +1378,21 @@  static int smsc95xx_enter_suspend1(struct usbnet *dev)
 
 	/* enter SUSPEND1 mode */
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Error reading PM_CTRL\n");
+	if (ret < 0)
 		return ret;
-	}
 
 	val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
 	val |= PM_CTL_SUS_MODE_1;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Error writing PM_CTRL\n");
+	if (ret < 0)
 		return ret;
-	}
 
 	/* clear wol status, enable energy detection */
 	val &= ~PM_CTL_WUPS_;
 	val |= (PM_CTL_WUPS_ED_ | PM_CTL_ED_EN_);
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	if (ret < 0)
-		netdev_warn(dev->net, "Error writing PM_CTRL\n");
 
 	return ret;
 }
@@ -1402,17 +1403,13 @@  static int smsc95xx_enter_suspend2(struct usbnet *dev)
 	int ret;
 
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Error reading PM_CTRL\n");
+	if (ret < 0)
 		return ret;
-	}
 
 	val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
 	val |= PM_CTL_SUS_MODE_2;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	if (ret < 0)
-		netdev_warn(dev->net, "Error writing PM_CTRL\n");
 
 	return ret;
 }
@@ -1442,32 +1439,24 @@  static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 
 		/* disable energy detect (link up) & wake up events */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error reading WUCSR\n");
+		if (ret < 0)
 			goto done;
-		}
 
 		val &= ~(WUCSR_MPEN_ | WUCSR_WAKE_EN_);
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error writing WUCSR\n");
+		if (ret < 0)
 			goto done;
-		}
 
 		ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error reading PM_CTRL\n");
+		if (ret < 0)
 			goto done;
-		}
 
 		val &= ~(PM_CTL_ED_EN_ | PM_CTL_WOL_EN_);
 
 		ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error writing PM_CTRL\n");
+		if (ret < 0)
 			goto done;
-		}
 
 		ret = smsc95xx_enter_suspend2(dev);
 		goto done;
@@ -1565,7 +1554,6 @@  static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 		for (i = 0; i < (wuff_filter_count * 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, filter_mask[i]);
 			if (ret < 0) {
-				netdev_warn(dev->net, "Error writing WUFF\n");
 				kfree(filter_mask);
 				goto done;
 			}
@@ -1574,67 +1562,51 @@  static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 
 		for (i = 0; i < (wuff_filter_count / 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, command[i]);
-			if (ret < 0) {
-				netdev_warn(dev->net, "Error writing WUFF\n");
+			if (ret < 0)
 				goto done;
-			}
 		}
 
 		for (i = 0; i < (wuff_filter_count / 4); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, offset[i]);
-			if (ret < 0) {
-				netdev_warn(dev->net, "Error writing WUFF\n");
+			if (ret < 0)
 				goto done;
-			}
 		}
 
 		for (i = 0; i < (wuff_filter_count / 2); i++) {
 			ret = smsc95xx_write_reg_nopm(dev, WUFF, crc[i]);
-			if (ret < 0) {
-				netdev_warn(dev->net, "Error writing WUFF\n");
+			if (ret < 0)
 				goto done;
-			}
 		}
 
 		/* clear any pending pattern match packet status */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error reading WUCSR\n");
+		if (ret < 0)
 			goto done;
-		}
 
 		val |= WUCSR_WUFR_;
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error writing WUCSR\n");
+		if (ret < 0)
 			goto done;
-		}
 	}
 
 	if (pdata->wolopts & WAKE_MAGIC) {
 		/* clear any pending magic packet status */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error reading WUCSR\n");
+		if (ret < 0)
 			goto done;
-		}
 
 		val |= WUCSR_MPR_;
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error writing WUCSR\n");
+		if (ret < 0)
 			goto done;
-		}
 	}
 
 	/* enable/disable wakeup sources */
 	ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Error reading WUCSR\n");
+	if (ret < 0)
 		goto done;
-	}
 
 	if (pdata->wolopts & (WAKE_BCAST | WAKE_MCAST | WAKE_ARP | WAKE_UCAST)) {
 		netdev_info(dev->net, "enabling pattern match wakeup\n");
@@ -1653,17 +1625,13 @@  static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 	}
 
 	ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Error writing WUCSR\n");
+	if (ret < 0)
 		goto done;
-	}
 
 	/* enable wol wakeup source */
 	ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Error reading PM_CTRL\n");
+	if (ret < 0)
 		goto done;
-	}
 
 	val |= PM_CTL_WOL_EN_;
 
@@ -1672,10 +1640,8 @@  static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
 		val |= PM_CTL_ED_EN_;
 
 	ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-	if (ret < 0) {
-		netdev_warn(dev->net, "Error writing PM_CTRL\n");
+	if (ret < 0)
 		goto done;
-	}
 
 	/* enable receiver to enable frame reception */
 	smsc95xx_start_rx_path(dev, 1);
@@ -1702,34 +1668,26 @@  static int smsc95xx_resume(struct usb_interface *intf)
 	if (pdata->wolopts) {
 		/* clear wake-up sources */
 		ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error reading WUCSR\n");
+		if (ret < 0)
 			return ret;
-		}
 
 		val &= ~(WUCSR_WAKE_EN_ | WUCSR_MPEN_);
 
 		ret = smsc95xx_write_reg_nopm(dev, WUCSR, val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error writing WUCSR\n");
+		if (ret < 0)
 			return ret;
-		}
 
 		/* clear wake-up status */
 		ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error reading PM_CTRL\n");
+		if (ret < 0)
 			return ret;
-		}
 
 		val &= ~PM_CTL_WOL_EN_;
 		val |= PM_CTL_WUPS_;
 
 		ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
-		if (ret < 0) {
-			netdev_warn(dev->net, "Error writing PM_CTRL\n");
+		if (ret < 0)
 			return ret;
-		}
 	}
 
 	ret = usbnet_resume(intf);