linux-arch.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: Tomeu Vizoso <tomeu.vizoso@collabora.com>,
	Mike Turquette <mturquette@linaro.org>
Cc: "Andrew Lunn" <andrew@lunn.ch>,
	"Ulf Hansson" <ulf.hansson@linaro.org>,
	"Prashant Gaikwad" <pgaikwad@nvidia.com>,
	"Tony Lindgren" <tony@atomide.com>,
	"Tomasz Figa" <t.figa@samsung.com>,
	alsa-devel@alsa-project.org, "Jaroslav Kysela" <perex@perex.cz>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Paul Mackerras" <paulus@samba.org>,
	"Sylwester Nawrocki" <s.nawrocki@samsung.com>,
	"Daniel Walker" <dwalker@fifo99.com>,
	linux-arch@vger.kernel.org,
	"Boris Brezillon" <boris.brezillon@free-electrons.com>,
	linux-samsung-soc@vger.kernel.org,
	"Kukjin Kim" <kgene.kim@samsung.com>,
	"Russell King" <linux@arm.linux.org.uk>,
	"Emilio López" <emilio@elopez.com.ar>,
	"Takashi Iwai" <tiwai@suse.de>,
	"Michal Simek" <michal.simek@xilinx.com>,
	"Kyungmin Park" <kyungmin.park@samsung.com>,
	"Kevin Hilman" <khilman@deeprootsystems.com>,
	linux-arm-kernel@lists.infradead.org,
	patches@opensource.wolfsonmicro.com
Subject: Re: [PATCH v4 6/6] clk: Add floor and ceiling constraints to clock rates
Date: Thu, 31 Jul 2014 11:51:20 -0600	[thread overview]
Message-ID: <53DA8218.7070300@wwwdotorg.org> (raw)
In-Reply-To: <53DA2CCB.2080300@collabora.com>

On 07/31/2014 05:47 AM, Tomeu Vizoso wrote:
> On 07/22/2014 07:50 PM, Stephen Warren wrote:
>> On 07/17/2014 08:13 AM, Tomeu Vizoso wrote:
>>> Adds a way for clock consumers to set maximum and minimum rates. This
>>> can be
>>> used for thermal drivers to set ceiling rates, or by misc. drivers to
>>> set
>>> floor rates to assure a minimum performance level.
>>
>>> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
>>
>>> +struct rate_constraint {
>>> +    struct hlist_node    node;
>>> +    const char        *dev_id;
>>> +    const char        *con_id;
>>> +    enum constraint_type    type;
>>> +    unsigned long        rate;
>>> +};
>>
>> I would personally still prefer either:
>>
>> a) The struct rate_constraints be stored in struct clk rather than
>> struct clk_core, so they're stored in the container that "set" the
>> constraints. This would mean iterating over a struct clk_core's
>> associated struct clks, then iterating over the struct rate_constraints
>> within each struct clk.
>>
>> or:
>>
>> b) struct rate_constraint to contain a pointer to the struct clk that
>> created the constraint, rather than copying the dev_id/con_id from that
>> struct clk into the struct rate_constraint.
>>
>> With either of those changes, the constraints are directly associated
>> with the clock client object that created the constraint much better
>> than right now, where the matching is only because the struct clk and
>> struct rate_constraint "happen to" have the same dev_id/con_id values.
>>
>> Still, this is a pretty minor issue, and overall this series looks
>> reasonable to me at a quick look.
>
> Yeah, I agree and personally prefer a), but given the little feedback
> that I have gotten so far on the refactoring, I'm starting to consider
> forgetting about the per-user clk struct and go instead with a
> request-based API similar to that of pm_qos, for setting floor and
> ceiling frequencies.

I would obviously encourage you to continue pushing for this feature, 
although I understand it can be difficult to be motivated when the 
patches don't get much feedback.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

  reply	other threads:[~2014-07-31 17:51 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-17 14:13 [PATCH v4 0/6] Per-user clock constraints Tomeu Vizoso
2014-07-17 14:13 ` [PATCH v4 1/6] clk: Add temporary mapping to the existing API Tomeu Vizoso
2014-07-17 14:13 ` [PATCH v4 2/6] ASoC: mxs-saif: fix mixed use of public and provider clk API Tomeu Vizoso
2014-07-17 15:06   ` Mark Brown
2014-07-17 14:13 ` [PATCH v4 3/6] clk: Move all drivers to use internal API Tomeu Vizoso
2014-07-17 14:13 ` [PATCH v4 4/6] clk: use struct clk only for external API Tomeu Vizoso
2014-07-17 14:13 ` [PATCH v4 5/6] clk: per-user clock accounting for debug Tomeu Vizoso
2014-07-17 14:13 ` [PATCH v4 6/6] clk: Add floor and ceiling constraints to clock rates Tomeu Vizoso
2014-07-22 17:50   ` Stephen Warren
2014-07-31 11:47     ` Tomeu Vizoso
2014-07-31 17:51       ` Stephen Warren [this message]
2014-07-31 18:54         ` Mike Turquette

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53DA8218.7070300@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=andrew@lunn.ch \
    --cc=boris.brezillon@free-electrons.com \
    --cc=dwalker@fifo99.com \
    --cc=emilio@elopez.com.ar \
    --cc=kgene.kim@samsung.com \
    --cc=khilman@deeprootsystems.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=michal.simek@xilinx.com \
    --cc=mturquette@linaro.org \
    --cc=patches@opensource.wolfsonmicro.com \
    --cc=paulus@samba.org \
    --cc=perex@perex.cz \
    --cc=pgaikwad@nvidia.com \
    --cc=s.nawrocki@samsung.com \
    --cc=t.figa@samsung.com \
    --cc=thierry.reding@gmail.com \
    --cc=tiwai@suse.de \
    --cc=tomeu.vizoso@collabora.com \
    --cc=tony@atomide.com \
    --cc=ulf.hansson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).