From: "Rafael J. Wysocki" <email@example.com> To: Mika Westerberg <firstname.lastname@example.org> Cc: "Rafael J. Wysocki" <email@example.com>, Bjorn Helgaas <firstname.lastname@example.org>, Len Brown <email@example.com>, Lukas Wunner <firstname.lastname@example.org>, Keith Busch <email@example.com>, Alex Williamson <firstname.lastname@example.org>, Alexandru Gagniuc <email@example.com>, ACPI Devel Maling List <firstname.lastname@example.org>, Linux PCI <email@example.com> Subject: Re: [PATCH v2 1/3] PCI / ACPI: Use cached ACPI device state to get PCI device power state Date: Tue, 25 Jun 2019 12:00:57 +0200 Message-ID: <CAJZ5v0gca0ey-NonUS4vgsv-kXwvnUzhGcDJpVotwscU3wc-nQ@mail.gmail.com> (raw) In-Reply-To: <20190625094559.GY2640@lahna.fi.intel.com> On Tue, Jun 25, 2019 at 11:46 AM Mika Westerberg <firstname.lastname@example.org> wrote: > > On Mon, Jun 24, 2019 at 01:14:47PM +0200, Rafael J. Wysocki wrote: > > > The problem here is that acpi_device_get_power() really only should be > > > used for two purposes: (1) To initialize adev->power.state, or to > > > update it via acpi_device_update_power(), and (2) by the > > > "real_power_state" sysfs attribute (of ACPI device objects). The > > > adev->power.state value should be used anywhere else, in principle, so > > > the Mika's patch is correct. > > > > Well, it is an improvement, but it is not sufficient. > > > > > [Note that adev->power.state cannot be updated after calling > > > acpi_device_get_power() to the value returned by it without updating > > > the reference counters of the power resources that are "on" *exactly* > > > because of the problem at hand here.] > > > > That is obviously correct, but -> > > > > > > but that's just an idle thought, not a suggestion. > > > > > > After the initialization of the ACPI subsystem, the authoritative > > > source of the ACPI device power state information is > > > adev->power.state. The ACPI subsystem is expected to update this > > > value as needed going forward (including system-wide transitions like > > > resume from S3). > > > > -> the "resume from S3 or hibernation" case needs special handling, because > > in that case the device power state need not reflect the information the ACPI > > subsystem has. That only matters if adev->power.state is ACPI_STATE_D0 and > > the device is actually *not* in D0, because in that case acpi_device_set_power() > > will not work. > > I guess you are talking about the special-cased devices that we leave in > D0 when system suspend (via firmware) is entered? > > > So that case is not covered currently (it should be rare in practice, > > though, if it happens at all), so something like the patch below (untested) may > > be needed in addition to the Mika's patch. > > Looks good to me. I actually decided to address this issue in acpi_device_set_power() as it may affect devices beyond PCI in principle. I will send a patch for that shortly. > > Still, there is also the "power state not matching" case in pci_pm_complete() that's > > need to be covered and the non-PCI ACPI PM has a similar issue in theory, so I > > need to think about this a bit more. > > Do you want me to hold off sending an updated version of the patch > series while we figure this one out or is it fine if I send it out now > and we can add further details on top? It is independent of the other fix, so it can be sent now just fine IMO.
next prev parent reply index Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-18 16:18 [PATCH v2 0/3] PCI / ACPI: Handle sibling devices sharing power resources Mika Westerberg 2019-06-18 16:18 ` [PATCH v2 1/3] PCI / ACPI: Use cached ACPI device state to get PCI device power state Mika Westerberg 2019-06-19 21:28 ` Bjorn Helgaas 2019-06-20 8:27 ` Mika Westerberg 2019-06-20 13:16 ` Bjorn Helgaas 2019-06-20 13:37 ` Mika Westerberg 2019-06-20 14:15 ` Bjorn Helgaas 2019-06-21 10:32 ` Rafael J. Wysocki 2019-06-21 13:09 ` Bjorn Helgaas 2019-06-22 8:51 ` Rafael J. Wysocki 2019-06-24 10:57 ` Mika Westerberg 2019-06-24 11:14 ` Rafael J. Wysocki 2019-06-25 9:45 ` Mika Westerberg 2019-06-25 10:00 ` Rafael J. Wysocki [this message] 2019-06-25 10:08 ` Mika Westerberg 2019-06-21 11:56 ` Rafael J. Wysocki 2019-06-24 10:58 ` Mika Westerberg 2019-06-18 16:18 ` [PATCH v2 2/3] ACPI / PM: Introduce concept of a _PR0 dependent device Mika Westerberg 2019-06-19 13:20 ` Rafael J. Wysocki 2019-06-19 13:34 ` Mika Westerberg 2019-06-18 16:18 ` [PATCH v2 3/3] PCI / ACPI: Add _PR0 dependent devices Mika Westerberg 2019-06-19 13:24 ` [PATCH v2 0/3] PCI / ACPI: Handle sibling devices sharing power resources Rafael J. Wysocki
Reply instructions: You may reply publically 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=CAJZ5v0gca0ey-NonUS4vgsv-kXwvnUzhGcDJpVotwscU3wc-nQ@mail.gmail.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ /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
Linux-ACPI Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-acpi/0 linux-acpi/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-acpi linux-acpi/ https://lore.kernel.org/linux-acpi \ firstname.lastname@example.org email@example.com public-inbox-index linux-acpi Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-acpi AGPL code for this site: git clone https://public-inbox.org/ public-inbox