linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] clk: qcom: ipq4019: fix apss cpu overclocking
@ 2020-06-04 12:09 Robert Marko
  2020-06-04 20:24 ` kernel test robot
  0 siblings, 1 reply; 7+ messages in thread
From: Robert Marko @ 2020-06-04 12:09 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, absahu,
	linux-arm-msm, linux-clk, linux-kernel
  Cc: Christian Lamparter, John Crispin, Robert Marko, Luka Perkov

From: Christian Lamparter <chunkeey@gmail.com>

There's an interaction issue between the clk changes:"
clk: qcom: ipq4019: Add the apss cpu pll divider clock node
clk: qcom: ipq4019: remove fixed clocks and add pll clocks
" and the cpufreq-dt.

cpufreq-dt is now spamming the kernel-log with the following:

[ 1099.190658] cpu cpu0: dev_pm_opp_set_rate: failed to find current OPP
for freq 761142857 (-34)

This only happens on certain devices like the Compex WPJ428
and AVM FritzBox!4040. However, other devices like the Asus
RT-AC58U and Meraki MR33 work just fine.

The issue stem from the fact that all higher CPU-Clocks
are achieved by switching the clock-parent to the P_DDRPLLAPSS
(ddrpllapss). Which is set by Qualcomm's proprietary bootcode
as part of the DDR calibration.

For example, the FB4040 uses 256 MiB Nanya NT5CC128M16IP clocked
at round 533 MHz (ddrpllsdcc = 190285714 Hz).

whereas the 128 MiB Nanya NT5CC64M16GP-DI in the ASUS RT-AC58U is
clocked at a slightly higher 537 MHz ( ddrpllsdcc = 192000000 Hz).

This patch attempts to fix the issue by modifying
clk_cpu_div_round_rate(), clk_cpu_div_set_rate(), clk_cpu_div_recalc_rate()
to use a new qcom_find_freq_close() function, which returns the closest
matching frequency, instead of the next higher. This way, the SoC in
the FB4040 (with its max clock speed of 710.4 MHz) will no longer
try to overclock to 761 MHz.

Fixes: d83dcacea18 ("clk: qcom: ipq4019: Add the apss cpu pll divider clock node")
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Signed-off-by: John Crispin <john@phrozen.org>
Tested-by: Robert Marko <robert.marko@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
 drivers/clk/qcom/gcc-ipq4019.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gcc-ipq4019.c b/drivers/clk/qcom/gcc-ipq4019.c
index ef5137fd50f3..eeed50573752 100644
--- a/drivers/clk/qcom/gcc-ipq4019.c
+++ b/drivers/clk/qcom/gcc-ipq4019.c
@@ -1243,6 +1243,29 @@ static const struct clk_fepll_vco gcc_fepll_vco = {
 	.reg = 0x2f020,
 };
 
+
+const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
+					     unsigned long rate)
+{
+	const struct freq_tbl *last = NULL;
+
+	for ( ; f->freq; f++) {
+		if (rate == f->freq)
+			return f;
+
+		if (f->freq > rate) {
+			if (!last ||
+			   (f->freq - rate) < (rate - last->freq))
+				return f;
+			else
+				return last;
+		}
+		last = f;
+	}
+
+	return last;
+}
+
 /*
  * Round rate function for APSS CPU PLL Clock divider.
  * It looks up the frequency table and returns the next higher frequency
@@ -1255,7 +1278,7 @@ static long clk_cpu_div_round_rate(struct clk_hw *hw, unsigned long rate,
 	struct clk_hw *p_hw;
 	const struct freq_tbl *f;
 
-	f = qcom_find_freq(pll->freq_tbl, rate);
+	f = qcom_find_freq_close(pll->freq_tbl, rate);
 	if (!f)
 		return -EINVAL;
 
@@ -1278,7 +1301,7 @@ static int clk_cpu_div_set_rate(struct clk_hw *hw, unsigned long rate,
 	u32 mask;
 	int ret;
 
-	f = qcom_find_freq(pll->freq_tbl, rate);
+	f = qcom_find_freq_close(pll->freq_tbl, rate);
 	if (!f)
 		return -EINVAL;
 
@@ -1305,6 +1328,7 @@ static unsigned long
 clk_cpu_div_recalc_rate(struct clk_hw *hw,
 			unsigned long parent_rate)
 {
+	const struct freq_tbl *f;
 	struct clk_fepll *pll = to_clk_fepll(hw);
 	u32 cdiv, pre_div;
 	u64 rate;
@@ -1325,7 +1349,11 @@ clk_cpu_div_recalc_rate(struct clk_hw *hw,
 	rate = clk_fepll_vco_calc_rate(pll, parent_rate) * 2;
 	do_div(rate, pre_div);
 
-	return rate;
+	f = qcom_find_freq_close(pll->freq_tbl, rate);
+	if (!f)
+		return rate;
+
+	return f->freq;
 };
 
 static const struct clk_ops clk_regmap_cpu_div_ops = {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: qcom: ipq4019: fix apss cpu overclocking
  2020-06-04 12:09 [PATCH] clk: qcom: ipq4019: fix apss cpu overclocking Robert Marko
@ 2020-06-04 20:24 ` kernel test robot
  2020-06-08  8:54   ` Robert Marko
  0 siblings, 1 reply; 7+ messages in thread
From: kernel test robot @ 2020-06-04 20:24 UTC (permalink / raw)
  To: Robert Marko, agross, bjorn.andersson, mturquette, sboyd, absahu,
	linux-arm-msm, linux-clk, linux-kernel
  Cc: kbuild-all, clang-built-linux, Christian Lamparter, John Crispin

[-- Attachment #1: Type: text/plain, Size: 2509 bytes --]

Hi Robert,

I love your patch! Perhaps something to improve:

[auto build test WARNING on clk/clk-next]
[also build test WARNING on v5.7 next-20200604]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Robert-Marko/clk-qcom-ipq4019-fix-apss-cpu-overclocking/20200605-002859
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project ac47588bc4ff5927a01ed6fcd269ce86aba52a7c)
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
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/clk/qcom/gcc-ipq4019.c:1247:24: warning: no previous prototype for function 'qcom_find_freq_close' [-Wmissing-prototypes]
const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
^
drivers/clk/qcom/gcc-ipq4019.c:1247:7: note: declare 'static' if the function is not intended to be used outside of this translation unit
const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
^
static
1 warning generated.

vim +/qcom_find_freq_close +1247 drivers/clk/qcom/gcc-ipq4019.c

  1245	
  1246	
> 1247	const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
  1248						     unsigned long rate)
  1249	{
  1250		const struct freq_tbl *last = NULL;
  1251	
  1252		for ( ; f->freq; f++) {
  1253			if (rate == f->freq)
  1254				return f;
  1255	
  1256			if (f->freq > rate) {
  1257				if (!last ||
  1258				   (f->freq - rate) < (rate - last->freq))
  1259					return f;
  1260				else
  1261					return last;
  1262			}
  1263			last = f;
  1264		}
  1265	
  1266		return last;
  1267	}
  1268	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 73615 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: qcom: ipq4019: fix apss cpu overclocking
  2020-06-04 20:24 ` kernel test robot
