Re: [PATCH v7 05/10] drm/rockchip: analogix_dp: Enable hclk for RK3588

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Power and clock leak for 'hclk' during suspend and display powerdown.

Pre-existing issues:
- [Critical] Dangling pointer dereference / Use-After-Free via `endpoint.local_node`.
--

commit 4b37db64b85dfb304df892e845f75e6e05adbfbe
Author: Damon Ding <damon.ding@rock-chips.com>

drm/rockchip: analogix_dp: Enable hclk for RK3588

This commit acquires and enables the HCLK_VO1 bus clock explicitly for
the RK3588 eDP controller. It fetches the optional hclk clock in the
driver probe to align with updated device tree bindings.

> diff --git a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> index 06072efd7fca3..d2af5eb29dbb5 100644
> --- a/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/analogix_dp-rockchip.c

[ ... ]

> @@ -327,6 +328,11 @@ static int rockchip_dp_of_probe(struct rockchip_dp_device *dp)
>  		return dev_err_probe(dev, PTR_ERR(dp->pclk),
>  				     "failed to get pclk property\n");
>  
> +	clk = devm_clk_get_optional_enabled(dev, "hclk");
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk),
> +				     "failed to get hclk property\n");
> +

[Severity: Medium]
Does this cause a clock leak during system suspend or when the eDP
controller is powered down?

Because hclk is acquired and enabled here but only stored in a local
variable, the driver has no way to reference it later to disable it
during suspend or powerdown.

Unlike pclk which is saved in the driver context and explicitly managed,
would leaving this clock permanently enabled prevent its parent power
domain from entering low-power states?

>  	dp->rst = devm_reset_control_get(dev, "dp");
>  	if (IS_ERR(dp->rst))

[ ... ]

[Severity: Critical]
This is a pre-existing issue, but can the usage of endpoint.local_node
in rockchip_dp_drm_encoder_enable() cause a use-after-free?

drm_of_encoder_active_endpoint() populates the of_endpoint structure,
setting endpoint.local_node to the endpoint node, but explicitly calls
of_node_put(ep) before returning. This drops the reference to the node.

When rockchip_dp_drm_encoder_enable() later passes this unreferenced
pointer to of_graph_get_remote_port_parent(), could this result in
dereferencing a dangling pointer if the node's reference count has
reached zero?

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