Re: [PATCH v11 1/3] arm64: dts: rockchip: Add missing hclk for RK3588 eDP0

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch adds the `HCLK_VO1` clock to the `edp0` node but incorrectly names it `"hclk"` instead of `"grf"`. This violates the `rockchip,analogix-dp.yaml` DT schema, which strictly mandates the third clock to be named `"grf"`. Furthermore, it causes the driver's explicit clock management to silently fail: `analogix_dp-rockchip.c` requests the clock using `devm_clk_get_optional(dev, "grf")`. Because the DT name (`"hclk"`) does not match, the driver receives `NULL` and its explicit `clk_prepare_enable()` calls become no-ops.
--

commit 1f0c2ce9ff98f7230f0fe6e03686f09557a50f13
Author: Damon Ding <damon.ding@rock-chips.com>

arm64: dts: rockchip: Add missing hclk for RK3588 eDP0

This commit adds the HCLK_VO1 clock to the RK3588 eDP0 node. This is
intended to allow the eDP controller to correctly access the video output
GRF, transitioning from implicit enablement to explicit definition.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> index 4fb8888c281c8..24a5ccbac08cb 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-base.dtsi
> @@ -1712,8 +1712,8 @@ hdmi0_out: port@1 {
>  	edp0: edp@fdec0000 {
>  		compatible = "rockchip,rk3588-edp";
>  		reg = <0x0 0xfdec0000 0x0 0x1000>;
> -		clocks = <&cru CLK_EDP0_24M>, <&cru PCLK_EDP0>;
> -		clock-names = "dp", "pclk";
> +		clocks = <&cru CLK_EDP0_24M>, <&cru PCLK_EDP0>, <&cru HCLK_VO1>;
> +		clock-names = "dp", "pclk", "hclk";

[Severity: Medium]
Does naming this third clock "hclk" conflict with the driver's expectations
and the device tree schema?

The rockchip,analogix-dp.yaml schema expects the third clock to be named "grf".
Additionally, looking at rockchip_dp_of_probe() in the analogix_dp-rockchip.c
driver, it explicitly requests "grf":

drivers/gpu/drm/rockchip/analogix_dp-rockchip.c:rockchip_dp_of_probe() {
    ...
    dp->grfclk = devm_clk_get_optional(dev, "grf");
    ...
}

Because the name is specified as "hclk" here instead of "grf", will
devm_clk_get_optional() return a NULL pointer? This would cause the driver's
subsequent explicit clk_prepare_enable() calls for this clock to become
silent no-ops, bypassing the intended explicit clock management.

>  		interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH 0>;
>  		phys = <&hdptxphy0>;
>  		phy-names = "dp";

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