[PATCH] e1000: the power down when running ifdown command

[PATCH] e1000: the power down when running ifdown command

[Naohiro Ooiwa]

Hi All

I resend my patch.
Sorry, my previous mail lacked an explanation.


The e1000 driver doesn't let the power down when running ifdown command.
So, I set to the D3hot state of a PCI device at the end of e1000_close().

With this modification, e1000 driver reduces power by ifdown.
It's about 6 watts when I measured a total power of one server machine
in my environment.

I tested this patch. The result is good enough to me.

Could you please check my patch ?
If I should have other considerations, please tell me.


Thanks you.
Naohiro Ooiwa



Signed-off-by: Naohiro Ooiwa <nooiwa@miraclelinux.com>
---
 drivers/net/e1000/e1000_main.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index bcd192c..12e1a42 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -26,6 +26,11 @@

 *******************************************************************************/

+/*
+ * define this if you want pci save power while ifdown.
+ */
+#define E1000_PCI_POWER_SAVE
+
 #include "e1000.h"
 #include <net/ip6_checksum.h>

@@ -1248,6 +1253,23 @@ static int e1000_open(struct net_device *netdev)
 	struct e1000_hw *hw = &adapter->hw;
 	int err;

+#ifdef E1000_PCI_POWER_SAVE
+	struct pci_dev *pdev = adapter->pdev;
+
+	pci_set_power_state(pdev, PCI_D0);
+	pci_restore_state(pdev);
+
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		err = pci_enable_device_mem(pdev);
+	if (err) {
+		printk(KERN_ERR "e1000: Cannot enable PCI device from power-save\n");
+		return err;
+	}
+	pci_set_master(pdev);
+#endif
+
 	/* disallow open during test */
 	if (test_bit(__E1000_TESTING, &adapter->flags))
 		return -EBUSY;
@@ -1265,6 +1287,7 @@ static int e1000_open(struct net_device *netdev)
 		goto err_setup_rx;

 	e1000_power_up_phy(adapter);
+	e1000_reset(adapter);

 	adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
 	if ((hw->mng_cookie.status &
@@ -1341,6 +1364,15 @@ static int e1000_close(struct net_device *netdev)
 		e1000_vlan_rx_kill_vid(netdev, adapter->mng_vlan_id);
 	}

+#ifdef E1000_PCI_POWER_SAVE
+#ifdef CONFIG_PM
+	pci_save_state(adapter->pdev);
+#endif
+	pci_disable_device(adapter->pdev);
+	pci_wake_from_d3(adapter->pdev, true);
+	pci_set_power_state(adapter->pdev, PCI_D3hot);
+#endif
+
 	return 0;
 }

-- 
1.5.4.1

--
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

--
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

Re: [PATCH] e1000: the power down when running ifdown command

[Stephen Hemminger]

On Sat, 31 Oct 2009 18:39:52 +0900
Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:

> Hi All
> 
> I resend my patch.
> Sorry, my previous mail lacked an explanation.
> 
> 
> The e1000 driver doesn't let the power down when running ifdown command.
> So, I set to the D3hot state of a PCI device at the end of e1000_close().
> 
> With this modification, e1000 driver reduces power by ifdown.
> It's about 6 watts when I measured a total power of one server machine
> in my environment.
> 
> I tested this patch. The result is good enough to me.
> 
> Could you please check my patch ?
> If I should have other considerations, please tell me.
> 


Does this work with Wake On Lan? 

------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay 
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference

Re: [PATCH] e1000: the power down when running ifdown command

[Naohiro Ooiwa]

Stephen Hemminger wrote:
> On Sat, 31 Oct 2009 18:39:52 +0900
> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
> 
>> Hi All
>>
>> I resend my patch.
>> Sorry, my previous mail lacked an explanation.
>>
>>
>> The e1000 driver doesn't let the power down when running ifdown command.
>> So, I set to the D3hot state of a PCI device at the end of e1000_close().
>>
>> With this modification, e1000 driver reduces power by ifdown.
>> It's about 6 watts when I measured a total power of one server machine
>> in my environment.
>>
>> I tested this patch. The result is good enough to me.
>>
>> Could you please check my patch ?
>> If I should have other considerations, please tell me.
>>

