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@xxxxxxxxxxxxxxxx>
> Signed-off-by: Chen-Yu Tsai <wenst@xxxxxxxxxxxx>

Reviewed-by: Manivannan Sadhasivam <mani@xxxxxxxxxx>

- Mani

> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxxxxxxxx>
> 
> ---
> 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
> 

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