Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753588Ab3FNRpL (ORCPT ); Fri, 14 Jun 2013 13:45:11 -0400 Received: from mail-oa0-f43.google.com ([209.85.219.43]:57662 "EHLO mail-oa0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753009Ab3FNRpD (ORCPT ); Fri, 14 Jun 2013 13:45:03 -0400 MIME-Version: 1.0 In-Reply-To: References: <20130401235256.GA31966@google.com> <20130614141101.GA29452@google.com> From: Bjorn Helgaas Date: Fri, 14 Jun 2013 11:44:42 -0600 Message-ID: Subject: Re: [PATCH] PCI: Remove not needed check in disable aspm link To: Yinghai Lu Cc: Jiang Liu , Roman Yepishev , "Rafael J. Wysocki" , "linux-pci@vger.kernel.org" , "linux-acpi@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Linus Torvalds , Andrew Morton , Greg Kroah-Hartman Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2117 Lines: 52 On Fri, Jun 14, 2013 at 10:57 AM, Yinghai Lu wrote: > On Fri, Jun 14, 2013 at 9:33 AM, Bjorn Helgaas wrote: >> On Fri, Jun 14, 2013 at 10:17 AM, Yinghai Lu wrote: >> >> Can you please refer to specific function names? I can't read your mind. >> >> You might be referring to quirk_disable_aspm_l0s(). This is a >> pci_fixup_final quirk that calls pci_disable_link_state(). In the >> current tree, we enumerate devices before requesting _OSC control. >> However, pci_fixup_final quirks are not run until the >> pci_apply_final_quirks() fs_initcall, which is after we request _OSC >> control. >> >> As far as I can tell, we never call pci_disable_link_state() before >> calling pcie_no_aspm(). > > ok, you are right, that is not pci_disable_link_state. > > It is pcie_aspm_init_link_state ==> pcie_aspm_sanity_check in booting path > that disable aspm. It has "if (aspm_disabled)" in it, and it cause > the difference. Yes, I agree, the pcie_aspm_init_link_state() path uses aspm_disabled before we set it: acpi_pci_root_add pci_acpi_scan_root pci_scan_child_bus pci_scan_slot pcie_aspm_init_link_state pcie_aspm_sanity_check if (aspm_disabled) # used before set ... acpi_pci_osc_control_set pcie_no_aspm aspm_disabled = 1 # set That might mean we do some ASPM configuration during enumeration (in pci_scan_slot()) even though the BIOS hasn't given us permission. It looks like we did that even in v3.7, since we did the enumeration before the _OSC there as well. That looks like a bug to me. I don't think the fact that we have been doing ASPM config during enumeration before _OSC is an argument for dropping the check in pci_disable_link_state(). Bjorn -- 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/