All of lore.kernel.org
 help / color / mirror / Atom feed
From: tomeu.vizoso@collabora.com (Tomeu Vizoso)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v9 5/6] clk: Add floor and ceiling constraints to clock rates
Date: Thu, 04 Sep 2014 10:53:05 +0200	[thread overview]
Message-ID: <54082871.8080808@collabora.com> (raw)
In-Reply-To: <20140904005305.11368.16931@quantum>

On 09/04/2014 02:53 AM, Mike Turquette wrote:
> Quoting Stephen Boyd (2014-09-03 16:39:37)
>> On 09/03/14 08:33, Tomeu Vizoso wrote:
>>> +int clk_set_ceiling_rate(struct clk *clk_user, unsigned long rate)
>>> +{
>>> +     struct clk_core *clk = clk_to_clk_core(clk_user);
>>> +
>>> +     WARN(rate > 0 && rate < clk_user->floor_constraint,
>>> +          "clk %s dev %s con %s: new ceiling %lu lower than existing floor %lu\n",
>>> +          __clk_get_name(clk), clk_user->dev_id, clk_user->con_id, rate,
>>> +          clk_user->floor_constraint);
>>> +
>>> +     clk_user->ceiling_constraint = rate;
>>> +     return clk_provider_set_rate(clk, clk_provider_get_rate(clk));
>>> +}
>>> +EXPORT_SYMBOL_GPL(clk_set_ceiling_rate);
>>
>> Maybe I'm late to this patch series given that Mike applied it, but I
>> wonder why we wouldn't just have one API that takes a min and a max,
>> i.e. clk_set_rate_range(clk, min, max)? Then clk_set_rate() is a small
>> wrapper on top that just sets min and max to the same value.
> 
> We certainly can have that. But being able to easily adjust a floor or
> ceiling value seems like a good thing to me, and that is what these
> functions do.
> 
> If we decide to have a clk_set_rate_range (where we perhaps pass zero in
> for a value that we do not wish to constrain) then I imagine that
> clk_set_ceiling_rate and clk_set_floor_rate will simply become a wrapper
> for that function. No harm having it both ways. If one way of doing
> things falls out of favor we can always cull it and update all the
> users.

I opted for separate functions because in the specific use cases I
thought of, any user will be interested in setting either a floor or a
ceiling constraint, but not both.

Regards,

Tomeu

WARNING: multiple messages have this Message-ID (diff)
From: Tomeu Vizoso <tomeu.vizoso@collabora.com>
To: Mike Turquette <mturquette@linaro.org>,
	Stephen Boyd <sboyd@codeaurora.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>,
	Peter De Schrijver <pdeschrijver@nvidia.com>,
	linux-kernel@vger.kernel.org, tomasz.figa@gmail.com,
	rabin@rab.in, Thierry Reding <thierry.reding@gmail.com>,
	Javier Martinez Canillas <javier.martinez@collabora.co.uk>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v9 5/6] clk: Add floor and ceiling constraints to clock rates
Date: Thu, 04 Sep 2014 10:53:05 +0200	[thread overview]
Message-ID: <54082871.8080808@collabora.com> (raw)
In-Reply-To: <20140904005305.11368.16931@quantum>

On 09/04/2014 02:53 AM, Mike Turquette wrote:
> Quoting Stephen Boyd (2014-09-03 16:39:37)
>> On 09/03/14 08:33, Tomeu Vizoso wrote:
>>> +int clk_set_ceiling_rate(struct clk *clk_user, unsigned long rate)
>>> +{
>>> +     struct clk_core *clk = clk_to_clk_core(clk_user);
>>> +
>>> +     WARN(rate > 0 && rate < clk_user->floor_constraint,
>>> +          "clk %s dev %s con %s: new ceiling %lu lower than existing floor %lu\n",
>>> +          __clk_get_name(clk), clk_user->dev_id, clk_user->con_id, rate,
>>> +          clk_user->floor_constraint);
>>> +
>>> +     clk_user->ceiling_constraint = rate;
>>> +     return clk_provider_set_rate(clk, clk_provider_get_rate(clk));
>>> +}
>>> +EXPORT_SYMBOL_GPL(clk_set_ceiling_rate);
>>
>> Maybe I'm late to this patch series given that Mike applied it, but I
>> wonder why we wouldn't just have one API that takes a min and a max,
>> i.e. clk_set_rate_range(clk, min, max)? Then clk_set_rate() is a small
>> wrapper on top that just sets min and max to the same value.
> 
> We certainly can have that. But being able to easily adjust a floor or
> ceiling value seems like a good thing to me, and that is what these
> functions do.
> 
> If we decide to have a clk_set_rate_range (where we perhaps pass zero in
> for a value that we do not wish to constrain) then I imagine that
> clk_set_ceiling_rate and clk_set_floor_rate will simply become a wrapper
> for that function. No harm having it both ways. If one way of doing
> things falls out of favor we can always cull it and update all the
> users.

I opted for separate functions because in the specific use cases I
thought of, any user will be interested in setting either a floor or a
ceiling constraint, but not both.

Regards,

Tomeu

  reply	other threads:[~2014-09-04  8:53 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-03 15:29 [PATCH v9 0/6] Per-user clock constraints Tomeu Vizoso
2014-09-03 15:29 ` Tomeu Vizoso
2014-09-03 15:29 ` [PATCH v9 1/6] clk: Add temporary mapping to the existing API Tomeu Vizoso
2014-09-03 15:29   ` Tomeu Vizoso
2014-09-03 15:31   ` [PATCH v9 2/6] clk: Move all drivers to use internal API Tomeu Vizoso
2014-09-03 15:31     ` Tomeu Vizoso
2014-09-08  6:13     ` Mike Turquette
2014-09-08  6:13       ` Mike Turquette
2014-09-09  6:15       ` Mike Turquette
2014-09-03 15:33 ` [PATCH v9 3/6] clk: use struct clk only for external API Tomeu Vizoso
2014-09-03 15:33   ` Tomeu Vizoso
2014-09-03 15:33   ` [PATCH v9 4/6] clk: per-user clock accounting for debug Tomeu Vizoso
2014-09-03 15:33     ` Tomeu Vizoso
2014-09-03 15:33   ` [PATCH v9 5/6] clk: Add floor and ceiling constraints to clock rates Tomeu Vizoso
2014-09-03 15:33     ` Tomeu Vizoso
2014-09-03 23:39     ` Stephen Boyd
2014-09-03 23:39       ` Stephen Boyd
2014-09-04  0:53       ` Mike Turquette
2014-09-04  0:53         ` Mike Turquette
2014-09-04  8:53         ` Tomeu Vizoso [this message]
2014-09-04  8:53           ` Tomeu Vizoso
2014-09-04 13:34       ` Tomeu Vizoso
2014-09-04 13:34         ` Tomeu Vizoso
2014-09-03 15:33   ` [PATCH v9 6/6] clk: Warn of unbalanced clk_prepare() calls Tomeu Vizoso
2014-09-03 15:33     ` Tomeu Vizoso
2014-09-03 17:26 ` [PATCH v9 0/6] Per-user clock constraints Mike Turquette
2014-09-04  8:30   ` Tomeu Vizoso
2014-09-04  8:30     ` Tomeu Vizoso

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=54082871.8080808@collabora.com \
    --to=tomeu.vizoso@collabora.com \
    --cc=linux-arm-kernel@lists.infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.