Re: [PATCH v2 4/5] PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks

[Manivannan Sadhasivam]

On Thu, Mar 05, 2026 at 02:26:22PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 3/5/2026 1:19 PM, Manivannan Sadhasivam wrote:
> > On Tue, Feb 17, 2026 at 04:49:09PM +0530, Krishna Chaitanya Chundru wrote:
> > > Some Qcom PCIe controller variants bring the PHY out of test power-down
> > > (PHY_TEST_PWR_DOWN) during init. When the link is later transitioned
> > > towards D3cold and the driver disables PCIe clocks and/or regulators
> > > without explicitly re-asserting PHY_TEST_PWR_DOWN, the PHY can remain
> > > partially powered, leading to avoidable power leakage.
> > > 
> > > Update the init-path comments to reflect that PARF_PHY_CTRL is used to
> > > power the PHY on. Also, for controller revisions that enable PHY power
> > > in init (2.3.2, 2.3.3, 2.7.0 and 2.9.0), explicitly power the PHY down
> > > via PARF_PHY_CTRL in the deinit path before disabling clocks/regulators.
> > > 
> > > This ensures the PHY is put into a defined low-power state prior to
> > > removing its supplies, preventing leakage when entering D3cold.
> > > 
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx>
> > > ---
> > >   drivers/pci/controller/dwc/pcie-qcom.c | 30 +++++++++++++++++++++++++++---
> > >   1 file changed, 27 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index 2c4dc7134e006d3530a809f1bcc1a6488d4632ad..b02c19bbdf2ea5db252c2a0281a569bb3a0cc497 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -513,7 +513,7 @@ static int qcom_pcie_post_init_2_1_0(struct qcom_pcie *pcie)
> > >   	u32 val;
> > >   	int ret;
> > > -	/* enable PCIe clocks and resets */
> > > +	/* PHY power ON */
> > This comment is confusing since we already have phy_power_on() API. What does
> > really happen in the 'test power down' case?
> QCOM PCIe controller wrapper has way to force the entire PHY into lowest
> power
> state by turning everything off, without this bit being cleared the phy will
> not be
> powered on even after phy_power_on(), we can think this as a kind of switch
> from the controller side to power on phy.
> 

We never cared to set/clear this bit so far. So I'm assuming that if we simply
set it during init, it will not do any harm and allow the PHY to fully power
down itself when phy_power_off() is called?

- Mani

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