Hi Boris, Am Freitag, 17. April 2015, 09:29:28 schrieb Boris Brezillon: > Clock rates are stored in an unsigned long field, but ->round_rate() > (which returns a rounded rate from a requested one) returns a long > value (errors are reported using negative error codes), which can lead > to long overflow if the clock rate exceed 2Ghz. > > Change ->round_rate() prototype to return 0 or an error code, and pass the > requested rate as a pointer so that it can be adjusted depending on > hardware capabilities. > > Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxxxxxxx> > --- On a rk3288-veyron-pinky with the fix described below: Tested-by: Heiko Stuebner <heiko@xxxxxxxxx> > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index fa5a00e..1462ddc 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1640,8 +1643,10 @@ static struct clk_core *clk_calc_new_rates(struct > clk_core *clk, &parent_hw); > parent = parent_hw ? parent_hw->core : NULL; > } else if (clk->ops->round_rate) { > - new_rate = clk->ops->round_rate(clk->hw, rate, > - &best_parent_rate); > + if (clk->ops->round_rate(clk->hw, &new_rate, > + &best_parent_rate)) > + return NULL; > + > if (new_rate < min_rate || new_rate > max_rate) > return NULL; > } else if (!parent || !(clk->flags & CLK_SET_RATE_PARENT)) { This is using new_rate uninitialized when calling into the round_rate callback. Which in turn pushed my PLLs up to 2.2GHz :-) I guess you'll need something like the following: diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index db4e4b2..afc7733 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1605,6 +1605,7 @@ static struct clk_core *clk_calc_new_rates(struct clk_core *clk, &parent_hw); parent = parent_hw ? parent_hw->core : NULL; } else if (clk->ops->round_rate) { + new_rate = rate; if (clk->ops->round_rate(clk->hw, &new_rate, &best_parent_rate)) return NULL; > diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c > index f8d3baf..bd408ef 100644 > --- a/drivers/clk/rockchip/clk-pll.c > +++ b/drivers/clk/rockchip/clk-pll.c > @@ -63,8 +63,8 @@ static const struct rockchip_pll_rate_table > *rockchip_get_pll_settings( return NULL; > } > > -static long rockchip_pll_round_rate(struct clk_hw *hw, > - unsigned long drate, unsigned long *prate) > +static int rockchip_pll_round_rate(struct clk_hw *hw, > + unsigned long *drate, unsigned long *prate) > { > struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > const struct rockchip_pll_rate_table *rate_table = pll->rate_table; > @@ -72,12 +72,15 @@ static long rockchip_pll_round_rate(struct clk_hw *hw, > > /* Assumming rate_table is in descending order */ > for (i = 0; i < pll->rate_count; i++) { > - if (drate >= rate_table[i].rate) > - return rate_table[i].rate; > + if (*drate >= rate_table[i].rate) { > + *drate = rate_table[i].rate; > + return 0; > + } > } > > /* return minimum supported value */ > - return rate_table[i - 1].rate; > + *drate = rate_table[i - 1].rate; > + return 0; > } > > /* The rockchip-part: Reviewed-by: Heiko Stuebner <heiko@xxxxxxxxx> And as I've stumbled onto this recently too, the clock-maintainership has expanded to Stephen Boyd and linux-clk@xxxxxxxxxxxxxxx . Heiko -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html