@ 2020-06-08  8:54   ` Robert Marko
  2020-06-08  9:07     ` Nathan Chancellor
  0 siblings, 1 reply; 7+ messages in thread
From: Robert Marko @ 2020-06-08  8:54 UTC (permalink / raw)
  To: kernel test robot
  Cc: Andy Gross, Bjorn Andersson, Michael Turquette, sboyd,
	Abhishek Sahu, linux-arm-msm, linux-clk, linux-kernel,
	kbuild-all, clang-built-linux, Christian Lamparter, John Crispin

Sorry for asking, but are these warnings relevant?
GCC9.3 does not throw them

Regards
Robert

On Thu, Jun 4, 2020 at 10:25 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Robert,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on clk/clk-next]
> [also build test WARNING on v5.7 next-20200604]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url:    https://github.com/0day-ci/linux/commits/Robert-Marko/clk-qcom-ipq4019-fix-apss-cpu-overclocking/20200605-002859
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> config: x86_64-allyesconfig (attached as .config)
> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project ac47588bc4ff5927a01ed6fcd269ce86aba52a7c)
> 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
>         # install x86_64 cross compiling tool for clang build
>         # apt-get install binutils-x86-64-linux-gnu
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>, old ones prefixed by <<):
>
> >> drivers/clk/qcom/gcc-ipq4019.c:1247:24: warning: no previous prototype for function 'qcom_find_freq_close' [-Wmissing-prototypes]
> const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
> ^
> drivers/clk/qcom/gcc-ipq4019.c:1247:7: note: declare 'static' if the function is not intended to be used outside of this translation unit
> const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
> ^
> static
> 1 warning generated.
>
> vim +/qcom_find_freq_close +1247 drivers/clk/qcom/gcc-ipq4019.c
>
>   1245
>   1246
> > 1247  const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
>   1248                                               unsigned long rate)
>   1249  {
>   1250          const struct freq_tbl *last = NULL;
>   1251
>   1252          for ( ; f->freq; f++) {
>   1253                  if (rate == f->freq)
>   1254                          return f;
>   1255
>   1256                  if (f->freq > rate) {
>   1257                          if (!last ||
>   1258                             (f->freq - rate) < (rate - last->freq))
>   1259                                  return f;
>   1260                          else
>   1261                                  return last;
>   1262                  }
>   1263                  last = f;
>   1264          }
>   1265
>   1266          return last;
>   1267  }
>   1268
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: qcom: ipq4019: fix apss cpu overclocking
  2020-06-08  8:54   ` Robert Marko
@ 2020-06-08  9:07     ` Nathan Chancellor
  2020-06-08  9:43       ` Robert Marko
  0 siblings, 1 reply; 7+ messages in thread
From: Nathan Chancellor @ 2020-06-08  9:07 UTC (permalink / raw)
  To: Robert Marko
  Cc: kernel test robot, Andy Gross, Bjorn Andersson,
	Michael Turquette, sboyd, Abhishek Sahu, linux-arm-msm,
	linux-clk, linux-kernel, kbuild-all, clang-built-linux,
	Christian Lamparter, John Crispin

On Mon, Jun 08, 2020 at 10:54:34AM +0200, Robert Marko wrote:
> On Thu, Jun 4, 2020 at 10:25 PM kernel test robot <lkp@intel.com> wrote:
> >
> > Hi Robert,
> >
> > I love your patch! Perhaps something to improve:
> >
> > [auto build test WARNING on clk/clk-next]
> > [also build test WARNING on v5.7 next-20200604]
> > [if your patch is applied to the wrong git tree, please drop us a note to help
> > improve the system. BTW, we also suggest to use '--base' option to specify the
> > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> >
> > url:    https://github.com/0day-ci/linux/commits/Robert-Marko/clk-qcom-ipq4019-fix-apss-cpu-overclocking/20200605-002859
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> > config: x86_64-allyesconfig (attached as .config)
> > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project ac47588bc4ff5927a01ed6fcd269ce86aba52a7c)
> > 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
> >         # install x86_64 cross compiling tool for clang build
> >         # apt-get install binutils-x86-64-linux-gnu
> >         # save the attached .config to linux build tree
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> >
> > All warnings (new ones prefixed by >>, old ones prefixed by <<):
> >
> > >> drivers/clk/qcom/gcc-ipq4019.c:1247:24: warning: no previous prototype for function 'qcom_find_freq_close' [-Wmissing-prototypes]
> > const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
> > ^
> > drivers/clk/qcom/gcc-ipq4019.c:1247:7: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
> > ^
> > static
> > 1 warning generated.
> >
> > vim +/qcom_find_freq_close +1247 drivers/clk/qcom/gcc-ipq4019.c
> >
> >   1245
> >   1246
> > > 1247  const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
> >   1248                                               unsigned long rate)
> >   1249  {
> >   1250          const struct freq_tbl *last = NULL;
> >   1251
> >   1252          for ( ; f->freq; f++) {
> >   1253                  if (rate == f->freq)
> >   1254                          return f;
> >   1255
> >   1256                  if (f->freq > rate) {
> >   1257                          if (!last ||
> >   1258                             (f->freq - rate) < (rate - last->freq))
> >   1259                                  return f;
> >   1260                          else
> >   1261                                  return last;
> >   1262                  }
> >   1263                  last = f;
> >   1264          }
> >   1265
> >   1266          return last;
> >   1267  }
> >   1268
> >
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 

<moved post to the bottom>

> Sorry for asking, but are these warnings relevant?
> GCC9.3 does not throw them
> 
> Regards
> Robert
> 

It should if you are using make W=1, this is not a clang specific
warning (it just happens that clang was the compiler for this report).

It looks like qcom_find_freq_close is only used in
drivers/clk/qcom/gcc-ipq4019.c, in which case it should be marked
static.

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: qcom: ipq4019: fix apss cpu overclocking
  2020-06-08  9:07     ` Nathan Chancellor
