lkml.org 
[lkml]   [2024]   [Feb]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH RESEND] clk: set initial best mux parent to current parent when determining rate
From
Date
Quoting Yang Xiwen (2024-02-28 18:33:11)
> On 2/29/2024 10:25 AM, Stephen Boyd wrote:
> >>>
> >>> Is the problem that we're not using abs_diff()?
> >>
> >>
> >> No, i think. It has nothing to do with the code here. It's because of
> >> the initial best_parent/best_parent_rate.
> >
> > Alright.

I will have to fix this as well in a different patch.

> >
> >>
> >>>
> >>> ----8<----
> >>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> >>> index a3bc7fb90d0f..91023345595f 100644
> >>> --- a/drivers/clk/clk.c
> >>> +++ b/drivers/clk/clk.c
> >>> @@ -542,7 +542,7 @@ static bool mux_is_better_rate(unsigned long rate, unsigned long now,
> >>> unsigned long best, unsigned long flags)
> >>> {
> >>> if (flags & CLK_MUX_ROUND_CLOSEST)
> >>> - return abs(now - rate) < abs(best - rate);
> >>> + return abs_diff(now, rate) < abs_diff(best, rate);
> >>
> >> Without this patch, the initial `best` rate would be always 0. This is
> >> wrong for most cases, 0Hz might (usually) be unavailable. We should use
> >> a valid rate(i.e. current rate) initially.
> >
> > Ok. But you set best to the parent rate. So why not use 'core->rate'
> > directly as 'best'?
>
>
> I can't remember exactly. I just add this piece of code and found it's
> working. Is this field already filled prior to setting rate? Anyway,
> your suggestion is very reasonable. Maybe dear clk maintainers can fix
> it as i'm not familiar with clk core code.

Yes the 'struct clk_rate_request' is pre-filled with many details,
including the rate of the clk and the current parent rate and parent hw
pointer. I'm pretty sure you're trying to fix this fixme from clk_test.c

static const struct clk_ops clk_dummy_single_parent_ops = {
/*
* FIXME: Even though we should probably be able to use
* __clk_mux_determine_rate() here, if we use it and call
* clk_round_rate() or clk_set_rate() with a rate lower than
* what all the parents can provide, it will return -EINVAL.
*
* This is due to the fact that it has the undocumented
* behaviour to always pick up the closest rate higher than the
* requested rate. If we get something lower, it thus considers
* that it's not acceptable and will return an error.
*
* It's somewhat inconsistent and creates a weird threshold
* between rates above the parent rate which would be rounded to
* what the parent can provide, but rates below will simply
* return an error.
*/

\
 
 \ /
  Last update: 2024-05-27 15:30    [W:0.046 / U:1.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site