Re: [PATCH] tty: serial: qcom-geni-serial: minor fixes to get_clk_div_rate()

From: Vijaya Krishna Nivarthi
Date: Mon Jun 06 2022 - 14:19:18 EST


Hi,


On 6/4/2022 12:10 AM, Doug Anderson wrote:
Hi,

On Fri, Jun 3, 2022 at 10:43 AM Vijaya Krishna Nivarthi
<quic_vnivarth@xxxxxxxxxxx> wrote:

Ah, or I guess what you're saying is that the table historically
contained "rounded" rates but that clk_round_rate() isn't returning
nice round rates. OK, but if we truly want to support an inexact
match, you'd want to pick the rate that reduces the error, not just
pick the first one. In other words, something like this (untested):

freq = clk_round_rate(clk, mult);
diff = abs(((long)mult - freq) / div);
if (diff < best_diff) {
best_diff = diff;
ser_clk = freq;
best_div = div;
}
I am not sure if its required that freq is a multiple of best_div now
that we don't have a multiple of desired_clk anyway.
How about just this (untested):

freq = clk_round_rate(clk, mult);
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;
}

I am afraid this still doesn't guarantee that ser_clk is a multiple of best_div

I tested it with a function simulates clk_round_rate.

static unsigned long clk_round_rate_test(struct clk *clk, unsigned long in_freq)
{
    unsigned long root_freq[6] = {105, 204, 303, 402, 501, 602};
    int i;

    for (i = 0; i < 6; i++) {
        if (root_freq[i] >= in_freq)
            return root_freq[i];
    }
    return root_freq[6];
}

    {
        unsigned long ser_clk;
        unsigned long desired_clk;
        unsigned long freq;
        int div_round_closest;
        unsigned long div;
        unsigned long mult;
        unsigned long candidate_div, candidate_freq;

        unsigned long diff, best_diff, best_div;
        unsigned long one;

        desired_clk = 100;
        one = 1;
        best_diff = ULONG_MAX;
        pr_err("\ndesired_clk-%d\n", desired_clk);
        for (div = 1; div <= 10; div++) {
            mult = div * desired_clk;

            freq = clk_round_rate_test(clk, mult);
            div_round_closest = DIV_ROUND_CLOSEST(freq, desired_clk);
            candidate_div = max(one, (unsigned long)div_round_closest);
            candidate_freq = freq / candidate_div;
            diff = abs((long)desired_clk - candidate_freq);
            pr_err("div-%d, mult-%d, freq-%d, div_round_closest-%d, candidate_div-%d, candidate_freq-%d, diff-%d\n",
                div, mult, freq, div_round_closest, candidate_div, candidate_freq, diff);
            if (diff < best_diff) {
                pr_err("This is best so far\n");
                best_diff = diff;
                ser_clk = freq;
                best_div = candidate_div;
            }
        }
        pr_err("\nbest_diff-%d, ser_clk-%d, best_div-%d\n",
            best_diff, ser_clk, best_div);
    }

And here is the output

[   17.835167] desired_clk-100
[   17.839567] div-1, mult-100, freq-105, div_round_closest-1, candidate_div-1, candidate_freq-105, diff-5
[   17.849220] This is best so far
[   17.852458] div-2, mult-200, freq-204, div_round_closest-2, candidate_div-2, candidate_freq-102, diff-2
[   17.862104] This is best so far
[   17.865345] div-3, mult-300, freq-303, div_round_closest-3, candidate_div-3, candidate_freq-101, diff-1
[   17.874995] This is best so far
[   17.878237] div-4, mult-400, freq-402, div_round_closest-4, candidate_div-4, candidate_freq-100, diff-0
[   17.887882] This is best so far
[   17.891118] div-5, mult-500, freq-501, div_round_closest-5, candidate_div-5, candidate_freq-100, diff-0
[   17.900770] div-6, mult-600, freq-602, div_round_closest-6, candidate_div-6, candidate_freq-100, diff-0
[   17.910415] div-7, mult-700, freq-602, div_round_closest-6, candidate_div-6, candidate_freq-100, diff-0
[   17.920057] div-8, mult-800, freq-602, div_round_closest-6, candidate_div-6, candidate_freq-100, diff-0
[   17.929703] div-9, mult-900, freq-602, div_round_closest-6, candidate_div-6, candidate_freq-100, diff-0
[   17.939353] div-10, mult-1000, freq-602, div_round_closest-6, candidate_div-6, candidate_freq-100, diff-0
[   17.949181]
[   17.949181] best_diff-0, ser_clk-402, best_div-4

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.

It seems like we need to run through all dividers to find one that can divide output freq from clk_round_rate and among those choose smallest delta.

In regular bootup 2nd loop was required only for 51200000. For others 1843200, 153600, 1st loop worked fine.

Thank you.


Here:

freq: a freq we can definitely make

candidate_div: the best number to divide freq by to get the desired clock.

candidate_freq: the frequency we'll end up if we divide freq by
candidate_div. We want this to be close to desired_clk.

diff: how far away the candidate_freq is away from what we want.

best_diff: how far away the best candidate was from what we wanted.

ser_clk: What we should pass to clk_set_rate() to get the best candidate.

best_div: What we should use as a divider to get the best candidate.


-Doug