Re: [PATCH V2 1/7] cpufreq: governor: Treat min_sampling_rate as a governor-specific tunable

From: Viresh Kumar <viresh.kumar@linaro.org>
To: "Rafael J. Wysocki" <rjw@rjwysocki.net>
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, 5 Feb 2016 08:17:13 +0530	[thread overview]
Message-ID: <20160205024713.GC3068@vireshk> (raw)
In-Reply-To: <5894332.OlEanUAMuU@vostro.rjw.lan>

On 05-02-16, 03:31, Rafael J. Wysocki wrote:
> 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.

I agree with that, no issues from my side.

But, this patch was actually required to kill the ugly macros. So, if
we are planning to take this series as is, then maybe we can keep it
for now and fix everything together with your patches :)

-- 
viresh