[PATCH v2] PCI/pwrctrl: Do not power off on pwrctrl device removal

[PATCH v2] PCI/pwrctrl: Do not power off on pwrctrl device removal

[Chen-Yu Tsai]

With the move to explicit pwrctrl power on/off APIs, the caller, i.e.
the PCI driver should manage the power state. The pwrctrl drivers should
not try to clean up or power off when they are removed, as this might
end up disabling an already disabled regulator, causing a big warning.
This can be triggered if a PCI controller driver's .remove() callback
calls pci_pwrctrl_destroy_devices() after pci_pwrctrl_power_off_devices().

Drop the devm cleanup parts that turn off regulators from the pwrctrl
drivers.

Fixes: b921aa3f8dec ("PCI/pwrctrl: Switch to pwrctrl create, power on/off, destroy APIs")
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

---
Changes since v1:
- Keep the release function for the PCI slot driver to free the
  regulators

I ran into this while integrating the new pci_pwrctrl_*() API into the
MediaTek driver. I am sending this separately since this change is
unrelated and does not conflict with or depend on the other changes for
the driver itself.

I think this should be merged for fixes.

---
 drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 12 ------------
 drivers/pci/pwrctrl/slot.c               |  1 -
 2 files changed, 13 deletions(-)

diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
index 0d0377283c37..c7e4beec160a 100644
--- a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
@@ -68,13 +68,6 @@ static int pwrseq_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)
 	return pwrseq_power_off(pwrseq->pwrseq);
 }
 
-static void devm_pwrseq_pwrctrl_power_off(void *data)
-{
-	struct pwrseq_pwrctrl *pwrseq = data;
-
-	pwrseq_pwrctrl_power_off(&pwrseq->pwrctrl);
-}
-
 static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
 {
 	const struct pwrseq_pwrctrl_pdata *pdata;
@@ -101,11 +94,6 @@ static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(pwrseq->pwrseq),
 				     "Failed to get the power sequencer\n");
 
-	ret = devm_add_action_or_reset(dev, devm_pwrseq_pwrctrl_power_off,
-				       pwrseq);
-	if (ret)
-		return ret;
-
 	pwrseq->pwrctrl.power_on = pwrseq_pwrctrl_power_on;
 	pwrseq->pwrctrl.power_off = pwrseq_pwrctrl_power_off;
 
diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
index 082af81efe25..b87639253ae2 100644
--- a/drivers/pci/pwrctrl/slot.c
+++ b/drivers/pci/pwrctrl/slot.c
@@ -63,7 +63,6 @@ static void devm_slot_pwrctrl_release(void *data)
 {
 	struct slot_pwrctrl *slot = data;
 
-	slot_pwrctrl_power_off(&slot->pwrctrl);
 	regulator_bulk_free(slot->num_supplies, slot->supplies);
 }
 
-- 
2.53.0.414.gf7e9f6c205-goog

Re: [PATCH v2] PCI/pwrctrl: Do not power off on pwrctrl device removal

[Manivannan Sadhasivam]

