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

[Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>]

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@oss.qualcomm.com>
>> ---
>>   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.

- Krishna Chaitanya.
> - Mani
>
>>   	val = readl(pcie->parf + PARF_PHY_CTRL);
>>   	val &= ~PHY_TEST_PWR_DOWN;
>>   	writel(val, pcie->parf + PARF_PHY_CTRL);
>> @@ -680,6 +680,12 @@ static int qcom_pcie_get_resources_2_3_2(struct qcom_pcie *pcie)
>>   static void qcom_pcie_deinit_2_3_2(struct qcom_pcie *pcie)
>>   {
>>   	struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
>> +	u32 val;
>> +
>> +	/* PHY Power down */
>> +	val = readl(pcie->parf + PARF_PHY_CTRL);
>> +	val |= PHY_TEST_PWR_DOWN;
>> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>>   
>>   	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>>   	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
>> @@ -712,7 +718,7 @@ static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie)
>>   {
>>   	u32 val;
>>   
>> -	/* enable PCIe clocks and resets */
>> +	/* PHY Power ON */
>>   	val = readl(pcie->parf + PARF_PHY_CTRL);
>>   	val &= ~PHY_TEST_PWR_DOWN;
>>   	writel(val, pcie->parf + PARF_PHY_CTRL);
>> @@ -844,6 +850,12 @@ static int qcom_pcie_get_resources_2_3_3(struct qcom_pcie *pcie)
>>   static void qcom_pcie_deinit_2_3_3(struct qcom_pcie *pcie)
>>   {
>>   	struct qcom_pcie_resources_2_3_3 *res = &pcie->res.v2_3_3;
>> +	u32 val;
>> +
>> +	/* PHY Power down */
>> +	val = readl(pcie->parf + PARF_PHY_CTRL);
>> +	val |= PHY_TEST_PWR_DOWN;
>> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>>   
>>   	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>>   }
>> @@ -994,7 +1006,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>>   	/* configure PCIe to RC mode */
>>   	writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE);
>>   
>> -	/* enable PCIe clocks and resets */
>> +	/* PHY power ON */
>>   	val = readl(pcie->parf + PARF_PHY_CTRL);
>>   	val &= ~PHY_TEST_PWR_DOWN;
>>   	writel(val, pcie->parf + PARF_PHY_CTRL);
>> @@ -1065,6 +1077,12 @@ static void qcom_pcie_host_post_init_2_7_0(struct qcom_pcie *pcie)
>>   static void qcom_pcie_deinit_2_7_0(struct qcom_pcie *pcie)
>>   {
>>   	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
>> +	u32 val;
>> +
>> +	/* PHY Power down */
>> +	val = readl(pcie->parf + PARF_PHY_CTRL);
>> +	val |= PHY_TEST_PWR_DOWN;
>> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>>   
>>   	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>>   
>> @@ -1169,6 +1187,12 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
>>   static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
>>   {
>>   	struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
>> +	u32 val;
>> +
>> +	/* PHY Power down */
>> +	val = readl(pcie->parf + PARF_PHY_CTRL);
>> +	val |= PHY_TEST_PWR_DOWN;
>> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>>   
>>   	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>>   }
>>
>> -- 
>> 2.34.1
>>