Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751501AbcLEDfY (ORCPT ); Sun, 4 Dec 2016 22:35:24 -0500 Received: from mail-pf0-f176.google.com ([209.85.192.176]:33574 "EHLO mail-pf0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751120AbcLEDfV (ORCPT ); Sun, 4 Dec 2016 22:35:21 -0500 Date: Mon, 5 Dec 2016 09:04:31 +0530 From: Viresh Kumar To: Bartosz Golaszewski Cc: Kevin Hilman , Michael Turquette , Sekhar Nori , Peter Ujfalusi , Russell King , Boris Brezillon , "Rafael J. Wysocki" , Richard Weinberger , David Woodhouse , Brian Norris , Marek Vasut , Cyrille Pitchen , LKML , arm-soc , linux-pm@vger.kernel.org Subject: Re: [PATCH v2 3/3] ARM: da850: fix da850_set_pll0rate() Message-ID: <20161205033431.GD29595@vireshk-i7> References: <1480693134-31324-1-git-send-email-bgolaszewski@baylibre.com> <1480693134-31324-4-git-send-email-bgolaszewski@baylibre.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1480693134-31324-4-git-send-email-bgolaszewski@baylibre.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2770 Lines: 80 Good to see you again Bartosz :) On 02-12-16, 16:38, Bartosz Golaszewski wrote: > This function is broken - its second argument is an index to the freq > table, not the requested clock rate in Hz. It leads to an oops when > called from clk_set_rate() since this argument isn't bounds checked > either. > > Fix it by iterating over the array of supported frequencies and > selecting a one that matches or returning -EINVAL for unsupported > rates. > > Also: update the davinci cpufreq driver. It's the only user of this > clock and currently it passes the cpufreq table index to > clk_set_rate(), which is confusing. Make it pass the requested clock > rate in Hz. > > Signed-off-by: Bartosz Golaszewski > --- > arch/arm/mach-davinci/da850.c | 22 ++++++++++++++++++---- > drivers/cpufreq/davinci-cpufreq.c | 2 +- > 2 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c > index a55101c..92e3303 100644 > --- a/arch/arm/mach-davinci/da850.c > +++ b/arch/arm/mach-davinci/da850.c > @@ -1179,14 +1179,28 @@ static int da850_set_armrate(struct clk *clk, unsigned long index) > return clk_set_rate(pllclk, index); > } > > -static int da850_set_pll0rate(struct clk *clk, unsigned long index) > +static int da850_set_pll0rate(struct clk *clk, unsigned long rate) > { > - unsigned int prediv, mult, postdiv; > - struct da850_opp *opp; > struct pll_data *pll = clk->pll_data; > + struct cpufreq_frequency_table *freq; > + unsigned int prediv, mult, postdiv; > + struct da850_opp *opp = NULL; > int ret; > > - opp = (struct da850_opp *) cpufreq_info.freq_table[index].driver_data; > + for (freq = da850_freq_table; > + freq->frequency != CPUFREQ_TABLE_END; freq++) { > + /* requested_rate is in Hz, freq->frequency is in KHz */ > + unsigned long freq_rate = freq->frequency * 1000; > + > + if (freq_rate == rate) { > + opp = (struct da850_opp *)freq->driver_data; > + break; > + } > + } > + > + if (opp == NULL) > + return -EINVAL; > + > prediv = opp->prediv; > mult = opp->mult; > postdiv = opp->postdiv; > diff --git a/drivers/cpufreq/davinci-cpufreq.c b/drivers/cpufreq/davinci-cpufreq.c > index b95a872..d54a27c 100644 > --- a/drivers/cpufreq/davinci-cpufreq.c > +++ b/drivers/cpufreq/davinci-cpufreq.c > @@ -55,7 +55,7 @@ static int davinci_target(struct cpufreq_policy *policy, unsigned int idx) > return ret; > } > > - ret = clk_set_rate(armclk, idx); > + ret = clk_set_rate(armclk, new_freq * 1000); > if (ret) > return ret; I haven't checked the internal of davinci or the way rate is changed now, but from cpufreq's perspective, fee free to add my: Acked-by: Viresh Kumar -- viresh