Re: [PATCH RFC] power: sequencing: rename pwrseq_power_on/off() to pwrseq_vote_on/off()

On Tue, Jul 28, 2026 at 12:14=E2=80=AFAM Bjorn Helgaas <[email protected]>=
 wrote:
>
> On Mon, Jul 27, 2026 at 11:57:38AM +0200, Bartosz Golaszewski 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 cal=
l
> > 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.
>
> I see the possible confusion with get/put interfaces, but "vote"
> doesn't seem exactly right because it really is refcounting, not a
> majority vote thing.
>
> Maybe enable/disable is a possibility, since regulator_enable() and
> regulator_disable() have the same kind of refcount behavior?

I agree with this. clk_prepare_enable() / clk_disable_unprepare() have
the same naming scheme. There's no absolute control unless the consumer
has exclusive access, and even then there are limitations.

I think having the same naming convention across the multiple kinds of
hardware resources would help developers align their expectations. That
is to say, they are refcounting within the kernel, subject to hardware
limitations (always-on regulators, clks controlled by hardware that
could still run despite the kernel turning it off, etc.).


ChenYu



> Whatever you end up with:
>
> Acked-by: Bjorn Helgaas <[email protected]>   # drivers/pci
>
> > Change the name of the two functions to pwrseq_vote_on/off() which
> > better reflects their purpose and semantics. No functional change
> > 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.
> >
> > Signed-off-by: Bartosz Golaszewski <[email protected]=
m>
> > ---
> > I've floated the idea to rename the two pwrseq functions from "power
> > on/off" to "vote on/off". Here's a concrete proposal.
> >
> > I'm sending it as an RFC but if there's agreement, I can queue it as is=
.
> >
> > 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_1V=
[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..faba5a8e3337e878c6dac0c=
c91d23b3577cab6fd 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 inte=
rested 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_pu=
t() and
> > +pwrseq_vote_on(). 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_=
off()
> > +pwrseq_vote_off(). Note that there is no guarantee that pwrseq_vote_of=
f()
> >  will have any effect as there may be multiple users of the underlying =
resources
> >  who may keep them active.
> >
> > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
> > index e6d107f67759f54eee2647d92bb8bfd4e68c9358..5aaabc9fb1b66eb6b24da25=
f77158690a2f03c84 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_vote_off(power->pwrseq);
> >               set_bit(QCA_BT_OFF, &qca->flags);
> >               return;
> >          }
> > @@ -2319,7 +2319,7 @@ static int qca_regulator_enable(struct qca_serdev=
 *qcadev)
