* [PATCH] r8169: add module param for control of ASPM disable
@ 2011-11-11 23:05 Todd Broch
2011-11-12 4:59 ` Matthew Garrett
0 siblings, 1 reply; 6+ messages in thread
From: Todd Broch @ 2011-11-11 23:05 UTC (permalink / raw)
To: Realtek linux nic maintainers, Francois Romieu; +Cc: netdev, Todd Broch
ASPM has been disabled in this driver by default as its been
implicated in stability issues on at least one platform. This CL adds
a module parameter to allow control of ASPM disable. The default
value is to enable ASPM again as its provides signficant (200mW) power
savings on the platform I tested.
Signed-off-by: Todd Broch <tbroch@chromium.org>
---
drivers/net/r8169.c | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 5f838ef..05769fa0 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -671,6 +671,10 @@ struct rtl8169_private {
#define RTL_FIRMWARE_UNKNOWN ERR_PTR(-EAGAIN);
};
+static int aspm_disable = 0;
+module_param(aspm_disable, bool, 0444);
+MODULE_PARM_DESC(aspm_disable, "Disable ASPM completely.");
+
MODULE_AUTHOR("Realtek and the Linux r8169 crew <netdev@vger.kernel.org>");
MODULE_DESCRIPTION("RealTek RTL-8169 Gigabit Ethernet driver");
module_param(use_dac, int, 0);
@@ -3291,8 +3295,12 @@ rtl8169_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
/* disable ASPM completely as that cause random device stop working
* problems as well as full system hangs for some PCIe devices users */
- pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1 |
- PCIE_LINK_STATE_CLKPM);
+ if (aspm_disable) {
+ pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S |
+ PCIE_LINK_STATE_L1 |
+ PCIE_LINK_STATE_CLKPM);
+ dprintk("ASPM disabled");
+ }
/* enable device (incl. PCI PM wakeup and hotplug setup) */
rc = pci_enable_device(pdev);
--
1.7.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] r8169: add module param for control of ASPM disable
2011-11-11 23:05 [PATCH] r8169: add module param for control of ASPM disable Todd Broch
@ 2011-11-12 4:59 ` Matthew Garrett
2011-11-12 10:46 ` Francois Romieu
0 siblings, 1 reply; 6+ messages in thread
From: Matthew Garrett @ 2011-11-12 4:59 UTC (permalink / raw)
To: Todd Broch; +Cc: Realtek linux nic maintainers, Francois Romieu, netdev
On Fri, Nov 11, 2011 at 03:05:55PM -0800, Todd Broch wrote:
> ASPM has been disabled in this driver by default as its been
> implicated in stability issues on at least one platform. This CL adds
> a module parameter to allow control of ASPM disable. The default
> value is to enable ASPM again as its provides signficant (200mW) power
> savings on the platform I tested.
The default is then to break some hardware. That's not reasonable.
Instead, we need to identify which chip versions are broken and limit
the blacklist to them.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] r8169: add module param for control of ASPM disable
2011-11-12 4:59 ` Matthew Garrett
@ 2011-11-12 10:46 ` Francois Romieu
[not found] ` <CA+iF6Rog3ptpmQZzhcRODmZUKN18_uw5t9xfpQjbJ86qKUA0eQ@mail.gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Francois Romieu @ 2011-11-12 10:46 UTC (permalink / raw)
To: Todd Broch
Cc: Matthew Garrett, Realtek linux nic maintainers, netdev,
Hayes Wang
Matthew Garrett <mjg59@srcf.ucam.org> :
> On Fri, Nov 11, 2011 at 03:05:55PM -0800, Todd Broch wrote:
> > ASPM has been disabled in this driver by default as its been
> > implicated in stability issues on at least one platform. This CL adds
> > a module parameter to allow control of ASPM disable. The default
> > value is to enable ASPM again as its provides signficant (200mW) power
> > savings on the platform I tested.
>
> The default is then to break some hardware. That's not reasonable.
> Instead, we need to identify which chip versions are broken and limit
> the blacklist to them.
Yes, assuming it's only a device problem.
Todd, which chipset(s) (XID or such) was it tested with ?
It would be nice to know the kind of use as well (desktop / laptop / server).
Btw, what is a 'CL' ?
--
Ueimor
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-11-17 21:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-11 23:05 [PATCH] r8169: add module param for control of ASPM disable Todd Broch
2011-11-12 4:59 ` Matthew Garrett
2011-11-12 10:46 ` Francois Romieu
[not found] ` <CA+iF6Rog3ptpmQZzhcRODmZUKN18_uw5t9xfpQjbJ86qKUA0eQ@mail.gmail.com>
2011-11-15 16:32 ` Matthew Garrett
2011-11-17 9:37 ` Francois Romieu
[not found] ` <CA+iF6RqKScip+6w-tzo3sF+hEsYDUefED29KEBrx5TDaZaTEbA@mail.gmail.com>
2011-11-17 21:26 ` Matthew Garrett
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).