@ 2020-06-08  9:43       ` Robert Marko
  0 siblings, 0 replies; 7+ messages in thread
From: Robert Marko @ 2020-06-08  9:43 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: kernel test robot, Andy Gross, Bjorn Andersson,
	Michael Turquette, sboyd, Abhishek Sahu, linux-arm-msm,
	linux-clk, linux-kernel, kbuild-all, clang-built-linux,
	Christian Lamparter, John Crispin

On Mon, Jun 8, 2020 at 11:07 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Mon, Jun 08, 2020 at 10:54:34AM +0200, Robert Marko wrote:
> > On Thu, Jun 4, 2020 at 10:25 PM kernel test robot <lkp@intel.com> wrote:
> > >
> > > Hi Robert,
> > >
> > > I love your patch! Perhaps something to improve:
> > >
> > > [auto build test WARNING on clk/clk-next]
> > > [also build test WARNING on v5.7 next-20200604]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help
> > > improve the system. BTW, we also suggest to use '--base' option to specify the
> > > base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
> > >
> > > url:    https://github.com/0day-ci/linux/commits/Robert-Marko/clk-qcom-ipq4019-fix-apss-cpu-overclocking/20200605-002859
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
> > > config: x86_64-allyesconfig (attached as .config)
> > > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project ac47588bc4ff5927a01ed6fcd269ce86aba52a7c)
> > > 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
> > >         # install x86_64 cross compiling tool for clang build
> > >         # apt-get install binutils-x86-64-linux-gnu
> > >         # save the attached .config to linux build tree
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > > All warnings (new ones prefixed by >>, old ones prefixed by <<):
> > >
> > > >> drivers/clk/qcom/gcc-ipq4019.c:1247:24: warning: no previous prototype for function 'qcom_find_freq_close' [-Wmissing-prototypes]
> > > const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
> > > ^
> > > drivers/clk/qcom/gcc-ipq4019.c:1247:7: note: declare 'static' if the function is not intended to be used outside of this translation unit
> > > const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
> > > ^
> > > static
> > > 1 warning generated.
> > >
> > > vim +/qcom_find_freq_close +1247 drivers/clk/qcom/gcc-ipq4019.c
> > >
> > >   1245
> > >   1246
> > > > 1247  const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
> > >   1248                                               unsigned long rate)
> > >   1249  {
> > >   1250          const struct freq_tbl *last = NULL;
> > >   1251
> > >   1252          for ( ; f->freq; f++) {
> > >   1253                  if (rate == f->freq)
> > >   1254                          return f;
> > >   1255
> > >   1256                  if (f->freq > rate) {
> > >   1257                          if (!last ||
> > >   1258                             (f->freq - rate) < (rate - last->freq))
> > >   1259                                  return f;
> > >   1260                          else
> > >   1261                                  return last;
> > >   1262                  }
> > >   1263                  last = f;
> > >   1264          }
> > >   1265
> > >   1266          return last;
> > >   1267  }
> > >   1268
> > >
> > > ---
> > > 0-DAY CI Kernel Test Service, Intel Corporation
> > > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> >
>
> <moved post to the bottom>
>
> > Sorry for asking, but are these warnings relevant?
> > GCC9.3 does not throw them
> >
> > Regards
> > Robert
> >
>
> It should if you are using make W=1, this is not a clang specific
> warning (it just happens that clang was the compiler for this report).

Thanks, W=1 does indeed show the warning along with a not evaluated return.
>
> It looks like qcom_find_freq_close is only used in
> drivers/clk/qcom/gcc-ipq4019.c, in which case it should be marked
> static.
Thanks, it does indeed solve the warning.
I will send a v2 today.

Regards,
Robert
>
> Cheers,
> Nathan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] clk: qcom: ipq4019: fix apss cpu overclocking
  2020-06-08  9:47 Robert Marko
@ 2020-06-09 21:15 ` Stephen Boyd
  0 siblings, 0 replies; 7+ messages in thread
