Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 1/2] e1000e: Only disable ASPM on 82573L devices

4 views
Skip to first unread message

Matthew Garrett

unread,
Feb 9, 2010, 6:10:01 PM2/9/10
to
The 82537 errata and comment in e1000e_disable_l1aspm both agree that
only 82537L devices are affected. Limit the L1 disable to them.

Signed-off-by: Matthew Garrett <m...@redhat.com>
---
drivers/net/e1000e/netdev.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 57f149b..27eed81 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4642,6 +4642,10 @@ static void e1000e_disable_l1aspm(struct pci_dev *pdev)
* Unfortunately this feature saves about 1W power consumption when
* active.
*/
+
+ if (pdev->device != E1000_DEV_ID_82573L)
+ return;
+
pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &val);
if (val & 0x2) {
--
1.6.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Matthew Garrett

unread,
Feb 11, 2010, 11:30:02 AM2/11/10
to

Brandeburg, Jesse

unread,
Feb 11, 2010, 12:20:01 PM2/11/10
to

On Thu, 11 Feb 2010, Matthew Garrett wrote:

> The 82537 errata and comment in e1000e_disable_l1aspm both agree that
> only 82537L devices are affected. Limit the L1 disable to them.

Hi Matthew, its a nitpick, but can you please change the part number to
the correct one? it should be s/82537/82573


> Signed-off-by: Matthew Garrett <m...@redhat.com>
> ---
> drivers/net/e1000e/netdev.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 57f149b..27eed81 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -4642,6 +4642,10 @@ static void e1000e_disable_l1aspm(struct pci_dev *pdev)
> * Unfortunately this feature saves about 1W power consumption when
> * active.
> */
> +
> + if (pdev->device != E1000_DEV_ID_82573L)
> + return;
> +
> pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
> pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &val);
> if (val & 0x2) {
>
--

Matthew Garrett

unread,
Feb 11, 2010, 12:50:02 PM2/11/10
to
On Thu, Feb 11, 2010 at 09:17:12AM -0800, Brandeburg, Jesse wrote:
>
>
> On Thu, 11 Feb 2010, Matthew Garrett wrote:
>
> > The 82537 errata and comment in e1000e_disable_l1aspm both agree that
> > only 82537L devices are affected. Limit the L1 disable to them.
>
> Hi Matthew, its a nitpick, but can you please change the part number to
> the correct one? it should be s/82537/82573

Whoops! Sorry, I'll do that.

--
Matthew Garrett | mj...@srcf.ucam.org

Matthew Garrett

unread,
Feb 11, 2010, 1:20:01 PM2/11/10
to
The 82537 errata and comment in e1000e_disable_l1aspm both agree that
only 82537L devices are affected. Limit the L1 disable to them.

Signed-off-by: Matthew Garrett <m...@redhat.com>


---
drivers/net/e1000e/netdev.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 57f149b..27eed81 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -4642,6 +4642,10 @@ static void e1000e_disable_l1aspm(struct pci_dev *pdev)
* Unfortunately this feature saves about 1W power consumption when
* active.
*/
+
+ if (pdev->device != E1000_DEV_ID_82573L)
+ return;
+
pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &val);
if (val & 0x2) {
--

1.6.6.1

David Miller

unread,
Feb 12, 2010, 3:40:02 PM2/12/10
to

Intel folks, are you looking at these patches?

Thanks.

Brandeburg, Jesse

unread,
Feb 12, 2010, 4:00:02 PM2/12/10
to

On Fri, 12 Feb 2010, David Miller wrote:
> Intel folks, are you looking at these patches?

Hi Dave, yes we're looking at them but due to Bruce being out, we need to
wait until next week to reply. If you can wait, we'll make sure they come
through Jeff or receive comments after full review.

It's a bit of a scary area to mess with as there were lots of ASPM related
bugs in the hardware in this hardware generation (system chipsets and
82573).

Allan, Bruce W

unread,
Feb 19, 2010, 5:00:02 PM2/19/10
to
On Thursday, February 11, 2010 10:15 AM, Matthew Garrett wrote:
> The 82537 errata and comment in e1000e_disable_l1aspm both agree that
> only 82537L devices are affected. Limit the L1 disable to them.
>
> Signed-off-by: Matthew Garrett <m...@redhat.com>
> ---
> drivers/net/e1000e/netdev.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
> index 57f149b..27eed81 100644
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -4642,6 +4642,10 @@ static void e1000e_disable_l1aspm(struct
> pci_dev *pdev)
> * Unfortunately this feature saves about 1W power consumption when
> * active.
> */
> +
> + if (pdev->device != E1000_DEV_ID_82573L)
> + return;
> +
> pos = pci_find_capability(pdev, PCI_CAP_ID_EXP);
> pci_read_config_word(pdev, pos + PCI_EXP_LNKCTL, &val);
> if (val & 0x2) {

Hi Matthew,

Exactly which erratum are you referring to? Erratum 17 in the 82573 Specification Update? If that is the case, I see the possibility of another interpretation of the erratum which suggests the possibility of the same issue on other variants of the 82573 when using standard frame sizes and ASPM enabled. Not to mention, I believe there may be other parts (82574 perhaps) that will have issues with L1 ASPM enabled. I will follow-up with the folks who did the investigation that resulted in the erratum in order to get a clearer picture of all this, and take a look into other parts that may likewise be affected.

Thanks,
Bruce.--

Matthew Garrett

unread,
Feb 19, 2010, 5:10:02 PM2/19/10
to
On Fri, Feb 19, 2010 at 01:53:01PM -0800, Allan, Bruce W wrote:

> Exactly which erratum are you referring to? Erratum 17 in the 82573
> Specification Update? If that is the case, I see the possibility of
> another interpretation of the erratum which suggests the possibility
> of the same issue on other variants of the 82573 when using standard
> frame sizes and ASPM enabled. Not to mention, I believe there may be
> other parts (82574 perhaps) that will have issues with L1 ASPM
> enabled. I will follow-up with the folks who did the investigation
> that resulted in the erratum in order to get a clearer picture of all
> this, and take a look into other parts that may likewise be affected.

Ah, yes - I see that it could be interpreted that way. The description
seems to suggest that it's only relevant if ERT is enabled, which is
required for jumbo frames. I'm not entirely clear on whether ERT is
enabled in other circumstances? If not, we ought to be able to limit
this to the L device - if not, it should be done on E and V as well.

The 82574 specification update doesn't mention any ASPM errata, but if
you're able to check then that would be great. My main aim here is to
try to get it turned back on on hardware where this works, since it's a
measurable power saving.

--
Matthew Garrett | mj...@srcf.ucam.org
--

0 new messages