Patchwork [07/12] e1000e: Use standard #defines for PCIe Capability ASPM fields

login
register
mail settings
Submitter Bjorn Helgaas
Date Dec. 5, 2012, 8:57 p.m.
Message ID <20121205205755.13851.77284.stgit@bhelgaas.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/203951/
State Not Applicable
Headers show

Comments

Bjorn Helgaas - Dec. 5, 2012, 8:57 p.m.
Use the standard #defines for PCIe Capability ASPM fields.

Previously we used PCIE_LINK_STATE_L0S and PCIE_LINK_STATE_L1 directly, but
these are defined for the Linux ASPM interfaces, e.g.,
pci_disable_link_state(), and only coincidentally match the actual register
bits.  PCIE_LINK_STATE_CLKPM, also part of that interface, does not match
the register bit.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Bruce Allan <bruce.w.allan@intel.com>
CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
CC: Don Skidmore <donald.c.skidmore@intel.com>
CC: Greg Rose <gregory.v.rose@intel.com>
CC: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
CC: Alex Duyck <alexander.h.duyck@intel.com>
CC: John Ronciak <john.ronciak@intel.com>
CC: e1000-devel@lists.sourceforge.net
---
 drivers/net/ethernet/intel/e1000e/netdev.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Kirsher - Dec. 5, 2012, 10:05 p.m.
On Wed, 2012-12-05 at 13:57 -0700, Bjorn Helgaas wrote:
> Use the standard #defines for PCIe Capability ASPM fields.
> 
> Previously we used PCIE_LINK_STATE_L0S and PCIE_LINK_STATE_L1
> directly, but
> these are defined for the Linux ASPM interfaces, e.g.,
> pci_disable_link_state(), and only coincidentally match the actual
> register
> bits.  PCIE_LINK_STATE_CLKPM, also part of that interface, does not
> match
> the register bit.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
> CC: Bruce Allan <bruce.w.allan@intel.com>
> CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
> CC: Don Skidmore <donald.c.skidmore@intel.com>
> CC: Greg Rose <gregory.v.rose@intel.com>
> CC: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> CC: Alex Duyck <alexander.h.duyck@intel.com>
> CC: John Ronciak <john.ronciak@intel.com>
> CC: e1000-devel@lists.sourceforge.net
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-) 

Thanks I will add this patch to my queue for e1000e.  Since this is
patch 7 of 12, are there PCI dependent patches in the series that need
to be applied before?
Bjorn Helgaas - Dec. 5, 2012, 10:07 p.m.
On Wed, Dec 5, 2012 at 3:05 PM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> On Wed, 2012-12-05 at 13:57 -0700, Bjorn Helgaas wrote:
>> Use the standard #defines for PCIe Capability ASPM fields.
>>
>> Previously we used PCIE_LINK_STATE_L0S and PCIE_LINK_STATE_L1
>> directly, but
>> these are defined for the Linux ASPM interfaces, e.g.,
>> pci_disable_link_state(), and only coincidentally match the actual
>> register
>> bits.  PCIE_LINK_STATE_CLKPM, also part of that interface, does not
>> match
>> the register bit.
>>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
>> CC: Bruce Allan <bruce.w.allan@intel.com>
>> CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
>> CC: Don Skidmore <donald.c.skidmore@intel.com>
>> CC: Greg Rose <gregory.v.rose@intel.com>
>> CC: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
>> CC: Alex Duyck <alexander.h.duyck@intel.com>
>> CC: John Ronciak <john.ronciak@intel.com>
>> CC: e1000-devel@lists.sourceforge.net
>> ---
>>  drivers/net/ethernet/intel/e1000e/netdev.c |   11 +++++++++--
>>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> Thanks I will add this patch to my queue for e1000e.  Since this is
> patch 7 of 12, are there PCI dependent patches in the series that need
> to be applied before?

