From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752240AbcGVPNl (ORCPT ); Fri, 22 Jul 2016 11:13:41 -0400 Received: from mail-pa0-f53.google.com ([209.85.220.53]:36684 "EHLO mail-pa0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751987AbcGVPNa (ORCPT ); Fri, 22 Jul 2016 11:13:30 -0400 Date: Fri, 22 Jul 2016 08:13:27 -0700 From: Viresh Kumar To: Steve Muckle Cc: "Rafael J. Wysocki" , "Rafael J. Wysocki" , Lists linaro-kernel , Linux PM , Linux Kernel Mailing List , Peter Zijlstra , Ingo Molnar , Vincent Guittot , Morten Rasmussen , Dietmar Eggemann , Juri Lelli , Patrick Bellasi Subject: Re: [PATCH V2] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index() Message-ID: <20160722151327.GP3122@ubuntu> References: <065301260510fbca81f5481b27b0de956073068a.1469137133.git.viresh.kumar@linaro.org> <1490801.bHMYROVPDC@vostro.rjw.lan> <20160721232228.GT27987@graphite.smuckle.net> <20160721234558.GX27987@graphite.smuckle.net> <20160722000903.GY27987@graphite.smuckle.net> <20160722003405.GZ27987@graphite.smuckle.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160722003405.GZ27987@graphite.smuckle.net> 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 On 21-07-16, 17:34, Steve Muckle wrote: > On Fri, Jul 22, 2016 at 02:18:54AM +0200, Rafael J. Wysocki wrote: > > > My thinking was that one of these two would be preferable: > > > > > > - Forcing ->target() drivers to install a ->resolve_freq callback, > > > enforcing this at cpufreq driver init time. > > > > That would have been possible, but your series didn't do that. > > > > > My understanding is > > > ->target() drivers are deprecated anyway > > > > No, they aren't. > > Ok. I didn't follow Documentation/cpu-freq/cpu-drivers.txt section 1.5 > then - it suggests something about target() is deprecated, perhaps it's > out of date. They are kind of deprecated for the new uesrs, but we still have handful of users of it. > Sorry, that should've been "check that either ->target_index() or > ->resolve_freq() is implemented." > > Implementing resolve_freq for the target() drivers and requiring it at > driver init time is probably the better way to go though. Perhaps I can > work on this at some point. As I said earlier as well in one of the emails, if you are worried about the extra 'if' check in the hot path, then wouldn't this fix it for you? diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 3dd4884c6f9e..91d8ec4c8eb7 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -517,7 +517,7 @@ unsigned int cpufreq_driver_resolve_freq(struct cpufreq_policy *policy, return policy->freq_table[idx].frequency; } - if (cpufreq_driver->resolve_freq) + if (likely(cpufreq_driver->resolve_freq)) return cpufreq_driver->resolve_freq(policy, target_freq); return target_freq; -- viresh