lkml.org 
[lkml]   [2010]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH] PCI: Disable ASPM if BIOS asks us to
    Date
    On Thursday, December 02, 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.

    Good catch, but I'd rather use a separate function pcie_disable_aspm() to handle
    the "BIOS tells us there's no ASPM" case and leave pcie_no_aspm() as is.
    The, you might avoid a couple of changes.

    > Signed-off-by: Matthew Garrett <mjg@redhat.com>
    > ---
    > drivers/pci/pci-acpi.c | 2 +-
    > drivers/pci/pcie/aspm.c | 20 +++++++++++++++-----
    > drivers/pci/pcie/portdrv_core.c | 2 +-
    > drivers/pci/pcie/portdrv_pci.c | 2 +-
    > include/linux/pci-aspm.h | 4 ++--
    > 5 files changed, 20 insertions(+), 10 deletions(-)
    >
    > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
    > index 24e19c5..e2c47cb 100644
    > --- a/drivers/pci/pci-acpi.c
    > +++ b/drivers/pci/pci-acpi.c
    > @@ -399,7 +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_no_aspm();
    > + pcie_no_aspm(true);
    > }
    >
    > ret = register_acpi_bus_type(&acpi_pci_bus);
    > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
    > index 7122281..08a2232 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);
    >
    > @@ -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,10 +906,13 @@ static int __init pcie_aspm_disable(char *str)
    >
    > __setup("pcie_aspm=", pcie_aspm_disable);
    >
    > -void pcie_no_aspm(void)
    > +void pcie_no_aspm(bool clear)
    > {
    > - if (!aspm_force)
    > + if (!aspm_force) {
    > aspm_disabled = 1;
    > + if (clear)
    > + aspm_clear_state = clear;

    Well, is the if(clear) really necessary?

    > + }
    > }
    >
    > /**
    > diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c
    > index a9c222d..bb0fda1 100644
    > --- a/drivers/pci/pcie/portdrv_core.c
    > +++ b/drivers/pci/pcie/portdrv_core.c
    > @@ -244,7 +244,7 @@ static int get_port_device_capability(struct pci_dev *dev)
    > err = pcie_port_platform_notify(dev, &cap_mask);
    > if (pcie_ports_auto) {
    > if (err) {
    > - pcie_no_aspm();
    > + pcie_no_aspm(false);
    > return 0;
    > }
    > } else {
    > diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
    > index f9033e1..87f5343 100644
    > --- a/drivers/pci/pcie/portdrv_pci.c
    > +++ b/drivers/pci/pcie/portdrv_pci.c
    > @@ -328,7 +328,7 @@ static int __init pcie_portdrv_init(void)
    > int retval;
    >
    > if (pcie_ports_disabled) {
    > - pcie_no_aspm();
    > + pcie_no_aspm(false);
    > return -EACCES;
    > }
    >
    > diff --git a/include/linux/pci-aspm.h b/include/linux/pci-aspm.h
    > index 91ba0b3..c7176a6 100644
    > --- a/include/linux/pci-aspm.h
    > +++ b/include/linux/pci-aspm.h
    > @@ -27,7 +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_no_aspm(void);
    > +extern void pcie_no_aspm(bool clear_state);
    > #else
    > static inline void pcie_aspm_init_link_state(struct pci_dev *pdev)
    > {
    > @@ -42,7 +42,7 @@ static inline void pci_disable_link_state(struct pci_dev *pdev, int state)
    > {
    > }
    >
    > -static inline void pcie_no_aspm(void)
    > +static inline void pcie_no_aspm(bool clear_state)
    > {
    > }
    > #endif
    >



    \
     
     \ /
      Last update: 2010-12-02 23:27    [W:0.045 / U:31.284 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site