| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150117015701.GB27202@codeaurora.org>
Date: Fri, 16 Jan 2015 17:57:01 -0800
From: Stephen Boyd <sboyd@...eaurora.org>
To: Tomeu Vizoso <tomeu.vizoso@...labora.com>
Cc: linux-kernel@...r.kernel.org,
Mike Turquette <mturquette@...aro.org>,
Javier Martinez Canillas <javier.martinez@...labora.co.uk>,
Jonathan Corbet <corbet@....net>,
Tony Lindgren <tony@...mide.com>,
Russell King <linux@....linux.org.uk>,
Ralf Baechle <ralf@...ux-mips.org>,
Boris Brezillon <boris.brezillon@...e-electrons.com>,
Emilio López <emilio@...pez.com.ar>,
Maxime Ripard <maxime.ripard@...e-electrons.com>,
Tero Kristo <t-kristo@...com>,
Manuel Lauss <manuel.lauss@...il.com>,
Alex Elder <elder@...aro.org>,
Matt Porter <mporter@...aro.org>,
Tim Kryger <tim.kryger@...aro.org>,
Zhangfei Gao <zhangfei.gao@...aro.org>,
Haojian Zhuang <haojian.zhuang@...aro.org>,
Bintian Wang <bintian.wang@...wei.com>,
Chao Xie <chao.xie@...vell.com>, linux-doc@...r.kernel.org,
linux-omap@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mips@...ux-mips.org
Subject: Re: [PATCH RESEND v8 2/2] clk: Add floor and ceiling constraints to
clock rates
On 01/12, Tomeu Vizoso wrote:
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 7eddfd8..2793bd7 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -1013,8 +1015,8 @@ static unsigned long clk_core_round_rate_nolock(struct clk_core *clk,
>
> if (clk->ops->determine_rate) {
> parent_hw = parent ? parent->hw : NULL;
> - return clk->ops->determine_rate(clk->hw, rate, &parent_rate,
> - &parent_hw);
> + return clk->ops->determine_rate(clk->hw, rate, 0, ULONG_MAX,
> + &parent_rate, &parent_hw);
> } else if (clk->ops->round_rate)
> return clk->ops->round_rate(clk->hw, rate, &parent_rate);
> else if (clk->flags & CLK_SET_RATE_PARENT)
> @@ -1453,8 +1458,20 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *clk,
>
> /* find the closest rate and parent clk/rate */
> if (clk->ops->determine_rate) {
> + hlist_for_each_entry(clk_user, &clk->clks, child_node) {
> + floor_rate = max(floor_rate,
> + clk_user->floor_constraint);
> + }
> +
> + hlist_for_each_entry(clk_user, &clk->clks, child_node) {
> + ceiling_rate = min(ceiling_rate,
> + clk_user->ceiling_constraint);
> + }
I would think we need to do this in the clk_round_rate() path as
well. We can't just pass 0 and ULONG_MAX there or we'll determine
one rate here and another rate in round_rate(), violating the
contract between set_rate() and round_rate().
> +
> parent_hw = parent ? parent->hw : NULL;
> new_rate = clk->ops->determine_rate(clk->hw, rate,
> + floor_rate,
> + ceiling_rate,
> &best_parent_rate,
> &parent_hw);
> parent = parent_hw ? parent_hw->core : NULL;
We should enforce a constraint if the clk is using the
round_rate() op too. If the .round_rate() op returns some rate
within range it should be ok. Otherwise we can fail the rate
change because it's out of range.
We'll also need to introduce some sort of
clk_core_determine_rate(core, rate, min, max) so that clock
providers can ask parent clocks to find a rate within some range
that they can tolerate. If we update __clk_mux_determine_rate()
we can see how that would work out.
> @@ -1660,13 +1657,92 @@ int clk_set_rate(struct clk *clk, unsigned long rate)
[...]
> + */
> +int clk_set_rate(struct clk *clk, unsigned long rate)
> +{
> + return clk_core_set_rate(clk->core, rate);
clk could be NULL.
> +}
> EXPORT_SYMBOL_GPL(clk_set_rate);
>
> +int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max)
> +{
> + int ret = 0;
Check for NULL clk.
> +
> +/**
> + * clk_set_floor_rate - set a minimum clock rate for a clock source
> + * @clk: clock source
> + * @rate: desired minimum clock rate in Hz
> + *
> + * Returns success (0) or negative errno.
> + */
> +int clk_set_floor_rate(struct clk *clk, unsigned long rate)
> +{
> + return clk_set_rate_range(clk, rate, clk->ceiling_constraint);
clk could be NULL.
> +}
> +EXPORT_SYMBOL_GPL(clk_set_floor_rate);
> +
> +/**
> + * clk_set_ceiling_rate - set a maximum clock rate for a clock source
> + * @clk: clock source
> + * @rate: desired maximum clock rate in Hz
> + *
> + * Returns success (0) or negative errno.
> + */
> +int clk_set_ceiling_rate(struct clk *clk, unsigned long rate)
> +{
> + return clk_set_rate_range(clk, clk->floor_constraint, rate);
clk could be NULL.
> +}
> +EXPORT_SYMBOL_GPL(clk_set_ceiling_rate);
> +
> static struct clk_core *clk_core_get_parent(struct clk_core *core)
> {
> struct clk_core *parent;
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index 2e65419..ae5c800 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -175,9 +175,12 @@ struct clk_ops {
> unsigned long parent_rate);
> long (*round_rate)(struct clk_hw *hw, unsigned long rate,
> unsigned long *parent_rate);
> - long (*determine_rate)(struct clk_hw *hw, unsigned long rate,
> - unsigned long *best_parent_rate,
> - struct clk_hw **best_parent_hw);
> + long (*determine_rate)(struct clk_hw *hw,
> + unsigned long rate,
> + unsigned long floor_rate,
> + unsigned long ceiling_rate,
I wonder if we should call this min_rate and max_rate?
> + unsigned long *best_parent_rate,
> + struct clk_hw **best_parent_hw);
> int (*set_parent)(struct clk_hw *hw, u8 index);
> u8 (*get_parent)(struct clk_hw *hw);
> int (*set_rate)(struct clk_hw *hw, unsigned long rate,
> diff --git a/include/linux/clk.h b/include/linux/clk.h
> index c7f258a..f99ae67 100644
> --- a/include/linux/clk.h
> +++ b/include/linux/clk.h
> @@ -302,6 +302,34 @@ long clk_round_rate(struct clk *clk, unsigned long rate);
> int clk_set_rate(struct clk *clk, unsigned long rate);
>
> /**
> + * clk_set_rate_range - set a rate range for a clock source
> + * @clk: clock source
> + * @min: desired minimum clock rate in Hz
> + * @max: desired maximum clock rate in Hz
> + *
> + * Returns success (0) or negative errno.
> + */
> +int clk_set_rate_range(struct clk *clk, unsigned long min, unsigned long max);
> +
> +/**
> + * clk_set_floor_rate - set a minimum clock rate for a clock source
> + * @clk: clock source
> + * @rate: desired minimum clock rate in Hz
> + *
> + * Returns success (0) or negative errno.
> + */
> +int clk_set_floor_rate(struct clk *clk, unsigned long rate);
And this called clk_set_max_rate()?
> +
> +/**
> + * clk_set_ceiling_rate - set a maximum clock rate for a clock source
> + * @clk: clock source
> + * @rate: desired maximum clock rate in Hz
> + *
> + * Returns success (0) or negative errno.
> + */
> +int clk_set_ceiling_rate(struct clk *clk, unsigned long rate);
And this called clk_set_min_rate()?
> +
> +/**
> * clk_set_parent - set the parent clock source for this clock
> * @clk: clock source
> * @parent: parent clock source
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists