linux-kernel - RE: [PATCH] tty: serial: qcom-geni-serial: minor fixes to get_clk_div

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BL0PR02MB4564C633F46BBCC315D86322FAB39@BL0PR02MB4564.namprd02.prod.outlook.com>
Date:   Tue, 21 Jun 2022 17:58:47 +0000
From:   "Vijaya Krishna Nivarthi (Temp)" <vnivarth@....qualcomm.com>
To:     Doug Anderson <dianders@...omium.org>,
        "Vijaya Krishna Nivarthi (Temp) (QUIC)" <quic_vnivarth@...cinc.com>
CC:     Andy Gross <agross@...nel.org>,
        "bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        "Mukesh Savaliya (QUIC)" <quic_msavaliy@...cinc.com>,
        Matthias Kaehlcke <mka@...omium.org>,
        Stephen Boyd <swboyd@...omium.org>
Subject: RE: [PATCH] tty: serial: qcom-geni-serial: minor fixes to
 get_clk_div_rate()

Hi,

For desired_clk = 100 and clock rates like 1st from below, DIV_ROUND_UP seems to cause missing candidate solutions.

static unsigned long clk_round_rate_test(struct clk *clk, unsigned long in_freq)
{
	//unsigned long root_freq[] = {301, 702, 1004};
	//unsigned long root_freq[] = {301, 702, 1004, 2000, 3000};
	//unsigned long root_freq[] = {50, 97, 99};
	//unsigned long root_freq[] = {50, 97, 99, 200};
	//unsigned long root_freq[] = {92, 110, 193, 230};
	//unsigned long root_freq[] = {92, 110, 193, 230, 300, 401};
	//unsigned long root_freq[] = {92, 110, 193, 230, 295, 296, 297, 401};
	//unsigned long root_freq[] = {92, 110, 193, 230, 295, 296, 297, 300, 401};
	//unsigned long root_freq[] = {197, 198, 199};
	unsigned long root_freq[] = {197, 198, 199, 200};
	int i;
	size_t n = sizeof root_freq / sizeof *root_freq;

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

I modified to handle such cases, optimised little and uploaded a patch.
It seems to work for all the cases like above.

Thank you,
Vijay/



> 
> Ah, it took me a while to understand why two loops. It's because in one case
> you're trying multiplies and in the other you're bumping up to the next
> closest clock rate. I don't think you really need to do that. Just test the (rate -
> 2%) and the rate. How about this (only lightly tested):
> 
>     ser_clk = 0;
>     maxdiv = CLK_DIV_MSK >> CLK_DIV_SHFT;
>     div = 1;
>     while (div < maxdiv) {
>         mult = (unsigned long long)div * desired_clk;
>         if (mult != (unsigned long)mult)
>             break;
> 
>         two_percent = mult / 50;
> 
>         /*
>          * Loop requesting (freq - 2%) and possibly (freq).
>          *
>          * We'll keep track of the lowest freq inexact match we found
>          * but always try to find a perfect match. NOTE: this algorithm
>          * could miss a slightly better freq if there's more than one
>          * freq between (freq - 2%) and (freq) but (freq) can't be made
>          * exactly, but that's OK.
>          *
>          * This absolutely relies on the fact that the Qualcomm clock
>          * driver always rounds up.
>          */
>         test_freq = mult - two_percent;
>         while (test_freq <= mult) {
>             freq = clk_round_rate(clk, test_freq);
> 
>             /*
>              * A dead-on freq is an insta-win. This implicitly
>              * handles when "freq == mult"
>              */
>             if (!(freq % desired_clk)) {
>                 *clk_div = freq / desired_clk;
>                 return freq;
>             }
> 
>             /*
>              * Only time clock framework doesn't round up is if
>              * we're past the max clock rate. We're done searching
>              * if that's the case.
>              */
>             if (freq < test_freq)
>                 return ser_clk;
> 
>             /* Save the first (lowest freq) within 2% */
>             if (!ser_clk && freq <= mult + two_percent) {
>                 ser_clk = freq;
>                 *clk_div = div;
>             }
> 
>             /*
>              * If we already rounded up past mult then this will
>              * cause the loop to exit. If not then this will run
>              * the loop a second time with exactly mult.
>              */
>             test_freq = max(freq + 1, mult);
>         }
> 
>         /*
>          * test_freq will always be bigger than mult by at least 1.
>          * That means we can get the next divider with a DIV_ROUND_UP.
>          * This has the advantage of skipping by a whole bunch of divs
>          * If the clock framework already bypassed them.
>          */
>         div = DIV_ROUND_UP(test_freq, desired_clk);
>         }
> 
>     return ser_clk;