From: Aaron Sierra <asierra@xes-inc.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: linux-pci@vger.kernel.org,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Len Brown <lenb@kernel.org>
Subject: Re: [PATCH v2 1/2] PCI/ACPI: Move _OSC test for native services to top
Date: Wed, 13 Feb 2019 11:11:09 -0600 (CST) [thread overview]
Message-ID: <64301353.124952.1550077869482.JavaMail.zimbra@xes-inc.com> (raw)
In-Reply-To: <20190130224415.GK229773@google.com>
----- Original Message -----
> From: "Bjorn Helgaas" <helgaas@kernel.org>
> To: "Aaron Sierra" <asierra@xes-inc.com>
> Sent: Wednesday, January 30, 2019 4:44:15 PM
> Subject: Re: [PATCH v2 1/2] PCI/ACPI: Move _OSC test for native services to top
> 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@xes-inc.com>
>> ---
>> 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.
>
Bjorn, thanks for the review. I certainly did not mean to change behavior
to the extent that you describe. This patch is also not really needed by
the second patch in the series, so I will drop this from v3. Sorry for
the noise.
-Aaron
next prev parent reply other threads:[~2019-02-13 17:20 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-15 22:23 [PATCH] PCI/ACPI: Improve _OSC control request granularity Aaron Sierra
2018-10-25 16:01 ` [PATCH v2 0/2] " Aaron Sierra
2018-10-25 16:01 ` [PATCH v2 1/2] PCI/ACPI: Move _OSC test for native services to top Aaron Sierra
2019-01-30 22:44 ` Bjorn Helgaas
2019-02-13 17:11 ` Aaron Sierra [this message]
2018-10-25 16:01 ` [PATCH v2 2/2] PCI/ACPI: Improve _OSC control request granularity Aaron Sierra
2019-01-30 22:57 ` Bjorn Helgaas
2019-02-13 17:31 ` Aaron Sierra
2019-02-13 17:36 ` Bjorn Helgaas
2019-02-13 21:32 ` [PATCH v3] " Aaron Sierra
2019-04-16 17:52 ` Aaron Sierra
2019-04-16 18:15 ` Bjorn Helgaas
2019-06-26 17:20 ` Bjorn Helgaas
2019-06-27 2:38 ` Aaron Sierra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=64301353.124952.1550077869482.JavaMail.zimbra@xes-inc.com \
--to=asierra@xes-inc.com \
--cc=helgaas@kernel.org \
--cc=lenb@kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=rjw@rjwysocki.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).