Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752522AbcLEJS6 (ORCPT ); Mon, 5 Dec 2016 04:18:58 -0500 Received: from mail-ua0-f177.google.com ([209.85.217.177]:36711 "EHLO mail-ua0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751244AbcLEJSB (ORCPT ); Mon, 5 Dec 2016 04:18:01 -0500 MIME-Version: 1.0 In-Reply-To: <130825f1-5c8f-f6bc-1f27-929ff591a9c3@ti.com> References: <1480693134-31324-1-git-send-email-bgolaszewski@baylibre.com> <1480693134-31324-4-git-send-email-bgolaszewski@baylibre.com> <130825f1-5c8f-f6bc-1f27-929ff591a9c3@ti.com> From: Bartosz Golaszewski Date: Mon, 5 Dec 2016 10:07:46 +0100 Message-ID: Subject: Re: [PATCH v2 3/3] ARM: da850: fix da850_set_pll0rate() To: Sekhar Nori Cc: Kevin Hilman , Michael Turquette , Peter Ujfalusi , Russell King , Viresh Kumar , Boris Brezillon , "Rafael J. Wysocki" , Richard Weinberger , David Woodhouse , Brian Norris , Marek Vasut , Cyrille Pitchen , LKML , arm-soc , linux-pm Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2473 Lines: 60 2016-12-05 9:45 GMT+01:00 Sekhar Nori : > On Friday 02 December 2016 09:08 PM, 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; > > A small optimization here. Instead of multiplying potentially every > frequency in the table by 1000, you could divide the incoming rate down > to KHz. This will also avoid the need for 'freq_rate'. Should have > noticed this earlier. Sorry about that. > > Thanks, > Sekhar I thought about it, but figured the multiplication would be safer. I will change it if you prefer this version. Best regards, Bartosz Golaszewski