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
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