Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756738AbcCCT0W (ORCPT ); Thu, 3 Mar 2016 14:26:22 -0500 Received: from mail-lb0-f193.google.com ([209.85.217.193]:36633 "EHLO mail-lb0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751435AbcCCT0U (ORCPT ); Thu, 3 Mar 2016 14:26:20 -0500 MIME-Version: 1.0 In-Reply-To: <20160303055359.GI2891@vireshk-i7> References: <2495375.dFbdlAZmA6@vostro.rjw.lan> <3567040.O8oI5B24Ng@vostro.rjw.lan> <20160303055359.GI2891@vireshk-i7> Date: Thu, 3 Mar 2016 20:26:18 +0100 X-Google-Sender-Auth: SjNxqMmDykecwzuYqkIkDJfeSEE Message-ID: Subject: Re: [PATCH 3/6] cpufreq: governor: New data type for management part of dbs_data From: "Rafael J. Wysocki" To: Viresh Kumar Cc: "Rafael J. Wysocki" , Linux PM list , Juri Lelli , Steve Muckle , ACPI Devel Maling List , Linux Kernel Mailing List , Peter Zijlstra , Srinivas Pandruvada , Vincent Guittot , Michael Turquette 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: 2073 Lines: 50 On Thu, Mar 3, 2016 at 6:53 AM, Viresh Kumar wrote: > On 02-03-16, 03:08, Rafael J. Wysocki wrote: >> From: Rafael J. Wysocki >> >> In addition to fields representing governor tunables, struct dbs_data >> contains some fields needed for the management of objects of that >> type. As it turns out, that part of struct dbs_data may be shared >> with (future) governors that won't use the common code used by >> "ondemand" and "conservative", so move it to a separate struct type >> and modify the code using struct dbs_data to follow. >> >> Signed-off-by: Rafael J. Wysocki >> --- >> drivers/cpufreq/cpufreq_conservative.c | 15 +++-- >> drivers/cpufreq/cpufreq_governor.c | 90 ++++++++++++++++++++------------- >> drivers/cpufreq/cpufreq_governor.h | 36 +++++++------ >> drivers/cpufreq/cpufreq_ondemand.c | 19 ++++-- >> 4 files changed, 97 insertions(+), 63 deletions(-) >> >> Index: linux-pm/drivers/cpufreq/cpufreq_governor.h >> =================================================================== >> --- linux-pm.orig/drivers/cpufreq/cpufreq_governor.h >> +++ linux-pm/drivers/cpufreq/cpufreq_governor.h >> @@ -41,6 +41,13 @@ >> /* Ondemand Sampling types */ >> enum {OD_NORMAL_SAMPLE, OD_SUB_SAMPLE}; >> >> +struct gov_tunables { >> + struct kobject kobj; >> + struct list_head policy_list; >> + struct mutex update_lock; >> + int usage_count; >> +}; > > Everything else looks fine, but I don't think that you have named it > properly. Every thing else present in struct dbs_data are tunables, > but not this. And so gov_tunables doesn't suit at all here.. So this is a totally bicycle shed discussion argument which makes it seriously irritating. Does it really matter so much how this structure is called? Essentially, it is something to build your tunables structure around and you can treat it as a counterpart of a C++ abstract class. So the name *does* make sense in that context. That said, what about gov_attr_set? Thanks, Rafael