All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Ferry Toth <fntoth@gmail.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Linux PCI <linux-pci@vger.kernel.org>,
	Linux ACPI <linux-acpi@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Bjorn Helgaas <helgaas@kernel.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: Re: [PATCH v2 2/7] PCI: ACPI: PM: Do not use pci_platform_pm_ops for ACPI
Date: Mon, 27 Sep 2021 15:46:40 +0200	[thread overview]
Message-ID: <CAJZ5v0gE5qagoiKWR1hAxLdSJepiCvT4GHaZu4M4pKR8v83zmA@mail.gmail.com> (raw)
In-Reply-To: <24c2c653-4087-fba1-5745-a62a8ae715c6@gmail.com>

On Fri, Sep 24, 2021 at 11:17 PM Ferry Toth <fntoth@gmail.com> wrote:
>
> Hi
>
> Op 24-09-2021 om 14:02 schreef Rafael J. Wysocki:
> > On Thu, Sep 23, 2021 at 10:32 PM Ferry Toth <fntoth@gmail.com> wrote:
> >> Hi
> >>
> >> Op 23-09-2021 om 15:51 schreef Ferry Toth:
> >>> Repost (with formatting removed, sorry for the noise)
> >>> Op 23-09-2021 om 13:30 schreef Rafael J. Wysocki:
> >>>> On Wed, Sep 22, 2021 at 11:31 PM Ferry Toth<fntoth@gmail.com>  wrote:
> >>>>> Hi,
> >>>>> Op 20-09-2021 om 21:17 schreef Rafael J. Wysocki:
> >>>>>> From: Rafael J. Wysocki<rafael.j.wysocki@intel.com>
> >>>>>>
> >>>>>> Using struct pci_platform_pm_ops for ACPI adds unnecessary
> >>>>>> indirection to the interactions between the PCI core and ACPI PM,
> >>>>>> which is also subject to retpolines.
> >>>>>>
> >>>>>> Moreover, it is not particularly clear from the current code that,
> >>>>>> as far as PCI PM is concerned, "platform" really means just ACPI
> >>>>>> except for the special casess when Intel MID PCI PM is used or when
> >>>>>> ACPI support is disabled (through the kernel config or command line,
> >>>>>> or because there are no usable ACPI tables on the system).
> >>>>>>
> >>>>>> To address the above, rework the PCI PM code to invoke ACPI PM
> >>>>>> functions directly as needed and drop the acpi_pci_platform_pm
> >>>>>> object that is not necessary any more.
> >>>>>>
> >>>>>> Accordingly, update some of the ACPI PM functions in question to do
> >>>>>> extra checks in case the ACPI support is disabled (which previously
> >>>>>> was taken care of by avoiding to set the pci_platform_ops pointer
> >>>>>> in those cases).
> >>>>>>
> >>>>>> Signed-off-by: Rafael J. Wysocki<rafael.j.wysocki@intel.com>
> >>>>>> ---
> >>>>>>
> >>>>>> v1 -> v2:
> >>>>>>        * Rebase on top of the new [1/7] and move dropping struct
> >>>>>>          pci_platform_pm_ops to a separate patch.
> >>>>> I wanted to test this series on 5.15-rc2 but this patch 2/7 doesn't
> >>>>> apply (after 1/7 applied). Should I apply this on another tree?
> >>>> This is on top of
> >>>> https://patchwork.kernel.org/project/linux-acpi/patch/2793105.e9J7NaK4W3@kreacher/
> >>>>
> >>>> which is not yet in any tree.
> >>>>
> >>>> Sorry for the confusion.
> >>> No problem at all. If I can I will try to report back tonight. Else,
> >>> will be delayed 2 due to a short break.
> >> With those 3 extra patches followed by 7 from this series it builds. But
> >> on boot I get:
> >> dwc3 dwc3.0.auto: this is not a DesignWare USB3 DRD Core
> >> Then after this it reboots. Nothing in the logs. Nothing else on
> >> console, I guess something goes wrong early.
> > It appears so.
> >
> > Can you please try just the 3 extra patches this series is on top of?
> > The problem is more likely to be located in one of them.
> Boots fine with just the 3 so up to and including "ACPI: glue: Look for
> ACPI bus type only if ACPI companion is not known". From the log I get:
>
>
> Intel MID platform detected, using MID PCI ops
> PCI: Using configuration type 1 for base access
> ..
> PCI: Using host bridge windows from ACPI; if necessary, use "pci=nocrs"
> and report a bug
> ..
> PCI: Probing PCI hardware
> PCI: root bus 00: using default resources
> PCI: Probing PCI hardware (bus 00)
> PCI: pci_cache_line_size set to 64 bytes
> ..
> pnp: PnP ACPI init
> ..
> pnp: PnP ACPI: found 2 devices
> ..
> xhci-hcd xhci-hcd.1.auto: xHCI Host Controller
> xhci-hcd xhci-hcd.1.auto: new USB bus registered, assigned bus number 1
> xhci-hcd xhci-hcd.1.auto: hcc params 0x0220f06c hci version 0x100 quirks
> 0x0000000002010010
> xhci-hcd xhci-hcd.1.auto: irq 14, io mem 0xf9100000
> usb usb1: New USB device found, idVendor=1d6b, idProduct=0002,
> bcdDevice= 5.15
> usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1
> usb usb1: Product: xHCI Host Controller
> usb usb1: Manufacturer: Linux 5.15.0-rc2-edison-acpi-standard xhci-hcd
> usb usb1: SerialNumber: xhci-hcd.1.auto
> hub 1-0:1.0: USB hub found
> hub 1-0:1.0: 1 port detected
>
> I continued up to "PCI: ACPI: PM: Do not use pci_platform_pm_ops for
> ACPI", still boots.
>
> In the logs I still see "Intel MID platform detected, using MID PCI ops".
>
> Unfortunately no more time today, and tomorrow short holiday starts. I
> will continue after returning next Sat.

