Re: [PATCH V5 2/4] PCI: imx6: Add skip_pwrctrl_off flag support

[Frank Li]

On Fri, Jul 10, 2026 at 02:16:35AM +0000, Sherry Sun wrote:
> > Subject: Re: [PATCH V5 2/4] PCI: imx6: Add skip_pwrctrl_off flag support
> >
> > On Thu, Jul 09, 2026 at 06:15:53PM +0800, Sherry Sun (OSS) wrote:
> > > From: Sherry Sun <sherry.sun@xxxxxxx>
> > >
> > > Use dw_pcie_rp::skip_pwrctrl_off to avoid powering off devices during
> > > suspend to preserve wakeup capability of the devices and also not to
> > > power on the devices in the init path.
> > >
> > > This allows controller power-off to be skipped when some devices (e.g.
> > > M.2 Key E cards without auxiliary power) need to support PCIe L2 link
> > > state and wake-up mechanisms.
> > >
> > > Signed-off-by: Sherry Sun <sherry.sun@xxxxxxx>
> > > ---
> > >  drivers/pci/controller/dwc/pci-imx6.c | 16 ++++++++++------
> > >  1 file changed, 10 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 92f8e4a299e8..afcf3b6bf3cd 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -1382,10 +1382,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp
> > *pp)
> > >  		}
> > >  	}
> > >
> > > -	ret = pci_pwrctrl_power_on_devices(dev);
> > > -	if (ret) {
> > > -		dev_err(dev, "failed to power on pwrctrl devices\n");
> > > -		goto err_reg_disable;
> > > +	if (!pp->skip_pwrctrl_off) {
> >
> > if pci_pwrctrl_power_on_devices is true, where call
> > pci_pwrctrl_power_on_devices()
>
> Hi Frank,
> The skip_pwrctrl_off flag defaults to false during the pcie bus probing phase,
> so the pci_pwrctrl_power_on_devices() is called at least once to ensure all
> regulators are enabled.
> The value of skip_pwrctrl_off is only changed in dw_pcie_suspend_noirq(),
> and is refreshed by calling the pci_host_common_d3cold_possible() each
> time a suspend occurs.
> This is why we use this flag at runtime to avoid powering off devices during
> suspend to preserve wakeup capability of the devices.

Okay, It would be better this part code can move into common dwc later.
this is not specific for imx.

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

Frank

>
> >
> > > +		ret = pci_pwrctrl_power_on_devices(dev);
> > > +		if (ret) {
> > > +			dev_err(dev, "failed to power on pwrctrl devices\n");
> > > +			goto err_reg_disable;
> > > +		}
> > >  	}
> > >
> > >  	ret = imx_pcie_clk_enable(imx_pcie); @@ -1454,7 +1456,8 @@ static
> > > int imx_pcie_host_init(struct dw_pcie_rp *pp)
> > >  err_clk_disable:
> > >  	imx_pcie_clk_disable(imx_pcie);
> > >  err_pwrctrl_power_off:
> > > -	pci_pwrctrl_power_off_devices(dev);
> > > +	if (!pp->skip_pwrctrl_off)
> > > +		pci_pwrctrl_power_off_devices(dev);
> > >  err_reg_disable:
> > >  	if (imx_pcie->vpcie)
> > >  		regulator_disable(imx_pcie->vpcie);
> > > @@ -1473,7 +1476,8 @@ static void imx_pcie_host_exit(struct dw_pcie_rp
> > *pp)
> > >  	}
> > >  	imx_pcie_clk_disable(imx_pcie);
> > >
> > > -	pci_pwrctrl_power_off_devices(pci->dev);
> > > +	if (!pci->pp.skip_pwrctrl_off)
> > > +		pci_pwrctrl_power_off_devices(pci->dev);
> >
> > Not sure if there are counter in side power_(on|off) function, if not, just skip
> > power_off is enough.
>
> It depends on the pwrctrl drivers implementation, for the generic pwrctrl, it will
> call regulator_bulk_enable/disable() to handle this, so has enable/use counter.
>
> As mentioned above, the value of skip_pwrctrl_off is refreshed in
> dw_pcie_suspend_noirq() for each suspend/resume, so we need this runtime
> check to avoid powering off some specific devices during suspend.
>
> Best Regards
> Sherry