| Message ID | 1355140301-10518-1-git-send-email-steve.glendinning@shawell.net |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Monday 10 December 2012 11:51:41 Steve Glendinning wrote: > This is a work in-progress patch. It's not yet complete but > I thought I'd share it for comments, feedback and testing. > > This patch enables dynamic autosuspend for all devices > supported by the driver, but it will only actually work on > LAN9500A (as this has a new SUSPEND3 mode for this purpose). So this is a problem with remote wakeup on older hardware? > Unfortunately we don't know if the connected device supports > this feature until we query its ID register at runtime. > > On unsupported devices (LAN9500/9512/9514) this patch claims > to support the feature but if enabled it will always return > failure to the autosuspend call (and fill up the kernel log > with a message every 2s). > > Suggestions on how best to indicate this capability at runtime > instead of compile-time would be appreciated, so we don't have > to repeatedly fail if accidentally enabled. Or maybe this is > actually the best way? If this is a problem with remote wakeup, you should up the pm counter (usb_autopm_get_noresume()) in .manage_power That was the reason I implemented this is a callback and not as a helper in usbnet. Regards Oliver -- 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
On Mon, Dec 10, 2012 at 7:51 PM, Steve Glendinning <steve.glendinning@shawell.net> wrote: > This is a work in-progress patch. It's not yet complete but > I thought I'd share it for comments, feedback and testing. > > This patch enables dynamic autosuspend for all devices > supported by the driver, but it will only actually work on > LAN9500A (as this has a new SUSPEND3 mode for this purpose). > Unfortunately we don't know if the connected device supports > this feature until we query its ID register at runtime. > > On unsupported devices (LAN9500/9512/9514) this patch claims > to support the feature but if enabled it will always return > failure to the autosuspend call (and fill up the kernel log > with a message every 2s). > > Suggestions on how best to indicate this capability at runtime > instead of compile-time would be appreciated, so we don't have > to repeatedly fail if accidentally enabled. Or maybe this is > actually the best way? The ID register can be read inside bind(), so you may set smsc95xx_info.manage_power as smsc95xx_manage_power only for LAN9500A devices. One disadvantage of above idea is that the link down can't trigger runtime suspend via mange_power way(USB auto-suspend), but we still can introduce explicit link change based runtime suspend for non-LAN9500A devices. > > We should also be able to identify devices supporting > autosuspend from the USB VID/PID if this would help. > > UPDATE: reposting this to a wider audience due to lack of > feedback last time round > > Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net> > --- > drivers/net/usb/smsc95xx.c | 136 +++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 135 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c > index 9b73670..d9c6674 100644 > --- a/drivers/net/usb/smsc95xx.c > +++ b/drivers/net/usb/smsc95xx.c > @@ -55,6 +55,13 @@ > #define FEATURE_PHY_NLP_CROSSOVER (0x02) > #define FEATURE_AUTOSUSPEND (0x04) > > +#define SUSPEND_SUSPEND0 (0x01) > +#define SUSPEND_SUSPEND1 (0x02) > +#define SUSPEND_SUSPEND2 (0x04) > +#define SUSPEND_SUSPEND3 (0x08) > +#define SUSPEND_ALLMODES (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \ > + SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3) > + > struct smsc95xx_priv { > u32 mac_cr; > u32 hash_hi; > @@ -62,6 +69,7 @@ struct smsc95xx_priv { > u32 wolopts; > spinlock_t mac_cr_lock; > u8 features; > + u8 suspend_flags; > }; > > static bool turbo_mode = true; > @@ -1341,6 +1349,8 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev) > if (ret < 0) > netdev_warn(dev->net, "Error reading PM_CTRL\n"); > > + pdata->suspend_flags |= SUSPEND_SUSPEND0; > + > return ret; > } > > @@ -1393,11 +1403,14 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev) > if (ret < 0) > netdev_warn(dev->net, "Error writing PM_CTRL\n"); > > + pdata->suspend_flags |= SUSPEND_SUSPEND1; > + > return ret; > } > > static int smsc95xx_enter_suspend2(struct usbnet *dev) > { > + struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); > u32 val; > int ret; > > @@ -1414,9 +1427,96 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev) > if (ret < 0) > netdev_warn(dev->net, "Error writing PM_CTRL\n"); > > + pdata->suspend_flags |= SUSPEND_SUSPEND2; > + > return ret; > } > > +static int smsc95xx_enter_suspend3(struct usbnet *dev) > +{ > + struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); > + u32 val; > + int ret; > + > + ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val); > + if (ret < 0) { > + netdev_warn(dev->net, "Error reading RX_FIFO_INF"); > + return ret; > + } > + > + if (val & 0xFFFF) { > + netdev_info(dev->net, "rx fifo not empty in autosuspend"); > + return -EBUSY; > + } > + > + ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val); > + if (ret < 0) { > + netdev_warn(dev->net, "Error reading PM_CTRL"); > + return ret; > + } > + > + val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_); > + val |= PM_CTL_SUS_MODE_3 | PM_CTL_RES_CLR_WKP_STS; > + > + ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val); > + if (ret < 0) { > + netdev_warn(dev->net, "Error writing PM_CTRL"); > + return ret; > + } > + > + /* clear wol status */ > + val &= ~PM_CTL_WUPS_; > + val |= PM_CTL_WUPS_WOL_; > + > + ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val); > + if (ret < 0) { > + netdev_warn(dev->net, "Error writing PM_CTRL"); > + return ret; > + } > + > + pdata->suspend_flags |= SUSPEND_SUSPEND3; > + > + return 0; > +} > + > +static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up) > +{ > + int ret; > + > + if (!netif_running(dev->net)) { > + /* interface is ifconfig down so fully power down hw */ > + netdev_dbg(dev->net, "autosuspend entering SUSPEND2"); > + return smsc95xx_enter_suspend2(dev); > + } > + > + if (!link_up) { > + /* link is down so enter EDPD mode */ > + netdev_dbg(dev->net, "autosuspend entering SUSPEND1"); > + > + /* enable PHY wakeup events for if cable is attached */ > + ret = smsc95xx_enable_phy_wakeup_interrupts(dev, > + PHY_INT_MASK_ANEG_COMP_); > + if (ret < 0) { > + netdev_warn(dev->net, "error enabling PHY wakeup ints"); > + return ret; > + } > + > + netdev_info(dev->net, "entering SUSPEND1 mode"); > + return smsc95xx_enter_suspend1(dev); > + } > + > + /* enable PHY wakeup events so we remote wakeup if cable is pulled */ > + ret = smsc95xx_enable_phy_wakeup_interrupts(dev, > + PHY_INT_MASK_LINK_DOWN_); > + if (ret < 0) { > + netdev_warn(dev->net, "error enabling PHY wakeup ints"); > + return ret; > + } > + > + netdev_dbg(dev->net, "autosuspend entering SUSPEND3"); > + return smsc95xx_enter_suspend3(dev); > +} > + > static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message) > { > struct usbnet *dev = usb_get_intfdata(intf); > @@ -1424,15 +1524,35 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message) > u32 val, link_up; > int ret; > > + /* TODO: don't indicate this feature to usb framework if > + * our current hardware doesn't have the capability > + */ > + if ((message.event == PM_EVENT_AUTO_SUSPEND) && > + (!(pdata->features & FEATURE_AUTOSUSPEND))) { > + netdev_warn(dev->net, "autosuspend not supported"); > + return -EBUSY; > + } > + > ret = usbnet_suspend(intf, message); > if (ret < 0) { > netdev_warn(dev->net, "usbnet_suspend error\n"); > return ret; > } > > + if (pdata->suspend_flags) { > + netdev_warn(dev->net, "error during last resume"); > + pdata->suspend_flags = 0; > + } > + > /* determine if link is up using only _nopm functions */ > link_up = smsc95xx_link_ok_nopm(dev); > > + if (message.event == PM_EVENT_AUTO_SUSPEND) { > + ret = smsc95xx_autosuspend(dev, link_up); > + goto done; > + } > + > + /* if we get this far we're not autosuspending */ > /* if no wol options set, or if link is down and we're not waking on > * PHY activity, enter lowest power SUSPEND2 mode > */ > @@ -1694,12 +1814,18 @@ static int smsc95xx_resume(struct usb_interface *intf) > { > struct usbnet *dev = usb_get_intfdata(intf); > struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); > + u8 suspend_flags = pdata->suspend_flags; > int ret; > u32 val; > > BUG_ON(!dev); > > - if (pdata->wolopts) { > + netdev_dbg(dev->net, "resume suspend_flags=0x%02x", suspend_flags); > + > + /* do this first to ensure it's cleared even in error case */ > + pdata->suspend_flags = 0; > + > + if (suspend_flags & SUSPEND_ALLMODES) { > /* clear wake-up sources */ > ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val); > if (ret < 0) { > @@ -1891,6 +2017,12 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev, > return skb; > } > > +static int smsc95xx_manage_power(struct usbnet *dev, int on) > +{ > + dev->intf->needs_remote_wakeup = on; > + return 0; > +} > + > static const struct driver_info smsc95xx_info = { > .description = "smsc95xx USB 2.0 Ethernet", > .bind = smsc95xx_bind, > @@ -1900,6 +2032,7 @@ static const struct driver_info smsc95xx_info = { > .rx_fixup = smsc95xx_rx_fixup, > .tx_fixup = smsc95xx_tx_fixup, > .status = smsc95xx_status, > + .manage_power = smsc95xx_manage_power, > .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_LINK_INTR, > }; > > @@ -2007,6 +2140,7 @@ static struct usb_driver smsc95xx_driver = { > .reset_resume = smsc95xx_resume, > .disconnect = usbnet_disconnect, > .disable_hub_initiated_lpm = 1, > + .supports_autosuspend = 1, > }; > > module_usb_driver(smsc95xx_driver); > -- > 1.7.10.4 > -- 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
On 10 December 2012 12:09, Oliver Neukum <oliver@neukum.org> wrote: > So this is a problem with remote wakeup on older hardware? Exactly, the older hardware revisions can't reliably do it. >> Unfortunately we don't know if the connected device supports >> this feature until we query its ID register at runtime. >> >> Suggestions on how best to indicate this capability at runtime >> instead of compile-time would be appreciated, so we don't have >> to repeatedly fail if accidentally enabled. Or maybe this is >> actually the best way? > > If this is a problem with remote wakeup, you should up the > pm counter (usb_autopm_get_noresume()) in .manage_power > That was the reason I implemented this is a callback and not as > a helper in usbnet. Thanks, so something like this should do the job? static int smsc95xx_manage_power(struct usbnet *dev, int on) { struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); dev->intf->needs_remote_wakeup = on; if (pdata->features & FEATURE_AUTOSUSPEND) return 0; /* this chip revision doesn't support autosuspend */ netdev_info(dev->net, "hardware doesn't support USB autosuspend\n"); if (on) usb_autopm_get_interface_no_resume(dev->intf); else usb_autopm_put_interface_no_suspend(dev->intf); return 0; } -- 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
On Mon, Dec 10, 2012 at 10:18 PM, Steve Glendinning <steve@shawell.net> wrote: > On 10 December 2012 12:09, Oliver Neukum <oliver@neukum.org> wrote: >> So this is a problem with remote wakeup on older hardware? > > Exactly, the older hardware revisions can't reliably do it. > >>> Unfortunately we don't know if the connected device supports >>> this feature until we query its ID register at runtime. >>> >>> Suggestions on how best to indicate this capability at runtime >>> instead of compile-time would be appreciated, so we don't have >>> to repeatedly fail if accidentally enabled. Or maybe this is >>> actually the best way? >> >> If this is a problem with remote wakeup, you should up the >> pm counter (usb_autopm_get_noresume()) in .manage_power >> That was the reason I implemented this is a callback and not as >> a helper in usbnet. > > Thanks, so something like this should do the job? This will do, but not simple as clearing .manage_power function pointer in bind(), and still disable runtime suspend for link off case since these devices which don't support suspend 3 can generate remote wakeup for link change event. I suggest to introduce link-off triggered runtime suspend for these usbnet devices(non-LAN9500A device, devices which don't support USB auto-suspend), and I have posted one patch set before[1]. If no one objects that, I'd like to post them again with some fix and update for checking link after link_reset(). [1], http://marc.info/?w=2&r=1&s=usbnet%3A+support+runtime+PM+triggered+by+&q=t > > static int smsc95xx_manage_power(struct usbnet *dev, int on) > { > struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); > > dev->intf->needs_remote_wakeup = on; > > if (pdata->features & FEATURE_AUTOSUSPEND) > return 0; > > /* this chip revision doesn't support autosuspend */ > netdev_info(dev->net, "hardware doesn't support USB autosuspend\n"); > > if (on) > usb_autopm_get_interface_no_resume(dev->intf); > else > usb_autopm_put_interface_no_suspend(dev->intf); > > return 0; > } Thanks, -- Ming Lei -- 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
On Tuesday 11 December 2012 10:24:57 Ming Lei wrote: > On Mon, Dec 10, 2012 at 10:18 PM, Steve Glendinning <steve@shawell.net> wrote: > > Thanks, so something like this should do the job? > > This will do, but not simple as clearing .manage_power function > pointer in bind(), and still disable runtime suspend for link off case > since these devices which don't support suspend 3 can generate > remote wakeup for link change event. So they can autosuspend if the interface is up and no cable is plugged in? > I suggest to introduce link-off triggered runtime suspend for these > usbnet devices(non-LAN9500A device, devices which don't support > USB auto-suspend), and I have posted one patch set before[1]. > If no one objects that, I'd like to post them again with some fix and > update for checking link after link_reset(). If you can get rid of a periodic work this would be great. Regards Oliver -- 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
On Tue, Dec 11, 2012 at 6:27 PM, Oliver Neukum <oliver@neukum.org> wrote: > On Tuesday 11 December 2012 10:24:57 Ming Lei wrote: >> On Mon, Dec 10, 2012 at 10:18 PM, Steve Glendinning <steve@shawell.net> wrote: > >> > Thanks, so something like this should do the job? >> >> This will do, but not simple as clearing .manage_power function >> pointer in bind(), and still disable runtime suspend for link off case >> since these devices which don't support suspend 3 can generate >> remote wakeup for link change event. > > So they can autosuspend if the interface is up and no cable is plugged > in? From the open datasheet, that is the suspend 1 mode, which is supported by all LAN95xx devices. Steve, correct me if I am wrong. > >> I suggest to introduce link-off triggered runtime suspend for these >> usbnet devices(non-LAN9500A device, devices which don't support >> USB auto-suspend), and I have posted one patch set before[1]. >> If no one objects that, I'd like to post them again with some fix and >> update for checking link after link_reset(). > > If you can get rid of a periodic work this would be great. For the LAN95xx devices, the periodic work isn't needed because they may generate remote wakeup when link change is detected. In fact, I have test data which can show a much power save on OMAP3 based beagle board plus asix usbnet device with the periodic work. IMO, the power save after introducing periodic timer depends on the arch or platform, there should be much power save if the CPU power consumption is very less. So how about letting module parameter switch on/off the periodic work? Thanks, -- Ming Lei -- 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
On 11 December 2012 12:53, Ming Lei <ming.lei@canonical.com> wrote: > On Tue, Dec 11, 2012 at 6:27 PM, Oliver Neukum <oliver@neukum.org> wrote: >> So they can autosuspend if the interface is up and no cable is plugged >> in? > > From the open datasheet, that is the suspend 1 mode, which is supported > by all LAN95xx devices. Steve, correct me if I am wrong. All parts support SUSPEND1, but some parts can't 100% reliably wake on ENERGYON - some link partners will wake them but others won't. The driver already detects parts that work reliably with all link partners and sets the FEATURE_PHY_NLP_CROSSOVER feature flag. I didn't block these devices from configuring WOL, because they do work in *some* cases and the user is explicitly requesting to wake the system so we try to do that (and sometimes succeed). >>> I suggest to introduce link-off triggered runtime suspend for these >>> usbnet devices(non-LAN9500A device, devices which don't support >>> USB auto-suspend), and I have posted one patch set before[1]. >>> If no one objects that, I'd like to post them again with some fix and >>> update for checking link after link_reset(). >> >> If you can get rid of a periodic work this would be great. > > For the LAN95xx devices, the periodic work isn't needed because > they may generate remote wakeup when link change is detected. As above, some parts will do this but some will not. I think we should only consider sleeping the part if we're sure it'll wake up when a cable is connected!
On Tuesday 11 December 2012 20:53:19 Ming Lei wrote: > In fact, I have test data which can show a much power save > on OMAP3 based beagle board plus asix usbnet device with > the periodic work. IMO, the power save after introducing periodic > timer depends on the arch or platform, there should be much power > save if the CPU power consumption is very less. So how about letting > module parameter switch on/off the periodic work? You could ask on linux-power and netdev. But there would be an obvious question: Why kernel space? Regards Oliver -- 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
CC linux-power On Tue, Dec 11, 2012 at 11:19 PM, Oliver Neukum <oliver@neukum.org> wrote: > On Tuesday 11 December 2012 20:53:19 Ming Lei wrote: >> In fact, I have test data which can show a much power save >> on OMAP3 based beagle board plus asix usbnet device with >> the periodic work. IMO, the power save after introducing periodic >> timer depends on the arch or platform, there should be much power >> save if the CPU power consumption is very less. So how about letting >> module parameter switch on/off the periodic work? > > You could ask on linux-power and netdev. But there would be an > obvious question: Why kernel space? How does user space utility know one interface doesn't support remote wakeup for link change? and how to do it in user space? Or could we persuade user space guys to do it? Or could the previous user space utility can support power save on these devices? At least, one advantage of doing it in kernel space is that we can let current and previous user space utility support power save on these devices when the link is off. Also, suppose user space utility may close interface automatically when the link becomes off, some configurations(such as IP address) of the network interface will be lost after it is brought up again next time once the link becomes on. The problem might break some application. Thanks, -- Ming Lei -- 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
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index 9b73670..d9c6674 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -55,6 +55,13 @@ #define FEATURE_PHY_NLP_CROSSOVER (0x02) #define FEATURE_AUTOSUSPEND (0x04) +#define SUSPEND_SUSPEND0 (0x01) +#define SUSPEND_SUSPEND1 (0x02) +#define SUSPEND_SUSPEND2 (0x04) +#define SUSPEND_SUSPEND3 (0x08) +#define SUSPEND_ALLMODES (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \ + SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3) + struct smsc95xx_priv { u32 mac_cr; u32 hash_hi; @@ -62,6 +69,7 @@ struct smsc95xx_priv { u32 wolopts; spinlock_t mac_cr_lock; u8 features; + u8 suspend_flags; }; static bool turbo_mode = true; @@ -1341,6 +1349,8 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev) if (ret < 0) netdev_warn(dev->net, "Error reading PM_CTRL\n"); + pdata->suspend_flags |= SUSPEND_SUSPEND0; + return ret; } @@ -1393,11 +1403,14 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev) if (ret < 0) netdev_warn(dev->net, "Error writing PM_CTRL\n"); + pdata->suspend_flags |= SUSPEND_SUSPEND1; + return ret; } static int smsc95xx_enter_suspend2(struct usbnet *dev) { + struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); u32 val; int ret; @@ -1414,9 +1427,96 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev) if (ret < 0) netdev_warn(dev->net, "Error writing PM_CTRL\n"); + pdata->suspend_flags |= SUSPEND_SUSPEND2; + return ret; } +static int smsc95xx_enter_suspend3(struct usbnet *dev) +{ + struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); + u32 val; + int ret; + + ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val); + if (ret < 0) { + netdev_warn(dev->net, "Error reading RX_FIFO_INF"); + return ret; + } + + if (val & 0xFFFF) { + netdev_info(dev->net, "rx fifo not empty in autosuspend"); + return -EBUSY; + } + + ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val); + if (ret < 0) { + netdev_warn(dev->net, "Error reading PM_CTRL"); + return ret; + } + + val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_); + val |= PM_CTL_SUS_MODE_3 | PM_CTL_RES_CLR_WKP_STS; + + ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val); + if (ret < 0) { + netdev_warn(dev->net, "Error writing PM_CTRL"); + return ret; + } + + /* clear wol status */ + val &= ~PM_CTL_WUPS_; + val |= PM_CTL_WUPS_WOL_; + + ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val); + if (ret < 0) { + netdev_warn(dev->net, "Error writing PM_CTRL"); + return ret; + } + + pdata->suspend_flags |= SUSPEND_SUSPEND3; + + return 0; +} + +static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up) +{ + int ret; + + if (!netif_running(dev->net)) { + /* interface is ifconfig down so fully power down hw */ + netdev_dbg(dev->net, "autosuspend entering SUSPEND2"); + return smsc95xx_enter_suspend2(dev); + } + + if (!link_up) { + /* link is down so enter EDPD mode */ + netdev_dbg(dev->net, "autosuspend entering SUSPEND1"); + + /* enable PHY wakeup events for if cable is attached */ + ret = smsc95xx_enable_phy_wakeup_interrupts(dev, + PHY_INT_MASK_ANEG_COMP_); + if (ret < 0) { + netdev_warn(dev->net, "error enabling PHY wakeup ints"); + return ret; + } + + netdev_info(dev->net, "entering SUSPEND1 mode"); + return smsc95xx_enter_suspend1(dev); + } + + /* enable PHY wakeup events so we remote wakeup if cable is pulled */ + ret = smsc95xx_enable_phy_wakeup_interrupts(dev, + PHY_INT_MASK_LINK_DOWN_); + if (ret < 0) { + netdev_warn(dev->net, "error enabling PHY wakeup ints"); + return ret; + } + + netdev_dbg(dev->net, "autosuspend entering SUSPEND3"); + return smsc95xx_enter_suspend3(dev); +} + static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message) { struct usbnet *dev = usb_get_intfdata(intf); @@ -1424,15 +1524,35 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message) u32 val, link_up; int ret; + /* TODO: don't indicate this feature to usb framework if + * our current hardware doesn't have the capability + */ + if ((message.event == PM_EVENT_AUTO_SUSPEND) && + (!(pdata->features & FEATURE_AUTOSUSPEND))) { + netdev_warn(dev->net, "autosuspend not supported"); + return -EBUSY; + } + ret = usbnet_suspend(intf, message); if (ret < 0) { netdev_warn(dev->net, "usbnet_suspend error\n"); return ret; } + if (pdata->suspend_flags) { + netdev_warn(dev->net, "error during last resume"); + pdata->suspend_flags = 0; + } + /* determine if link is up using only _nopm functions */ link_up = smsc95xx_link_ok_nopm(dev); + if (message.event == PM_EVENT_AUTO_SUSPEND) { + ret = smsc95xx_autosuspend(dev, link_up); + goto done; + } + + /* if we get this far we're not autosuspending */ /* if no wol options set, or if link is down and we're not waking on * PHY activity, enter lowest power SUSPEND2 mode */ @@ -1694,12 +1814,18 @@ static int smsc95xx_resume(struct usb_interface *intf) { struct usbnet *dev = usb_get_intfdata(intf); struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); + u8 suspend_flags = pdata->suspend_flags; int ret; u32 val; BUG_ON(!dev); - if (pdata->wolopts) { + netdev_dbg(dev->net, "resume suspend_flags=0x%02x", suspend_flags); + + /* do this first to ensure it's cleared even in error case */ + pdata->suspend_flags = 0; + + if (suspend_flags & SUSPEND_ALLMODES) { /* clear wake-up sources */ ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val); if (ret < 0) { @@ -1891,6 +2017,12 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev, return skb; } +static int smsc95xx_manage_power(struct usbnet *dev, int on) +{ + dev->intf->needs_remote_wakeup = on; + return 0; +} + static const struct driver_info smsc95xx_info = { .description = "smsc95xx USB 2.0 Ethernet", .bind = smsc95xx_bind, @@ -1900,6 +2032,7 @@ static const struct driver_info smsc95xx_info = { .rx_fixup = smsc95xx_rx_fixup, .tx_fixup = smsc95xx_tx_fixup, .status = smsc95xx_status, + .manage_power = smsc95xx_manage_power, .flags = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_LINK_INTR, }; @@ -2007,6 +2140,7 @@ static struct usb_driver smsc95xx_driver = { .reset_resume = smsc95xx_resume, .disconnect = usbnet_disconnect, .disable_hub_initiated_lpm = 1, + .supports_autosuspend = 1, }; module_usb_driver(smsc95xx_driver);
This is a work in-progress patch. It's not yet complete but I thought I'd share it for comments, feedback and testing. This patch enables dynamic autosuspend for all devices supported by the driver, but it will only actually work on LAN9500A (as this has a new SUSPEND3 mode for this purpose). Unfortunately we don't know if the connected device supports this feature until we query its ID register at runtime. On unsupported devices (LAN9500/9512/9514) this patch claims to support the feature but if enabled it will always return failure to the autosuspend call (and fill up the kernel log with a message every 2s). Suggestions on how best to indicate this capability at runtime instead of compile-time would be appreciated, so we don't have to repeatedly fail if accidentally enabled. Or maybe this is actually the best way? We should also be able to identify devices supporting autosuspend from the USB VID/PID if this would help. UPDATE: reposting this to a wider audience due to lack of feedback last time round Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net> --- drivers/net/usb/smsc95xx.c | 136 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 135 insertions(+), 1 deletion(-)