[PATCH] e100: Don't enable WoL by default on Toshiba devices

[PATCH] e100: Don't enable WoL by default on Toshiba devices

[Ondrej Zary]

Enabling WoL on some Toshiba laptops (such as Portege R100) causes battery
drain after shutdown (WoL is active even on battery). These laptops have the
WoL bit set in EEPROM ID, causing e100 driver to enable WoL by default.

Check subsystem vendor ID and if it's Toshiba, don't enable WoL by default
from EEPROM settings.

Fixes https://bugs.launchpad.net/ubuntu/+source/linux/+bug/110784

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
---
 drivers/net/ethernet/intel/e100.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index 781065e..daf8fcc 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -2949,9 +2949,12 @@ static int e100_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		}
 	}
 
-	/* Wol magic packet can be enabled from eeprom */
-	if ((nic->mac >= mac_82558_D101_A4) &&
-	   (nic->eeprom[eeprom_id] & eeprom_id_wol)) {
+	/* Wol magic packet can be enabled from eeprom
+	 * Don't enable WoL by default on Toshiba laptops as it causes battery
+	 * drain after shutdown */
+	if ((pdev->subsystem_vendor != PCI_VENDOR_ID_TOSHIBA) &&
+	    (nic->mac >= mac_82558_D101_A4) &&
+	    (nic->eeprom[eeprom_id] & eeprom_id_wol)) {
 		nic->flags |= wol_magic;
 		device_set_wakeup_enable(&pdev->dev, true);
 	}
-- 
Ondrej Zary

Re: [PATCH] e100: Don't enable WoL by default on Toshiba devices

[David Miller]

From: Ondrej Zary <linux@rainbow-software.org>
Date: Wed, 12 Nov 2014 23:47:25 +0100

> Enabling WoL on some Toshiba laptops (such as Portege R100) causes battery
> drain after shutdown (WoL is active even on battery). These laptops have the
> WoL bit set in EEPROM ID, causing e100 driver to enable WoL by default.
> 
> Check subsystem vendor ID and if it's Toshiba, don't enable WoL by default
> from EEPROM settings.
> 
> Fixes https://bugs.launchpad.net/ubuntu/+source/linux/+bug/110784
> 
> Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

Jeff, are you gonna pick this up?

Re: [PATCH] e100: Don't enable WoL by default on Toshiba devices

[Jeff Kirsher]

[-- Attachment #1.1: Type: text/plain, Size: 730 bytes --]

On Wed, 2014-11-12 at 18:18 -0500, David Miller wrote:
> From: Ondrej Zary <linux@rainbow-software.org>
> Date: Wed, 12 Nov 2014 23:47:25 +0100
> 
> > Enabling WoL on some Toshiba laptops (such as Portege R100) causes battery
> > drain after shutdown (WoL is active even on battery). These laptops have the
> > WoL bit set in EEPROM ID, causing e100 driver to enable WoL by default.
> > 
> > Check subsystem vendor ID and if it's Toshiba, don't enable WoL by default
> > from EEPROM settings.
> > 
> > Fixes https://bugs.launchpad.net/ubuntu/+source/linux/+bug/110784
> > 
> > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> 
> Jeff, are you gonna pick this up?

Yes, sorry I did not catch it earlier.

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 358 bytes --]

------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

Re: [PATCH] e100: Don't enable WoL by default on Toshiba devices

[Ondrej Zary]

On Thursday 13 November 2014 04:38:47 Jeff Kirsher wrote:
> On Wed, 2014-11-12 at 18:18 -0500, David Miller wrote:
> > From: Ondrej Zary <linux@rainbow-software.org>
> > Date: Wed, 12 Nov 2014 23:47:25 +0100
> >
> > > Enabling WoL on some Toshiba laptops (such as Portege R100) causes
> > > battery drain after shutdown (WoL is active even on battery). These
> > > laptops have the WoL bit set in EEPROM ID, causing e100 driver to
> > > enable WoL by default.
> > >
> > > Check subsystem vendor ID and if it's Toshiba, don't enable WoL by
> > > default from EEPROM settings.
> > >
> > > Fixes https://bugs.launchpad.net/ubuntu/+source/linux/+bug/110784
> > >
> > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> >
> > Jeff, are you gonna pick this up?
>
> Yes, sorry I did not catch it earlier.

Will this go in 3.19?

-- 
Ondrej Zary

Re: [PATCH] e100: Don't enable WoL by default on Toshiba devices

[Ondrej Zary]

On Thursday 13 November 2014, Jeff Kirsher wrote:
> On Wed, 2014-11-12 at 18:18 -0500, David Miller wrote:
> > From: Ondrej Zary <linux@rainbow-software.org>
> > Date: Wed, 12 Nov 2014 23:47:25 +0100
> >
> > > Enabling WoL on some Toshiba laptops (such as Portege R100) causes
> > > battery drain after shutdown (WoL is active even on battery). These
> > > laptops have the WoL bit set in EEPROM ID, causing e100 driver to
> > > enable WoL by default.
> > >
> > > Check subsystem vendor ID and if it's Toshiba, don't enable WoL by
> > > default from EEPROM settings.
> > >
> > > Fixes https://bugs.launchpad.net/ubuntu/+source/linux/+bug/110784
> > >
> > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> >
> > Jeff, are you gonna pick this up?
>
> Yes, sorry I did not catch it earlier.

What happened to this patch? I don't see it in net.git or net-next.git 
(checked both davem's and jkirsher's)

-- 
Ondrej Zary

Re: [PATCH] e100: Don't enable WoL by default on Toshiba devices

[Jeff Kirsher]

[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]

On Thu, 2015-01-15 at 14:40 +0100, Ondrej Zary wrote:
> On Thursday 13 November 2014, Jeff Kirsher wrote:
> > On Wed, 2014-11-12 at 18:18 -0500, David Miller wrote:
> > > From: Ondrej Zary <linux@rainbow-software.org>
> > > Date: Wed, 12 Nov 2014 23:47:25 +0100
> > >
> > > > Enabling WoL on some Toshiba laptops (such as Portege R100)
> causes
> > > > battery drain after shutdown (WoL is active even on battery).
> These
> > > > laptops have the WoL bit set in EEPROM ID, causing e100 driver
> to
> > > > enable WoL by default.
> > > >
> > > > Check subsystem vendor ID and if it's Toshiba, don't enable WoL
> by
> > > > default from EEPROM settings.
> > > >
> > > > Fixes
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/110784
> > > >
> > > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> > >
> > > Jeff, are you gonna pick this up?
> >
> > Yes, sorry I did not catch it earlier.
> 
> What happened to this patch? I don't see it in net.git or
> net-next.git 
> (checked both davem's and jkirsher's)

Sorry, I thought I had replied with a NAK on this patch after further
review of the changes.

We don't fix BIOS issues in the driver especially regarding feature
enablement like WoL.  We would end up with dozens of these kinds of fixes
if we to allow this.

You should go back to the OEM and ask for a BIOS update to resolve this or
configure udev so that ethtool disables WoL.  


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] e100: Don't enable WoL by default on Toshiba devices