From: Stephen Boyd @ 2020-06-09 21:15 UTC (permalink / raw)
  To: Robert Marko, agross, bjorn.andersson, linux-arm-msm, linux-clk,
	linux-kernel, mturquette
  Cc: Christian Lamparter, John Crispin, Robert Marko, Luka Perkov

Quoting Robert Marko (2020-06-08 02:47:15)
> From: Christian Lamparter <chunkeey@gmail.com>
> 
> There's an interaction issue between the clk changes:"
> clk: qcom: ipq4019: Add the apss cpu pll divider clock node
> clk: qcom: ipq4019: remove fixed clocks and add pll clocks
> " and the cpufreq-dt.
> 
> cpufreq-dt is now spamming the kernel-log with the following:
> 
> [ 1099.190658] cpu cpu0: dev_pm_opp_set_rate: failed to find current OPP
> for freq 761142857 (-34)
> 
> This only happens on certain devices like the Compex WPJ428
> and AVM FritzBox!4040. However, other devices like the Asus
> RT-AC58U and Meraki MR33 work just fine.
> 
> The issue stem from the fact that all higher CPU-Clocks
> are achieved by switching the clock-parent to the P_DDRPLLAPSS
> (ddrpllapss). Which is set by Qualcomm's proprietary bootcode
> as part of the DDR calibration.
> 
> For example, the FB4040 uses 256 MiB Nanya NT5CC128M16IP clocked
> at round 533 MHz (ddrpllsdcc = 190285714 Hz).
> 
> whereas the 128 MiB Nanya NT5CC64M16GP-DI in the ASUS RT-AC58U is
> clocked at a slightly higher 537 MHz ( ddrpllsdcc = 192000000 Hz).
> 
> This patch attempts to fix the issue by modifying
> clk_cpu_div_round_rate(), clk_cpu_div_set_rate(), clk_cpu_div_recalc_rate()
> to use a new qcom_find_freq_close() function, which returns the closest
> matching frequency, instead of the next higher. This way, the SoC in
> the FB4040 (with its max clock speed of 710.4 MHz) will no longer
> try to overclock to 761 MHz.

