lkml.org 
[lkml]   [2022]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [PATCH] tty: serial: qcom-geni-serial: Fix get_clk_div_rate() which otherwise could return a sub-optimal clock rate.
Date
Hi,


> > +
> > + /*
> > + * A dead-on freq is an insta-win, look for it only in 1st run
> > + */
> > + if (*exact_match) {
> > + if (!(freq % desired_clk)) {
> > + ser_clk = freq;
> > + *clk_div = freq / desired_clk;
> > + return ser_clk;
> > + }
> > + }
>
> The "*exact_match" if test isn't needed here. It's not saving you any
> significant amount of time. You're still doing an "if" test, right?
> ...so you're basically saving a mod operation by adding a pointer dereference
> and complexity? I don't think that's the right tradeoff.
>
>

Removed exact_match check from here.


> >
> > - if (!ser_clk) {
> > - pr_err("%s: Can't find matching DFS entry for baud %d\n",
> > - __func__, baud);
> > + /* try to find exact clock rate or within 2% tolerance */
> > + ser_clk = 0;
> > + exact_match = true;
> > + desired_tol = 2;
>
> Don't need a "desired_tol" variable. Just pass 2 into the function.
>
>

Done


> > + ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, desired_tol,
> &exact_match);
> > + if (ser_clk) {
> > + if (!exact_match)
> > + pr_warn("Cannot find exact match clk_rate,
> > + using one within 2 percent tolerance\n");
>
> IMO get rid of this printout. Just return what you found if it's not 0. It's
> perfectly fine. ...that means you can fully get rid of the "exact_match"
> variable.
>
>

Done.
But retained exact_match as bool instead of pointer to help early out in 2nd call.

> > return ser_clk;
> > }
> >
> > - *clk_div = ser_clk / desired_clk;
> > - if (!(*clk_div))
> > - *clk_div = 1;
> > + /* try within 5% tolerance now, no need to look for exact match */
> > + exact_match = false;
> > + desired_tol = 5;
> > +
> > + ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, desired_tol,
> &exact_match);
> > + if (ser_clk)
> > + pr_warn("Cannot find exact match clk_rate, using one within 5
> percent tolerance\n");
> > + else
> > + pr_err("Cannot find suitable clk_rate, giving up\n");
>
> Just keep the error message but not the warning. ...and ideally use "dev_err"
> and print out the clock you were trying to achieve.

Done. Retained pr_err since dev wasn’t readily available here.

Thank you.
\
 
 \ /
  Last update: 2022-06-29 12:16    [W:0.187 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site