Re: [PATCH v8 3/8] clk: introduce new helper clk_hw_get_children_lcm() to calculate LCM of all child rates

[Miquel Raynal]

Hi Brian,

On 27/03/2026 at 15:31:58 -04, Brian Masney <bmasney@xxxxxxxxxx> wrote:

> Introduce a new helper that recursively walks through all children and
> their descendants, calculating the lowest common multiple (LCM) of their
> rates. For the requesting child, it uses the requested rate; for other
> enabled children, it uses their current rate. This is useful for
> determining what parent rate can satisfy all children through simple
> integer dividers.
>
> Link: https://lore.kernel.org/linux-clk/aUSWU7UymULCXOeF@xxxxxxxxxx/
> Link: https://lpc.events/event/19/contributions/2152/
> Signed-off-by: Brian Masney <bmasney@xxxxxxxxxx>
> ---

Thanks a lot for this proposal, it is very interesting.

>  drivers/clk/clk.c            | 54 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk-provider.h |  2 ++
>  2 files changed, 56 insertions(+)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 47093cda9df32223c1120c3710261296027c4cd3..bb602c07df9a2b523a72e1e05cc3a27956e7198b 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -14,6 +14,7 @@
>  #include <linux/err.h>
>  #include <linux/hashtable.h>
>  #include <linux/init.h>
> +#include <linux/lcm.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> @@ -838,6 +839,59 @@ void clk_hw_set_rate_range(struct clk_hw *hw, unsigned long min_rate,
>  }
>  EXPORT_SYMBOL_GPL(clk_hw_set_rate_range);
>  
> +/**
> + * clk_hw_get_children_lcm - Calculate LCM of all children's rates recursively
> + * @hw: The parent clock hardware
> + * @requesting_hw: The child clock that is requesting a rate change (can be NULL)
> + * @requesting_rate: The target rate for the requesting clock
> + *
> + * This helper recursively walks through all children and their descendants,
> + * calculating the lowest common multiple (LCM) of their rates. For the
> + * requesting child, it uses the requested rate; for other enabled children, it
> + * uses their current rate. This is useful for determining what parent rate can
> + * satisfy all children through simple integer dividers.
> + *
> + * Returns: The LCM of all non-zero rates found in the subtree, or 0 if no valid rates.
> + */
> +unsigned long clk_hw_get_children_lcm(struct clk_hw *hw, struct clk_hw *requesting_hw,
> +				      unsigned long requesting_rate)
> +{
> +	unsigned long lcm_rate = 0;
> +	unsigned long child_rate;
> +	struct clk_core *child;
> +
> +	lockdep_assert_held(&prepare_lock);
> +
> +	hlist_for_each_entry(child, &hw->core->children, child_node) {
> +		/* Use requesting rate for the requesting child, current rate for others */
> +		if (child->hw == requesting_hw) {

This is not working on my end (imx8mp) because the clock requesting the
rate is not identified correctly. Here you use the hw clock, but in the
imx8mp case there are many composite clocks which are made of inner
core clocks (like gate, divider and mux). I believe we should iterate over
core clocks instead, eg:

                if (requesting_hw && child == requesting_hw->core) {

> +			child_rate = requesting_rate;
> +		} else {
> +			if (!clk_hw_is_enabled(child->hw))
> +				continue;
> +
> +			child_rate = clk_hw_get_rate(child->hw);
> +		}

Thanks,
Miquèl