[Ondrej Zary]

On Thursday 15 January 2015, Jeff Kirsher wrote:
> On Thu, 2015-01-15 at 14:40 +0100, Ondrej Zary wrote:
> > On Thursday 13 November 2014, Jeff Kirsher wrote:
> > > On Wed, 2014-11-12 at 18:18 -0500, David Miller wrote:
> > > > From: Ondrej Zary <linux@rainbow-software.org>
> > > > Date: Wed, 12 Nov 2014 23:47:25 +0100
> > > >
> > > > > Enabling WoL on some Toshiba laptops (such as Portege R100)
> >
> > causes
> >
> > > > > battery drain after shutdown (WoL is active even on battery).
> >
> > These
> >
> > > > > laptops have the WoL bit set in EEPROM ID, causing e100 driver
> >
> > to
> >
> > > > > enable WoL by default.
> > > > >
> > > > > Check subsystem vendor ID and if it's Toshiba, don't enable WoL
> >
> > by
> >
> > > > > default from EEPROM settings.
> > > > >
> > > > > Fixes
> >
> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/110784
> >
> > > > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> > > >
> > > > Jeff, are you gonna pick this up?
> > >
> > > Yes, sorry I did not catch it earlier.
> >
> > What happened to this patch? I don't see it in net.git or
> > net-next.git
> > (checked both davem's and jkirsher's)
>
> Sorry, I thought I had replied with a NAK on this patch after further
> review of the changes.
>
> We don't fix BIOS issues in the driver especially regarding feature
> enablement like WoL.  We would end up with dozens of these kinds of fixes
> if we to allow this.
>
> You should go back to the OEM and ask for a BIOS update to resolve this or
> configure udev so that ethtool disables WoL.

