From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750798AbcBECaJ (ORCPT ); Thu, 4 Feb 2016 21:30:09 -0500 Received: from v094114.home.net.pl ([79.96.170.134]:52037 "HELO v094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750723AbcBECaH (ORCPT ); Thu, 4 Feb 2016 21:30:07 -0500 From: "Rafael J. Wysocki" To: Viresh Kumar Cc: juri.lelli@arm.com, linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, skannan@codeaurora.org, peterz@infradead.org, mturquette@baylibre.com, steve.muckle@linaro.org, vincent.guittot@linaro.org, morten.rasmussen@arm.com, dietmar.eggemann@arm.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH V2 1/7] cpufreq: governor: Treat min_sampling_rate as a governor-specific tunable Date: Fri, 05 Feb 2016 03:31:12 +0100 Message-ID: <5894332.OlEanUAMuU@vostro.rjw.lan> User-Agent: KMail/4.11.5 (Linux/4.5.0-rc1+; KDE/4.11.5; x86_64; ; ) In-Reply-To: <92be8bede3340aa93549b468528f60244fbc50f2.1454507872.git.viresh.kumar@linaro.org> References: <92be8bede3340aa93549b468528f60244fbc50f2.1454507872.git.viresh.kumar@linaro.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday, February 03, 2016 07:32:17 PM Viresh Kumar wrote: > The min_sampling_rate governor tunable is a field in struct dbs_data, so > it has to be handled in a special way separate from the rest of governor > tunables. In particular, that requires a special macro to be present > for creating its show/store sysfs attribute callbacks. > > However, there is no real need for the data structures and code in > question to be arranged this way and if min_sampling_rate is moved to > data structures holding the other governor tunables, the sysfs attribute > creation macros that work with those tunables will also work with > min_sampling_rate and the special macro for it won't be necessary any > more. That will make it easier to modify the governor code going > forward, so do it. > > [ Rafael: Written changelog ] > Signed-off-by: Viresh Kumar I'm having some second thoughts about the utility of this patch to be honest. I actually would like to move some tunables in the opposite direction. That is, from struct od_dbs_tuners and struct cs_dbs_tuners to struct dbs_data. The tuners field in that will then become something like gov_tunables (in analogy with gov_ops in struct common_dbs_data) and it will point to governor-specific tunables. The reason why I'd like to do that is to make it easier to get rid of the super-ugly governor == GOV_CONSERVATIVE etc tests in the common code. Also I think that governor-specific tunables should be defined in the .c file for that governor rather than in the common header. We will need two set of macros for their sysfs attributes then, but that's not a big deal IMO. Thanks, Rafael