Yes.  It does depend on a previous patch that adds the #defines to
include/uapi/linux/pci_regs.h
(http://marc.info/?l=linux-pci&m=135474107109010&w=2).

I plan to merge that during the v3.8 merge window, so you can merge it
after that.  Or, if it won't cause you conflicts, I can include the
e1000e change in my PCI tree.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jeff Kirsher - Dec. 5, 2012, 10:24 p.m.
On Wed, 2012-12-05 at 15:07 -0700, Bjorn Helgaas wrote:
> On Wed, Dec 5, 2012 at 3:05 PM, Jeff Kirsher
> <jeffrey.t.kirsher@intel.com> wrote:
> > On Wed, 2012-12-05 at 13:57 -0700, Bjorn Helgaas wrote:
> >> Use the standard #defines for PCIe Capability ASPM fields.
> >>
> >> Previously we used PCIE_LINK_STATE_L0S and PCIE_LINK_STATE_L1
> >> directly, but
> >> these are defined for the Linux ASPM interfaces, e.g.,
> >> pci_disable_link_state(), and only coincidentally match the actual
> >> register
> >> bits.  PCIE_LINK_STATE_CLKPM, also part of that interface, does not
> >> match
> >> the register bit.
> >>
> >> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> >> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> >> CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
> >> CC: Bruce Allan <bruce.w.allan@intel.com>
> >> CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
> >> CC: Don Skidmore <donald.c.skidmore@intel.com>
> >> CC: Greg Rose <gregory.v.rose@intel.com>
> >> CC: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> >> CC: Alex Duyck <alexander.h.duyck@intel.com>
> >> CC: John Ronciak <john.ronciak@intel.com>
> >> CC: e1000-devel@lists.sourceforge.net
> >> ---
> >>  drivers/net/ethernet/intel/e1000e/netdev.c |   11 +++++++++--
> >>  1 files changed, 9 insertions(+), 2 deletions(-)
> >
> > Thanks I will add this patch to my queue for e1000e.  Since this is
> > patch 7 of 12, are there PCI dependent patches in the series that need
> > to be applied before?
> 
> Yes.  It does depend on a previous patch that adds the #defines to
> include/uapi/linux/pci_regs.h
> (http://marc.info/?l=linux-pci&m=135474107109010&w=2).
> 
> I plan to merge that during the v3.8 merge window, so you can merge it
> after that.  Or, if it won't cause you conflicts, I can include the
> e1000e change in my PCI tree.
> 
> Bjorn

yeah, it does not apply cleanly (because of patches currently in the
queue from Bruce).  So I will take care of pushing your e1000e patch
through Dave's tree.
Bjorn Helgaas - Jan. 4, 2013, 7:33 p.m.
On Wed, Dec 5, 2012 at 3:24 PM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> On Wed, 2012-12-05 at 15:07 -0700, Bjorn Helgaas wrote:
>> On Wed, Dec 5, 2012 at 3:05 PM, Jeff Kirsher
>> <jeffrey.t.kirsher@intel.com> wrote:
>> > On Wed, 2012-12-05 at 13:57 -0700, Bjorn Helgaas wrote:
>> >> Use the standard #defines for PCIe Capability ASPM fields.
>> >>
>> >> Previously we used PCIE_LINK_STATE_L0S and PCIE_LINK_STATE_L1
>> >> directly, but
>> >> these are defined for the Linux ASPM interfaces, e.g.,
>> >> pci_disable_link_state(), and only coincidentally match the actual
>> >> register
>> >> bits.  PCIE_LINK_STATE_CLKPM, also part of that interface, does not
>> >> match
>> >> the register bit.
>> >>
>> >> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> >> CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> >> CC: Jesse Brandeburg <jesse.brandeburg@intel.com>
>> >> CC: Bruce Allan <bruce.w.allan@intel.com>
>> >> CC: Carolyn Wyborny <carolyn.wyborny@intel.com>
>> >> CC: Don Skidmore <donald.c.skidmore@intel.com>
>> >> CC: Greg Rose <gregory.v.rose@intel.com>
>> >> CC: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
>> >> CC: Alex Duyck <alexander.h.duyck@intel.com>
>> >> CC: John Ronciak <john.ronciak@intel.com>
>> >> CC: e1000-devel@lists.sourceforge.net
>> >> ---
>> >>  drivers/net/ethernet/intel/e1000e/netdev.c |   11 +++++++++--
>> >>  1 files changed, 9 insertions(+), 2 deletions(-)
>> >
>> > Thanks I will add this patch to my queue for e1000e.  Since this is
>> > patch 7 of 12, are there PCI dependent patches in the series that need
>> > to be applied before?
>>
>> Yes.  It does depend on a previous patch that adds the #defines to
>> include/uapi/linux/pci_regs.h
>> (http://marc.info/?l=linux-pci&m=135474107109010&w=2).
>>
>> I plan to merge that during the v3.8 merge window, so you can merge it
>> after that.  Or, if it won't cause you conflicts, I can include the
>> e1000e change in my PCI tree.
>>
>> Bjorn
>
> yeah, it does not apply cleanly (because of patches currently in the
> queue from Bruce).  So I will take care of pushing your e1000e patch
> through Dave's tree.

The pci_regs.h change this depends on appeared in v3.8-rc2, so this
patch can be merged any time, at least as far as that particular
issue.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" 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/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index f444eb0..b8561ef 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -5613,15 +5613,22 @@  static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
 #else
 static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
 {
+	u16 aspm_ctl = 0;
+
+	if (state & PCIE_LINK_STATE_L0S)
+		aspm_ctl |= PCI_EXP_LNKCTL_ASPM_L0S;
+	if (state & PCIE_LINK_STATE_L1)
+		aspm_ctl |= PCI_EXP_LNKCTL_ASPM_L1;
+
 	/*
 	 * Both device and parent should have the same ASPM setting.
 	 * Disable ASPM in downstream component first and then upstream.
 	 */
-	pcie_capability_clear_word(pdev, PCI_EXP_LNKCTL, state);
+	pcie_capability_clear_word(pdev, PCI_EXP_LNKCTL, aspm_ctl);
 
 	if (pdev->bus->self)
 		pcie_capability_clear_word(pdev->bus->self, PCI_EXP_LNKCTL,
-					   state);
+					   aspm_ctl);
 }
 #endif
 static void e1000e_disable_aspm(struct pci_dev *pdev, u16 state)