This is not a BIOS bug. When the machine is powered off in BIOS (or GRUB), 
everything is OK.

The bug is that e100 driver enables WoL based on some bit in EEPROM that 
happens to be set on at least some Toshiba laptops and user has no way to 
change it. Windows driver does not do this. Other Linux ethernet drivers 
don't do this. When user wants WoL, (s)he enables it in BIOS and OS. Maybe 
this (mis)feature should be removed from the driver.

-- 
Ondrej Zary

------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

Re: [PATCH] e100: Don't enable WoL by default on Toshiba devices

[Jeff Kirsher]

[-- Attachment #1.1: Type: text/plain, Size: 2489 bytes --]

On Thu, 2015-01-15 at 16:18 +0100, Ondrej Zary wrote:
> On Thursday 15 January 2015, Jeff Kirsher wrote:
> > On Thu, 2015-01-15 at 14:40 +0100, Ondrej Zary wrote:
> > > On Thursday 13 November 2014, Jeff Kirsher wrote:
> > > > On Wed, 2014-11-12 at 18:18 -0500, David Miller wrote:
> > > > > From: Ondrej Zary <linux@rainbow-software.org>
> > > > > Date: Wed, 12 Nov 2014 23:47:25 +0100
> > > > >
> > > > > > Enabling WoL on some Toshiba laptops (such as Portege R100)
> > >
> > > causes
> > >
> > > > > > battery drain after shutdown (WoL is active even on battery).
> > >
> > > These
> > >
> > > > > > laptops have the WoL bit set in EEPROM ID, causing e100 driver
> > >
> > > to
> > >
> > > > > > enable WoL by default.
> > > > > >
> > > > > > Check subsystem vendor ID and if it's Toshiba, don't enable WoL
> > >
> > > by
> > >
> > > > > > default from EEPROM settings.
> > > > > >
> > > > > > Fixes
> > >
> > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/110784
> > >
> > > > > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> > > > >
> > > > > Jeff, are you gonna pick this up?
> > > >
> > > > Yes, sorry I did not catch it earlier.
> > >
> > > What happened to this patch? I don't see it in net.git or
> > > net-next.git
> > > (checked both davem's and jkirsher's)
> >
> > Sorry, I thought I had replied with a NAK on this patch after further
> > review of the changes.
> >
> > We don't fix BIOS issues in the driver especially regarding feature
> > enablement like WoL.  We would end up with dozens of these kinds of fixes
> > if we to allow this.
> >
> > You should go back to the OEM and ask for a BIOS update to resolve this or
> > configure udev so that ethtool disables WoL.
> 
> This is not a BIOS bug. When the machine is powered off in BIOS (or GRUB), 
> everything is OK.
> 
> The bug is that e100 driver enables WoL based on some bit in EEPROM that 
> happens to be set on at least some Toshiba laptops and user has no way to 
> change it.

Yes, the EEPROM can be modified/updated through the BIOS update I
suggested earlier.  So again, a BIOS issue.

OR you can configure udev so that ethtool disables WoL if you do not
want to pursue a EEPROM update through a BIOS update.

>  Windows driver does not do this. Other Linux ethernet drivers 
> don't do this. When user wants WoL, (s)he enables it in BIOS and OS. Maybe 
> this (mis)feature should be removed from the driver.
> 



[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 392 bytes --]

------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
http://p.sf.net/sfu/gigenet

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

Re: [PATCH] e100: Don't enable WoL by default on Toshiba devices

[Ondrej Zary]

On Thursday 15 January 2015 16:31:04 Jeff Kirsher wrote:
> On Thu, 2015-01-15 at 16:18 +0100, Ondrej Zary wrote:
> > On Thursday 15 January 2015, Jeff Kirsher wrote:
> > > On Thu, 2015-01-15 at 14:40 +0100, Ondrej Zary wrote:
> > > > On Thursday 13 November 2014, Jeff Kirsher wrote:
> > > > > On Wed, 2014-11-12 at 18:18 -0500, David Miller wrote:
> > > > > > From: Ondrej Zary <linux@rainbow-software.org>
> > > > > > Date: Wed, 12 Nov 2014 23:47:25 +0100
> > > > > >
> > > > > > > Enabling WoL on some Toshiba laptops (such as Portege R100)
> > > >
> > > > causes
> > > >
> > > > > > > battery drain after shutdown (WoL is active even on battery).
> > > >
> > > > These
> > > >
> > > > > > > laptops have the WoL bit set in EEPROM ID, causing e100 driver
> > > >
> > > > to
> > > >
> > > > > > > enable WoL by default.
> > > > > > >
> > > > > > > Check subsystem vendor ID and if it's Toshiba, don't enable WoL
> > > >
> > > > by
> > > >
> > > > > > > default from EEPROM settings.
> > > > > > >
> > > > > > > Fixes
> > > >
> > > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/110784
> > > >
> > > > > > > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> > > > > >
> > > > > > Jeff, are you gonna pick this up?
> > > > >
> > > > > Yes, sorry I did not catch it earlier.
> > > >
> > > > What happened to this patch? I don't see it in net.git or
> > > > net-next.git
> > > > (checked both davem's and jkirsher's)
> > >
> > > Sorry, I thought I had replied with a NAK on this patch after further
> > > review of the changes.
> > >
> > > We don't fix BIOS issues in the driver especially regarding feature
> > > enablement like WoL.  We would end up with dozens of these kinds of
> > > fixes if we to allow this.
> > >
> > > You should go back to the OEM and ask for a BIOS update to resolve this
> > > or configure udev so that ethtool disables WoL.
> >
> > This is not a BIOS bug. When the machine is powered off in BIOS (or
> > GRUB), everything is OK.
> >
> > The bug is that e100 driver enables WoL based on some bit in EEPROM that
> > happens to be set on at least some Toshiba laptops and user has no way to
> > change it.
>
> Yes, the EEPROM can be modified/updated through the BIOS update I
> suggested earlier.  So again, a BIOS issue.

The machine has already the latest BIOS and it's unlikely that Toshiba will 
release a new version for old laptops.

> OR you can configure udev so that ethtool disables WoL if you do not
> want to pursue a EEPROM update through a BIOS update.

I can configure udev but what about other people that install Linux and expect 
it to work (not drain battery after shutdown)? Without needing to know what 
udev, ethtool or WoL is.

> >  Windows driver does not do this. Other Linux ethernet drivers
> > don't do this. When user wants WoL, (s)he enables it in BIOS and OS.
> > Maybe this (mis)feature should be removed from the driver.


-- 
Ondrej Zary