On 29.02.2024 06:38, Satya Priya Kakitapalli wrote: > From: Taniya Das <quic_tdas@xxxxxxxxxxx> > > Regera PLL ops are required to control the Regera PLL from clock > controller drivers, thus add support for the same. > > Signed-off-by: Taniya Das <quic_tdas@xxxxxxxxxxx> > Signed-off-by: Satya Priya Kakitapalli <quic_skakitap@xxxxxxxxxxx> > --- [...] > +static int clk_regera_pll_enable(struct clk_hw *hw) This function is 1:1 clk_zonda_pll_enable() logic-wise, except for the `if (val & ZONDA_STAY_IN_CFA)` part. Would it be an issue on Regera? > +static void clk_regera_pll_disable(struct clk_hw *hw) This again is clk_zonda_pll_disable(), except the very last value written to PLL_OPMODE is different. Could you commonize them? > + > +static void zonda_pll_adjust_l_val(unsigned long rate, unsigned long prate, > + u32 *l) (Ugly wrapping, please touch it up) ..should it apply to zonda as the name suggests? Also, any explanations? > + u64 remainder, quotient; > + > + quotient = rate; > + remainder = do_div(quotient, prate); > + *l = quotient; > + > + if ((remainder * 2) / prate) > + *l = *l + 1; > +} > + > +static int clk_regera_pll_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long prate) > +{ > + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); > + unsigned long rrate; > + u32 l, alpha_width = pll_alpha_width(pll); > + u64 a; > + int ret; > + > + rrate = alpha_pll_round_rate(rate, prate, &l, &a, alpha_width); > + > + ret = alpha_pll_check_rate_margin(hw, rrate, rate); > + if (ret < 0) > + return ret; > + > + if (a && (a & BIT(15))) What is BIT(15)? Also, the left part of the condition is totally bogus, if a is 0 then a & BIT(15) will surely be zero as well. Konrad