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@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
> > Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxxxxxxxxxxx
> > https://lists.infradead.org/mailman/listinfo/linux-phy
>
> --
> With best wishes
> Dmitry