Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754488Ab0LFWVX (ORCPT ); Mon, 6 Dec 2010 17:21:23 -0500 Received: from ogre.sisk.pl ([217.79.144.158]:59260 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753335Ab0LFWVV (ORCPT ); Mon, 6 Dec 2010 17:21:21 -0500 From: "Rafael J. Wysocki" To: Matthew Garrett Subject: Re: [PATCH v2] PCI: Disable ASPM if BIOS asks us to Date: Mon, 6 Dec 2010 23:20:36 +0100 User-Agent: KMail/1.13.5 (Linux/2.6.37-rc4+; KDE/4.4.4; x86_64; ; ) Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, jbarnes@virtuousgeek.org References: <1291662056-6055-1-git-send-email-mjg@redhat.com> In-Reply-To: <1291662056-6055-1-git-send-email-mjg@redhat.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-2" Content-Transfer-Encoding: 7bit Message-Id: <201012062320.36304.rjw@sisk.pl> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4931 Lines: 137 On Monday, December 06, 2010, Matthew Garrett wrote: > We currently refuse to touch the ASPM registers if the BIOS tells us that > ASPM isn't supported. This can cause problems if the BIOS has (for any > reason) enabled ASPM on some devices anyway. Change the code such that we > explicitly clear ASPM if the FADT indicates that ASPM isn't supported, > and make sure we tidy up appropriately on device removal in order to deal > with the hotplug case. If ASPM is disabled because the BIOS doesn't hand > over control then we won't touch the registers. > > Signed-off-by: Matthew Garrett Acked-by: Rafael J. Wysocki > --- > > Implement Rafael's suggestion to use two separate functions, and also > ensure that we clear the clkpm bit as well as the ASPM bits. > > drivers/pci/pci-acpi.c | 1 + > drivers/pci/pcie/aspm.c | 21 +++++++++++++++++---- > include/linux/pci-aspm.h | 5 ++++- > 3 files changed, 22 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > index 24e19c5..d7ea699 100644 > --- a/drivers/pci/pci-acpi.c > +++ b/drivers/pci/pci-acpi.c > @@ -399,6 +399,7 @@ static int __init acpi_pci_init(void) > > if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) { > printk(KERN_INFO"ACPI FADT declares the system doesn't support PCIe ASPM, so disable it\n"); > + pcie_clear_aspm(); > pcie_no_aspm(); > } > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index 7122281..8112415 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -68,7 +68,7 @@ struct pcie_link_state { > struct aspm_latency acceptable[8]; > }; > > -static int aspm_disabled, aspm_force; > +static int aspm_disabled, aspm_force, aspm_clear_state; > static DEFINE_MUTEX(aspm_lock); > static LIST_HEAD(link_list); > > @@ -139,7 +139,7 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable) > { > /* Don't enable Clock PM if the link is not Clock PM capable */ > if (!link->clkpm_capable && enable) > - return; > + enable = 0; > /* Need nothing if the specified equals to current state */ > if (link->clkpm_enabled == enable) > return; > @@ -498,6 +498,10 @@ static int pcie_aspm_sanity_check(struct pci_dev *pdev) > struct pci_dev *child; > int pos; > u32 reg32; > + > + if (aspm_clear_state) > + return -EINVAL; > + > /* > * Some functions in a slot might not all be PCIe functions, > * very strange. Disable ASPM for the whole slot > @@ -563,12 +567,15 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev) > struct pcie_link_state *link; > int blacklist = !!pcie_aspm_sanity_check(pdev); > > - if (aspm_disabled || !pci_is_pcie(pdev) || pdev->link_state) > + if (!pci_is_pcie(pdev) || pdev->link_state) > return; > if (pdev->pcie_type != PCI_EXP_TYPE_ROOT_PORT && > pdev->pcie_type != PCI_EXP_TYPE_DOWNSTREAM) > return; > > + if (aspm_disabled && !aspm_clear_state) > + return; > + > /* VIA has a strange chipset, root port is under a bridge */ > if (pdev->pcie_type == PCI_EXP_TYPE_ROOT_PORT && > pdev->bus->self) > @@ -641,7 +648,7 @@ void pcie_aspm_exit_link_state(struct pci_dev *pdev) > struct pci_dev *parent = pdev->bus->self; > struct pcie_link_state *link, *root, *parent_link; > > - if (aspm_disabled || !pci_is_pcie(pdev) || > + if ((aspm_disabled && !aspm_clear_state) || !pci_is_pcie(pdev) || > !parent || !parent->link_state) > return; > if ((parent->pcie_type != PCI_EXP_TYPE_ROOT_PORT) && > @@ -899,6 +906,12 @@ static int __init pcie_aspm_disable(char *str) > > __setup("pcie_aspm=", pcie_aspm_disable); > > +void pci_clear_aspm(void) > +{ > + if (!aspm_force) > + aspm_clear_state = 1; > +} > + > void pcie_no_aspm(void) > { > if (!aspm_force) > diff --git a/include/linux/pci-aspm.h b/include/linux/pci-aspm.h > index 91ba0b3..ce68105 100644 > --- a/include/linux/pci-aspm.h > +++ b/include/linux/pci-aspm.h > @@ -27,6 +27,7 @@ extern void pcie_aspm_init_link_state(struct pci_dev *pdev); > extern void pcie_aspm_exit_link_state(struct pci_dev *pdev); > extern void pcie_aspm_pm_state_change(struct pci_dev *pdev); > extern void pci_disable_link_state(struct pci_dev *pdev, int state); > +extern void pcie_clear_aspm(void); > extern void pcie_no_aspm(void); > #else > static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) > @@ -41,7 +42,9 @@ static inline void pcie_aspm_pm_state_change(struct pci_dev *pdev) > static inline void pci_disable_link_state(struct pci_dev *pdev, int state) > { > } > - > +static inline void pcie_clear_aspm(void) > +{ > +} > static inline void pcie_no_aspm(void) > { > } > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/