> > + nf = fvco * m_table[m];
> > + n = nf / F_27M;
> > + if ((n * F_27M) == nf)
> > + break;
> > + }
> > + m = m_table[m];
> > +
> > + if (!m) {
>
> This can be resolved with a 'return' from a subfunction or a goto.
Sorry, Could you explain more clear?
Did you means like:
If (!m) {
ret = -EINVAL;
goto err_not_found;
}
...
return freq;
err_not_found:
...
return ret;
}
>
> > + pr_err("%s: %s freq:%lu not found a valid setting\n",
> > + __func__, clk_hw_get_name(&clk->hw), freq);
> > + return -EINVAL;
> > + }
> > +
> > + /* save parameters */
> > + clk->p[SEL_FRA] = 0;
> > + clk->p[DIVR] = r;
> > + clk->p[DIVN] = n;
> > + clk->p[DIVM] = m;
> > +
> > + return freq;
> > +}
> > +
> > + df_quotient = df / m;
> > + df_remainder = ((df % m) * 1000) / m;
> > +
> > + if (freq > df_quotient) {
> > + df_quotient = freq - df_quotient - 1;
> > + df_remainder = 1000 - df_remainder;
>
> Where does 1000 come from?
1000 is come from "borrow 1" in last operation.
>
> > + } else {
> > + df_quotient = df_quotient - freq;
> > + }
> > +
> > +static int sp_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> > + unsigned long prate)
> > +{
> > + struct sp_pll *clk = to_sp_pll(hw);
> > + unsigned long flags;
> > + u32 reg;
> > +
> > + spin_lock_irqsave(clk->lock, flags);
>
> Please push the lock down. Maybe that means having two conditionals?
> Either way, it is too large because it calls into plla_set_rate() and
> plltv_set_rate() with the lock held.
Sorry, I don't understand "push the lock down. Maybe that means having two conditionals?" what means.
The plla_set_rate() & plltv_set_rate() is only include simple operations & HW reg writes.
I think it should be called with lock held.
static void plla_set_rate(struct sp_pll *clk)
{
const u32 *pp = pa[clk->p[0]].regs;
int i;
for (i = 0; i < ARRAY_SIZE(pa->regs); i++)
writel(0xffff0000 | pp[i], clk->reg + (i * 4));
}
static void plltv_set_rate(struct sp_pll *clk)
{
u32 reg;
reg = HWM_FIELD_PREP(MASK_SEL_FRA, clk->p[SEL_FRA]);
reg |= HWM_FIELD_PREP(MASK_SDM_MOD, clk->p[SDM_MOD]);
reg |= HWM_FIELD_PREP(MASK_PH_SEL, clk->p[PH_SEL]);
reg |= HWM_FIELD_PREP(MASK_NFRA, clk->p[NFRA]);
writel(reg, clk->reg);
reg = HWM_FIELD_PREP(MASK_DIVR, clk->p[DIVR]);
writel(reg, clk->reg + 4);
reg = HWM_FIELD_PREP(MASK_DIVN, clk->p[DIVN] - 1);
reg |= HWM_FIELD_PREP(MASK_DIVM, clk->p[DIVM] - 1);
writel(reg, clk->reg + 8);
}
