On Tue, 28 Jun 2016 21:31:06 +0200
Ond?ej Jirman [off-list ref] wrote:

On 28.6.2016 17:37, Jean-Francois Moine wrote:

quoted

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 <redacted>
---
 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]

quoted

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]

quoted

+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)

quoted

+	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/