> >       int ret;
> >
> >       if (power->pwrseq)
> > -             return pwrseq_power_on(power->pwrseq);
> > +             return pwrseq_vote_on(power->pwrseq);
> >
> >       /* Already enabled */
> >       if (power->vregs_on)
> > diff --git a/drivers/gpu/drm/imagination/pvr_power.c b/drivers/gpu/drm/=
imagination/pvr_power.c
> > index a71d5b35601e7b996428312028a4c0542a5b1350..c6dfa2685ee64086c53281d=
06563e6169109c00f 100644
> > --- a/drivers/gpu/drm/imagination/pvr_power.c
> > +++ b/drivers/gpu/drm/imagination/pvr_power.c
> > @@ -352,12 +352,12 @@ static int pvr_power_init_pwrseq(struct pvr_devic=
e *pvr_dev)
> >
> >  static int pvr_power_on_sequence_pwrseq(struct pvr_device *pvr_dev)
> >  {
> > -     return pwrseq_power_on(pvr_dev->pwrseq);
> > +     return pwrseq_vote_on(pvr_dev->pwrseq);
> >  }
> >
> >  static int pvr_power_off_sequence_pwrseq(struct pvr_device *pvr_dev)
> >  {
> > -     return pwrseq_power_off(pvr_dev->pwrseq);
> > +     return pwrseq_vote_off(pvr_dev->pwrseq);
> >  }
> >
> >  const struct pvr_power_sequence_ops pvr_power_sequence_ops_pwrseq =3D =
{
> > diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wirel=
ess/ath/ath10k/snoc.c
> > index 3106502275781d32d17a45a661362206144df6b3..f87e56576e29124902dccd1=
a21600fd3407b2922 100644
> > --- a/drivers/net/wireless/ath/ath10k/snoc.c
> > +++ b/drivers/net/wireless/ath/ath10k/snoc.c
> > @@ -1025,7 +1025,7 @@ static int ath10k_hw_power_on(struct ath10k *ar)
> >
> >       ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power on\n");
> >
> > -     ret =3D pwrseq_power_on(ar_snoc->pwrseq);
> > +     ret =3D pwrseq_vote_on(ar_snoc->pwrseq);
> >       if (ret)
> >               return ret;
> >
> > @@ -1042,7 +1042,7 @@ static int ath10k_hw_power_on(struct ath10k *ar)
> >  vreg_off:
> >       regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
> >  pwrseq_off:
> > -     pwrseq_power_off(ar_snoc->pwrseq);
> > +     pwrseq_vote_off(ar_snoc->pwrseq);
> >
> >       return ret;
> >  }
> > @@ -1060,7 +1060,7 @@ static int ath10k_hw_power_off(struct ath10k *ar)
> >       ret_vreg =3D regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->=
vregs);
> >
> >       if (ar_snoc->pwrseq)
> > -             ret_seq =3D pwrseq_power_off(ar_snoc->pwrseq);
> > +             ret_seq =3D pwrseq_vote_off(ar_snoc->pwrseq);
> >
> >       return ret_vreg ? : ret_seq;
> >  }
> > diff --git a/drivers/pci/pwrctrl/generic.c b/drivers/pci/pwrctrl/generi=
c.c
> > index a7e599d841e6289db161b19a93ba5a61d9e0ab1c..8872f6501352d7273a9b4f2=
0589c2f74d6accc65 100644
> > --- a/drivers/pci/pwrctrl/generic.c
> > +++ b/drivers/pci/pwrctrl/generic.c
> > @@ -29,7 +29,7 @@ static int slot_pwrctrl_power_on(struct pci_pwrctrl *=
pwrctrl)
> >       int ret;
> >
> >       if (slot->pwrseq) {
> > -             pwrseq_power_on(slot->pwrseq);
> > +             pwrseq_vote_on(slot->pwrseq);
> >               return 0;
> >       }
> >
> > @@ -48,7 +48,7 @@ static int slot_pwrctrl_power_off(struct pci_pwrctrl =
*pwrctrl)
> >                                               struct slot_pwrctrl, pwrc=
trl);
> >
> >       if (slot->pwrseq) {
> > -             pwrseq_power_off(slot->pwrseq);
> > +             pwrseq_vote_off(slot->pwrseq);
> >               return 0;
> >       }
> >
> > diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c b/drivers/pci/pwr=
ctrl/pci-pwrctrl-pwrseq.c
> > index a308bf4b5fc01d23b75db4cd396201c3ba3fe695..86822f0f645eb757455f575=
000f81122bce82e60 100644
> > --- a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> > +++ b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> > @@ -56,7 +56,7 @@ static int pwrseq_pwrctrl_power_on(struct pci_pwrctrl=
 *pwrctrl)
