[PATCH v1 0/2] Add the PM suspend/resume callbacks for qcom-combo phys

[PATCH v1 0/2] Add the PM suspend/resume callbacks for qcom-combo phys

[Prashanth K]

Currently the phy clocks are left ON during system suspend, this leading to
power leakages. Prevent this by disabling the phy clocks during PM suspend.

Prashanth K (2):
  phy: qcom: qmp-combo: Add PM suspend/resume callbacks
  phy: qcom: qmp-usb: Add PM suspend/resume callbacks

 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 17 +++++++++++++++++
 drivers/phy/qualcomm/phy-qcom-qmp-usb.c   | 17 +++++++++++++++++
 2 files changed, 34 insertions(+)

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v1 1/2] phy: qcom: qmp-combo: Add PM suspend/resume callbacks

[Prashanth K]

Currently when we perform system suspend, the phy clocks aren't
turned off leading to power leakage. Prevent this by adding PM
suspend/resume callbacks for the qmp-combo-phy which toggles
clocks on/off correspondingly.

Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index c39ced168d03..f624195c9b84 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -4463,9 +4463,26 @@ static int __maybe_unused qmp_combo_runtime_resume(struct device *dev)
 	return 0;
 }
 
+static int __maybe_unused qmp_combo_pm_suspend(struct device *dev)
+{
+	if (pm_runtime_suspended(dev))
+		return 0;
+
+	return qmp_combo_runtime_suspend(dev);
+}
+
+static int __maybe_unused qmp_combo_pm_resume(struct device *dev)
+{
+	if (pm_runtime_suspended(dev))
+		return 0;
+
+	return qmp_combo_runtime_resume(dev);
+}
+
 static const struct dev_pm_ops qmp_combo_pm_ops = {
 	SET_RUNTIME_PM_OPS(qmp_combo_runtime_suspend,
 			   qmp_combo_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(qmp_combo_pm_suspend, qmp_combo_pm_resume)
 };
 
 static int qmp_combo_reset_init(struct qmp_combo *qmp)
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v1 2/2] phy: qcom: qmp-usb: Add PM suspend/resume callbacks

[Prashanth K]

Currently when we perform system suspend, the phy clocks aren't
turned off, this leads to power leakage. Prevent this by adding
PM suspend/resume callbacks for the qmp-usb phy which toggles
clocks on/off correspondingly.

Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usb.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
index b0790bcf0bc8..d2863633007d 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb.c
@@ -2150,9 +2150,26 @@ static int __maybe_unused qmp_usb_runtime_resume(struct device *dev)
 	return 0;
 }
 
+static int __maybe_unused qmp_usb_pm_suspend(struct device *dev)
+{
+	if (pm_runtime_suspended(dev))
+		return 0;
+
+	return qmp_usb_runtime_suspend(dev);
+}
+
+static int __maybe_unused qmp_usb_pm_resume(struct device *dev)
+{
+	if (pm_runtime_suspended(dev))
+		return 0;
+
+	return qmp_usb_runtime_resume(dev);
+}
+
 static const struct dev_pm_ops qmp_usb_pm_ops = {
 	SET_RUNTIME_PM_OPS(qmp_usb_runtime_suspend,
 			   qmp_usb_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(qmp_usb_pm_suspend, qmp_usb_pm_resume)
 };
 
 static int qmp_usb_reset_init(struct qmp_usb *qmp,
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v1 1/2] phy: qcom: qmp-combo: Add PM suspend/resume callbacks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] phy: qcom-qmp-combo: race and unclocked MMIO in PM suspend/resume
--

commit e464964d5e47ce46857ee85893848203f7194310
Author: Prashanth K <prashanth.k@oss.qualcomm.com>

phy: qcom: qmp-combo: Add PM suspend/resume callbacks

This commit adds system sleep suspend and resume callbacks for the
qmp-combo phy driver to toggle phy clocks off and on across system
suspend when the device is not runtime suspended.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 187f0cd11f6c3..3b4a1b174c882 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -4758,9 +4758,26 @@ static int __maybe_unused qmp_combo_runtime_resume(struct device *dev)
>  	return 0;
>  }
>  
> +static int __maybe_unused qmp_combo_pm_suspend(struct device *dev)
> +{
> +	if (pm_runtime_suspended(dev))
> +		return 0;
> +
> +	return qmp_combo_runtime_suspend(dev);
> +}
> +
> +static int __maybe_unused qmp_combo_pm_resume(struct device *dev)
> +{
> +	if (pm_runtime_suspended(dev))
> +		return 0;
> +
> +	return qmp_combo_runtime_resume(dev);
> +}

[Severity: High]
Can qmp_combo_pm_suspend() and qmp_combo_pm_resume() race with the type-c
switch and mux callbacks or leave the phy clocks disabled while type-c
events access mmio registers?

In qmp_combo_probe(), runtime pm is forbidden by default via
pm_runtime_forbid(), so pm_runtime_suspended() is false. During system
suspend with usb in host mode and wakeup enabled, dwc3_suspend_common()
keeps the phy initialized, leaving qmp->init_count > 0.

When qmp_combo_pm_suspend() runs, it calls qmp_combo_runtime_suspend()
without holding qmp->phy_mutex:

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_runtime_suspend() {
	if (!qmp->init_count) {
		dev_vdbg(dev, "PHY not initialized, bailing out\n");
		return 0;
	}

	qmp_combo_enable_autonomous_mode(qmp);

	clk_disable_unprepare(qmp->pipe_clk);
	clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
}

