[linux-sunxi] [PATCH 1/3] clk: sunxi: Add a driver for the CCU

[Jean-Francois Moine]

On Tue, 28 Jun 2016 21:31:06 +0200
Ondřej Jirman <megi at xff.cz> wrote:

> 
> On 28.6.2016 17:37, Jean-Francois Moine wrote:
> > Most of the clocks in the Allwinner's SoCs are configured in the CCU
> > (Clock Configuration Unit).
> > 
> > The PLL clocks are driven from the main clock. Their rates are controlled
> > by a set of multiply and divide factors, named from the Allwinner's
> > documentation:
> > - multipliers: 'n' and 'k'
> > - dividers: 'd1', 'd2', 'm' and 'p'
> > 
> > The peripheral clocks may receive their inputs from one or more parents,
> > thanks to a mux. Their rates are controlled by a set of divide factors
> > only, named 'm' and 'p'.
> > 
> > This driver also handles:
> > - fixed clocks,
> > - the phase delays for the MMCs,
> > - the clock gates,
> > - the bus gates,
> > - and the resets.
> > 
> > Signed-off-by: Jean-Francois Moine <moinejf at free.fr>
> > ---
> >  drivers/clk/sunxi/Makefile |   2 +
> >  drivers/clk/sunxi/ccu.c    | 980 +++++++++++++++++++++++++++++++++++++++++++++
> >  drivers/clk/sunxi/ccu.h    | 153 +++++++
> >  3 files changed, 1135 insertions(+)
> >  create mode 100644 drivers/clk/sunxi/ccu.c
> >  create mode 100644 drivers/clk/sunxi/ccu.h
	[snip]
> > diff --git a/drivers/clk/sunxi/ccu.c b/drivers/clk/sunxi/ccu.c
> > new file mode 100644
> > index 0000000..5749f9c
> > --- /dev/null
> > +++ b/drivers/clk/sunxi/ccu.c
	[snip]
> > +static void ccu_pll_set_flat_factors(struct ccu *ccu, u32 mask, u32 val)
> > +{
> > +	u32 reg, m_val, p_val;
> > +	u32 m_mask = (1 << ccu->m_width) - 1;
> > +	u32 p_mask = (1 << ccu->p_width) - 1;
> 
> I see you have shifted the factor values to be set in the caller of this
> function, but then you have to shift masks too, to match. It might be
> clearer to do both shifts in this function.

Oops, yes. Thanks. (I did not test yet your patch about the CPU
frequency)

> > +	reg = readl(ccu->base + ccu->reg);
> > +	m_val = reg & m_mask;
> > +	p_val = reg & p_mask;
> > +
> > +	spin_lock(&ccu_lock);
> > +
> > +	/* increase p, then m */
> > +	if (ccu->p_width && p_val < (val & p_mask)) {
> > +		reg &= ~p_mask;
> > +		reg |= val & p_mask;
> > +		writel(reg, ccu->base + ccu->reg);
> > +		udelay(10);
> > +	}
> > +	if (ccu->m_width && m_val < (val & m_mask)) {
> > +		reg &= ~m_mask;
> > +		reg |= val & m_mask;
> > +		writel(reg, ccu->base + ccu->reg);
> > +		udelay(10);
> > +	}
> > +
> > +	/* set other factors */
> > +	reg &= ~(mask & ~(p_mask | m_mask));
> > +	reg |= val & ~(p_mask | m_mask);
> > +	writel(reg, ccu->base + ccu->reg);
> > +
> > +	/* decrease m */
> > +	if (ccu->m_width && m_val > (val & m_mask)) {
> > +		reg &= ~m_mask;
> > +		reg |= val & m_mask;
> > +		writel(reg, ccu->base + ccu->reg);
> > +		udelay(10);
> > +	}
> > +
> > +	/* wait for PLL stable */
> > +	if (ccu->lock_reg) {
> > +		u32 lock;
> > +
> > +		lock = BIT(ccu->lock_bit);
> > +		WARN_ON(readl_relaxed_poll_timeout(ccu->base + ccu->lock_reg,
> > +							reg, !(reg & lock),
> > +							100, 70000));
> > +	}
	[snip]
> 
> Any reason why you're waiting for the lock after decreasing m and not
> before? PLL output may till run too fast before reducing the postdivider
> potentially causing a crash. But I might be misunderstanding what m is
> exactly.

I just followed what is done in the function
 sunxi_clk_factors_set_flat_facotrs() from Allwinnertech:

	1).try to increase factor p first
	2).try to increase factor m first
	3. write factor n & k
	4. do pair things for 2). decease factor m
	5. wait for PLL state stable
	6.do pair things for 1).  decease factor p

As this is written, it seems m is a pre-divider.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/