[PATCH v5 0/5] phy: qcom: Fix possible NULL-deref and runtime PM race conditions

[PATCH v5 0/5] phy: qcom: Fix possible NULL-deref and runtime PM race conditions

[Loic Poulain]

Address potential NULL pointer dereferences and race conditions related
to runtime PM in several Qualcomm PHY drivers. In all cases, enabling
runtime PM before the PHY instance is fully initialized can lead to
crashes during early runtime suspend callbacks.

- Attach driver data before enabling runtime PM.
- Introduce initialization flags where needed to avoid dereferencing
uninitialized pointers.
- Reorder pm_runtime_enable() and pm_runtime_forbid() calls to prevent
unnecessary suspend/resume cycles during driver probe.
- Use devres-managed PM runtime helpers for proper cleanup.

Why it happens?

The PHY is a supplier of the USB device. A USB device cannot be probed
until all its suppliers are ready. As long as the PHY is not ready, the
device core keeps retrying the probe, which fails with -EPROBE_DEFER.

At some point the PHY probe finally runs, but the device core may still be
attempting to probe the USB device concurrently.
 
Inside __driver_probe_device(), we have:

    ret = really_probe(dev, drv);
    pm_request_idle(dev);

    if (dev->parent)
        pm_runtime_put(dev->parent);

    pm_runtime_put_suppliers(dev);
    return ret;

This means that whenever a USB probe attempt completes, whether with an
error or not, its suppliers are released via pm_runtime_put_suppliers().
Releasing suppliers may in turn trigger a runtime suspend.
 
In our case, since the PHY is a supplier of the USB device, the USB core
keeps 'looping' in __driver_probe_device() returning -EPROBE_DEFER until
the PHY becomes ready. As a result, pm_runtime_put_suppliers() may run
concurrently with the PHY's probe function. If this happens after
runtime PM has been enabled for the PHY, but before the driver has
forbidden suspend or taken a PM reference, the PHY may end up being
runtime-suspended 'unexpectedly'

---
Changes in v5:
- Re-introduce phy: qcom: qmp-usb-legacy: Fix possible NULL-deref (sashiko)
- Link to v4: https://lore.kernel.org/r/20260720-qcom-usb-phy-fix-null-v4-0-4d2a0f06d53a@oss.qualcomm.com

Changes in V4:
- Instead of moving pm/forbid, increment the pm usage counter before
  enabling runtime pm and decrement it after the PHY has been created. (Johan)
- Drop now unnecessary dev_set_drvdata() move in snps-femto-v2
- Drop 4/5 (qmp-usb-legacy: Prevent unnecessary PM runtime suspend at boot)
  which is not required anymore. 
- Rebase
- Link to v3: https://lore.kernel.org/all/20260205160240.748371-1-loic.poulain@oss.qualcomm.com/

Changes in v3:
Rebase on next and remove 2/6 (obsolete)

Changes in v2:
Split patches 2/4 and 3/4 so that the null‑pointer dereference fix and
the runtime‑PM enable/forbid reordering are logically separated.

Loic Poulain (4):
  phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot
  phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
  phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime
    suspend
  phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime
    suspend

 drivers/phy/qualcomm/phy-qcom-qmp-combo.c     | 10 ++++-----
 .../phy/qualcomm/phy-qcom-qmp-usb-legacy.c    | 21 ++++++++++++-------
 drivers/phy/qualcomm/phy-qcom-qmp-usbc.c      | 10 ++++-----
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 15 ++++++-------
 4 files changed, 32 insertions(+), 24 deletions(-)

--
2.34.1

To: Vinod Koul <vkoul@kernel.org>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Dmitry Baryshkov <lumag@kernel.org>
To: Wesley Cheng <quic_wcheng@quicinc.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-phy@lists.infradead.org
Cc: linux-kernel@vger.kernel.org

