RE: [PATCH V3 1/8] PCI: imx6: Add skip_pwrctrl_off flag support

[Sherry Sun]

> Subject: Re: [PATCH V3 1/8] PCI: imx6: Add skip_pwrctrl_off flag support
> 
> On Fri, Jun 26, 2026 at 10:31:19AM +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.
> 
> Need empty line here.

Ok, will fix.

> 
> > This allows controller power-off to be skipped when some devices(e.g.
> > M.2 cards key E without auxiliary power) required to support PCIe L2
> > link state and wake-up mechanisms.
> >
> > Move pci_pwrctrl_create_devices() to imx_pcie_probe() so that it is
> > only called once during probe, similar to other regulator_get calls.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@xxxxxxx>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 43
> > ++++++++++++++++-----------
> >  1 file changed, 25 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 0fa716d1ed75..0685573fee71 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -1382,16 +1382,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp
> *pp)
> >  		}
> >  	}
> >
> > -	ret = pci_pwrctrl_create_devices(dev);
> > -	if (ret) {
> > -		dev_err(dev, "failed to create pwrctrl devices\n");
> > -		goto err_reg_disable;
> > -	}
> > -
> 
> Please two patch do that. one patch move pci_pwrctrl_create_devices() to
> probe
> 
> one patch check skip_power_off.
> 
> > -	ret = pci_pwrctrl_power_on_devices(dev);
> > -	if (ret) {
> > -		dev_err(dev, "failed to power on pwrctrl devices\n");
> > -		goto err_pwrctrl_destroy;
> > +	if (!pp->skip_pwrctrl_off) {
> > +		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); @@ -1460,10 +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);
> > -err_pwrctrl_destroy:
> > -	if (ret != -EPROBE_DEFER)
> > -		pci_pwrctrl_destroy_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);
> > @@ -1482,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);
> >  	if (imx_pcie->vpcie)
> >  		regulator_disable(imx_pcie->vpcie);
> >  }
> > @@ -1954,11 +1949,15 @@ static int imx_pcie_probe(struct
> platform_device *pdev)
> >  	if (ret)
> >  		return ret;
> >
> > +	ret = pci_pwrctrl_create_devices(dev);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "failed to create pwrctrl
> > +devices\n");
> > +
> >  	pci->use_parent_dt_ranges = true;
> >  	if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
> >  		ret = imx_add_pcie_ep(imx_pcie, pdev);
> >  		if (ret < 0)
> > -			return ret;
> > +			goto err_pwrctrl_destroy;
> >
> >  		/*
> >  		 * FIXME: Only single Device (EPF) is supported due to the
> @@
> > -1973,7 +1972,7 @@ static int imx_pcie_probe(struct platform_device
> *pdev)
> >  		pci->pp.use_atu_msg = true;
> >  		ret = dw_pcie_host_init(&pci->pp);
> >  		if (ret < 0)
> > -			return ret;
> > +			goto err_pwrctrl_destroy;
> >
> >  		if (pci_msi_enabled()) {
> >  			u8 offset = dw_pcie_find_capability(pci,
> PCI_CAP_ID_MSI); @@
> > -1985,16 +1984,24 @@ static int imx_pcie_probe(struct platform_device
> *pdev)
> >  	}
> >
> >  	return 0;
> > +
> > +err_pwrctrl_destroy:
> > +	if (ret != -EPROBE_DEFER)
> > +		pci_pwrctrl_destroy_devices(dev);
> > +	return ret;
> 
> Mani said he will fix DEFER problem soon.

Yes, so for now I'll make a patch to move pci_pwrctrl_create_devices() into the
probe() and temporarily add the err_pwrctrl_destroy error label until Mani fixes it.
Let me know if any other suggestions, thanks!

Best Regards
Sherry

> 
> Frank
> 
> >  }
> >
> >  static void imx_pcie_shutdown(struct platform_device *pdev)  {
> >  	struct imx_pcie *imx_pcie = platform_get_drvdata(pdev);
> > +	struct dw_pcie *pci = imx_pcie->pci;
> > +	struct dw_pcie_rp *pp = &pci->pp;
> >
> >  	/* bring down link, so bootloader gets clean state in case of reboot */
> >  	imx_pcie_assert_core_reset(imx_pcie);
> >  	imx_pcie_assert_perst(imx_pcie, true);
> > -	pci_pwrctrl_power_off_devices(&pdev->dev);
> > +	if (!pp->skip_pwrctrl_off)
> > +		pci_pwrctrl_power_off_devices(&pdev->dev);
> >  	pci_pwrctrl_destroy_devices(&pdev->dev);
> >  }
> >
> > --
> > 2.50.1
> >
> >