Re: [PATCH] clk: check for disabled clock-provider in of_clk_get_hw_from_clkspec

From: Cristian Ciocaltea
Date: Sat Feb 22 2025 - 10:34:11 EST


Hi,

On 2/21/25 12:54 AM, Heiko Stuebner wrote:
> of_clk_get_hw_from_clkspec checks all available clock-providers by
> compairing their of-nodes to the one from the clkspec. If no matching
> clock-provider is found, the function returns EPROBE_DEFER to cause a
> re-check at a later date.
>
> If a matching clock-provider is found, a authoritative answer can be

Nit: s/a authoritative/an authoritative/

> retrieved from it whether the clock exists or not.
>
> This does not take into account that the clock-provider may never appear,
> because it's node is disabled. This can happen for example when a clock
> is optional, provided by a separate block which just never gets enabled.
>
> One example of this happening is the rk3588's VOP, which has optional
> additional display-clock-supplies coming from PLLs inside the hdmiphy
> blocks. These can be used for better rates, but the system will also
> work without them.
>
> The problem around that is described in the followups to:
> https://lore.kernel.org/dri-devel/20250215-vop2-hdmi1-disp-modes-v1-3-81962a7151d6@xxxxxxxxxxxxx/
>
> As we already know the of-node of the presumed clock-provider, just add
> a check via of_device_is_available whether this is a "valid" device node.
> This prevents ethernal defer-loops.
>
> Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx>
> ---
> drivers/clk/clk.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index cf7720b9172f..50faafbf5dda 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -5258,6 +5258,10 @@ of_clk_get_hw_from_clkspec(struct of_phandle_args *clkspec)
> if (!clkspec)
> return ERR_PTR(-EINVAL);
>
> + /* Check if node in clkspec is in disabled/fail state */
> + if (!of_device_is_available(clkspec->np))
> + return ERR_PTR(-ENOENT);
> +
> mutex_lock(&of_clk_mutex);
> list_for_each_entry(provider, &of_clk_providers, link) {
> if (provider->node == clkspec->np) {

Without this patch applied, when disabling hdmi0 and hdptxphy0 nodes on
Rock5B, rockchip-drm module fails to probe and hdmi1 output cannot be
used anymore:

rockchip-drm display-subsystem: [drm] *ERROR* failed to get pll_hdmiphy0

After applying the patch, I confirm the error is gone and hdmi1 is
functional again. Hence,

Tested-by: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx>

Thanks,
Cristian