From: Tomeu Vizoso <tomeu.vizoso@collabora•com>
To: Stephen Warren <swarren@wwwdotorg•org>,
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,
"Viresh Kumar" <viresh.linux@gmail•com>,
"David Brown" <davidb@codeaurora•org>,
"Anatolij Gustschin" <agust@denx•de>,
"Dinh Nguyen" <dinguyen@altera•com>,
"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail•com>,
"Jason Cooper" <jason@lakedaemon•net>,
"Arnd Bergmann" <arnd@arndb•de>,
linux-arm-msm@vger•kernel.org, spear-devel@list•st.com,
"Barry Song" <baohua@kernel•org>,
"Mark Brown" <broonie@kernel•org>,
linux-rpi-kernel@lists•infradead.org,
"Ben Dooks" <ben-linux@fluff•org>,
linux-tegra@vger•kernel.org, linux-omap@vger•kernel.org,
"Sascha Hauer" <kernel@pengutronix•de>,
"Shawn Guo" <shawn.guo@freescale•com>,
"Paul Walmsley" <paul@pwsan•com>,
"Peter De Schrijver" <pdeschrijver@nvidia•com>,
"Liam Girdwood" <lgirdwood@gmail•com>,
linux-kernel@vger•kernel.org, rabin@rab•in,
"Bryan Huntsman" <bryanh@codeaurora•org>,
"Santosh Shilimkar" <santosh.shilimkar@ti•com>,
"Benoît Cousson" <bcousson@baylibre•com>,
"Maxime Ripard" <maxime.ripard@free-electrons•com>,
linux-media@vger•kernel.org, linuxppc-dev@lists•ozlabs.org,
"Javier Martinez Canillas" <javier.martinez@collabora•co.uk>,
"Mauro Carvalho Chehab" <m.chehab@samsung•com>
Subject: Re: [PATCH v4 6/6] clk: Add floor and ceiling constraints to clock rates
Date: Thu, 31 Jul 2014 13:47:23 +0200 [thread overview]
Message-ID: <53DA2CCB.2080300@collabora.com> (raw)
In-Reply-To: <53CEA483.6090704@wwwdotorg.org>
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.
Regards,
Tomeu
next prev parent reply other threads:[~2014-07-31 11:58 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 [this message]
2014-07-31 17:51 ` Stephen Warren
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=53DA2CCB.2080300@collabora.com \
--to=tomeu.vizoso@collabora$(echo .)com \
--cc=agust@denx$(echo .)de \
--cc=alsa-devel@alsa-project$(echo .)org \
--cc=andrew@lunn$(echo .)ch \
--cc=arnd@arndb$(echo .)de \
--cc=baohua@kernel$(echo .)org \
--cc=bcousson@baylibre$(echo .)com \
--cc=ben-linux@fluff$(echo .)org \
--cc=boris.brezillon@free-electrons$(echo .)com \
--cc=broonie@kernel$(echo .)org \
--cc=bryanh@codeaurora$(echo .)org \
--cc=davidb@codeaurora$(echo .)org \
--cc=dinguyen@altera$(echo .)com \
--cc=dwalker@fifo99$(echo .)com \
--cc=emilio@elopez$(echo .)com.ar \
--cc=jason@lakedaemon$(echo .)net \
--cc=javier.martinez@collabora$(echo .)co.uk \
--cc=kernel@pengutronix$(echo .)de \
--cc=kgene.kim@samsung$(echo .)com \
--cc=khilman@deeprootsystems$(echo .)com \
--cc=kyungmin.park@samsung$(echo .)com \
--cc=lgirdwood@gmail$(echo .)com \
--cc=linux-arch@vger$(echo .)kernel.org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-arm-msm@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-media@vger$(echo .)kernel.org \
--cc=linux-omap@vger$(echo .)kernel.org \
--cc=linux-rpi-kernel@lists$(echo .)infradead.org \
--cc=linux-samsung-soc@vger$(echo .)kernel.org \
--cc=linux-tegra@vger$(echo .)kernel.org \
--cc=linux@arm$(echo .)linux.org.uk \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=m.chehab@samsung$(echo .)com \
--cc=maxime.ripard@free-electrons$(echo .)com \
--cc=michal.simek@xilinx$(echo .)com \
--cc=mturquette@linaro$(echo .)org \
--cc=patches@opensource$(echo .)wolfsonmicro.com \
--cc=paul@pwsan$(echo .)com \
--cc=paulus@samba$(echo .)org \
--cc=pdeschrijver@nvidia$(echo .)com \
--cc=perex@perex$(echo .)cz \
--cc=pgaikwad@nvidia$(echo .)com \
--cc=rabin@rab$(echo .)in \
--cc=s.nawrocki@samsung$(echo .)com \
--cc=santosh.shilimkar@ti$(echo .)com \
--cc=sebastian.hesselbarth@gmail$(echo .)com \
--cc=shawn.guo@freescale$(echo .)com \
--cc=spear-devel@list$(echo .)st.com \
--cc=swarren@wwwdotorg$(echo .)org \
--cc=t.figa@samsung$(echo .)com \
--cc=thierry.reding@gmail$(echo .)com \
--cc=tiwai@suse$(echo .)de \
--cc=tony@atomide$(echo .)com \
--cc=ulf.hansson@linaro$(echo .)org \
--cc=viresh.linux@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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