| Message ID | 1657221457-32494-1-git-send-email-quic_vnivarth@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V3] tty: serial: qcom-geni-serial: Fix get_clk_div_rate() which otherwise could return a sub-optimal clock rate. | expand |
Hi Vijaya,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tty/tty-testing]
[also build test WARNING on linus/master v5.19-rc5 next-20220707]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Vijaya-Krishna-Nivarthi/tty-serial-qcom-geni-serial-Fix-get_clk_div_rate-which-otherwise-could-return-a-sub-optimal-clock-rate/20220708-031921
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: nios2-randconfig-r024-20220707 (https://download.01.org/0day-ci/archive/20220708/202207081234.3SPgl4KO-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/fbd8a1a4b7d91ea5caa048e4557ab18b0d08ea86
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Vijaya-Krishna-Nivarthi/tty-serial-qcom-geni-serial-Fix-get_clk_div_rate-which-otherwise-could-return-a-sub-optimal-clock-rate/20220708-031921
git checkout fbd8a1a4b7d91ea5caa048e4557ab18b0d08ea86
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=nios2 SHELL=/bin/bash drivers/tty/serial/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:29,
from include/linux/clk.h:13,
from drivers/tty/serial/qcom_geni_serial.c:4:
drivers/tty/serial/qcom_geni_serial.c: In function 'get_clk_div_rate':
>> drivers/tty/serial/qcom_geni_serial.c:1006:26: warning: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'unsigned int' [-Wformat=]
1006 | pr_debug("desired_clk-%lu, ser_clk-%lu, clk_div-%lu\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/printk.h:370:21: note: in definition of macro 'pr_fmt'
370 | #define pr_fmt(fmt) fmt
| ^~~
include/linux/dynamic_debug.h:152:9: note: in expansion of macro '__dynamic_func_call'
152 | __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~~~~
include/linux/dynamic_debug.h:162:9: note: in expansion of macro '_dynamic_func_call'
162 | _dynamic_func_call(fmt, __dynamic_pr_debug, \
| ^~~~~~~~~~~~~~~~~~
include/linux/printk.h:604:9: note: in expansion of macro 'dynamic_pr_debug'
604 | dynamic_pr_debug(fmt, ##__VA_ARGS__)
| ^~~~~~~~~~~~~~~~
drivers/tty/serial/qcom_geni_serial.c:1006:17: note: in expansion of macro 'pr_debug'
1006 | pr_debug("desired_clk-%lu, ser_clk-%lu, clk_div-%lu\n",
| ^~~~~~~~
drivers/tty/serial/qcom_geni_serial.c:1006:67: note: format string is defined here
1006 | pr_debug("desired_clk-%lu, ser_clk-%lu, clk_div-%lu\n",
| ~~^
| |
| long unsigned int
| %u
vim +1006 drivers/tty/serial/qcom_geni_serial.c
983
984 static unsigned long get_clk_div_rate(struct clk *clk, unsigned int baud,
985 unsigned int sampling_rate, unsigned int *clk_div)
986 {
987 unsigned long ser_clk;
988 unsigned long desired_clk;
989
990 desired_clk = baud * sampling_rate;
991 if (!desired_clk) {
992 pr_err("%s: Invalid frequency\n", __func__);
993 return 0;
994 }
995
996 /*
997 * try to find a clock rate within 2% tolerance, then within
998 */
999 ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, 2);
1000 if (!ser_clk)
1001 ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, 5);
1002
1003 if (!ser_clk)
1004 pr_err("Couldn't find suitable clock rate for %lu\n", desired_clk);
1005 else
> 1006 pr_debug("desired_clk-%lu, ser_clk-%lu, clk_div-%lu\n",
1007 desired_clk, ser_clk, *clk_div);
1008
1009 return ser_clk;
1010 }
1011
Hi Vijaya,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tty/tty-testing]
[also build test WARNING on linus/master v5.19-rc5 next-20220707]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Vijaya-Krishna-Nivarthi/tty-serial-qcom-geni-serial-Fix-get_clk_div_rate-which-otherwise-could-return-a-sub-optimal-clock-rate/20220708-031921
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: hexagon-randconfig-r023-20220707 (https://download.01.org/0day-ci/archive/20220708/202207081429.4VXpQCls-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 562c3467a6738aa89203f72fc1d1343e5baadf3c)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/fbd8a1a4b7d91ea5caa048e4557ab18b0d08ea86
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Vijaya-Krishna-Nivarthi/tty-serial-qcom-geni-serial-Fix-get_clk_div_rate-which-otherwise-could-return-a-sub-optimal-clock-rate/20220708-031921
git checkout fbd8a1a4b7d91ea5caa048e4557ab18b0d08ea86
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/tty/serial/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/tty/serial/qcom_geni_serial.c:1007:26: warning: format specifies type 'unsigned long' but the argument has type 'unsigned int' [-Wformat]
desired_clk, ser_clk, *clk_div);
^~~~~~~~
include/linux/printk.h:610:38: note: expanded from macro 'pr_debug'
no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:131:17: note: expanded from macro 'no_printk'
printk(fmt, ##__VA_ARGS__); \
~~~ ^~~~~~~~~~~
include/linux/printk.h:480:60: note: expanded from macro 'printk'
#define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
~~~ ^~~~~~~~~~~
include/linux/printk.h:452:19: note: expanded from macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
~~~~ ^~~~~~~~~~~
1 warning generated.
vim +1007 drivers/tty/serial/qcom_geni_serial.c
983
984 static unsigned long get_clk_div_rate(struct clk *clk, unsigned int baud,
985 unsigned int sampling_rate, unsigned int *clk_div)
986 {
987 unsigned long ser_clk;
988 unsigned long desired_clk;
989
990 desired_clk = baud * sampling_rate;
991 if (!desired_clk) {
992 pr_err("%s: Invalid frequency\n", __func__);
993 return 0;
994 }
995
996 /*
997 * try to find a clock rate within 2% tolerance, then within
998 */
999 ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, 2);
1000 if (!ser_clk)
1001 ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, 5);
1002
1003 if (!ser_clk)
1004 pr_err("Couldn't find suitable clock rate for %lu\n", desired_clk);
1005 else
1006 pr_debug("desired_clk-%lu, ser_clk-%lu, clk_div-%lu\n",
> 1007 desired_clk, ser_clk, *clk_div);
1008
1009 return ser_clk;
1010 }
1011
On Fri, Jul 08, 2022 at 12:47:37AM +0530, Vijaya Krishna Nivarthi wrote: > In the logic around call to clk_round_rate(), for some corner conditions, > get_clk_div_rate() could return an sub-optimal clock rate. Also, if an > exact clock rate was not found lowest clock was being returned. > > Search for suitable clock rate in 2 steps > a) exact match or within 2% tolerance > b) within 5% tolerance > This also takes care of corner conditions. > > Fixes: c2194bc999d4 ("tty: serial: qcom-geni-serial: Remove uart frequency table. Instead, find suitable frequency with call to clk_round_rate") > Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com> > --- > v3: simplified algorithm further, fixed robot compile warnings > v2: removed minor optimisations to make more readable > v1: intial patch contained slightly complicated logic > --- > drivers/tty/serial/qcom_geni_serial.c | 88 +++++++++++++++++++++-------------- > 1 file changed, 53 insertions(+), 35 deletions(-) > > diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c > index 2e23b65..ac2df1c 100644 > --- a/drivers/tty/serial/qcom_geni_serial.c > +++ b/drivers/tty/serial/qcom_geni_serial.c > @@ -943,52 +943,71 @@ static int qcom_geni_serial_startup(struct uart_port *uport) > return 0; > } > > -static unsigned long get_clk_div_rate(struct clk *clk, unsigned int baud, > - unsigned int sampling_rate, unsigned int *clk_div) > +static unsigned long find_clk_rate_in_tol(struct clk *clk, unsigned int desired_clk, > + unsigned int *clk_div, unsigned int percent_tol) > { > - unsigned long ser_clk; > - unsigned long desired_clk; > - unsigned long freq, prev; > + unsigned long freq; > unsigned long div, maxdiv; > - int64_t mult; > - > - desired_clk = baud * sampling_rate; > - if (!desired_clk) { > - pr_err("%s: Invalid frequency\n", __func__); > - return 0; > - } > + u64 mult; > + unsigned long offset, abs_tol, achieved; > > + abs_tol = div_u64((u64)desired_clk * percent_tol, 100); > maxdiv = CLK_DIV_MSK >> CLK_DIV_SHFT; > - prev = 0; > - > - for (div = 1; div <= maxdiv; div++) { > - mult = div * desired_clk; > - if (mult > ULONG_MAX) > + div = 1; > + while (div <= maxdiv) { > + mult = (u64)div * desired_clk; > + if (mult != (unsigned long)mult) > break; > > - freq = clk_round_rate(clk, (unsigned long)mult); > - if (!(freq % desired_clk)) { > - ser_clk = freq; > - break; > - } > + offset = div * abs_tol; > + freq = clk_round_rate(clk, mult - offset); > > - if (!prev) > - ser_clk = freq; > - else if (prev == freq) > + /* Can only get lower if we're done */ > + if (freq < mult - offset) > break; > > - prev = freq; > + /* > + * Re-calculate div in case rounding skipped rates but we > + * ended up at a good one, then check for a match. > + */ > + div = DIV_ROUND_CLOSEST(freq, desired_clk); > + achieved = DIV_ROUND_CLOSEST(freq, div); > + if (achieved <= desired_clk + abs_tol && > + achieved >= desired_clk - abs_tol) { > + *clk_div = div; > + return freq; > + } > + > + div = DIV_ROUND_UP(freq, desired_clk); > } > > - if (!ser_clk) { > - pr_err("%s: Can't find matching DFS entry for baud %d\n", > - __func__, baud); > - return ser_clk; > + return 0; > +} > + > +static unsigned long get_clk_div_rate(struct clk *clk, unsigned int baud, > + unsigned int sampling_rate, unsigned int *clk_div) > +{ > + unsigned long ser_clk; > + unsigned long desired_clk; > + > + desired_clk = baud * sampling_rate; > + if (!desired_clk) { > + pr_err("%s: Invalid frequency\n", __func__); Note, this is a driver, ALWAYS use dev_err() and friends instead. Also do not allow userspace to flood the kernel logs like this looks is possible, this should just be dev_dbg(). And of course, never use __func__, it's not needed anymore for dev_dbg(). > + return 0; Why if you have a error, are you returning 0? > } > > - *clk_div = ser_clk / desired_clk; > - if (!(*clk_div)) > - *clk_div = 1; > + /* > + * try to find a clock rate within 2% tolerance, then within > + */ > + ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, 2); > + if (!ser_clk) > + ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, 5); > + > + if (!ser_clk) > + pr_err("Couldn't find suitable clock rate for %lu\n", desired_clk); return an error? dev_err(). > + else > + pr_debug("desired_clk-%lu, ser_clk-%lu, clk_div-%lu\n", > + desired_clk, ser_clk, *clk_div); dev_dbg()? Also, as the kernel test robot says, this does not build cleanly :( thanks, greg k-h
Hi Vijaya,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on tty/tty-testing]
[also build test WARNING on linus/master v5.19-rc5 next-20220708]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Vijaya-Krishna-Nivarthi/tty-serial-qcom-geni-serial-Fix-get_clk_div_rate-which-otherwise-could-return-a-sub-optimal-clock-rate/20220708-031921
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: arc-randconfig-r004-20220707 (https://download.01.org/0day-ci/archive/20220710/202207100234.kIBTfg5O-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/fbd8a1a4b7d91ea5caa048e4557ab18b0d08ea86
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Vijaya-Krishna-Nivarthi/tty-serial-qcom-geni-serial-Fix-get_clk_div_rate-which-otherwise-could-return-a-sub-optimal-clock-rate/20220708-031921
git checkout fbd8a1a4b7d91ea5caa048e4557ab18b0d08ea86
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash drivers/tty/serial/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:29,
from include/linux/clk.h:13,
from drivers/tty/serial/qcom_geni_serial.c:4:
drivers/tty/serial/qcom_geni_serial.c: In function 'get_clk_div_rate':
>> include/linux/kern_levels.h:5:25: warning: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'unsigned int' [-Wformat=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
include/linux/printk.h:452:25: note: in definition of macro 'printk_index_wrap'
452 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
include/linux/printk.h:131:17: note: in expansion of macro 'printk'
131 | printk(fmt, ##__VA_ARGS__); \
| ^~~~~~
include/linux/printk.h:610:9: note: in expansion of macro 'no_printk'
610 | no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~
include/linux/kern_levels.h:15:25: note: in expansion of macro 'KERN_SOH'
15 | #define KERN_DEBUG KERN_SOH "7" /* debug-level messages */
| ^~~~~~~~
include/linux/printk.h:610:19: note: in expansion of macro 'KERN_DEBUG'
610 | no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~~~
drivers/tty/serial/qcom_geni_serial.c:1006:17: note: in expansion of macro 'pr_debug'
1006 | pr_debug("desired_clk-%lu, ser_clk-%lu, clk_div-%lu\n",
| ^~~~~~~~
vim +5 include/linux/kern_levels.h
314ba3520e513a Joe Perches 2012-07-30 4
04d2c8c83d0e3a Joe Perches 2012-07-30 @5 #define KERN_SOH "\001" /* ASCII Start Of Header */
04d2c8c83d0e3a Joe Perches 2012-07-30 6 #define KERN_SOH_ASCII '\001'
04d2c8c83d0e3a Joe Perches 2012-07-30 7
On 7/8/2022 6:41 PM, Greg KH wrote: > On Fri, Jul 08, 2022 at 12:47:37AM +0530, Vijaya Krishna Nivarthi wrote: >> In the logic around call to clk_round_rate(), for some corner conditions, >> get_clk_div_rate() could return an sub-optimal clock rate. Also, if an >> exact clock rate was not found lowest clock was being returned. >> >> Search for suitable clock rate in 2 steps >> a) exact match or within 2% tolerance >> b) within 5% tolerance >> This also takes care of corner conditions. >> >> Fixes: c2194bc999d4 ("tty: serial: qcom-geni-serial: Remove uart frequency table. Instead, find suitable frequency with call to clk_round_rate") >> Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com> >> --- >> v3: simplified algorithm further, fixed robot compile warnings >> v2: removed minor optimisations to make more readable >> v1: intial patch contained slightly complicated logic >> --- >> drivers/tty/serial/qcom_geni_serial.c | 88 +++++++++++++++++++++-------------- >> 1 file changed, 53 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c >> index 2e23b65..ac2df1c 100644 >> --- a/drivers/tty/serial/qcom_geni_serial.c >> +++ b/drivers/tty/serial/qcom_geni_serial.c >> @@ -943,52 +943,71 @@ static int qcom_geni_serial_startup(struct uart_port *uport) >> return 0; >> } >> >> -static unsigned long get_clk_div_rate(struct clk *clk, unsigned int baud, >> - unsigned int sampling_rate, unsigned int *clk_div) >> +static unsigned long find_clk_rate_in_tol(struct clk *clk, unsigned int desired_clk, >> + unsigned int *clk_div, unsigned int percent_tol) >> { >> - unsigned long ser_clk; >> - unsigned long desired_clk; >> - unsigned long freq, prev; >> + unsigned long freq; >> unsigned long div, maxdiv; >> - int64_t mult; >> - >> - desired_clk = baud * sampling_rate; >> - if (!desired_clk) { >> - pr_err("%s: Invalid frequency\n", __func__); >> - return 0; >> - } >> + u64 mult; >> + unsigned long offset, abs_tol, achieved; >> >> + abs_tol = div_u64((u64)desired_clk * percent_tol, 100); >> maxdiv = CLK_DIV_MSK >> CLK_DIV_SHFT; >> - prev = 0; >> - >> - for (div = 1; div <= maxdiv; div++) { >> - mult = div * desired_clk; >> - if (mult > ULONG_MAX) >> + div = 1; >> + while (div <= maxdiv) { >> + mult = (u64)div * desired_clk; >> + if (mult != (unsigned long)mult) >> break; >> >> - freq = clk_round_rate(clk, (unsigned long)mult); >> - if (!(freq % desired_clk)) { >> - ser_clk = freq; >> - break; >> - } >> + offset = div * abs_tol; >> + freq = clk_round_rate(clk, mult - offset); >> >> - if (!prev) >> - ser_clk = freq; >> - else if (prev == freq) >> + /* Can only get lower if we're done */ >> + if (freq < mult - offset) >> break; >> >> - prev = freq; >> + /* >> + * Re-calculate div in case rounding skipped rates but we >> + * ended up at a good one, then check for a match. >> + */ >> + div = DIV_ROUND_CLOSEST(freq, desired_clk); >> + achieved = DIV_ROUND_CLOSEST(freq, div); >> + if (achieved <= desired_clk + abs_tol && >> + achieved >= desired_clk - abs_tol) { >> + *clk_div = div; >> + return freq; >> + } >> + >> + div = DIV_ROUND_UP(freq, desired_clk); >> } >> >> - if (!ser_clk) { >> - pr_err("%s: Can't find matching DFS entry for baud %d\n", >> - __func__, baud); >> - return ser_clk; >> + return 0; >> +} >> + >> +static unsigned long get_clk_div_rate(struct clk *clk, unsigned int baud, >> + unsigned int sampling_rate, unsigned int *clk_div) >> +{ >> + unsigned long ser_clk; >> + unsigned long desired_clk; >> + >> + desired_clk = baud * sampling_rate; >> + if (!desired_clk) { >> + pr_err("%s: Invalid frequency\n", __func__); > Note, this is a driver, ALWAYS use dev_err() and friends instead. > > Also do not allow userspace to flood the kernel logs like this looks is > possible, this should just be dev_dbg(). > > And of course, never use __func__, it's not needed anymore for > dev_dbg(). Ok. > >> + return 0; > Why if you have a error, are you returning 0? Yes, and it has been so earlier too. 0 is an invalid clock rate and will be handled accordingly by caller. >> } >> >> - *clk_div = ser_clk / desired_clk; >> - if (!(*clk_div)) >> - *clk_div = 1; >> + /* >> + * try to find a clock rate within 2% tolerance, then within >> + */ >> + ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, 2); >> + if (!ser_clk) >> + ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, 5); >> + >> + if (!ser_clk) >> + pr_err("Couldn't find suitable clock rate for %lu\n", desired_clk); > return an error? > > dev_err(). As mentioned, we didn't (and don't) return error from here but 0. > >> + else >> + pr_debug("desired_clk-%lu, ser_clk-%lu, clk_div-%lu\n", >> + desired_clk, ser_clk, *clk_div); > dev_dbg()? Ok. > > Also, as the kernel test robot says, this does not build cleanly :( change to dev_dbg should take care of these. Will do. Thank you. Vijay/ > > thanks, > > greg k-h
diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c index 2e23b65..ac2df1c 100644 --- a/drivers/tty/serial/qcom_geni_serial.c +++ b/drivers/tty/serial/qcom_geni_serial.c @@ -943,52 +943,71 @@ static int qcom_geni_serial_startup(struct uart_port *uport) return 0; } -static unsigned long get_clk_div_rate(struct clk *clk, unsigned int baud, - unsigned int sampling_rate, unsigned int *clk_div) +static unsigned long find_clk_rate_in_tol(struct clk *clk, unsigned int desired_clk, + unsigned int *clk_div, unsigned int percent_tol) { - unsigned long ser_clk; - unsigned long desired_clk; - unsigned long freq, prev; + unsigned long freq; unsigned long div, maxdiv; - int64_t mult; - - desired_clk = baud * sampling_rate; - if (!desired_clk) { - pr_err("%s: Invalid frequency\n", __func__); - return 0; - } + u64 mult; + unsigned long offset, abs_tol, achieved; + abs_tol = div_u64((u64)desired_clk * percent_tol, 100); maxdiv = CLK_DIV_MSK >> CLK_DIV_SHFT; - prev = 0; - - for (div = 1; div <= maxdiv; div++) { - mult = div * desired_clk; - if (mult > ULONG_MAX) + div = 1; + while (div <= maxdiv) { + mult = (u64)div * desired_clk; + if (mult != (unsigned long)mult) break; - freq = clk_round_rate(clk, (unsigned long)mult); - if (!(freq % desired_clk)) { - ser_clk = freq; - break; - } + offset = div * abs_tol; + freq = clk_round_rate(clk, mult - offset); - if (!prev) - ser_clk = freq; - else if (prev == freq) + /* Can only get lower if we're done */ + if (freq < mult - offset) break; - prev = freq; + /* + * Re-calculate div in case rounding skipped rates but we + * ended up at a good one, then check for a match. + */ + div = DIV_ROUND_CLOSEST(freq, desired_clk); + achieved = DIV_ROUND_CLOSEST(freq, div); + if (achieved <= desired_clk + abs_tol && + achieved >= desired_clk - abs_tol) { + *clk_div = div; + return freq; + } + + div = DIV_ROUND_UP(freq, desired_clk); } - if (!ser_clk) { - pr_err("%s: Can't find matching DFS entry for baud %d\n", - __func__, baud); - return ser_clk; + return 0; +} + +static unsigned long get_clk_div_rate(struct clk *clk, unsigned int baud, + unsigned int sampling_rate, unsigned int *clk_div) +{ + unsigned long ser_clk; + unsigned long desired_clk; + + desired_clk = baud * sampling_rate; + if (!desired_clk) { + pr_err("%s: Invalid frequency\n", __func__); + return 0; } - *clk_div = ser_clk / desired_clk; - if (!(*clk_div)) - *clk_div = 1; + /* + * try to find a clock rate within 2% tolerance, then within + */ + ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, 2); + if (!ser_clk) + ser_clk = find_clk_rate_in_tol(clk, desired_clk, clk_div, 5); + + if (!ser_clk) + pr_err("Couldn't find suitable clock rate for %lu\n", desired_clk); + else + pr_debug("desired_clk-%lu, ser_clk-%lu, clk_div-%lu\n", + desired_clk, ser_clk, *clk_div); return ser_clk; } @@ -1021,8 +1040,7 @@ static void qcom_geni_serial_set_termios(struct uart_port *uport, if (ver >= QUP_SE_VERSION_2_5) sampling_rate /= 2; - clk_rate = get_clk_div_rate(port->se.clk, baud, - sampling_rate, &clk_div); + clk_rate = get_clk_div_rate(port->se.clk, baud, sampling_rate, &clk_div); if (!clk_rate) goto out_restart_rx;
In the logic around call to clk_round_rate(), for some corner conditions, get_clk_div_rate() could return an sub-optimal clock rate. Also, if an exact clock rate was not found lowest clock was being returned. Search for suitable clock rate in 2 steps a) exact match or within 2% tolerance b) within 5% tolerance This also takes care of corner conditions. Fixes: c2194bc999d4 ("tty: serial: qcom-geni-serial: Remove uart frequency table. Instead, find suitable frequency with call to clk_round_rate") Signed-off-by: Vijaya Krishna Nivarthi <quic_vnivarth@quicinc.com> --- v3: simplified algorithm further, fixed robot compile warnings v2: removed minor optimisations to make more readable v1: intial patch contained slightly complicated logic --- drivers/tty/serial/qcom_geni_serial.c | 88 +++++++++++++++++++++-------------- 1 file changed, 53 insertions(+), 35 deletions(-)