This disables qmp->pipe_clk and qmp->clks while qmp->init_count remains
non-zero.

Meanwhile, the type-c orientation and mode callbacks
qmp_combo_typec_switch_set() and qmp_combo_typec_mux_set() do not have a
device link to qmp->dev and can be invoked from workqueues or kthread
workers (such as pmic_glink_ucsi_notify() -> ucsi_handle_connector_change()
or tcpm) during system suspend, s2idle, or before qmp_combo_pm_resume()
runs:

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_typec_switch_set() {
	guard(mutex)(&qmp->phy_mutex);
	qmp->orientation = orientation;

	if (qmp->init_count) {
		if (qmp->usb_init_count || qmp->usb4_init_count)
			qmp_combo_usb_power_off(qmp->usb_phy);
		qmp_combo_com_exit(qmp, true);

		qmp_combo_com_init(qmp, true);
		...
	}
}

And similarly in qmp_combo_typec_mux_set():

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_typec_mux_set() {
	guard(mutex)(&qmp->phy_mutex);
	...
	if (!qmp->init_count)
		return 0;

	return qmp_combo_reconfigure_phy(qmp, new_mode);
}

If a type-c event arrives concurrently with qmp_combo_pm_suspend() or
qmp_combo_pm_resume(), or while qmp->dev is system-suspended with
qmp->init_count > 0, won't these paths access qmp->pcs and other phy mmio
registers while qmp->pipe_clk and qmp->clks are disabled, causing an
unclocked register access crash, underflowing the clock enable counts in
qmp_combo_com_exit(), and leaving the clocks enabled again before
qmp_combo_pm_resume() enables them a second time?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022104321.306679-1-prashanth.k@oss.qualcomm.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v1 1/2] phy: qcom: qmp-combo: Add PM suspend/resume callbacks

[Konrad Dybcio]

On 10/22/26 12:43 PM, Prashanth K wrote:
> Currently when we perform system suspend, the phy clocks aren't
> turned off leading to power leakage. Prevent this by adding PM
> suspend/resume callbacks for the qmp-combo-phy which toggles
> clocks on/off correspondingly.
> 
> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
> ---

Well this still leaves pm_runtime_forbid() in the probe function..

>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index c39ced168d03..f624195c9b84 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -4463,9 +4463,26 @@ static int __maybe_unused qmp_combo_runtime_resume(struct device *dev)
>  	return 0;
>  }
>  
> +static int __maybe_unused qmp_combo_pm_suspend(struct device *dev)
> +{
> +	if (pm_runtime_suspended(dev))
> +		return 0;
> +
> +	return qmp_combo_runtime_suspend(dev);
> +}
> +
> +static int __maybe_unused qmp_combo_pm_resume(struct device *dev)
> +{
> +	if (pm_runtime_suspended(dev))
> +		return 0;
> +
> +	return qmp_combo_runtime_resume(dev);
> +}
> +
>  static const struct dev_pm_ops qmp_combo_pm_ops = {
>  	SET_RUNTIME_PM_OPS(qmp_combo_runtime_suspend,
>  			   qmp_combo_runtime_resume, NULL)
> +	SET_SYSTEM_SLEEP_PM_OPS(qmp_combo_pm_suspend, qmp_combo_pm_resume)

You can instead do

+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)

and as a related fix, we should probably throw a mutex guard into the
runtime pm functions

Konrad

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v1 1/2] phy: qcom: qmp-combo: Add PM suspend/resume callbacks

[Prashanth K]

On 10/23/2026 3:53 PM, Konrad Dybcio wrote:
> On 10/22/26 12:43 PM, Prashanth K wrote:
>> Currently when we perform system suspend, the phy clocks aren't
>> turned off leading to power leakage. Prevent this by adding PM
>> suspend/resume callbacks for the qmp-combo-phy which toggles
>> clocks on/off correspondingly.
>>
>> Signed-off-by: Prashanth K <prashanth.k@oss.qualcomm.com>
>> ---
> 
> Well this still leaves pm_runtime_forbid() in the probe function..
> 

Not sure about the history behind this, I assume its for controlling RT
behavior from user-space.
>>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> index c39ced168d03..f624195c9b84 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> @@ -4463,9 +4463,26 @@ static int __maybe_unused qmp_combo_runtime_resume(struct device *dev)
>>  	return 0;
>>  }
>>  
>> +static int __maybe_unused qmp_combo_pm_suspend(struct device *dev)
>> +{
>> +	if (pm_runtime_suspended(dev))
>> +		return 0;
>> +
>> +	return qmp_combo_runtime_suspend(dev);
>> +}
>> +
>> +static int __maybe_unused qmp_combo_pm_resume(struct device *dev)
>> +{
>> +	if (pm_runtime_suspended(dev))
>> +		return 0;
>> +
>> +	return qmp_combo_runtime_resume(dev);
>> +}
>> +
>>  static const struct dev_pm_ops qmp_combo_pm_ops = {
>>  	SET_RUNTIME_PM_OPS(qmp_combo_runtime_suspend,
>>  			   qmp_combo_runtime_resume, NULL)
>> +	SET_SYSTEM_SLEEP_PM_OPS(qmp_combo_pm_suspend, qmp_combo_pm_resume)
> 
> You can instead do
> 
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +				pm_runtime_force_resume)
> 
> and as a related fix, we should probably throw a mutex guard into the
> runtime pm functions
> 
> Konrad

Yes we can do that as well. Will update in next patchset.

Thanks!
Prashanth K

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy