Re: [PATCH v6 5/7] clk: introduce new flag CLK_V2_RATE_NEGOTIATION for sensitive clocks

[Maxime Ripard]

On Fri, Mar 13, 2026 at 12:43:12PM -0400, Brian Masney wrote:
> As demonstrated by the kunit tests, clk_calc_subtree() in the clk core
> can overwrite a sibling clk with the parent rate. Clocks that are used
> for some subsystems like DRM and sound are particularly sensitive to
> this issue.
> 
> I consider this to be a logic bug in the clk subsystem, however this
> functionality has existed since the clk core was introduced with
> commit b2476490ef11 ("clk: introduce the common clock framework"),
> and some boards are unknowingly dependent on this behavior.
> 
> Let's add support for a v2 rate negotiation logic that addresses the
> logic error. Clks can opt into this new behavior by adding the flag
> CLK_V2_RATE_NEGOTIATION, or globally on all clks with the kernel
> parameter clk_v2_rate_negotiation.
> 
> Link: https://lore.kernel.org/linux-clk/aUSWU7UymULCXOeF@xxxxxxxxxx/
> Link: https://lpc.events/event/19/contributions/2152/
> Signed-off-by: Brian Masney <bmasney@xxxxxxxxxx>
> ---
>  drivers/clk/clk.c            | 70 ++++++++++++++++++++++++++++++++++++--------
>  include/linux/clk-provider.h |  3 ++
>  2 files changed, 60 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 051fe755e3bf1b0a06db254b92f8a02889456db9..64c6de5ff5df2117b8d1aca663d40b41d974bf92 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -886,6 +886,43 @@ unsigned long clk_hw_get_children_lcm(struct clk_hw *hw, struct clk_hw *requesti
>  }
>  EXPORT_SYMBOL_GPL(clk_hw_get_children_lcm);
>  
> +static bool clk_v2_rate_negotiation_enabled;
> +static int __init clk_v2_rate_negotiation_setup(char *__unused)
> +{
> +	clk_v2_rate_negotiation_enabled = true;
> +	return 1;
> +}
> +__setup("clk_v2_rate_negotiation", clk_v2_rate_negotiation_setup);
> +
> +/**
> + * clk_has_v2_rate_negotiation - Check if a clk should use v2 rate negotiation
> + * @core: The clock core to check
> + *
> + * This function recursively checks if the clk or any of its descendants have
> + * the CLK_V2_RATE_NEGOTIATION flag set. The v2 behavior can also be enabled
> + * globally by adding clk_v2_rate_negotiation to the kernel command line.
> + *
> + * Returns: true if the v2 logic should be used; false otherwise
> + */
> +bool clk_has_v2_rate_negotiation(const struct clk_core *core)
> +{
> +	struct clk_core *child;
> +
> +	if (clk_v2_rate_negotiation_enabled)
> +		return true;
> +
> +	if (core->flags & CLK_V2_RATE_NEGOTIATION)
> +		return true;
> +
> +	hlist_for_each_entry(child, &core->children, child_node) {
> +		if (clk_has_v2_rate_negotiation(child))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +EXPORT_SYMBOL_GPL(clk_has_v2_rate_negotiation);
> +

Do we really need to export it? I'd expect it to be abstracted away for
consumers and providers?

Maxime

Attachment: signature.asc
Description: PGP signature