On Thu, Feb 26, 2026 at 05:22:33PM +0800, Chen-Yu Tsai wrote:
> With the move to explicit pwrctrl power on/off APIs, the caller, i.e.
> the PCI driver should manage the power state. The pwrctrl drivers should
> not try to clean up or power off when they are removed, as this might
> end up disabling an already disabled regulator, causing a big warning.
> This can be triggered if a PCI controller driver's .remove() callback
> calls pci_pwrctrl_destroy_devices() after pci_pwrctrl_power_off_devices().
> 
> Drop the devm cleanup parts that turn off regulators from the pwrctrl
> drivers.
> 
> Fixes: b921aa3f8dec ("PCI/pwrctrl: Switch to pwrctrl create, power on/off, destroy APIs")
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> 
> ---
> Changes since v1:
> - Keep the release function for the PCI slot driver to free the
>   regulators
> 
> I ran into this while integrating the new pci_pwrctrl_*() API into the
> MediaTek driver. I am sending this separately since this change is
> unrelated and does not conflict with or depend on the other changes for
> the driver itself.
> 
> I think this should be merged for fixes.
> 
> ---
>  drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 12 ------------
>  drivers/pci/pwrctrl/slot.c               |  1 -
>  2 files changed, 13 deletions(-)
> 
> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> index 0d0377283c37..c7e4beec160a 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> @@ -68,13 +68,6 @@ static int pwrseq_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)
>  	return pwrseq_power_off(pwrseq->pwrseq);
>  }
>  
> -static void devm_pwrseq_pwrctrl_power_off(void *data)
> -{
> -	struct pwrseq_pwrctrl *pwrseq = data;
> -
> -	pwrseq_pwrctrl_power_off(&pwrseq->pwrctrl);
> -}
> -
>  static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
>  {
>  	const struct pwrseq_pwrctrl_pdata *pdata;
> @@ -101,11 +94,6 @@ static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(pwrseq->pwrseq),
>  				     "Failed to get the power sequencer\n");
>  
> -	ret = devm_add_action_or_reset(dev, devm_pwrseq_pwrctrl_power_off,
> -				       pwrseq);
> -	if (ret)
> -		return ret;
> -
>  	pwrseq->pwrctrl.power_on = pwrseq_pwrctrl_power_on;
>  	pwrseq->pwrctrl.power_off = pwrseq_pwrctrl_power_off;
>  
> diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
> index 082af81efe25..b87639253ae2 100644
> --- a/drivers/pci/pwrctrl/slot.c
> +++ b/drivers/pci/pwrctrl/slot.c
> @@ -63,7 +63,6 @@ static void devm_slot_pwrctrl_release(void *data)
>  {
>  	struct slot_pwrctrl *slot = data;
>  
> -	slot_pwrctrl_power_off(&slot->pwrctrl);
>  	regulator_bulk_free(slot->num_supplies, slot->supplies);
>  }
>  
> -- 
> 2.53.0.414.gf7e9f6c205-goog
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2] PCI/pwrctrl: Do not power off on pwrctrl device removal

[Chen-Yu Tsai]

Ping?

What's going on with this patch? The slot driver got renamed and this
fix is still not merged.

