Messages in this thread | | | From | qinjian[覃健] <> | Subject | RE: [PATCH v15 05/10] clk: Add Sunplus SP7021 clock driver | Date | Tue, 17 May 2022 09:30:24 +0000 |
| |
> > + 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); }
| |