---
Loic Poulain (5):
      phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot
      phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
      phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
      phy: qcom: qmp-usb-legacy: Prevent unnecessary PM runtime suspend at boot
      phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

 drivers/phy/qualcomm/phy-qcom-qmp-combo.c      | 19 ++++++++++++-----
 drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 28 +++++++++++++++++++++-----
 drivers/phy/qualcomm/phy-qcom-qmp-usbc.c       | 18 +++++++++++++----
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c  | 17 +++++++++++++---
 4 files changed, 65 insertions(+), 17 deletions(-)
---
base-commit: 910b828b22b7b91054b3bd4be676a017444b0e00
change-id: 20260720-qcom-usb-phy-fix-null-ff097f56d1e4

Best regards,
-- 
Loic Poulain <loic.poulain@oss.qualcomm.com>

[PATCH v5 1/5] phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot

[Loic Poulain]

Runtime PM has to be enabled before creating the PHYs, since phy_create()
only enables runtime PM on the PHY devices if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHYs are not yet registered.

Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHYs have been created to
prevent the device from being runtime suspended during that window.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index cdcfad2e86b1d37650c4d7b0433319837ee9c9d4..affc8e040bb968a5b9d0b6f6fc2694c471368729 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -4934,10 +4934,16 @@ static int qmp_combo_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
+	/*
+	 * Enable runtime PM before creating the PHYs, phy_create() only enables
+	 * it on the PHY devices if already enabled on the parent. Hold a usage
+	 * reference so callbacks cannot run until the PHY is ready.
+	 */
+	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
-		goto err_node_put;
+		goto err_pm_put;
 	/*
 	 * Prevent runtime pm from being ON by default. Users can enable
 	 * it using power/control in sysfs.
@@ -4946,14 +4952,13 @@ static int qmp_combo_probe(struct platform_device *pdev)
 
 	ret = qmp_combo_register_clocks(qmp, usb_np, dp_np);
 	if (ret)
-		goto err_node_put;
-
+		goto err_pm_put;
 
 	qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
 	if (IS_ERR(qmp->usb_phy)) {
 		ret = PTR_ERR(qmp->usb_phy);
 		dev_err(dev, "failed to create USB PHY: %d\n", ret);
-		goto err_node_put;
+		goto err_pm_put;
 	}
 
 	phy_set_drvdata(qmp->usb_phy, qmp);
@@ -4962,11 +4967,13 @@ static int qmp_combo_probe(struct platform_device *pdev)
 	if (IS_ERR(qmp->dp_phy)) {
 		ret = PTR_ERR(qmp->dp_phy);
 		dev_err(dev, "failed to create DP PHY: %d\n", ret);
-		goto err_node_put;
+		goto err_pm_put;
 	}
 
 	phy_set_drvdata(qmp->dp_phy, qmp);
 
+	pm_runtime_put(dev);
+
 	if (usb_np == dev->of_node)
 		phy_provider = devm_of_phy_provider_register(dev, qmp_combo_phy_xlate);
 	else
@@ -4977,6 +4984,8 @@ static int qmp_combo_probe(struct platform_device *pdev)
 
 	return PTR_ERR_OR_ZERO(phy_provider);
 
+err_pm_put:
+	pm_runtime_put_noidle(dev);
 err_node_put:
 	of_node_put(usb_np);
 	of_node_put(dp_np);

-- 
2.34.1

[PATCH v5 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot

[Loic Poulain]

Runtime PM has to be enabled before creating the PHYs, since phy_create()
only enables runtime PM on the PHY devices if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHYs are not yet registered.

Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHYs have been created to
prevent the device from being runtime suspended during that window.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
index ab3055bb5b0c198832ae06dfcd04fd34395e271d..4317224070fd8dec98090e9200ac7d0e97265979 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
@@ -1959,10 +1959,16 @@ static int qmp_usbc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
+	/*
+	 * Enable runtime PM before creating the PHYs, phy_create() only enables
+	 * it on the PHY devices if already enabled on the parent. Hold a usage
+	 * reference so callbacks cannot run until the PHY is ready.
+	 */
+	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
-		goto err_node_put;
+		goto err_pm_put;
 	/*
 	 * Prevent runtime pm from being ON by default. Users can enable
 	 * it using power/control in sysfs.
@@ -1971,13 +1977,13 @@ static int qmp_usbc_probe(struct platform_device *pdev)
 
 	ret = qmp_usbc_register_clocks(qmp, np);
 	if (ret)
-		goto err_node_put;
+		goto err_pm_put;
 
 	qmp->usb_phy = devm_phy_create(dev, np, &qmp_usbc_usb_phy_ops);
 	if (IS_ERR(qmp->usb_phy)) {
 		ret = PTR_ERR(qmp->usb_phy);
 		dev_err(dev, "failed to create PHY: %d\n", ret);
-		goto err_node_put;
+		goto err_pm_put;
 	}
 
 	phy_set_drvdata(qmp->usb_phy, qmp);
@@ -1987,17 +1993,21 @@ static int qmp_usbc_probe(struct platform_device *pdev)
 		if (IS_ERR(qmp->dp_phy)) {
 			ret = PTR_ERR(qmp->dp_phy);
 			dev_err(dev, "failed to create PHY: %d\n", ret);
-			goto err_node_put;
+			goto err_pm_put;
 		}
 		phy_set_drvdata(qmp->dp_phy, qmp);
 	}
 
+	pm_runtime_put(dev);
+
 	of_node_put(np);
 
 	phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
 
 	return PTR_ERR_OR_ZERO(phy_provider);
 
+err_pm_put:
+	pm_runtime_put_noidle(dev);
 err_node_put:
 	of_node_put(np);
 	return ret;

-- 
2.34.1

Re: [PATCH v5 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot

[Dmitry Baryshkov]

On Tue, Jul 21, 2026 at 03:05:46PM +0200, Loic Poulain wrote:
> Runtime PM has to be enabled before creating the PHYs, since phy_create()
> only enables runtime PM on the PHY devices if it is already enabled on
> this parent device. This opens a small window where the device can be
> runtime suspended after pm_runtime_enable() and before the later
> pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
> the PHYs are not yet registered.
> 
> Take a runtime PM usage reference with pm_runtime_get_noresume() before
> enabling runtime PM and release it once the PHYs have been created to
> prevent the device from being runtime suspended during that window.
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> index ab3055bb5b0c198832ae06dfcd04fd34395e271d..4317224070fd8dec98090e9200ac7d0e97265979 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> @@ -1959,10 +1959,16 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_node_put;
>  
> +	/*
> +	 * Enable runtime PM before creating the PHYs, phy_create() only enables
> +	 * it on the PHY devices if already enabled on the parent. Hold a usage
> +	 * reference so callbacks cannot run until the PHY is ready.
> +	 */
> +	pm_runtime_get_noresume(dev);
>  	pm_runtime_set_active(dev);
>  	ret = devm_pm_runtime_enable(dev);
>  	if (ret)
> -		goto err_node_put;
> +		goto err_pm_put;
>  	/*
>  	 * Prevent runtime pm from being ON by default. Users can enable
>  	 * it using power/control in sysfs.
> @@ -1971,13 +1977,13 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  
>  	ret = qmp_usbc_register_clocks(qmp, np);
>  	if (ret)
> -		goto err_node_put;
> +		goto err_pm_put;
>  
>  	qmp->usb_phy = devm_phy_create(dev, np, &qmp_usbc_usb_phy_ops);
>  	if (IS_ERR(qmp->usb_phy)) {
>  		ret = PTR_ERR(qmp->usb_phy);
>  		dev_err(dev, "failed to create PHY: %d\n", ret);
> -		goto err_node_put;
> +		goto err_pm_put;
>  	}
>  
>  	phy_set_drvdata(qmp->usb_phy, qmp);
> @@ -1987,17 +1993,21 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  		if (IS_ERR(qmp->dp_phy)) {
>  			ret = PTR_ERR(qmp->dp_phy);
>  			dev_err(dev, "failed to create PHY: %d\n", ret);
> -			goto err_node_put;
> +			goto err_pm_put;
>  		}
>  		phy_set_drvdata(qmp->dp_phy, qmp);
>  	}
>  
> +	pm_runtime_put(dev);

Should be _put_sync() ?

> +
>  	of_node_put(np);
>  
>  	phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
>  
>  	return PTR_ERR_OR_ZERO(phy_provider);
>  
> +err_pm_put:
> +	pm_runtime_put_noidle(dev);

Why is it _noidle()?

>  err_node_put:
>  	of_node_put(np);
>  	return ret;
> 
> -- 
> 2.34.1
> 
> 
> -- 
> linux-phy mailing list
> linux-phy@lists.infradead.org
> https://lists.infradead.org/mailman/listinfo/linux-phy

-- 
With best wishes
Dmitry

Re: [PATCH v5 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot

[Loic Poulain]

Hi Dmitry,


On Tue, Jul 21, 2026 at 3:32 PM Dmitry Baryshkov
<dmitry.baryshkov@oss.qualcomm.com> wrote:
>
> On Tue, Jul 21, 2026 at 03:05:46PM +0200, Loic Poulain wrote:
> > Runtime PM has to be enabled before creating the PHYs, since phy_create()
> > only enables runtime PM on the PHY devices if it is already enabled on
> > this parent device. This opens a small window where the device can be
> > runtime suspended after pm_runtime_enable() and before the later
> > pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
> > the PHYs are not yet registered.
> >
> > Take a runtime PM usage reference with pm_runtime_get_noresume() before
> > enabling runtime PM and release it once the PHYs have been created to
> > prevent the device from being runtime suspended during that window.
> >
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> > Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> > ---
> >  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> > index ab3055bb5b0c198832ae06dfcd04fd34395e271d..4317224070fd8dec98090e9200ac7d0e97265979 100644
> > --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> > @@ -1959,10 +1959,16 @@ static int qmp_usbc_probe(struct platform_device *pdev)
> >       if (ret)
> >               goto err_node_put;
> >
> > +     /*
> > +      * Enable runtime PM before creating the PHYs, phy_create() only enables
> > +      * it on the PHY devices if already enabled on the parent. Hold a usage
> > +      * reference so callbacks cannot run until the PHY is ready.
> > +      */
> > +     pm_runtime_get_noresume(dev);
> >       pm_runtime_set_active(dev);
> >       ret = devm_pm_runtime_enable(dev);
> >       if (ret)
> > -             goto err_node_put;
> > +             goto err_pm_put;
> >       /*
> >        * Prevent runtime pm from being ON by default. Users can enable
> >        * it using power/control in sysfs.
> > @@ -1971,13 +1977,13 @@ static int qmp_usbc_probe(struct platform_device *pdev)
> >
> >       ret = qmp_usbc_register_clocks(qmp, np);
> >       if (ret)
> > -             goto err_node_put;
> > +             goto err_pm_put;
> >
> >       qmp->usb_phy = devm_phy_create(dev, np, &qmp_usbc_usb_phy_ops);
> >       if (IS_ERR(qmp->usb_phy)) {
> >               ret = PTR_ERR(qmp->usb_phy);
> >               dev_err(dev, "failed to create PHY: %d\n", ret);
> > -             goto err_node_put;
> > +             goto err_pm_put;
> >       }
> >
> >       phy_set_drvdata(qmp->usb_phy, qmp);
> > @@ -1987,17 +1993,21 @@ static int qmp_usbc_probe(struct platform_device *pdev)
> >               if (IS_ERR(qmp->dp_phy)) {
> >                       ret = PTR_ERR(qmp->dp_phy);
> >                       dev_err(dev, "failed to create PHY: %d\n", ret);
> > -                     goto err_node_put;
> > +                     goto err_pm_put;
> >               }
> >               phy_set_drvdata(qmp->dp_phy, qmp);
> >       }
> >
> > +     pm_runtime_put(dev);
>
> Should be _put_sync() ?

Well, yes for avoiding inconsistent state on error path, but it might
be cleaner to move the async put after
devm_of_phy_provider_register(), once we know the probe has succeeded,
and then rely on put_noidle() for all error paths.

>
> > +
> >       of_node_put(np);
> >
> >       phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
> >
> >       return PTR_ERR_OR_ZERO(phy_provider);
> >
> > +err_pm_put:
> > +     pm_runtime_put_noidle(dev);
>
> Why is it _noidle()?

The device starts in the active state (pm_runtime_set_active()). Here,
I use put_noidle() only to balance the PM usage counter while keeping
the device active. This restores the runtime PM state to what it was
before the probe (pm_runtime_get_noresume), allowing the device to be
reprobed from the same state.


>
> >  err_node_put:
> >       of_node_put(np);
> >       return ret;
> >
> > --
> > 2.34.1
> >
> >
> > --
> > linux-phy mailing list
> > linux-phy@lists.infradead.org
> > https://lists.infradead.org/mailman/listinfo/linux-phy
>
> --
> With best wishes
> Dmitry

[PATCH v5 3/5] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend

[Loic Poulain]

There is a small window where the runtime suspend callback may run
after pm_runtime_enable() and before pm_runtime_forbid(). In this
case, a crash occurs because runtime suspend/resume dereferences
qmp->phy pointer, which is not yet initialized:
        `if (!qmp->phy->init_count) {`

This can also happen if user re-enables runtime-pm via the sysfs
attribute before qmp phy is initialized.

Similarly to other qcom phy drivers, introduce a qmp->phy_initialized
variable that can be used to avoid relying on the possibly uninitialized
phy pointer.

Fixes: e464a3180a43 ("phy: qcom-qmp-usb: split off the legacy USB+dp_com support")
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
index 8bf951b0490cfd811635df8940de1b789e21b46c..fc490589c8e4888770807a60170905a1c7610421 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
@@ -542,6 +542,8 @@ struct qmp_usb {
 
 	enum phy_mode mode;
 
+	bool phy_initialized;
+
 	struct phy *phy;
 
 	struct clk_fixed_rate pipe_clk_fixed;
@@ -895,6 +897,7 @@ static int qmp_usb_legacy_power_off(struct phy *phy)
 
 static int qmp_usb_legacy_enable(struct phy *phy)
 {
+	struct qmp_usb *qmp = phy_get_drvdata(phy);
 	int ret;
 
 	ret = qmp_usb_legacy_init(phy);
@@ -904,14 +907,19 @@ static int qmp_usb_legacy_enable(struct phy *phy)
 	ret = qmp_usb_legacy_power_on(phy);
 	if (ret)
 		qmp_usb_legacy_exit(phy);
+	else
+		qmp->phy_initialized = true;
 
 	return ret;
 }
 
 static int qmp_usb_legacy_disable(struct phy *phy)
 {
+	struct qmp_usb *qmp = phy_get_drvdata(phy);
 	int ret;
 
+	qmp->phy_initialized = false;
+
 	ret = qmp_usb_legacy_power_off(phy);
 	if (ret)
 		return ret;
@@ -988,7 +996,7 @@ static int __maybe_unused qmp_usb_legacy_runtime_suspend(struct device *dev)
 
 	dev_vdbg(dev, "Suspending QMP phy, mode:%d\n", qmp->mode);
 
-	if (!qmp->phy->init_count) {
+	if (!qmp->phy_initialized) {
 		dev_vdbg(dev, "PHY not initialized, bailing out\n");
 		return 0;
 	}
@@ -1009,7 +1017,7 @@ static int __maybe_unused qmp_usb_legacy_runtime_resume(struct device *dev)
 
 	dev_vdbg(dev, "Resuming QMP phy, mode:%d\n", qmp->mode);
 
-	if (!qmp->phy->init_count) {
+	if (!qmp->phy_initialized) {
 		dev_vdbg(dev, "PHY not initialized, bailing out\n");
 		return 0;
 	}

-- 
2.34.1

[PATCH v5 4/5] phy: qcom: qmp-usb-legacy: Prevent unnecessary PM runtime suspend at boot

[Loic Poulain]

Runtime PM has to be enabled before creating the PHY, since phy_create()
only enables runtime PM on the PHY device if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHY is not yet registered.

Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHY has been created to
prevent the device from being runtime suspended during that window.

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
index fc490589c8e4888770807a60170905a1c7610421..eb1826fb26f0868f7758245b0cbe1a0590fea9a3 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
@@ -1285,10 +1285,16 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
+	/*
+	 * Enable runtime PM before creating the PHY, phy_create() only enables
+	 * it on the PHY device if already enabled on the parent. Hold a usage
+	 * reference so callbacks cannot run before qmp->phy is assigned.
+	 */
+	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
-		goto err_node_put;
+		goto err_pm_put;
 	/*
 	 * Prevent runtime pm from being ON by default. Users can enable
 	 * it using power/control in sysfs.
@@ -1297,23 +1303,27 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
 
 	ret = phy_pipe_clk_register(qmp, np);
 	if (ret)
-		goto err_node_put;
+		goto err_pm_put;
 
 	qmp->phy = devm_phy_create(dev, np, &qmp_usb_legacy_phy_ops);
 	if (IS_ERR(qmp->phy)) {
 		ret = PTR_ERR(qmp->phy);
 		dev_err(dev, "failed to create PHY: %d\n", ret);
-		goto err_node_put;
+		goto err_pm_put;
 	}
 
 	phy_set_drvdata(qmp->phy, qmp);
 
+	pm_runtime_put(dev);
+
 	of_node_put(np);
 
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 
 	return PTR_ERR_OR_ZERO(phy_provider);
 
+err_pm_put:
+	pm_runtime_put_noidle(dev);
 err_node_put:
 	of_node_put(np);
 	return ret;

-- 
2.34.1

[PATCH v5 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

[Loic Poulain]

Runtime PM must be enabled before creating the PHY, since phy_create()
only enables runtime PM on the PHY device if it is already enabled on
this parent device. However, the runtime PM callbacks dereference the
hsphy instance, which is not yet ready, leaving a window where a suspend
callback may trigger a NULL pointer dereference.

Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHY has been created, so that
no runtime suspend can run before the PHY is ready. This also prevents a
short window where an unnecessary runtime suspend can occur.

Use the devres-managed version to ensure PM runtime is symmetrically
disabled during driver removal for proper cleanup.

Fixes: 0d75f508a9d5 ("phy: qcom-snps: Add runtime suspend and resume handlers")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
index eb0b0f61d98e03963bf92e5c822334f52636abe7..b1a85518fed0a7a35516d16da3ebf69afe760e40 100644
--- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
+++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
@@ -599,8 +599,18 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, ret,
 				     "failed to get regulator supplies\n");
 
+	/*
+	 * Enable runtime PM before creating the PHY, phy_create() only enables
+	 * it on the PHY device if already enabled on the parent. Hold a usage
+	 * reference so callbacks cannot run before the PHY is ready.
+	 */
+	pm_runtime_get_noresume(dev);
 	pm_runtime_set_active(dev);
-	pm_runtime_enable(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret) {
+		pm_runtime_put_noidle(dev);
+		return ret;
+	}
 	/*
 	 * Prevent runtime pm from being ON by default. Users can enable
 	 * it using power/control in sysfs.
@@ -611,6 +621,7 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 	if (IS_ERR(generic_phy)) {
 		ret = PTR_ERR(generic_phy);
 		dev_err(dev, "failed to create phy, %d\n", ret);
+		pm_runtime_put_noidle(dev);
 		return ret;
 	}
 	hsphy->phy = generic_phy;
@@ -622,8 +633,8 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 	if (!IS_ERR(phy_provider))
 		dev_dbg(dev, "Registered Qcom-SNPS HS phy\n");
-	else
-		pm_runtime_disable(dev);
+
+	pm_runtime_put(dev);
 
 	return PTR_ERR_OR_ZERO(phy_provider);
 }

-- 
2.34.1