On Thu, Feb 26, 2026 at 5:30 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Thu, Feb 26, 2026 at 05:22:33PM +0800, Chen-Yu Tsai wrote:
> > With the move to explicit pwrctrl power on/off APIs, the caller, i.e.
> > the PCI driver should manage the power state. The pwrctrl drivers should
> > not try to clean up or power off when they are removed, as this might
> > end up disabling an already disabled regulator, causing a big warning.
> > This can be triggered if a PCI controller driver's .remove() callback
> > calls pci_pwrctrl_destroy_devices() after pci_pwrctrl_power_off_devices().
> >
> > Drop the devm cleanup parts that turn off regulators from the pwrctrl
> > drivers.
> >
> > Fixes: b921aa3f8dec ("PCI/pwrctrl: Switch to pwrctrl create, power on/off, destroy APIs")
> > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>
> Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
>
> - Mani
>
> > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> >
> > ---
> > Changes since v1:
> > - Keep the release function for the PCI slot driver to free the
> >   regulators
> >
> > I ran into this while integrating the new pci_pwrctrl_*() API into the
> > MediaTek driver. I am sending this separately since this change is
> > unrelated and does not conflict with or depend on the other changes for
> > the driver itself.
> >
> > I think this should be merged for fixes.
> >
> > ---
> >  drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 12 ------------
> >  drivers/pci/pwrctrl/slot.c               |  1 -
> >  2 files changed, 13 deletions(-)
> >
> > diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> > index 0d0377283c37..c7e4beec160a 100644
> > --- a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> > +++ b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> > @@ -68,13 +68,6 @@ static int pwrseq_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)
> >       return pwrseq_power_off(pwrseq->pwrseq);
> >  }
> >
> > -static void devm_pwrseq_pwrctrl_power_off(void *data)
> > -{
> > -     struct pwrseq_pwrctrl *pwrseq = data;
> > -
> > -     pwrseq_pwrctrl_power_off(&pwrseq->pwrctrl);
> > -}
> > -
> >  static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
> >  {
> >       const struct pwrseq_pwrctrl_pdata *pdata;
> > @@ -101,11 +94,6 @@ static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
> >               return dev_err_probe(dev, PTR_ERR(pwrseq->pwrseq),
> >                                    "Failed to get the power sequencer\n");
> >
> > -     ret = devm_add_action_or_reset(dev, devm_pwrseq_pwrctrl_power_off,
> > -                                    pwrseq);
> > -     if (ret)
> > -             return ret;
> > -
> >       pwrseq->pwrctrl.power_on = pwrseq_pwrctrl_power_on;
> >       pwrseq->pwrctrl.power_off = pwrseq_pwrctrl_power_off;
> >
> > diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
> > index 082af81efe25..b87639253ae2 100644
> > --- a/drivers/pci/pwrctrl/slot.c
> > +++ b/drivers/pci/pwrctrl/slot.c
> > @@ -63,7 +63,6 @@ static void devm_slot_pwrctrl_release(void *data)
> >  {
> >       struct slot_pwrctrl *slot = data;
> >
> > -     slot_pwrctrl_power_off(&slot->pwrctrl);
> >       regulator_bulk_free(slot->num_supplies, slot->supplies);
> >  }
> >
> > --
> > 2.53.0.414.gf7e9f6c205-goog
> >
>
> --
> மணிவண்ணன் சதாசிவம்

Re: [PATCH v2] PCI/pwrctrl: Do not power off on pwrctrl device removal

[Manivannan Sadhasivam]

On Mon, Mar 23, 2026 at 07:41:17PM +0800, Chen-Yu Tsai wrote:
> Ping?
> 
> What's going on with this patch? The slot driver got renamed and this
> fix is still not merged.
> 

Bjorn, can you please pick this up for v7.0-rcS?

- Mani

> On Thu, Feb 26, 2026 at 5:30 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > On Thu, Feb 26, 2026 at 05:22:33PM +0800, Chen-Yu Tsai wrote:
> > > With the move to explicit pwrctrl power on/off APIs, the caller, i.e.
> > > the PCI driver should manage the power state. The pwrctrl drivers should
> > > not try to clean up or power off when they are removed, as this might
> > > end up disabling an already disabled regulator, causing a big warning.
> > > This can be triggered if a PCI controller driver's .remove() callback
> > > calls pci_pwrctrl_destroy_devices() after pci_pwrctrl_power_off_devices().
> > >
> > > Drop the devm cleanup parts that turn off regulators from the pwrctrl
> > > drivers.
> > >
> > > Fixes: b921aa3f8dec ("PCI/pwrctrl: Switch to pwrctrl create, power on/off, destroy APIs")
> > > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> >
> > Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
> >
> > - Mani
> >
> > > Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> > >
> > > ---
> > > Changes since v1:
> > > - Keep the release function for the PCI slot driver to free the
> > >   regulators
> > >
> > > I ran into this while integrating the new pci_pwrctrl_*() API into the
> > > MediaTek driver. I am sending this separately since this change is
> > > unrelated and does not conflict with or depend on the other changes for
> > > the driver itself.
> > >
> > > I think this should be merged for fixes.
> > >
> > > ---
> > >  drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 12 ------------
> > >  drivers/pci/pwrctrl/slot.c               |  1 -
> > >  2 files changed, 13 deletions(-)
> > >
> > > diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> > > index 0d0377283c37..c7e4beec160a 100644
> > > --- a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> > > +++ b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> > > @@ -68,13 +68,6 @@ static int pwrseq_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)
> > >       return pwrseq_power_off(pwrseq->pwrseq);
> > >  }
> > >
> > > -static void devm_pwrseq_pwrctrl_power_off(void *data)
> > > -{
> > > -     struct pwrseq_pwrctrl *pwrseq = data;
> > > -
> > > -     pwrseq_pwrctrl_power_off(&pwrseq->pwrctrl);
> > > -}
> > > -
> > >  static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
> > >  {
> > >       const struct pwrseq_pwrctrl_pdata *pdata;
> > > @@ -101,11 +94,6 @@ static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
> > >               return dev_err_probe(dev, PTR_ERR(pwrseq->pwrseq),
> > >                                    "Failed to get the power sequencer\n");
> > >
> > > -     ret = devm_add_action_or_reset(dev, devm_pwrseq_pwrctrl_power_off,
> > > -                                    pwrseq);
> > > -     if (ret)
> > > -             return ret;
> > > -
> > >       pwrseq->pwrctrl.power_on = pwrseq_pwrctrl_power_on;
> > >       pwrseq->pwrctrl.power_off = pwrseq_pwrctrl_power_off;
> > >
> > > diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
> > > index 082af81efe25..b87639253ae2 100644
> > > --- a/drivers/pci/pwrctrl/slot.c
> > > +++ b/drivers/pci/pwrctrl/slot.c
> > > @@ -63,7 +63,6 @@ static void devm_slot_pwrctrl_release(void *data)
> > >  {
> > >       struct slot_pwrctrl *slot = data;
> > >
> > > -     slot_pwrctrl_power_off(&slot->pwrctrl);
> > >       regulator_bulk_free(slot->num_supplies, slot->supplies);
> > >  }
> > >
> > > --
> > > 2.53.0.414.gf7e9f6c205-goog
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2] PCI/pwrctrl: Do not power off on pwrctrl device removal

