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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] `PM_RUNTIME_ACQUIRE` falsely succeeds when runtime PM is disabled (e.g., during system sleep), leading to a system hang when disabled hardware registers are accessed.
--

--- Patch [3]: [PATCH v6 3/6] phy: fsl-imx8mq-usb: add runtime PM support ---
commit d85e441d52a7fee829fc860ee8795964a99e4555
Author: Xu Yang <xu.yang_2@nxp.com>

phy: fsl-imx8mq-usb: add runtime PM support

This commit adds runtime PM support to ensure the PHY is properly powered
and clocked during register access. It guards register accesses in PHY
operations and Type-C orientation switching.

> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 3a5788c609e1b..bc804d2b5aeeb 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c

[ ... ]

> @@ -136,17 +137,15 @@ static int tca_blk_typec_switch_set(struct typec_switch_dev *sw,
>  {
>  	struct imx8mq_usb_phy *imx_phy = typec_switch_get_drvdata(sw);
>  	struct tca_blk *tca = imx_phy->tca;
> -	int ret;
>  
>  	if (tca->orientation == orientation)
>  		return 0;
>  
> -	ret = clk_prepare_enable(imx_phy->clk);
> -	if (ret)
> -		return ret;
> +	PM_RUNTIME_ACQUIRE(&imx_phy->phy->dev, pm);
> +	if (PM_RUNTIME_ACQUIRE_ERR(&pm))
> +		return -ENXIO;

[Severity: High]
Does this check falsely succeed when runtime PM is disabled, such as during
system sleep?

During system suspend, pm_runtime_force_suspend() disables runtime PM
(disable_depth > 0) and turns off the PHY clocks. If a Type-C orientation
change occurs via interrupt at this time, PM_RUNTIME_ACQUIRE (which wraps
pm_runtime_get_active with RPM_TRANSPARENT) will return success despite the
device being suspended.

The driver would then proceed to tca_blk_orientation_set(), which accesses
unclocked PHY registers, potentially causing a synchronous external abort or
system hang.

Could PM_RUNTIME_ACQUIRE_IF_ENABLED() be used here instead to safely check
if the device is active and fail appropriately if runtime PM is disabled?

>  
>  	tca_blk_orientation_set(tca, orientation);
> -	clk_disable_unprepare(imx_phy->clk);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-imx8mp-usb-phy-improvement-v6-0-00d95e270e4c@nxp.com?part=3