mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: "Rafael J. Wysocki (Intel)" <rafael@kernel.org>
Cc: "Fabrice Gasnier" <fabrice.gasnier@st.com>,
	linux-pm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	"Ulf Hansson" <ulfh@kernel.org>,
	linux-kernel@vger.kernel.org, linux-iio@vger.kernel.org,
	"Andy Shevchenko" <andy@kernel.org>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"David Lechner" <dlechner@baylibre.com>,
	"Alexandre Torgue" <alexandre.torgue@foss.st.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Maxime Coquelin" <mcoquelin.stm32@gmail.com>
Subject: Re: [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency
Date: Thu, 5 Nov 2026 17:06:48 -0800	[thread overview]
Message-ID: <au0oKFb9CmNzc6Cg@google.com> (raw)
In-Reply-To: <CAJZ5v0g25xQeRaLJWJyAo2fcJko+gZ7w6JhX7FzMgRx2+uuimw@mail.gmail.com>

On Thu, Nov 05, 2026 at 10:18:52PM +0100, Rafael J. Wysocki (Intel) wrote:
> Of course, it will not prevent the parent from suspending if the child
> does not attempt to resume it for a time longer than its autosuspend
> delay.  That's by design and you didn't have to take the time to
> demonstrate it.

To be clear, I wasn't just trying to demonstrate for fun or because it
wasn't clear. I think there's value in putting concrete reproducible
test cases out there. (KUnit is an easier common ground than some
particular piece of obscure hardware.)

> The question is how often those kinds of patterns occur in practice.
> Or IOW, whether or not they occur often enough to justify adding
> complexity related specifically to addressing them.

Well, I tried to keep very close to the spirit of the actual real
reports in question. So when commit c537d3457542 says, "As an example
[...] cat in_voltageY_raw at a 0.2s rate", I think it's not a far
stretch to test 0.1s too. And it's actually quite odd (IMO) if 0.2s
behaves *better* than 0.1s or 0.09s, just because it hits the parent's
rpm_resume() more often.

> > On Thu, Nov 05, 2026 at 12:59:24PM +0100, Rafael J. Wysocki (Intel) wrote:
> > I would love to hear your counterarguments.
> 
> OK, I'll say more about that tomorrow (it's past 10 PM here, time to
> go away from the keyboard).

Great! And don't rush on my account. I'm interested in eventual
progress, even if not rapid. I'm sure you're busy.

Or, maybe I should have gone to LPC...

> > I don't believe your solution solves the problem,
> 
> That depends on what problem in particular you mean.
> 
> It addresses an inconsistency in rpm_resume() and as I said above,
> this needs to be done anyway because it is not only related to the
> cases when children or consumers are involved.  There are other cases
> in which rpm_resume() gets called for an active device and the lack of
> last_busy update causes the device to suspend earlier than expected.

Yes, on second thought, I agree that your patch addresses a real
problem, even if it's not (IMO) quite a full solution for commit
c537d3457542's.

> > and my solution does while also handling
> > the inconsistency you mention.
> 
> I'm not sure about that.

Sorry, I see that part was inaccurate.

> For example, say a device has an autosuspend delay of 2 s.  Say it is
> initially suspended and user space writes "on" to its "power/control",
> so the device resumes.  Now, 1 s later, user space writes "auto" to
> the device's "power/control" and the autosuspend delay is respected,
> so the device doesn't suspend.  Next, 1.5 s after the first resume,
> user space writes "on" to the device's "power/control" and the device
> does not resume because it is active (its last_busy is not updated).
> Finally, user space writes "auto" to the device's "power/control", say
> 1 s later, and the device suspends even though it can be expected to
> stay active for at least 1 s more.
> 
> How is that going to be addressed by propagating last_busy from
> children/consumers to parents/suppliers?

Agreed, your patch addresses some scenarios mine doesn't.

I also agree the power/control behavior is a bit awkward, although it's
not very convincing to me as a use case. (Arbitrary/periodic fiddling
with power/control seems more like a nuisance API than a real expected
use case.) Or if it were, I would suggest solving that by adding
pm_runtime_mark_last_busy() to pm_runtime_allow() :)

To me, a more convincing one is: a non-autosuspending child with an
autosuspending parent, in a situation otherwise similar to commit
c537d3457542. The child is not likely to intentionally update last_busy,
but only implicitly via rpm_resume(). When you solve the inconsistency
with your patch, then it enables my patch to be more effective w.r.t.
the parent.

> Note that I'm not dismissing your proposal, I'm just having doubts
> about the need to do this kind of thing in the core code.

OK. I agree it adds a bit of complexity, but at the moment, it seems to
me like the most thorough way to improve the ergonomics for clients
(drivers) here. And that's pretty much my entire motivation here: to
improve the ergonomics of runtime PM -- even if in this case, it
requires a little (core) complexity.

So, I'd love to hear what can make the ergonomics better here, with less
core complexity.

Thanks,
Brian

  reply	other threads:[~2026-11-06  1:06 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-03 22:14 [PATCH v2 0/5] PM: runtime: Improve autosuspend child to parent propagation Brian Norris
2026-11-03 22:14 ` [PATCH v2 1/5] PM: runtime: Only queue an idle check for RPM-linked suppliers (part 2) Brian Norris
2026-11-04 14:44   ` Rafael J. Wysocki (Intel)
2026-11-03 22:14 ` [PATCH v2 2/5] PM: runtime: Convert last_busy to atomic64_t Brian Norris
2026-11-04  8:21   ` Joshua Crofts
2026-11-03 22:14 ` [PATCH v2 3/5] PM: runtime: Propagate last_busy from dependent to dependency Brian Norris
2026-11-03 23:17   ` Jonathan Cameron
2026-11-04  0:32     ` Brian Norris
2026-11-04  1:40       ` Jonathan Cameron
2026-11-04 13:34   ` Rafael J. Wysocki (Intel)
2026-11-04 19:31     ` Brian Norris
2026-11-05 11:59       ` Rafael J. Wysocki (Intel)
2026-11-05 20:21         ` Brian Norris
2026-11-05 21:18           ` Rafael J. Wysocki (Intel)
2026-11-06  1:06             ` Brian Norris [this message]
2026-11-03 22:14 ` [PATCH v2 4/5] PM: runtime: Add tests for last_busy propagation Brian Norris
2026-11-03 23:21   ` Jonathan Cameron
2026-11-03 22:14 ` [PATCH v2 5/5] iio: adc: stm32-adc: Drop runtime_idle() Brian Norris

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=au0oKFb9CmNzc6Cg@google.com \
    --to=briannorris@chromium.org \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andy@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=fabrice.gasnier@st.com \
    --cc=jic23@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=nuno.sa@analog.com \
    --cc=rafael@kernel.org \
    --cc=ulfh@kernel.org \
    /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

all inboxes | Powered by JetHome®