Why are the OPP tables not properly indicating the frequencies that
should be chosen? The rounding policy should presumably be matching the
frequency exactly vs. relying on some sort of rounding policy to fix it.

> 
> Fixes: d83dcacea18 ("clk: qcom: ipq4019: Add the apss cpu pll divider clock node")
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
> Signed-off-by: John Crispin <john@phrozen.org>
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> ---
> Changes from v1 to v2:

Please update subject of the patch to indicate patch version (i.e.
PATCHv2 and next time PATCHv3, not just PATCH).

> * Resolve warnings discovered by the kbot
> * Return the return of regmap_update_bits instead of not using it at all
> 
>  drivers/clk/qcom/gcc-ipq4019.c | 36 ++++++++++++++++++++++++++++++----
>  1 file changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gcc-ipq4019.c b/drivers/clk/qcom/gcc-ipq4019.c
> index ef5137fd50f3..62fa17a4291c 100644
> --- a/drivers/clk/qcom/gcc-ipq4019.c
> +++ b/drivers/clk/qcom/gcc-ipq4019.c
> @@ -1243,6 +1243,29 @@ static const struct clk_fepll_vco gcc_fepll_vco = {
>         .reg = 0x2f020,
>  };
>  
> +
> +static const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,

Please call it qcom_find_freq_closest() instead.