> >       struct pwrseq_pwrctrl *pwrseq =3D container_of(pwrctrl,
> >                                          struct pwrseq_pwrctrl, pwrctrl=
);
> >
> > -     return pwrseq_power_on(pwrseq->pwrseq);
> > +     return pwrseq_vote_on(pwrseq->pwrseq);
> >  }
> >
> >  static int pwrseq_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)
> > @@ -64,7 +64,7 @@ static int pwrseq_pwrctrl_power_off(struct pci_pwrctr=
l *pwrctrl)
> >       struct pwrseq_pwrctrl *pwrseq =3D container_of(pwrctrl,
> >                                          struct pwrseq_pwrctrl, pwrctrl=
);
> >
> > -     return pwrseq_power_off(pwrseq->pwrseq);
> > +     return pwrseq_vote_off(pwrseq->pwrseq);
> >  }
> >
> >  static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
> > diff --git a/drivers/power/sequencing/core.c b/drivers/power/sequencing=
/core.c
> > index 02f42da915985339d3de507fc36dd158b0035a99..c630b14f6f2c478f05e4b28=
93b386aab0cddaaf5 100644
> > --- a/drivers/power/sequencing/core.c
> > +++ b/drivers/power/sequencing/core.c
> > @@ -708,7 +708,7 @@ void pwrseq_put(struct pwrseq_desc *desc)
> >       pwrseq =3D desc->pwrseq;
> >
> >       if (desc->powered_on)
> > -             pwrseq_power_off(desc);
> > +             pwrseq_vote_off(desc);
> >
> >       kfree(desc);
> >       module_put(pwrseq->owner);
> > @@ -874,7 +874,7 @@ static int pwrseq_unit_disable(struct pwrseq_device=
 *pwrseq,
> >  }
> >
> >  /**
> > - * pwrseq_power_on() - Issue a power-on request on behalf of the consu=
mer
> > + * pwrseq_vote_on() - Issue a power-on request on behalf of the consum=
er
> >   *                     device.
> >   * @desc: Descriptor referencing the power sequencer.
> >   *
> > @@ -887,7 +887,7 @@ static int pwrseq_unit_disable(struct pwrseq_device=
 *pwrseq,
> >   * Returns:
> >   * 0 on success, negative error number on failure.
> >   */
> > -int pwrseq_power_on(struct pwrseq_desc *desc)
> > +int pwrseq_vote_on(struct pwrseq_desc *desc)
> >  {
> >       struct pwrseq_device *pwrseq;
> >       struct pwrseq_target *target;
> > @@ -925,14 +925,14 @@ int pwrseq_power_on(struct pwrseq_desc *desc)
> >
> >       return ret;
> >  }
> > -EXPORT_SYMBOL_GPL(pwrseq_power_on);
> > +EXPORT_SYMBOL_GPL(pwrseq_vote_on);
> >
> >  /**
> > - * pwrseq_power_off() - Issue a power-off request on behalf of the con=
sumer
> > + * pwrseq_vote_off() - Issue a power-off request on behalf of the cons=
umer
> >   *                      device.
> >   * @desc: Descriptor referencing the power sequencer.
> >   *
> > - * This undoes the effects of pwrseq_power_on(). It issues a power-off=
 request
> > + * This undoes the effects of pwrseq_vote_on(). It issues a power-off =
request
> >   * on behalf of the consumer and when the last remaining user does so,=
 the
> >   * power-down sequence will be started. If one is in progress, the fun=
ction
> >   * will block until it's complete and then return.
> > @@ -940,7 +940,7 @@ EXPORT_SYMBOL_GPL(pwrseq_power_on);
> >   * Returns:
> >   * 0 on success, negative error number on failure.
> >   */
> > -int pwrseq_power_off(struct pwrseq_desc *desc)
> > +int pwrseq_vote_off(struct pwrseq_desc *desc)
> >  {
> >       struct pwrseq_device *pwrseq;
> >       struct pwrseq_unit *unit;
> > @@ -966,7 +966,7 @@ int pwrseq_power_off(struct pwrseq_desc *desc)
> >
> >       return ret;
> >  }
> > -EXPORT_SYMBOL_GPL(pwrseq_power_off);
> > +EXPORT_SYMBOL_GPL(pwrseq_vote_off);
> >
> >  /**
> >   * pwrseq_to_device() - Get the pwrseq device pointer from a descripto=
r.
> > diff --git a/include/linux/pwrseq/consumer.h b/include/linux/pwrseq/con=
sumer.h
> > index 3c907c9e1885dc2958043a9a733fbe20bdf95f6e..e4d2cb321354f0cb7e1d1d7=
2e6b5a96b94129da3 100644
> > --- a/include/linux/pwrseq/consumer.h
> > +++ b/include/linux/pwrseq/consumer.h
> > @@ -20,8 +20,8 @@ void pwrseq_put(struct pwrseq_desc *desc);
> >  struct pwrseq_desc * __must_check
> >  devm_pwrseq_get(struct device *dev, const char *target);
> >
> > -int pwrseq_power_on(struct pwrseq_desc *desc);
> > -int pwrseq_power_off(struct pwrseq_desc *desc);
> > +int pwrseq_vote_on(struct pwrseq_desc *desc);
> > +int pwrseq_vote_off(struct pwrseq_desc *desc);
> >
> >  struct device *pwrseq_to_device(struct pwrseq_desc *desc);
> >
> > @@ -43,12 +43,12 @@ devm_pwrseq_get(struct device *dev, const char *tar=
get)
> >       return ERR_PTR(-ENOSYS);
> >  }
> >
> > -static inline int pwrseq_power_on(struct pwrseq_desc *desc)
> > +static inline int pwrseq_vote_on(struct pwrseq_desc *desc)
> >  {
> >       return -ENOSYS;
> >  }
> >
> > -static inline int pwrseq_power_off(struct pwrseq_desc *desc)
> > +static inline int pwrseq_vote_off(struct pwrseq_desc *desc)
> >  {
> >       return -ENOSYS;
> >  }
> >
> > ---
> > base-commit: c5e32e86ca02b003f86e095d379b38148999293d
> > change-id: 20260727-pwrseq-vote-rename-b7cd21032668
> >
> > Best regards,
> > --
> > Bartosz Golaszewski <[email protected]>
> >