[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <24b641332461006bdedd5a4d682fb040.sboyd@kernel.org>
Date: Wed, 26 Feb 2025 14:32:28 -0800
From: Stephen Boyd <sboyd@...nel.org>
To: Heiko Stuebner <heiko@...ech.de>, mturquette@...libre.com
Cc: linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org, liujianfeng1994@...il.com, sebastian.reichel@...labora.com, cristian.ciocaltea@...labora.com, heiko@...ech.de
Subject: Re: [PATCH v2] clk: check for disabled clock-provider in of_clk_get_hw_from_clkspec
Quoting Heiko Stuebner (2025-02-22 14:37:33)
> 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, an authoritative answer can be
> 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@collabora.com/
>
> 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 eternal defer-loops.
>
> Reviewed-by: Sebastian Reichel <sebastian.reichel@...labora.com>
> Tested-by: Cristian Ciocaltea <cristian.ciocaltea@...labora.com>
> Signed-off-by: Heiko Stuebner <heiko@...ech.de>
> ---
Applied to clk-next (unless this needs to fix something urgent?)
Please write a unit test (or many). I also wonder if we should use a
different return value so that we don't try to look up the clk by name
(see clk_core_fill_parent_index()). We could go even further and stop
trying to find the clk over and over again too. Maybe -ENODEV can
indicate that and we can cache that parent entry value so we stop
trying.
Powered by blists - more mailing lists