> +                                            unsigned long rate)
> +{
> +       const struct freq_tbl *last = NULL;
> +
> +       for ( ; f->freq; f++) {
> +               if (rate == f->freq)
> +                       return f;
> +
> +               if (f->freq > rate) {
> +                       if (!last ||
> +                          (f->freq - rate) < (rate - last->freq))

> +                               return f;
> +                       else
> +                               return last;

	if (...)
		return ...;
	else

should be replaced with

	if (...)
		return ...;

	...

but I also wonder if it would be clearer with some sort of 'break' and
then 'return f' type of logic.

> +               }
> +               last = f;
> +       }
> +
> +       return last;
> +}

Please relocate this to common.c in the qcom directory.

> +
>  /*
>   * Round rate function for APSS CPU PLL Clock divider.
>   * It looks up the frequency table and returns the next higher frequency

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] clk: qcom: ipq4019: fix apss cpu overclocking
@ 2020-06-08  9:47 Robert Marko
  2020-06-09 21:15 ` Stephen Boyd
  0 siblings, 1 reply; 7+ messages in thread
From: Robert Marko @ 2020-06-08  9:47 UTC (permalink / raw)
  To: agross, bjorn.andersson, mturquette, sboyd, linux-arm-msm,
	linux-clk, linux-kernel
  Cc: Christian Lamparter, John Crispin, Robert Marko, Luka Perkov

From: Christian Lamparter <chunkeey@gmail.com>

There's an interaction issue between the clk changes:"
clk: qcom: ipq4019: Add the apss cpu pll divider clock node
clk: qcom: ipq4019: remove fixed clocks and add pll clocks
" and the cpufreq-dt.

cpufreq-dt is now spamming the kernel-log with the following:

[ 1099.190658] cpu cpu0: dev_pm_opp_set_rate: failed to find current OPP
for freq 761142857 (-34)

This only happens on certain devices like the Compex WPJ428
and AVM FritzBox!4040. However, other devices like the Asus
RT-AC58U and Meraki MR33 work just fine.

The issue stem from the fact that all higher CPU-Clocks
are achieved by switching the clock-parent to the P_DDRPLLAPSS
(ddrpllapss). Which is set by Qualcomm's proprietary bootcode
as part of the DDR calibration.

For example, the FB4040 uses 256 MiB Nanya NT5CC128M16IP clocked
at round 533 MHz (ddrpllsdcc = 190285714 Hz).

whereas the 128 MiB Nanya NT5CC64M16GP-DI in the ASUS RT-AC58U is
clocked at a slightly higher 537 MHz ( ddrpllsdcc = 192000000 Hz).

This patch attempts to fix the issue by modifying
clk_cpu_div_round_rate(), clk_cpu_div_set_rate(), clk_cpu_div_recalc_rate()
to use a new qcom_find_freq_close() function, which returns the closest
matching frequency, instead of the next higher. This way, the SoC in
the FB4040 (with its max clock speed of 710.4 MHz) will no longer
try to overclock to 761 MHz.

Fixes: d83dcacea18 ("clk: qcom: ipq4019: Add the apss cpu pll divider clock node")
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Signed-off-by: John Crispin <john@phrozen.org>
Signed-off-by: Robert Marko <robert.marko@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
Changes from v1 to v2:
* Resolve warnings discovered by the kbot
* Return the return of regmap_update_bits instead of not using it at all

 drivers/clk/qcom/gcc-ipq4019.c | 36 ++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/qcom/gcc-ipq4019.c b/drivers/clk/qcom/gcc-ipq4019.c
index ef5137fd50f3..62fa17a4291c 100644
--- a/drivers/clk/qcom/gcc-ipq4019.c
+++ b/drivers/clk/qcom/gcc-ipq4019.c
@@ -1243,6 +1243,29 @@ static const struct clk_fepll_vco gcc_fepll_vco = {
 	.reg = 0x2f020,
 };
 
+
+static const struct freq_tbl *qcom_find_freq_close(const struct freq_tbl *f,
+					     unsigned long rate)
+{
+	const struct freq_tbl *last = NULL;
+
+	for ( ; f->freq; f++) {
+		if (rate == f->freq)
+			return f;
+
+		if (f->freq > rate) {
+			if (!last ||
+			   (f->freq - rate) < (rate - last->freq))
+				return f;
+			else
+				return last;
+		}
+		last = f;
+	}
+
+	return last;
+}
+
 /*
  * Round rate function for APSS CPU PLL Clock divider.
  * It looks up the frequency table and returns the next higher frequency
@@ -1255,7 +1278,7 @@ static long clk_cpu_div_round_rate(struct clk_hw *hw, unsigned long rate,
 	struct clk_hw *p_hw;
 	const struct freq_tbl *f;
 
-	f = qcom_find_freq(pll->freq_tbl, rate);
+	f = qcom_find_freq_close(pll->freq_tbl, rate);
 	if (!f)
 		return -EINVAL;
 
@@ -1278,7 +1301,7 @@ static int clk_cpu_div_set_rate(struct clk_hw *hw, unsigned long rate,
 	u32 mask;
 	int ret;
 
-	f = qcom_find_freq(pll->freq_tbl, rate);
+	f = qcom_find_freq_close(pll->freq_tbl, rate);
 	if (!f)
 		return -EINVAL;
 
@@ -1292,7 +1315,7 @@ static int clk_cpu_div_set_rate(struct clk_hw *hw, unsigned long rate,
 	 */
 	udelay(1);
 
