Re: [PATCH v10 11/11] drm/rockchip: analogix_dp: Add support for RK3576

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Ignoring the return value of `dp->plat_data->power_on()` in `analogix_dp_resume()` leads to a bus hang and kernel panic if power-on fails.
--

commit 16c45b0fec5cc5f41b6c562365dd00c24ea63ad5
Author: Damon Ding <damon.ding@rock-chips.com>

drm/rockchip: analogix_dp: Add support for RK3576

RK3576 integrates Analogix eDP 1.3 TX and Samsung combo PHY
hardware blocks that fully match the proven RK3588 design. Add dedicated
chip data table and device tree matching entry to bring up basic eDP
functionality for the RK3576 platform.

> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index b1ed25cefe5ed..776954c7d0525 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
[ ... ]
> @@ -543,6 +551,7 @@ static const struct rockchip_dp_chip_data rk3588_edp[] = {
>  static const struct of_device_id rockchip_dp_dt_ids[] = {
>  	{.compatible = "rockchip,rk3288-dp", .data = &rk3288_dp },
>  	{.compatible = "rockchip,rk3399-edp", .data = &rk3399_edp },
> +	{.compatible = "rockchip,rk3576-edp", .data = &rk3576_edp },

[Severity: High]
This is a pre-existing issue, but adding a new platform here raises a
question about how power-on failures are handled.

Does this platform risk a bus hang and kernel panic if power-on fails during
resume?

If rockchip_dp_poweron() fails (e.g., if rockchip_dp_pre_init() fails),
it disables dp->pclk:

drivers/gpu/drm/rockchip/analogix_dp-rockchip.c:rockchip_dp_poweron() {
    ...
	ret = rockchip_dp_pre_init(dp);
	if (ret < 0) {
		DRM_DEV_ERROR(dp->dev, "failed to dp pre init %d\n", ret);
		clk_disable_unprepare(dp->pclk);
		return ret;
	}
    ...
}

However, the core driver ignores the return value of power_on() and continues
to access memory-mapped registers via analogix_dp_init_dp():

drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:analogix_dp_resume() {
    ...
	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);

	analogix_dp_init_dp(dp);
    ...
}

Since attempting to read or write memory-mapped I/O registers when the APB
bus clock (pclk) is disabled causes a synchronous external abort on ARM
platforms, could this cause a kernel panic if power-on fails?

>  	{.compatible = "rockchip,rk3588-edp", .data = &rk3588_edp },
>  	{}
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601065100.1103873-1-damon.ding@rock-chips.com?part=11