On 20/10/2026 at 11:57:08 +02, Miquel Raynal <[email protected]> wrote:
> Hi Brian, > > On 27/03/2026 at 15:31:58 -04, Brian Masney <[email protected]> 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/[email protected]/ >> Link: https://lpc.events/event/19/contributions/2152/ >> Signed-off-by: Brian Masney <[email protected]> >> --- > > 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 I meant "made of inner hw clocks" here. Just to be more clear, if my understanting is correct: - requesting_hw is the hw clock associated to the composite clock - child->hw is one of the inner composite clocks So the comparaison cannot work. Iterating over the core clock therefore solves this incompatibility. However, while trying to understand better the values of the pointers there, I realized the initialization of the various inner hw->core pointers are made all over the place (?) and in certain cases, they are even not done at all. For instance, a composite clock with a single parent would not go through __clk_hw_set_clk(), at least not during registration. Either this never happened before, or maybe some recalc_rate() spuriously set it by chance (because __clk_hw_set_clk() calls are made a bit everywhere), or maybe because this problem was not really a problem without the v2 rate parenting Brian is proposing. Anyway, I think this situation can be fixed easily, so I will propose something on top of this series. Thanks, Miquèl
