Re: [PATCH RESEND v8 2/2] clk: Add floor and ceiling constraints to clock rates
From: Stephen Boyd
Date: Fri Jan 16 2015 - 20:57:10 EST
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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/