-	return 0;
+	return ret;
 };
 
 /*
@@ -1305,6 +1328,7 @@ static unsigned long
 clk_cpu_div_recalc_rate(struct clk_hw *hw,
 			unsigned long parent_rate)
 {
+	const struct freq_tbl *f;
 	struct clk_fepll *pll = to_clk_fepll(hw);
 	u32 cdiv, pre_div;
 	u64 rate;
@@ -1325,7 +1349,11 @@ clk_cpu_div_recalc_rate(struct clk_hw *hw,
 	rate = clk_fepll_vco_calc_rate(pll, parent_rate) * 2;
 	do_div(rate, pre_div);
 
-	return rate;
+	f = qcom_find_freq_close(pll->freq_tbl, rate);
+	if (!f)
+		return rate;
+
+	return f->freq;
 };
 
 static const struct clk_ops clk_regmap_cpu_div_ops = {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-06-09 21:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04 12:09 [PATCH] clk: qcom: ipq4019: fix apss cpu overclocking Robert Marko
2020-06-04 20:24 ` kernel test robot
2020-06-08  8:54   ` Robert Marko
2020-06-08  9:07     ` Nathan Chancellor
2020-06-08  9:43       ` Robert Marko
2020-06-08  9:47 Robert Marko
2020-06-09 21:15 ` Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).