[PATCH] drm/bridge: analogix: Propagate PHY errors from resume

[PATCH] drm/bridge: analogix: Propagate PHY errors from resume

[Pengpeng Hou]

analogix_dp_resume() enables the controller clock and platform power
before setting up the DisplayPort PHY.  It currently ignores both PHY
operation results and initializes the controller even when the PHY is
unavailable.

Check phy_set_mode() and phy_power_on().  On failure, undo platform
power and the controller clock before returning the error to the
platform resume callback.

The issue was identified via static analysis and manually reviewed.

Fixes: 73f613908eb1 ("drm/bridge: analogix_dp: move platform and PHY power handling into runtime PM")
Fixes: 2c0883459ed6 ("drm/bridge: analogix_dp: Add support for phy configuration.")

Assisted-by: LLM
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 .../gpu/drm/bridge/analogix/analogix_dp_core.c    | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
index 30c0c0b41d5a..00844f68d748 100644
--- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
+++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
@@ -1461,12 +1461,23 @@ int analogix_dp_resume(struct analogix_dp_device *dp)
 	if (dp->plat_data->power_on)
 		dp->plat_data->power_on(dp->plat_data);
 
-	phy_set_mode(dp->phy, PHY_MODE_DP);
-	phy_power_on(dp->phy);
+	ret = phy_set_mode(dp->phy, PHY_MODE_DP);
+	if (ret)
+		goto power_off;
+
+	ret = phy_power_on(dp->phy);
+	if (ret)
+		goto power_off;
 
 	analogix_dp_init_dp(dp);
 
 	return 0;
+
+power_off:
+	if (dp->plat_data->power_off)
+		dp->plat_data->power_off(dp->plat_data);
+	clk_disable_unprepare(dp->clock);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(analogix_dp_resume);
 
-- 
2.43.0

Re: [PATCH] drm/bridge: analogix: Propagate PHY errors from resume

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The return value of the platform-specific dp->plat_data->power_on() callback is ignored in analogix_dp_resume(), potentially leading to hardware faults and unbalanced clock disables in the newly added error path.
--

commit 777e12c640d41a75f043282a8ce210a68d6eca97
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

drm/bridge: analogix: Propagate PHY errors from resume

This patch updates analogix_dp_resume() to check the return values of
phy_set_mode() and phy_power_on(). If either fails, it rolls back the power
configuration using a newly added power_off error path instead of ignoring
the error.

> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 30c0c0b41d5aa..00844f68d748e 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1461,12 +1461,23 @@ int analogix_dp_resume(struct analogix_dp_device *dp)
>  	if (dp->plat_data->power_on)
>  		dp->plat_data->power_on(dp->plat_data);
>  

[Severity: High]
This isn't a bug introduced by this patch, but should the return value of
dp->plat_data->power_on() be checked here?

If dp->plat_data->power_on() fails (for example, if clk_prepare_enable()
fails in rockchip_dp_poweron()), it returns a negative error code and
leaves the platform clock disabled. By ignoring this error, execution
proceeds to phy_set_mode() and analogix_dp_init_dp(), which can result in
accessing unpowered hardware registers. Accessing unpowered hardware
typically causes a synchronous external abort on Arm platforms.

Furthermore, this patch actively exacerbates this issue. If the platform
power initialization fails and subsequently phy_set_mode() or
phy_power_on() fails, the newly added goto power_off error path will
execute:

    power_off:
        if (dp->plat_data->power_off)
            dp->plat_data->power_off(dp->plat_data);
        clk_disable_unprepare(dp->clock);

Can this cause an unbalanced clock disable? The power_off path will blindly
call clk_disable_unprepare() on the platform clock that was never
successfully enabled.

> -	phy_set_mode(dp->phy, PHY_MODE_DP);
> -	phy_power_on(dp->phy);
> +	ret = phy_set_mode(dp->phy, PHY_MODE_DP);
> +	if (ret)
> +		goto power_off;
> +
> +	ret = phy_power_on(dp->phy);
> +	if (ret)
> +		goto power_off;
>  
>  	analogix_dp_init_dp(dp);
>  
>  	return 0;
> +
> +power_off:
> +	if (dp->plat_data->power_off)
> +		dp->plat_data->power_off(dp->plat_data);
> +	clk_disable_unprepare(dp->clock);
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828091334.5793-1-pengpeng@iscas.ac.cn?part=1