Hi Stephen

Thank you so much for your reply.

> Does this work with Wake On Lan? 

Yes, it works WOL.
But I worry that my test is enough.

They are following:
  - simple data transmission after ifdown;ifup.
  - enable wol, ifup network device, system shutdown, and make sure wol work.
  - enable wol, ifdown network device, system shutdown, and make sure wol work.
  - while [ 0 ] ; do ifdown eth0 ; ifup eth0 ; done
  - while [ 0 ] ; do modprobe e1000 ; rmmod e1000 ; done


> @@ -1265,6 +1287,7 @@ static int e1000_open(struct net_device *netdev)
>  		goto err_setup_rx;
>
>  	e1000_power_up_phy(adapter);
> +	e1000_reset(adapter);
>
>  	adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
>  	if ((hw->mng_cookie.status &

This code fix problem that e1000 driver doesn't work to auto-negotiation
once in a while.
Maybe, the cause is that set state to D0 just before it.
I found it by repeat of ifup and ifdown.

If you find out other points and any necessary tests from my patch,
please tell me. I will make sure them.

Thanks you.
Naohiro Ooiwa

Re: [PATCH] e1000: the power down when running ifdown command

[Jeff Kirsher]

2009/10/31 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
> Hi All
>
> I resend my patch.
> Sorry, my previous mail lacked an explanation.
>
>
> The e1000 driver doesn't let the power down when running ifdown command.
> So, I set to the D3hot state of a PCI device at the end of e1000_close().
>
> With this modification, e1000 driver reduces power by ifdown.
> It's about 6 watts when I measured a total power of one server machine
> in my environment.
>
> I tested this patch. The result is good enough to me.
>
> Could you please check my patch ?
> If I should have other considerations, please tell me.
>
>
> Thanks you.
> Naohiro Ooiwa
>
>
>
> Signed-off-by: Naohiro Ooiwa <nooiwa@miraclelinux.com>
> ---
>  drivers/net/e1000/e1000_main.c |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index bcd192c..12e1a42 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -26,6 +26,11 @@
>
>  *******************************************************************************/
>
> +/*
> + * define this if you want pci save power while ifdown.
> + */
> +#define E1000_PCI_POWER_SAVE
> +
>  #include "e1000.h"
>  #include <net/ip6_checksum.h>
>
> @@ -1248,6 +1253,23 @@ static int e1000_open(struct net_device *netdev)
>        struct e1000_hw *hw = &adapter->hw;
>        int err;
>
> +#ifdef E1000_PCI_POWER_SAVE
> +       struct pci_dev *pdev = adapter->pdev;
> +
> +       pci_set_power_state(pdev, PCI_D0);
> +       pci_restore_state(pdev);
> +
> +       if (adapter->need_ioport)
> +               err = pci_enable_device(pdev);
> +       else
> +               err = pci_enable_device_mem(pdev);
> +       if (err) {
> +               printk(KERN_ERR "e1000: Cannot enable PCI device from power-save\n");
> +               return err;
> +       }
> +       pci_set_master(pdev);
> +#endif
> +
>        /* disallow open during test */
>        if (test_bit(__E1000_TESTING, &adapter->flags))
>                return -EBUSY;
> @@ -1265,6 +1287,7 @@ static int e1000_open(struct net_device *netdev)
>                goto err_setup_rx;
>
>        e1000_power_up_phy(adapter);
> +       e1000_reset(adapter);
>
>        adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
>        if ((hw->mng_cookie.status &
> @@ -1341,6 +1364,15 @@ static int e1000_close(struct net_device *netdev)
>                e1000_vlan_rx_kill_vid(netdev, adapter->mng_vlan_id);
>        }
>
> +#ifdef E1000_PCI_POWER_SAVE
> +#ifdef CONFIG_PM
> +       pci_save_state(adapter->pdev);
> +#endif
> +       pci_disable_device(adapter->pdev);
> +       pci_wake_from_d3(adapter->pdev, true);
> +       pci_set_power_state(adapter->pdev, PCI_D3hot);
> +#endif
> +
>        return 0;
>  }
>

I have added this patch to my tree for testing.  This patch requires a
fair amount of regression testing, so once its passed testing I will
push the patch to David/netdev.

-- 
Cheers,
Jeff

Re: [PATCH] e1000: the power down when running ifdown command

[Dan Williams]

On Mon, 2009-11-02 at 16:26 -0800, Jeff Kirsher wrote:
> 2009/10/31 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
> > Hi All
> >
> > I resend my patch.
> > Sorry, my previous mail lacked an explanation.
> >
> >
> > The e1000 driver doesn't let the power down when running ifdown command.
> > So, I set to the D3hot state of a PCI device at the end of e1000_close().
> >
> > With this modification, e1000 driver reduces power by ifdown.
> > It's about 6 watts when I measured a total power of one server machine
> > in my environment.
> >
> > I tested this patch. The result is good enough to me.
> >
> > Could you please check my patch ?
> > If I should have other considerations, please tell me.
> >
> >
> > Thanks you.
> > Naohiro Ooiwa
> >
> >
> >
> > Signed-off-by: Naohiro Ooiwa <nooiwa@miraclelinux.com>
> > ---
> >  drivers/net/e1000/e1000_main.c |   32 ++++++++++++++++++++++++++++++++
> >  1 files changed, 32 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> > index bcd192c..12e1a42 100644
> > --- a/drivers/net/e1000/e1000_main.c
> > +++ b/drivers/net/e1000/e1000_main.c
> > @@ -26,6 +26,11 @@
> >
> >  *******************************************************************************/
> >
> > +/*
> > + * define this if you want pci save power while ifdown.
> > + */
> > +#define E1000_PCI_POWER_SAVE
> > +
> >  #include "e1000.h"
> >  #include <net/ip6_checksum.h>
> >
> > @@ -1248,6 +1253,23 @@ static int e1000_open(struct net_device *netdev)
> >        struct e1000_hw *hw = &adapter->hw;
> >        int err;
> >
> > +#ifdef E1000_PCI_POWER_SAVE
> > +       struct pci_dev *pdev = adapter->pdev;
> > +
> > +       pci_set_power_state(pdev, PCI_D0);
> > +       pci_restore_state(pdev);
> > +
> > +       if (adapter->need_ioport)
> > +               err = pci_enable_device(pdev);
> > +       else
> > +               err = pci_enable_device_mem(pdev);
> > +       if (err) {
> > +               printk(KERN_ERR "e1000: Cannot enable PCI device from power-save\n");
> > +               return err;
> > +       }
> > +       pci_set_master(pdev);
> > +#endif
> > +
> >        /* disallow open during test */
> >        if (test_bit(__E1000_TESTING, &adapter->flags))
> >                return -EBUSY;
> > @@ -1265,6 +1287,7 @@ static int e1000_open(struct net_device *netdev)
> >                goto err_setup_rx;
> >
> >        e1000_power_up_phy(adapter);
> > +       e1000_reset(adapter);
> >
> >        adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
> >        if ((hw->mng_cookie.status &
> > @@ -1341,6 +1364,15 @@ static int e1000_close(struct net_device *netdev)
> >                e1000_vlan_rx_kill_vid(netdev, adapter->mng_vlan_id);
> >        }
> >
> > +#ifdef E1000_PCI_POWER_SAVE
> > +#ifdef CONFIG_PM
> > +       pci_save_state(adapter->pdev);
> > +#endif
> > +       pci_disable_device(adapter->pdev);
> > +       pci_wake_from_d3(adapter->pdev, true);
> > +       pci_set_power_state(adapter->pdev, PCI_D3hot);
> > +#endif
> > +
> >        return 0;
> >  }
> >
> 
> I have added this patch to my tree for testing.  This patch requires a
> fair amount of regression testing, so once its passed testing I will
> push the patch to David/netdev.

Can we get rid of E1000_PCI_POWER_SAVE and just enable this
unconditionally once it's baked?  Having the define there is pretty
superfluous right now.  Either that, or make it
CONFIG_E1000_PCI_POWER_SAVE and make a Kconfig option for it that
depends on experimental or something.  Or a module parameter.  Or
something other than a dangling #define :)

Dan



------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay 
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference

Re: [PATCH] e1000: the power down when running ifdown command

[Naohiro Ooiwa]

Jeff Kirsher wrote:
> 2009/10/31 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
> 
> I have added this patch to my tree for testing.  This patch requires a
> fair amount of regression testing, so once its passed testing I will
> push the patch to David/netdev.

I appreciate the marge your tree.
If there is anything I can do, please let me know.

And I know this patch is good for e100 driver too.
I would really like to create patch for it.
How do you think about e100 driver.


Thanks,
Naohiro Ooiwa

Re: [PATCH] e1000: the power down when running ifdown command

[Jeff Kirsher]

2009/11/3 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
> Jeff Kirsher wrote:
>> 2009/10/31 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
>>
>> I have added this patch to my tree for testing.  This patch requires a
>> fair amount of regression testing, so once its passed testing I will
>> push the patch to David/netdev.
>
> I appreciate the marge your tree.
> If there is anything I can do, please let me know.
>
> And I know this patch is good for e100 driver too.
> I would really like to create patch for it.
> How do you think about e100 driver.
>
>
> Thanks,
> Naohiro Ooiwa
>

Patches are always welcome (referring to a e100 patch).

As far as the e1000 patch goes, it has a number of issues which were
found in testing.  Here are just a few problems we saw:
1. ethtool -t - crashes the system
2. ethtool eth0 - always shows link/speed as 1000/Full even when there
is no cable
3. ethtool -s eth0 autoneg on/off - system hang. Sometimes a copper
interface will show up as fiber after this.
4. ethtool -d/-S/-g etc - will corrupt the stats of the interface
while doing ifup/down

So it appears that more needs to be done to the driver to get this to
work as expected.

NAK

-- 
Cheers,
Jeff

Re: [PATCH] e1000: the power down when running ifdown command

[Naohiro Ooiwa]

Naohiro Ooiwa wrote:
> Stephen Hemminger wrote:
>> On Sat, 31 Oct 2009 18:39:52 +0900
>> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
>>
>> Does this work with Wake On Lan? 
> 
> Yes, it works WOL.

Sorry, I made a mistake.
The WOL doesn't work when my patch applied to kernel.
I wasn't myself.

I consider the WOL and I will resent the patch.
Thank you for your point.


thanks,
Naohiro Ooiwa


> But I worry that my test is enough.
> 
> They are following:
>   - simple data transmission after ifdown;ifup.
>   - enable wol, ifup network device, system shutdown, and make sure wol work.
>   - enable wol, ifdown network device, system shutdown, and make sure wol work.
>   - while [ 0 ] ; do ifdown eth0 ; ifup eth0 ; done
>   - while [ 0 ] ; do modprobe e1000 ; rmmod e1000 ; done
> 
> 
>> @@ -1265,6 +1287,7 @@ static int e1000_open(struct net_device *netdev)
>>  		goto err_setup_rx;
>>
>>  	e1000_power_up_phy(adapter);
>> +	e1000_reset(adapter);
>>
>>  	adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
>>  	if ((hw->mng_cookie.status &
> 
> This code fix problem that e1000 driver doesn't work to auto-negotiation
> once in a while.
> Maybe, the cause is that set state to D0 just before it.
> I found it by repeat of ifup and ifdown.
> 
> If you find out other points and any necessary tests from my patch,
> please tell me. I will make sure them.
> 
> Thanks you.
> Naohiro Ooiwa
> 
> --
> 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
> 

Re: [PATCH] e1000: the power down when running ifdown command

[Naohiro Ooiwa]

Jeff Kirsher wrote:
> 2009/11/3 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
>> Jeff Kirsher wrote:
>>> 2009/10/31 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
>>>
>>> I have added this patch to my tree for testing.  This patch requires a
>>> fair amount of regression testing, so once its passed testing I will
>>> push the patch to David/netdev.
>> I appreciate the marge your tree.
>> If there is anything I can do, please let me know.
>>
>> And I know this patch is good for e100 driver too.
>> I would really like to create patch for it.
>> How do you think about e100 driver.
>>
>>
>> Thanks,
>> Naohiro Ooiwa
>>
> 
> Patches are always welcome (referring to a e100 patch).

I am happy that you should say that.
I will try to create a patch for e100, e1000e, igb and ixgbe.
Before that, I should fix the following problems.


> As far as the e1000 patch goes, it has a number of issues which were
> found in testing.  Here are just a few problems we saw:
> 1. ethtool -t - crashes the system
> 2. ethtool eth0 - always shows link/speed as 1000/Full even when there
> is no cable
> 3. ethtool -s eth0 autoneg on/off - system hang. Sometimes a copper
> interface will show up as fiber after this.
> 4. ethtool -d/-S/-g etc - will corrupt the stats of the interface
> while doing ifup/down

Thank you for your tests.
Oh, My patch is full of problem.
The rest is my work.

I will resend the patch after test of all ethtool's options.
At that time, I will tell you contents of my tests.

And I sad WOL works on my patch in previous mail.
But WOL doesn't work. Sorry, I wasn't myself.
I will fix it too.

> 
> So it appears that more needs to be done to the driver to get this to
> work as expected.
> 
> NAK
> 

Re: [PATCH] e1000: the power down when running ifdown command

[Stephen Hemminger]

On Wed, 04 Nov 2009 19:23:43 +0900
Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:

> Naohiro Ooiwa wrote:
> > Stephen Hemminger wrote:
> >> On Sat, 31 Oct 2009 18:39:52 +0900
> >> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
> >>
> >> Does this work with Wake On Lan? 
> > 
> > Yes, it works WOL.
> 
> Sorry, I made a mistake.
> The WOL doesn't work when my patch applied to kernel.
> I wasn't myself.
> 
> I consider the WOL and I will resent the patch.
> Thank you for your point.
> 
> 
> thanks,
> Naohiro Ooiwa


Good, thank you for checking. I like the idea of powering down the
PHY. Some of the drivers have shutdown hooks to power PHY back on
during shutdown to enable WOL.


-- 

RE: [E1000-devel] [PATCH] e1000: the power down when running ifdown command

[Allan, Bruce W]

FWIW, in e1000e the PHY is already powered down when the interface is brought down if WoL and manageability are disabled.  There is an issue where the PHY remains powered on when the driver is removed for which I have a patch already queued up here at Intel.

>-----Original Message-----
>From: Stephen Hemminger [mailto:shemminger@vyatta.com]
>Sent: Wednesday, November 04, 2009 8:09 AM
>To: Naohiro Ooiwa
>Cc: e1000-devel@lists.sourceforge.net; netdev@vger.kernel.org; Brandeburg,
>Jesse; Ronciak, John; Kirsher, Jeffrey T; Andrew Morton;
>svaidy@linux.vnet.ibm.com; davem@davemloft.net
>Subject: Re: [E1000-devel] [PATCH] e1000: the power down when running
>ifdown command
>
>On Wed, 04 Nov 2009 19:23:43 +0900
>Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
>
>> Naohiro Ooiwa wrote:
>> > Stephen Hemminger wrote:
>> >> On Sat, 31 Oct 2009 18:39:52 +0900
>> >> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
>> >>
>> >> Does this work with Wake On Lan?
>> >
>> > Yes, it works WOL.
>>
>> Sorry, I made a mistake.
>> The WOL doesn't work when my patch applied to kernel.
>> I wasn't myself.
>>
>> I consider the WOL and I will resent the patch.
>> Thank you for your point.
>>
>>
>> thanks,
>> Naohiro Ooiwa
>
>
>Good, thank you for checking. I like the idea of powering down the
>PHY. Some of the drivers have shutdown hooks to power PHY back on
>during shutdown to enable WOL.
>
>
>--
>
>--------------------------------------------------------------------------
>----
>Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-
>Day
>trial. Simplify your report design, integration and deployment - and focus
>on
>what you do best, core application coding. Discover what's new with
>Crystal Reports now.  http://p.sf.net/sfu/bobj-july
>_______________________________________________
>E1000-devel mailing list
>E1000-devel@lists.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/e1000-devel

Re: [PATCH] e1000: the power down when running ifdown command

[Naohiro Ooiwa]

Stephen Hemminger wrote:
> On Wed, 04 Nov 2009 19:23:43 +0900
> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
> 
>> Naohiro Ooiwa wrote:
>>> Stephen Hemminger wrote:
>>>> On Sat, 31 Oct 2009 18:39:52 +0900
>>>> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
>>>>
>>>> Does this work with Wake On Lan? 
>>> Yes, it works WOL.
>> Sorry, I made a mistake.
>> The WOL doesn't work when my patch applied to kernel.
>> I wasn't myself.
>>
>> I consider the WOL and I will resent the patch.
>> Thank you for your point.
>>
>>
>> thanks,
>> Naohiro Ooiwa
> 
> 
> Good, thank you for checking. I like the idea of powering down the
> PHY. Some of the drivers have shutdown hooks to power PHY back on
> during shutdown to enable WOL.
> 

Thank you so much for your infomation.
Oh, The shutdown hooks is useful.


Thanks.
Naohiro Ooiwa

[PATCH] e1000: the power down when running ifdown command

[Naohiro Ooiwa]

I resend the mail to maintainers of the e1000 driver.
and Andrew CC-ed.

-------- Original Message --------

Hi all

I'm trying to modify e1000 driver for power saving.

The e1000 driver doesn't let the power down when running ifdown command.
So, I set to the D3hot state of a PCI device at the end of e1000_close().

With this modification, e1000 driver reduces power by ifdown
to the same level as link-down case.

I spoke it in Collaboration Summit 2009.
For details and result of power measurement,
please refer the 36-38 page of following document.
http://events.linuxfoundation.org/archive/lfcs09_ooiwa.pdf

Could you please check the my patch ?

Should I consider WOL ?
Dosen't E1000_PCI_POWER_SAVE need ?


Thanks you.
Naohiro Ooiwa

Signed-off-by: Naohiro Ooiwa <nooiwa@miraclelinux.com>
---
 drivers/net/e1000/e1000_main.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index bcd192c..12e1a42 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -26,6 +26,11 @@

 *******************************************************************************/

+/*
+ * define this if you want pci save power while ifdown.
+ */
+#define E1000_PCI_POWER_SAVE
+
 #include "e1000.h"
 #include <net/ip6_checksum.h>

@@ -1248,6 +1253,23 @@ static int e1000_open(struct net_device *netdev)
 	struct e1000_hw *hw = &adapter->hw;
 	int err;

+#ifdef E1000_PCI_POWER_SAVE
+	struct pci_dev *pdev = adapter->pdev;
+
+	pci_set_power_state(pdev, PCI_D0);
+	pci_restore_state(pdev);
+
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		err = pci_enable_device_mem(pdev);
+	if (err) {
+		printk(KERN_ERR "e1000: Cannot enable PCI device from power-save\n");
+		return err;
+	}
+	pci_set_master(pdev);
+#endif
+
 	/* disallow open during test */
 	if (test_bit(__E1000_TESTING, &adapter->flags))
 		return -EBUSY;
@@ -1265,6 +1287,7 @@ static int e1000_open(struct net_device *netdev)
 		goto err_setup_rx;

 	e1000_power_up_phy(adapter);
+	e1000_reset(adapter);

 	adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
 	if ((hw->mng_cookie.status &
@@ -1341,6 +1364,15 @@ static int e1000_close(struct net_device *netdev)
 		e1000_vlan_rx_kill_vid(netdev, adapter->mng_vlan_id);
 	}

+#ifdef E1000_PCI_POWER_SAVE
+#ifdef CONFIG_PM
+	pci_save_state(adapter->pdev);
+#endif
+	pci_disable_device(adapter->pdev);
+	pci_wake_from_d3(adapter->pdev, true);
+	pci_set_power_state(adapter->pdev, PCI_D3hot);
+#endif
+
 	return 0;
 }

-- 
1.5.4.1

--
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] e1000: the power down when running ifdown command

[Naohiro Ooiwa]

Hi all

I'm trying to modify e1000 driver for power saving.

The e1000 driver doesn't let the power down when running ifdown command.
So, I set to the D3hot state of a PCI device at the end of e1000_close().

With this modification, e1000 driver reduces power by ifdown
to the same level as link-down case.

I spoke it in Collaboration Summit 2009.
For details and result of power measurement,
please refer the 36-38 page of following document.
http://events.linuxfoundation.org/archive/lfcs09_ooiwa.pdf

Could you please check the my patch ?

Should I consider WOL ?
Dosen't E1000_PCI_POWER_SAVE need ?


Hi Vaidy

I was glad that you talked to me in Collaboration Summit.
I'm sorry for sending the patch so late.

I found a bug in the patch which I used in Collaboration Summit.
Sometimes, it's don't work to auto-negotiation by repeated ifup and ifdown.

I fixed it. I'd appreciate it if you could test.


Thanks you.
Naohiro Ooiwa

Signed-off-by: Naohiro Ooiwa <nooiwa@miraclelinux.com>
---
 drivers/net/e1000/e1000_main.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index bcd192c..12e1a42 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -26,6 +26,11 @@

 *******************************************************************************/

+/*
+ * define this if you want pci save power while ifdown.
+ */
+#define E1000_PCI_POWER_SAVE
+
 #include "e1000.h"
 #include <net/ip6_checksum.h>

@@ -1248,6 +1253,23 @@ static int e1000_open(struct net_device *netdev)
 	struct e1000_hw *hw = &adapter->hw;
 	int err;

+#ifdef E1000_PCI_POWER_SAVE
+	struct pci_dev *pdev = adapter->pdev;
+
+	pci_set_power_state(pdev, PCI_D0);
+	pci_restore_state(pdev);
+
+	if (adapter->need_ioport)
+		err = pci_enable_device(pdev);
+	else
+		err = pci_enable_device_mem(pdev);
+	if (err) {
+		printk(KERN_ERR "e1000: Cannot enable PCI device from power-save\n");
+		return err;
+	}
+	pci_set_master(pdev);
+#endif
+
 	/* disallow open during test */
 	if (test_bit(__E1000_TESTING, &adapter->flags))
 		return -EBUSY;
@@ -1265,6 +1287,7 @@ static int e1000_open(struct net_device *netdev)
 		goto err_setup_rx;

 	e1000_power_up_phy(adapter);
+	e1000_reset(adapter);

 	adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
 	if ((hw->mng_cookie.status &
@@ -1341,6 +1364,15 @@ static int e1000_close(struct net_device *netdev)
 		e1000_vlan_rx_kill_vid(netdev, adapter->mng_vlan_id);
 	}

+#ifdef E1000_PCI_POWER_SAVE
+#ifdef CONFIG_PM
+	pci_save_state(adapter->pdev);
+#endif
+	pci_disable_device(adapter->pdev);
+	pci_wake_from_d3(adapter->pdev, true);
+	pci_set_power_state(adapter->pdev, PCI_D3hot);
+#endif
+
 	return 0;
 }

-- 
1.5.4.1