Thanks for the testing and feedback, much appreciated!

I'm going to queue up the patches that you have tested with a
Tested-by tag from you if that's not an issue.

Also patches [3/7] ("PCI: PM: Drop struct pci_platform_pm_ops") and
[7/7] ("PCI: PM: Simplify acpi_pci_power_manageable()") are not likely
to introduce functional issue, because the former removes unused code
and the latter simply rearranges some computations, so I'm going to
queue up these two as well.

Patches [4-5/7] change behavior, if only slightly, so they need to be
double checked.

In turn, patch [6/7[ contains a bug - it makes
pci_platform_power_transition() call acpi_pci_set_power_state()
instead of platform_pci_set_power_state() which is probably why you
see the problem (nobody with an ACPI platform of any platform other
then MID would see it).  Sorry about that.

After queuing up the patches that are not problematic I'll prepare a
new 3-patch series to test on top of them.

Thanks!

  reply	other threads:[~2021-09-27 13:46 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-20 18:52 [PATCH v2 0/7] PCI: ACPI: Get rid of struct pci_platform_pm_ops and clean up code Rafael J. Wysocki
2021-09-20 19:16 ` [PATCH v2 1/7] PCI: PM: Do not use pci_platform_pm_ops for Intel MID PM Rafael J. Wysocki
2021-09-20 19:17 ` [PATCH v2 2/7] PCI: ACPI: PM: Do not use pci_platform_pm_ops for ACPI Rafael J. Wysocki
2021-09-22 21:30   ` Ferry Toth
2021-09-23 11:30     ` Rafael J. Wysocki
     [not found]       ` <013e3a7b-ec67-1a67-c2b9-e1fbb11c664e@gmail.com>
2021-09-23 13:35         ` Rafael J. Wysocki
2021-09-23 13:51       ` Ferry Toth
2021-09-23 20:32         ` Ferry Toth
2021-09-24 12:02           ` Rafael J. Wysocki
2021-09-24 14:52             ` Ferry Toth
2021-09-24 21:17             ` Ferry Toth
2021-09-27 13:46               ` Rafael J. Wysocki [this message]
2021-09-29 19:08   ` Nathan Chancellor
2021-09-29 19:13     ` Rafael J. Wysocki
2021-09-29 19:13       ` Rafael J. Wysocki
2021-09-20 19:17 ` [PATCH v2 3/7] PCI: PM: Drop struct pci_platform_pm_ops Rafael J. Wysocki
2021-09-20 19:17 ` [PATCH v2 4/7] PCI: PM: Rearrange pci_target_state() Rafael J. Wysocki
2021-09-20 19:17 ` [PATCH v2 5/7] PCI: PM: Make pci_choose_state() call pci_target_state() Rafael J. Wysocki
2021-09-20 19:17 ` [PATCH v2 6/7] PCI: PM: Do not call platform_pci_power_manageable() unnecessarily Rafael J. Wysocki
2021-09-20 19:17 ` [PATCH v2 7/7] PCI: PM: Simplify acpi_pci_power_manageable() Rafael J. Wysocki
2021-09-28 23:28 ` [PATCH v2 0/7] PCI: ACPI: Get rid of struct pci_platform_pm_ops and clean up code Bjorn Helgaas
2021-09-29 12:00   ` Rafael J. Wysocki
2021-09-29 15:07     ` Bjorn Helgaas
2021-09-29 17:14   ` Ferry Toth
2021-09-29 18:05 ` [PATCH v3 0/3] PCI: PM: Simplify and unify some helper functions Rafael J. Wysocki
2021-09-29 18:09   ` [PATCH v3 1/3] PCI: PM: Rearrange pci_target_state() Rafael J. Wysocki
2021-09-29 18:11   ` [PATCH v3 2/3] PCI: PM: Make pci_choose_state() call pci_target_state() Rafael J. Wysocki
2021-09-29 18:15   ` [PATCH v3 3/3] PCI: PM: Do not call platform_pci_power_manageable() unnecessarily Rafael J. Wysocki
2021-10-03 20:14   ` [PATCH v3 0/3] PCI: PM: Simplify and unify some helper functions Ferry Toth
2021-10-05 11:03     ` Rafael J. Wysocki

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=CAJZ5v0gE5qagoiKWR1hAxLdSJepiCvT4GHaZu4M4pKR8v83zmA@mail.gmail.com \
    --to=rafael@kernel.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=fntoth@gmail.com \
    --cc=helgaas@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rjw@rjwysocki.net \
    --subject='Re: [PATCH v2 2/7] PCI: ACPI: PM: Do not use pci_platform_pm_ops for ACPI' \
    /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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.