Hi,
On 6/8/2022 12:55 AM, Doug Anderson wrote:
Hi,
On Tue, Jun 7, 2022 at 10:40 AM Vijaya Krishna Nivarthi
<quic_vnivarth@xxxxxxxxxxx> wrote:
Hi,
On 6/7/2022 1:29 AM, Doug Anderson wrote:
My only concern continues to be...
Given ser_clk is the final frequency that this function is going to
return and best_div is going to be the clk_divider, is it ok if the
divider cant divide the frequency exactly?
In other words, Can this function output combinations like (402,4)
(501,5) ?
If ok, then we can go ahead with this patch or even previous perhaps.
I don't see why not. You're basically just getting a resulting clock
that's not an integral "Hz", right?
So if "baud" is 9600 and sampling_rate is 16 then desired_clk is (9600
* 16) = 153600
Let's imagine that we do all the math and we finally decide that our
best bet is with the rate 922000 and a divider of 6. That means that
the actual clock we'll make is 153666.67 when we _wanted_ 153600.
There's no reason it needs to be integral, though, and 153666.67 would
still be better than making 160000.
Thank you for clarification.
power?)
Actually power saving was the anticipation behind returning first
frequency in original patch, when we cant find exact frequency.
Right, except that if you just pick the first clock you find it would
be _wildly_ off. I guess if you really want to do this the right way,
you need to set a maximum tolerance and pick the first rate you find
that meets that tolerance. Random web search for "uart baud rate
tolerance" makes me believe that +/- 5% deviation is OK, but to be
safe you probably want something lower. Maybe 2%? So if the desired
clock is within 2% of a clock you can make, can you just pick that
one?
Ok, 2% seems good.
Please note that we go past cases when we have an divider that can
exactly divide the frequency(105/1, 204/2, 303/3) and end up with one
that doesn't.
Ah, good point. Luckily that's a 1-line fix, right?
Apologies, I could not figure out how.
Ah, sorry. Not quite 1 line, but this (untested)
freq = clk_round_rate(clk, mult);
if (freq % desired_clk == 0) {
ser_clk = freq;
best_div = freq / desired_clk;
break;
}
candidate_div = max(1, DIV_ROUND_CLOSEST(freq, desired_clk));
candidate_freq = freq / candidate_div;
diff = abs((long)desired_clk - candidate_freq);
if (diff < best_diff) {
best_diff = diff;
ser_clk = freq;
best_div = candidate_div;
}
But then once again, we would likely need 2 loops because while we are
ok with giving up on search for best_div on finding something within 2%
tolerance, we may not want to give up on exact match (freq % desired_clk
== 0 )
So how about something like this with 2 loops (more optimised than
previous version with 2 loops)? (untested)
maxdiv = CLK_DIV_MSK >> CLK_DIV_SHFT;
prev = 0;
/* run through quicker loop anticipating to find an exact match */
for (div = 1; div <= maxdiv; div++) {
mult = (unsigned long long)div * desired_clk;
if (mult > ULONG_MAX)
break;
freq = clk_round_rate(clk, max((unsigned long)mult, prev+1));
if (!(freq % desired_clk)) {
*clk_div = freq / desired_clk;
return freq;
}
if (prev && prev == freq)
break;
prev = freq;
}
pr_warn("Can't find exact match frequency and divider\n");
freq = 0;
best_diff = ULONG_MAX;
prev_candidate_div = -1;
while (true) {
prev = freq;
freq = clk_round_rate(clk, freq+1);
if (freq == prev)
break; /* end of table */
candidate_div = DIV_ROUND_CLOSEST(freq, desired_clk);
/*
* Since the frequencies are increasing, previous is better
* if we have same divider, proceed to next in table
*/
if (prev_candidate_div == candidate_div)
continue;
prev_candidate_div = candidate_div;
if (candidate_div)
candidate_freq = freq / candidate_div;
else
candidate_freq = freq;
diff = abs((long)desired_clk - candidate_freq);
if (diff < best_diff) {
best_diff = diff;
ser_clk = freq;
*clk_div = candidate_div;
if (diff * 50 < ser_clk) {
two_percent_tolerance = true;
break;
}
}
}
if (!two_percent_tolerance) {
pr_warn("Can't find frequency within 2 percent tolerance\n");
}
return ser_clk;
}
Thank you.
