Re: [PATCH v7 3/6] phy: fsl-imx8mq-usb: add runtime PM support

On Wed, Jul 29, 2026 at 06:55:20AM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Incorrect error checking of pm_runtime_get_sync() return value causes spurious warnings when the device is already active.
> - [Low] Failure of pm_runtime_get_sync() in remove() leads to an unbalanced clock disable by devres, triggering a kernel WARN.
> --
> 
> --- Patch [3]: [PATCH v7 3/6] phy: fsl-imx8mq-usb: add runtime PM support ---
> commit f054765f7ebd59735a4592c4d68b74584abdbeeb
> Author: Xu Yang <[email protected]>
> 
> phy: fsl-imx8mq-usb: add runtime PM support
> 
> Add runtime PM support to ensure the PHY clocks are properly gated
> when the PHY is not in use, reducing power consumption. Clock
> management is moved from power_on()/power_off() callbacks into the
> runtime_resume()/runtime_suspend() callbacks.
> 
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > index 3a5788c609e1b..74e665525ec48 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> [ ... ]
> > @@ -708,44 +696,101 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> [ ... ]
> >  static void imx8mq_usb_phy_remove(struct platform_device *pdev)
> >  {
> > +	struct device *dev = &pdev->dev;
> > +	int ret;
> > +
> > +	ret = pm_runtime_get_sync(dev);
> > +	if (ret)
> > +		dev_warn(dev, "failed to resume on remove: %d\n", ret);
> 
> [Severity: Medium]
> Is it intended to warn when pm_runtime_get_sync() returns 1?
> 
> Since pm_runtime_get_sync() returns 1 when the device is already active
> (or when CONFIG_PM is disabled), checking "if (ret)" instead of
> "if (ret < 0)" will trigger this warning even on successful unbinds
> where the device was already active.

My bad. It should be "if (ret < 0)". Will fix it in v8.

Thanks,
Xu Yang