Hi Aaron,
On Thu, Oct 25, 2018 at 11:01:31AM -0500, Aaron Sierra wrote:
> Move the simple test for when PCIe native services are disabled
> closer to the top, prior to where things get more complicated.
>
> Signed-off-by: Aaron Sierra <asierra@xxxxxxxxxxx>
> ---
> drivers/acpi/pci_root.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
> index 707aafc..eb9f14e 100644
> --- a/drivers/acpi/pci_root.c
> +++ b/drivers/acpi/pci_root.c
> @@ -441,6 +441,12 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> return;
> }
>
> + if (pcie_ports_disabled) {
> + dev_info(&device->dev,
> + "PCIe port services disabled; not requesting _OSC control\n");
> + return;
> + }
Today we always set "*no_aspm = 1" if _OSC fails, which means we later
call pcie_no_aspm().
After this patch, when pcie_ports_disabled is "true", we don't even try to
evaluate _OSC, and we will never set *no_aspm, so we will never call
pcie_no_aspm() when pcie_ports_disabled is "true", which happens in these
cases:
1) CONFIG_PCIEPORTBUS is unset, or
2) CONFIG_PCIEPORTBUS=y and we booted with "pcie_ports=compat"
Case 1) isn't a problem because pcie_no_aspm() is only implemented when
CONFIG_PCIEASPM=y, and CONFIG_PCIEASPM depends on CONFIG_PCIEPORTBUS, so in
this case today we only call the empty stub pcie_no_aspm() function.
But case 2) is a behavior change that seems unintended.
Even though CONFIG_PCIEASPM depends on CONFIG_PCIEPORTBUS, ASPM doesn't
actually *use* anything provided by PCIEPORTBUS, so I think the ASPM code
is still active and useful even when we boot with "pcie_ports=compat".
Whether CONFIG_PCIEASPM should depend on CONFIG_PCIEPORTBUS is another
question. I tend to think maybe it should not, but that's an orthogonal
question.
> /*
> * All supported architectures that use ACPI have support for
> * PCI domains, so we indicate this in _OSC support capabilities.
> @@ -468,11 +474,6 @@ static void negotiate_os_control(struct acpi_pci_root *root, int *no_aspm,
> return;
> }
>
> - if (pcie_ports_disabled) {
> - dev_info(&device->dev, "PCIe port services disabled; not requesting _OSC control\n");
> - return;
> - }
> -
> if ((support & ACPI_PCIE_REQ_SUPPORT) != ACPI_PCIE_REQ_SUPPORT) {
> decode_osc_support(root, "not requesting OS control; OS requires",
> ACPI_PCIE_REQ_SUPPORT);
> --
> 2.7.4
>