[Bjorn Helgaas]

On Thu, Feb 26, 2026 at 05:22:33PM +0800, Chen-Yu Tsai wrote:
> With the move to explicit pwrctrl power on/off APIs, the caller, i.e.
> the PCI driver should manage the power state. The pwrctrl drivers should
> not try to clean up or power off when they are removed, as this might
> end up disabling an already disabled regulator, causing a big warning.
> This can be triggered if a PCI controller driver's .remove() callback
> calls pci_pwrctrl_destroy_devices() after pci_pwrctrl_power_off_devices().
> 
> Drop the devm cleanup parts that turn off regulators from the pwrctrl
> drivers.
> 
> Fixes: b921aa3f8dec ("PCI/pwrctrl: Switch to pwrctrl create, power on/off, destroy APIs")
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Applied to pci/for-linus for v7.0, thanks!

> ---
> Changes since v1:
> - Keep the release function for the PCI slot driver to free the
>   regulators
> 
> I ran into this while integrating the new pci_pwrctrl_*() API into the
> MediaTek driver. I am sending this separately since this change is
> unrelated and does not conflict with or depend on the other changes for
> the driver itself.
> 
> I think this should be merged for fixes.
> 
> ---
>  drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 12 ------------
>  drivers/pci/pwrctrl/slot.c               |  1 -
>  2 files changed, 13 deletions(-)
> 
> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> index 0d0377283c37..c7e4beec160a 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
> @@ -68,13 +68,6 @@ static int pwrseq_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)
>  	return pwrseq_power_off(pwrseq->pwrseq);
>  }
>  
> -static void devm_pwrseq_pwrctrl_power_off(void *data)
> -{
> -	struct pwrseq_pwrctrl *pwrseq = data;
> -
> -	pwrseq_pwrctrl_power_off(&pwrseq->pwrctrl);
> -}
> -
>  static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
>  {
>  	const struct pwrseq_pwrctrl_pdata *pdata;
> @@ -101,11 +94,6 @@ static int pwrseq_pwrctrl_probe(struct platform_device *pdev)
>  		return dev_err_probe(dev, PTR_ERR(pwrseq->pwrseq),
>  				     "Failed to get the power sequencer\n");
>  
> -	ret = devm_add_action_or_reset(dev, devm_pwrseq_pwrctrl_power_off,
> -				       pwrseq);
> -	if (ret)
> -		return ret;
> -
>  	pwrseq->pwrctrl.power_on = pwrseq_pwrctrl_power_on;
>  	pwrseq->pwrctrl.power_off = pwrseq_pwrctrl_power_off;
>  
> diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
> index 082af81efe25..b87639253ae2 100644
> --- a/drivers/pci/pwrctrl/slot.c
> +++ b/drivers/pci/pwrctrl/slot.c
> @@ -63,7 +63,6 @@ static void devm_slot_pwrctrl_release(void *data)
>  {
>  	struct slot_pwrctrl *slot = data;
>  
> -	slot_pwrctrl_power_off(&slot->pwrctrl);
>  	regulator_bulk_free(slot->num_supplies, slot->supplies);
>  }
>  
> -- 
> 2.53.0.414.gf7e9f6c205-goog
>