Re: [PATCH v2] power: sequencing: rename pwrseq_power_on/off() to pwrseq_enable/disable()
Bartosz Golaszewski <[email protected]> Fri, 31 Jul 2026 08:04:30 -0500
| Newsgroups | org.infradead.lists.ath10k,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-doc,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.linux-pm,org.kernel.vger.linux-wireless |
|---|---|
| Message-ID | <CAMRc=McA3BqoazxvGDu1_WoT9x0ujXT_80jKPp+5Jb0Rda9zOA@mail.gmail.com> |
On Fri, 31 Jul 2026 10:43:19 +0200, Loic Poulain <[email protected]> said: > On Fri, Jul 31, 2026 at 10:29=E2=80=AFAM Bartosz Golaszewski > <[email protected]> wrote: >> >> The way power sequencing works means that a call to pwrseq_power_on() >> does not necessarily result in the pwrseq target being powered-on at >> that time: it may have already been powered on before. Similarly: a call >> to pwrseq_power_off() does not have to result in an actual powering off >> of resources: there may still be other users that requested a power-on >> before. >> >> We will also introduce the concept of "non-controllable" pwrseq targets >> soon which further increases the disconnect between the naming >> convention and the actual semantics. >> >> What consumers of pwrseq descriptors actually do is: they *vote* for a >> powering on of a given target or retract that vote. These operations >> could be called get/put in line with runtime PM but this could become >> confusing since we already provide pwrseq_get/put() for a different >> purpose. pwrseq_vote_on/off() also have been rejected as unusual in >> the tree. >> >> Change the name of the two functions to pwrseq_enable/disable() which >> better reflects their purpose and semantics and also mirrors other >> enable-counted resources like regulators and clocks. No functional chang= e >> intended. >> >> If at any point users need to know *when* the exact power event happens, >> we can provide that information in the form of a notifier. >> >> Acked-by: Jeff Johnson <[email protected]> >> Acked-by: Bjorn Helgaas <[email protected]> >> Signed-off-by: Bartosz Golaszewski <[email protected]= > >> --- >> I've floated the idea to rename the two pwrseq functions from "power >> on/off" to "vote on/off". I've sent a patch but the overwhelming >> consensus seem to be that enable/disable naming is preferred. >> >> I'm doing it in a single commit that should go through the pwrseq tree >> with Acks from subsystem maintainers. >> >> [1] https://lore.kernel.org/all/CAMRc=3DMefxn81d0VUwmQgwFtffkL1=3DEr_1VQ= [email protected]/ >> --- >> Changes in v2: >> - Change the naming convention to enable/disable >> - Link to v1: https://patch.msgid.link/20260727-pwrseq-vote-rename-v1-1-= [email protected] >> --- >> Documentation/driver-api/pwrseq.rst | 4 ++-- >> drivers/bluetooth/hci_qca.c | 4 ++-- >> drivers/gpu/drm/imagination/pvr_power.c | 4 ++-- >> drivers/net/wireless/ath/ath10k/snoc.c | 6 +++--- >> drivers/pci/pwrctrl/generic.c | 4 ++-- >> drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 4 ++-- >> drivers/power/sequencing/core.c | 16 ++++++++-------- >> include/linux/pwrseq/consumer.h | 8 ++++---- >> 8 files changed, 25 insertions(+), 25 deletions(-) >> >> diff --git a/Documentation/driver-api/pwrseq.rst b/Documentation/driver-= api/pwrseq.rst >> index ad18b2326b689a41471216f3889480b11768ca82..0c315ab1138b02540fae60b3= 9928c580027c0f7c 100644 >> --- a/Documentation/driver-api/pwrseq.rst >> +++ b/Documentation/driver-api/pwrseq.rst >> @@ -50,9 +50,9 @@ Consumer interface >> The consumer API is aimed to be as simple as possible. The driver inter= ested in >> getting a descriptor from the power sequencer should call pwrseq_get() = and >> specify the name of the target it wants to reach in the sequence after = calling >> -pwrseq_power_up(). The descriptor can be released by calling pwrseq_put= () and >> +pwrseq_enable(). The descriptor can be released by calling pwrseq_put()= and >> the consumer can request the powering down of its target with >> -pwrseq_power_off(). Note that there is no guarantee that pwrseq_power_o= ff() >> +pwrseq_disble(). Note that there is no guarantee that pwrseq_disble() >> will have any effect as there may be multiple users of the underlying r= esources >> who may keep them active. >> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >> index 345f602e9ce233a0934a73a85d1e7c17c4f3210d..e713fc06b73402ae3d57794e= d0870df8394a796a 100644 >> --- a/drivers/bluetooth/hci_qca.c >> +++ b/drivers/bluetooth/hci_qca.c >> @@ -2259,7 +2259,7 @@ static void qca_power_off(struct hci_uart *hu) >> } >> >> if (power && power->pwrseq) { >> - pwrseq_power_off(power->pwrseq); >> + pwrseq_disble(power->pwrseq); > > Nit: s/pwrseq_disble/pwrseq_disable ? > Ah dang it, how did I miss it, I test-built it. :/ Thanks for catching it. Bart