Messages in this thread | | | Date | Thu, 7 Nov 2019 18:57:51 +0100 | From | Michael Tretter <> | Subject | Re: [PATCH] clk: zynqmp: Add support for clock with CLK_DIVIDER_POWER_OF_TWO flag |
| |
On Thu, 07 Nov 2019 00:55:02 -0800, Rajan Vaja wrote: > From: Tejas Patel <tejas.patel@xilinx.com> > > Existing clock divider functions is not checking for > base of divider. So, if any clock divider is power of 2 > then clock rate calculation will be wrong. > > Add support to calculate divider value for the clocks > with CLK_DIVIDER_POWER_OF_TWO flag. > > Signed-off-by: Tejas Patel <tejas.patel@xilinx.com> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com> > Signed-off-by: Rajan Vaja <rajan.vaja@xilinx.com> > --- > drivers/clk/zynqmp/divider.c | 36 +++++++++++++++++++++++++++++++----- > 1 file changed, 31 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/zynqmp/divider.c b/drivers/clk/zynqmp/divider.c > index d8f5b70d..ce63cf5 100644 > --- a/drivers/clk/zynqmp/divider.c > +++ b/drivers/clk/zynqmp/divider.c > @@ -2,7 +2,7 @@ > /* > * Zynq UltraScale+ MPSoC Divider support > * > - * Copyright (C) 2016-2018 Xilinx > + * Copyright (C) 2016-2019 Xilinx > * > * Adjustable divider clock implementation > */ > @@ -44,9 +44,26 @@ struct zynqmp_clk_divider { > }; > > static inline int zynqmp_divider_get_val(unsigned long parent_rate, > - unsigned long rate) > + unsigned long rate, u16 flags) > { > - return DIV_ROUND_CLOSEST(parent_rate, rate); > + int up, down; > + unsigned long up_rate, down_rate; > + > + if (flags & CLK_DIVIDER_POWER_OF_TWO) { > + up = DIV_ROUND_UP_ULL((u64)parent_rate, rate); > + down = parent_rate / rate;
Maybe use DIV_ROUND_DOWN_ULL()?
> + > + up = __roundup_pow_of_two(up); > + down = __rounddown_pow_of_two(down); > + > + up_rate = DIV_ROUND_UP_ULL((u64)parent_rate, up); > + down_rate = DIV_ROUND_UP_ULL((u64)parent_rate, down); > + > + return (rate - up_rate) <= (down_rate - rate) ? up : down;
The calculation looks correct. Maybe there could be a common helper with the _div_round_closest() function?
> + > + } else { > + return DIV_ROUND_CLOSEST(parent_rate, rate); > + } > } > > /** > @@ -78,6 +95,9 @@ static unsigned long zynqmp_clk_divider_recalc_rate(struct clk_hw *hw, > else > value = div >> 16; > > + if (divider->flags & CLK_DIVIDER_POWER_OF_TWO) > + value = 1 << value;
Not sure, but I think a small helper similar to _get_div() would help with the readability. Just hide the difference between the normal and power of two divisors behind some helper functions.
Michael
> + > if (!value) { > WARN(!(divider->flags & CLK_DIVIDER_ALLOW_ZERO), > "%s: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set\n", > @@ -120,10 +140,13 @@ static long zynqmp_clk_divider_round_rate(struct clk_hw *hw, > else > bestdiv = bestdiv >> 16; > > + if (divider->flags & CLK_DIVIDER_POWER_OF_TWO) > + bestdiv = 1 << bestdiv; > + > return DIV_ROUND_UP_ULL((u64)*prate, bestdiv); > } > > - bestdiv = zynqmp_divider_get_val(*prate, rate); > + bestdiv = zynqmp_divider_get_val(*prate, rate, divider->flags); > > if ((clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT) && divider->is_frac) > bestdiv = rate % *prate ? 1 : bestdiv; > @@ -151,7 +174,7 @@ static int zynqmp_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate, > int ret; > const struct zynqmp_eemi_ops *eemi_ops = zynqmp_pm_get_eemi_ops(); > > - value = zynqmp_divider_get_val(parent_rate, rate); > + value = zynqmp_divider_get_val(parent_rate, rate, divider->flags); > if (div_type == TYPE_DIV1) { > div = value & 0xFFFF; > div |= 0xffff << 16; > @@ -160,6 +183,9 @@ static int zynqmp_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate, > div |= value << 16; > } > > + if (divider->flags & CLK_DIVIDER_POWER_OF_TWO) > + div = __ffs(div); > + > ret = eemi_ops->clock_setdivider(clk_id, div); > > if (ret)
| |