[PATCH v2 0/3] clk: meson: Add clock controller

[PATCH v2 0/3] clk: meson: Add clock controller

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

This patchset introduces preliminary support for the clock controller
found on Amlogic MesonX SoCs and enables it for the Meson8b.

DTS patches with support for Meson8b SoCs will follow.

Changelog:

v1->v2:
   * fix double CLK_IGNORE_UNUSED in clk-pll.c
   * fix comment in meson8b-clkc.h

Carlo Caione (3):
  clk: meson: Add support for Meson clock controller
  clk: meson: Document bindings for Meson8b clock controller
  clk: meson8b: Add support for Meson8b clocks

 .../bindings/clock/amlogic,meson8b-clkc.txt        |  40 +++
 drivers/clk/Makefile                               |   1 +
 drivers/clk/meson/Makefile                         |   6 +
 drivers/clk/meson/clk-cpu.c                        | 291 +++++++++++++++++++++
 drivers/clk/meson/clk-pll.c                        | 231 ++++++++++++++++
 drivers/clk/meson/clkc.c                           | 234 +++++++++++++++++
 drivers/clk/meson/clkc.h                           | 187 +++++++++++++
 drivers/clk/meson/meson8b-clkc.c                   | 182 +++++++++++++
 include/dt-bindings/clock/meson8b-clkc.h           |  20 ++
 9 files changed, 1192 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
 create mode 100644 drivers/clk/meson/Makefile
 create mode 100644 drivers/clk/meson/clk-cpu.c
 create mode 100644 drivers/clk/meson/clk-pll.c
 create mode 100644 drivers/clk/meson/clkc.c
 create mode 100644 drivers/clk/meson/clkc.h
 create mode 100644 drivers/clk/meson/meson8b-clkc.c
 create mode 100644 include/dt-bindings/clock/meson8b-clkc.h

-- 
1.9.1

[PATCH v2 1/3] clk: meson: Add support for Meson clock controller

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

This patchset adds the infrastructure for registering and managing the
core clocks found on Amlogic MesonX SoCs. In particular:

- PLLs
- CPU clock
- Fixed rate clocks, fixed factor clocks, ...

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 drivers/clk/Makefile                     |   1 +
 drivers/clk/meson/Makefile               |   5 +
 drivers/clk/meson/clk-cpu.c              | 291 +++++++++++++++++++++++++++++++
 drivers/clk/meson/clk-pll.c              | 231 ++++++++++++++++++++++++
 drivers/clk/meson/clkc.c                 | 234 +++++++++++++++++++++++++
 drivers/clk/meson/clkc.h                 | 187 ++++++++++++++++++++
 include/dt-bindings/clock/meson8b-clkc.h |  20 +++
 7 files changed, 969 insertions(+)
 create mode 100644 drivers/clk/meson/Makefile
 create mode 100644 drivers/clk/meson/clk-cpu.c
 create mode 100644 drivers/clk/meson/clk-pll.c
 create mode 100644 drivers/clk/meson/clkc.c
 create mode 100644 drivers/clk/meson/clkc.h
 create mode 100644 include/dt-bindings/clock/meson8b-clkc.h

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 3d00c25..9c4ddb1 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -55,6 +55,7 @@ ifeq ($(CONFIG_COMMON_CLK), y)
 obj-$(CONFIG_ARCH_MMP)			+= mmp/
 endif
 obj-$(CONFIG_PLAT_ORION)		+= mvebu/
+obj-$(CONFIG_ARCH_MESON)		+= meson/
 obj-$(CONFIG_ARCH_MXS)			+= mxs/
 obj-$(CONFIG_MACH_PISTACHIO)		+= pistachio/
 obj-$(CONFIG_COMMON_CLK_PXA)		+= pxa/
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
new file mode 100644
index 0000000..66c6d8d
--- /dev/null
+++ b/drivers/clk/meson/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for Meson specific clk
+#
+
+obj-y += clkc.o clk-pll.o clk-cpu.o
diff --git a/drivers/clk/meson/clk-cpu.c b/drivers/clk/meson/clk-cpu.c
new file mode 100644
index 0000000..88f4606
--- /dev/null
+++ b/drivers/clk/meson/clk-cpu.c
@@ -0,0 +1,291 @@
+/*
+ * Copyright (c) 2015 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo@endlessm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * CPU clock path:
+ *
+ *                           +-[/N]-----|3|
+ *             MUX2  +--[/2]-+----------|2| MUX1
+ * [sys_pll]---|1|   |--[/3]------------|1|-|1|
+ *             | |---+------------------|0| | |----- [a5_clk]
+ *          +--|0|                          | |
+ * [xtal]---+-------------------------------|0|
+ *
+ *
+ *
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#define MESON_CPU_CLK_CNTL1		0x00
+#define MESON_CPU_CLK_CNTL		0x40
+
+#define MESON_CPU_CLK_MUX1		BIT(7)
+#define MESON_CPU_CLK_MUX2		BIT(0)
+
+#define MESON_N_WIDTH			9
+#define MESON_N_SHIFT			20
+#define MESON_SEL_WIDTH			2
+#define MESON_SEL_SHIFT			2
+
+#define MULT_ROUND_UP(r, m) ((r) * (m) + (m) - 1)
+
+#include "clkc.h"
+
+struct meson_clk_cpu {
+	struct notifier_block	clk_nb;
+	struct clk_hw		hw;
+	void __iomem		*base;
+	u16			reg_off;
+	spinlock_t		*lock;
+};
+#define to_meson_clk_cpu_hw(_hw) container_of(_hw, struct meson_clk_cpu, hw)
+#define to_meson_clk_cpu_nb(_nb) container_of(_nb, struct meson_clk_cpu, clk_nb)
+
+static bool is_valid_div(int i)
+{
+	if ((i % 2) && (i != 1) && (i != 3))
+		return 0;
+	return 1;
+}
+
+static bool is_best_div(unsigned long rate, unsigned long now,
+			unsigned long best)
+{
+	return abs(rate - now) < abs(rate - best);
+}
+
+static int meson_clk_cpu_get_div(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *best_parent_rate)
+{
+	unsigned long maxdiv, parent_rate, now;
+	unsigned long best = 0;
+	unsigned long parent_rate_saved = *best_parent_rate;
+	int i, bestdiv = 0;
+
+	maxdiv = 2 * PMASK(MESON_N_WIDTH);
+	maxdiv = min(ULONG_MAX / rate, maxdiv);
+
+	for (i = 1; i <= maxdiv; i++) {
+		if (!is_valid_div(i))
+			continue;
+		if (rate * i == parent_rate_saved) {
+			*best_parent_rate = parent_rate_saved;
+			return i;
+		}
+		parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
+				MULT_ROUND_UP(rate, i));
+		now = DIV_ROUND_UP(parent_rate, i);
+		if (is_best_div(rate, now, best)) {
+			bestdiv = i;
+			best = now;
+			*best_parent_rate = parent_rate;
+		}
+	}
+
+	return bestdiv;
+}
+
+static long meson_clk_cpu_round_rate(struct clk_hw *hw, unsigned long rate,
+				     unsigned long *prate)
+{
+	int div;
+
+	div = meson_clk_cpu_get_div(hw, rate, prate);
+
+	return DIV_ROUND_UP(*prate, div);
+}
+
+static int meson_clk_cpu_set_rate(struct clk_hw *hw, unsigned long rate,
+				  unsigned long parent_rate)
+{
+	struct meson_clk_cpu *clk_cpu = to_meson_clk_cpu_hw(hw);
+	unsigned int div, sel, N = 0;
+	u32 reg;
+
+	div = DIV_ROUND_UP(parent_rate, rate);
+
+	if (div <= 3) {
+		sel = div - 1;
+	} else {
+		sel = 3;
+		N = div / 2;
+	}
+
+	reg = readl(clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL1);
+	reg = PARM_SET(MESON_N_WIDTH, MESON_N_SHIFT, reg, N);
+	writel(reg, clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL1);
+
+	reg = readl(clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL);
+	reg = PARM_SET(MESON_SEL_WIDTH, MESON_SEL_SHIFT, reg, sel);
+	writel(reg, clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL);
+
+	return 0;
+}
+
+static unsigned long meson_clk_cpu_recalc_rate(struct clk_hw *hw,
+					       unsigned long parent_rate)
+{
+	struct meson_clk_cpu *clk_cpu = to_meson_clk_cpu_hw(hw);
+	unsigned int N, sel;
+	unsigned int div = 1;
+	u32 reg;
+
+	reg = readl(clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL1);
+	N = PARM_GET(MESON_N_WIDTH, MESON_N_SHIFT, reg);
+
+	reg = readl(clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL);
+	sel = PARM_GET(MESON_SEL_WIDTH, MESON_SEL_SHIFT, reg);
+
+	if (sel < 3)
+		div = sel + 1;
+	else
+		div = 2 * N;
+
+	return parent_rate / div;
+}
+
+static int meson_clk_cpu_pre_rate_change(struct meson_clk_cpu *clk_cpu,
+					 struct clk_notifier_data *ndata)
+{
+	u32 cpu_clk_cntl;
+
+	spin_lock(clk_cpu->lock);
+
+	/* switch MUX1 to xtal */
+	cpu_clk_cntl = readl(clk_cpu->base + clk_cpu->reg_off
+				+ MESON_CPU_CLK_CNTL);
+	cpu_clk_cntl &= ~MESON_CPU_CLK_MUX1;
+	writel(cpu_clk_cntl, clk_cpu->base + clk_cpu->reg_off
+				+ MESON_CPU_CLK_CNTL);
+	udelay(100);
+
+	/* switch MUX2 to sys-pll */
+	cpu_clk_cntl |= MESON_CPU_CLK_MUX2;
+	writel(cpu_clk_cntl, clk_cpu->base + clk_cpu->reg_off
+				+ MESON_CPU_CLK_CNTL);
+
+	spin_unlock(clk_cpu->lock);
+
+	return 0;
+}
+
+static int meson_clk_cpu_post_rate_change(struct meson_clk_cpu *clk_cpu,
+					  struct clk_notifier_data *ndata)
+{
+	u32 cpu_clk_cntl;
+
+	spin_lock(clk_cpu->lock);
+
+	/* switch MUX1 to divisors' output */
+	cpu_clk_cntl = readl(clk_cpu->base + clk_cpu->reg_off
+				+ MESON_CPU_CLK_CNTL);
+	cpu_clk_cntl |= MESON_CPU_CLK_MUX1;
+	writel(cpu_clk_cntl, clk_cpu->base + clk_cpu->reg_off
+				+ MESON_CPU_CLK_CNTL);
+	udelay(100);
+
+	spin_unlock(clk_cpu->lock);
+
+	return 0;
+}
+
+/*
+ * This clock notifier is called when the frequency of the of the parent
+ * PLL clock is to be changed. We use the xtal input as temporary parent
+ * while the PLL frequency is stabilized.
+ */
+static int meson_clk_cpu_notifier_cb(struct notifier_block *nb,
+				     unsigned long event, void *data)
+{
+	struct clk_notifier_data *ndata = data;
+	struct meson_clk_cpu *clk_cpu = to_meson_clk_cpu_nb(nb);
+	int ret = 0;
+
+	if (event == PRE_RATE_CHANGE)
+		ret = meson_clk_cpu_pre_rate_change(clk_cpu, ndata);
+	else if (event == POST_RATE_CHANGE)
+		ret = meson_clk_cpu_post_rate_change(clk_cpu, ndata);
+
+	return notifier_from_errno(ret);
+}
+
+static const struct clk_ops meson_clk_cpu_ops = {
+	.recalc_rate	= meson_clk_cpu_recalc_rate,
+	.round_rate	= meson_clk_cpu_round_rate,
+	.set_rate	= meson_clk_cpu_set_rate,
+};
+
+struct clk *meson_clk_register_cpu(struct clk_conf *clk_conf,
+				   void __iomem *reg_base,
+				   spinlock_t *lock)
+{
+	struct clk *clk;
+	struct clk *pclk;
+	struct meson_clk_cpu *clk_cpu;
+	struct clk_init_data init;
+	int ret;
+
+	clk_cpu = kzalloc(sizeof(*clk_cpu), GFP_KERNEL);
+	if (!clk_cpu)
+		return ERR_PTR(-ENOMEM);
+
+	clk_cpu->lock = lock;
+	clk_cpu->base = reg_base;
+	clk_cpu->reg_off = clk_conf->reg_off;
+	clk_cpu->clk_nb.notifier_call = meson_clk_cpu_notifier_cb;
+
+	init.name = clk_conf->clk_name;
+	init.ops = &meson_clk_cpu_ops;
+	init.flags = clk_conf->flags | CLK_GET_RATE_NOCACHE;
+	init.flags |= CLK_SET_RATE_PARENT;
+	init.parent_names = clk_conf->clks_parent;
+	init.num_parents = 1;
+
+	clk_cpu->hw.init = &init;
+
+	pclk = __clk_lookup(clk_conf->clks_parent[0]);
+	if (!pclk) {
+		pr_err("%s: could not lookup parent clock %s\n",
+				__func__, clk_conf->clks_parent[0]);
+		return ERR_PTR(-EINVAL);
+	}
+
+	ret = clk_notifier_register(pclk, &clk_cpu->clk_nb);
+	if (ret) {
+		pr_err("%s: failed to register clock notifier for %s\n",
+				__func__, clk_conf->clk_name);
+		return ERR_PTR(-EINVAL);
+	}
+
+	clk = clk_register(NULL, &clk_cpu->hw);
+	if (IS_ERR(clk)) {
+		clk_notifier_unregister(pclk, &clk_cpu->clk_nb);
+		kfree(clk_cpu);
+	}
+
+	return clk;
+}
+
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
new file mode 100644
index 0000000..662d694
--- /dev/null
+++ b/drivers/clk/meson/clk-pll.c
@@ -0,0 +1,231 @@
+/*
+ * Copyright (c) 2015 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo@endlessm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+/*
+ * In the most basic form, a Meson PLL is composed as follows:
+ *
+ *                     PLL
+ *      +------------------------------+
+ *      |                              |
+ * in -----[ /N ]---[ *M ]---[ >>OD ]----->> out
+ *      |         ^        ^           |
+ *      +------------------------------+
+ *                |        |
+ *               FREF     VCO
+ *
+ * out = (in * M / N) >> OD
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#include "clkc.h"
+
+#define MESON_PLL_RESET				BIT(29)
+#define MESON_PLL_LOCK				BIT(31)
+
+struct meson_clk_pll {
+	struct clk_hw	hw;
+	void __iomem	*base;
+	struct pll_conf	*conf;
+	unsigned int	rate_count;
+	spinlock_t	*lock;
+};
+#define to_meson_clk_pll(_hw) container_of(_hw, struct meson_clk_pll, hw)
+
+static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
+{
+	struct meson_clk_pll *pll = to_meson_clk_pll(hw);
+	struct parm *p;
+	unsigned long parent_rate_mhz = parent_rate / 1000000;
+	unsigned long rate_mhz;
+	u16 n, m, od;
+	u32 reg;
+
+	p = &pll->conf->n;
+	reg = readl(pll->base + p->reg_off);
+	n = PARM_GET(p->width, p->shift, reg);
+
+	p = &pll->conf->m;
+	reg = readl(pll->base + p->reg_off);
+	m = PARM_GET(p->width, p->shift, reg);
+
+	p = &pll->conf->od;
+	reg = readl(pll->base + p->reg_off);
+	od = PARM_GET(p->width, p->shift, reg);
+
+	rate_mhz = (parent_rate_mhz * m / n) >> od;
+
+	return rate_mhz * 1000000;
+}
+
+static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate,
+				      unsigned long *parent_rate)
+{
+	struct meson_clk_pll *pll = to_meson_clk_pll(hw);
+	struct pll_rate_table *rate_table = pll->conf->rate_table;
+	int i;
+
+	for (i = 0; i < pll->rate_count; i++) {
+		if (rate <= rate_table[i].rate)
+			return rate_table[i].rate;
+	}
+
+	/* else return the smallest value */
+	return rate_table[0].rate;
+}
+
+static struct pll_rate_table *meson_clk_get_pll_settings(struct meson_clk_pll *pll,
+							 unsigned long rate)
+{
+	struct pll_rate_table *rate_table = pll->conf->rate_table;
+	int i;
+
+	for (i = 0; i < pll->rate_count; i++) {
+		if (rate == rate_table[i].rate)
+			return &rate_table[i];
+	}
+	return NULL;
+}
+
+static int meson_clk_pll_wait_lock(struct meson_clk_pll *pll,
+				   struct parm *p_n)
+{
+	int delay = 24000000;
+	u32 reg;
+
+	while (delay > 0) {
+		reg = readl(pll->base + p_n->reg_off);
+
+		if (reg & MESON_PLL_LOCK)
+			return 0;
+		delay--;
+	}
+	return -ETIMEDOUT;
+}
+
+static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+				  unsigned long parent_rate)
+{
+	struct meson_clk_pll *pll = to_meson_clk_pll(hw);
+	struct parm *p;
+	struct pll_rate_table *rate_set;
+	unsigned long old_rate;
+	int ret = 0;
+	u32 reg;
+
+	if (parent_rate == 0 || rate == 0)
+		return -EINVAL;
+
+	old_rate = rate;
+
+	rate_set = meson_clk_get_pll_settings(pll, rate);
+	if (!rate_set)
+		return -EINVAL;
+
+	/* PLL reset */
+	p = &pll->conf->n;
+	reg = readl(pll->base + p->reg_off);
+	writel(reg | MESON_PLL_RESET, pll->base + p->reg_off);
+
+	reg = PARM_SET(p->width, p->shift, reg, rate_set->n);
+	writel(reg, pll->base + p->reg_off);
+
+	p = &pll->conf->m;
+	reg = readl(pll->base + p->reg_off);
+	reg = PARM_SET(p->width, p->shift, reg, rate_set->m);
+	writel(reg, pll->base + p->reg_off);
+
+	p = &pll->conf->od;
+	reg = readl(pll->base + p->reg_off);
+	reg = PARM_SET(p->width, p->shift, reg, rate_set->od);
+	writel(reg, pll->base + p->reg_off);
+
+	p = &pll->conf->n;
+	ret = meson_clk_pll_wait_lock(pll, p);
+	if (ret) {
+		pr_warn("%s: pll did not lock, trying to restore old rate %lu\n",
+			__func__, old_rate);
+		meson_clk_pll_set_rate(hw, old_rate, parent_rate);
+	}
+
+	return ret;
+}
+
+static const struct clk_ops meson_clk_pll_ops = {
+	.recalc_rate	= meson_clk_pll_recalc_rate,
+	.round_rate	= meson_clk_pll_round_rate,
+	.set_rate	= meson_clk_pll_set_rate,
+};
+
+static const struct clk_ops meson_clk_pll_ro_ops = {
+	.recalc_rate	= meson_clk_pll_recalc_rate,
+};
+
+struct clk *meson_clk_register_pll(struct clk_conf *clk_conf,
+				   void __iomem *reg_base,
+				   spinlock_t *lock)
+{
+	struct clk *clk;
+	struct meson_clk_pll *clk_pll;
+	struct clk_init_data init;
+
+	clk_pll = kzalloc(sizeof(*clk_pll), GFP_KERNEL);
+	if (!clk_pll)
+		return ERR_PTR(-ENOMEM);
+
+	clk_pll->base = reg_base + clk_conf->reg_off;
+	clk_pll->lock = lock;
+	clk_pll->conf = clk_conf->conf.pll;
+
+	init.name = clk_conf->clk_name;
+	init.flags = clk_conf->flags | CLK_GET_RATE_NOCACHE;
+
+	/* We usually don't touch PLLs */
+	init.flags |= CLK_IGNORE_UNUSED;
+
+	init.parent_names = &clk_conf->clks_parent[0];
+	init.num_parents = 1;
+	init.ops = &meson_clk_pll_ro_ops;
+
+	/* If no rate_table is specified we assume the PLL is read-only */
+	if (clk_pll->conf->rate_table) {
+		int len;
+
+		for (len = 0; clk_pll->conf->rate_table[len].rate != 0; )
+			len++;
+
+		 clk_pll->rate_count = len;
+		 init.ops = &meson_clk_pll_ops;
+	}
+
+	clk_pll->hw.init = &init;
+
+	clk = clk_register(NULL, &clk_pll->hw);
+	if (IS_ERR(clk))
+		kfree(clk_pll);
+
+	return clk;
+}
diff --git a/drivers/clk/meson/clkc.c b/drivers/clk/meson/clkc.c
new file mode 100644
index 0000000..8805772
--- /dev/null
+++ b/drivers/clk/meson/clkc.c
@@ -0,0 +1,234 @@
+/*
+ * Copyright (c) 2015 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo@endlessm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/mfd/syscon.h>
+#include <linux/slab.h>
+
+#include "clkc.h"
+
+static DEFINE_SPINLOCK(clk_lock);
+
+static struct clk **clks;
+static struct clk_onecell_data clk_data;
+
+struct clk __init **meson_clk_init(struct device_node *np,
+				   unsigned long nr_clks)
+{
+	clks = kcalloc(nr_clks, sizeof(struct clk *), GFP_KERNEL);
+	if (!clks) {
+		pr_err("%s: could not allocate clock lookup table\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	clk_data.clks = clks;
+	clk_data.clk_num = nr_clks;
+	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
+
+	return clks;
+}
+
+static void meson_clk_add_lookup(struct clk *clk, unsigned int id)
+{
+	if (clks && id)
+		clks[id] = clk;
+}
+
+static struct clk __init *meson_clk_register_composite(struct clk_conf *clk_conf,
+						       void __iomem *clk_base)
+{
+	struct clk *clk;
+	struct clk_mux *mux = NULL;
+	struct clk_divider *div = NULL;
+	struct clk_gate *gate = NULL;
+	const struct clk_ops *mux_ops = NULL;
+	struct composite_conf *composite_conf;
+
+	composite_conf = clk_conf->conf.composite;
+
+	if (clk_conf->num_parents > 1) {
+		mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+		if (!mux)
+			return ERR_PTR(-ENOMEM);
+
+		mux->reg = clk_base + clk_conf->reg_off
+				+ composite_conf->mux_parm.reg_off;
+		mux->shift = composite_conf->mux_parm.shift;
+		mux->mask = BIT(composite_conf->mux_parm.width) - 1;
+		mux->flags = composite_conf->mux_flags;
+		mux->lock = &clk_lock;
+		mux->table = composite_conf->mux_table;
+		mux_ops = (composite_conf->mux_flags & CLK_MUX_READ_ONLY) ?
+			  &clk_mux_ro_ops : &clk_mux_ops;
+	}
+
+	if (MESON_PARM_APPLICABLE(&composite_conf->div_parm)) {
+		div = kzalloc(sizeof(*div), GFP_KERNEL);
+		if (!div)
+			return ERR_PTR(-ENOMEM);
+
+		div->reg = clk_base + clk_conf->reg_off
+				+ composite_conf->div_parm.reg_off;
+		div->shift = composite_conf->div_parm.shift;
+		div->width = composite_conf->div_parm.width;
+		div->lock = &clk_lock;
+		div->flags = composite_conf->div_flags;
+		div->table = composite_conf->div_table;
+	}
+
+	if (MESON_PARM_APPLICABLE(&composite_conf->gate_parm)) {
+		gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+		if (!gate)
+			return ERR_PTR(-ENOMEM);
+
+		gate->reg = clk_base + clk_conf->reg_off
+				+ composite_conf->div_parm.reg_off;
+		gate->bit_idx = composite_conf->gate_parm.shift;
+		gate->flags = composite_conf->gate_flags;
+		gate->lock = &clk_lock;
+	}
+
+	clk = clk_register_composite(NULL, clk_conf->clk_name,
+				    clk_conf->clks_parent,
+				    clk_conf->num_parents,
+				    mux ? &mux->hw : NULL, mux_ops,
+				    div ? &div->hw : NULL, &clk_divider_ops,
+				    gate ? &gate->hw : NULL, &clk_gate_ops,
+				    clk_conf->flags);
+
+	return clk;
+}
+
+static struct clk __init *meson_clk_register_fixed_factor(struct clk_conf *clk_conf,
+							  void __iomem *clk_base)
+{
+	struct clk *clk;
+	struct fixed_fact_conf *fixed_fact_conf;
+	struct parm *p;
+	unsigned int mult, div;
+	u32 reg;
+
+	fixed_fact_conf = &clk_conf->conf.fixed_fact;
+
+	mult = clk_conf->conf.fixed_fact.mult;
+	div = clk_conf->conf.fixed_fact.div;
+
+	if (!mult) {
+		mult = 1;
+		p = &fixed_fact_conf->mult_parm;
+		if (MESON_PARM_APPLICABLE(p)) {
+			reg = readl(clk_base + clk_conf->reg_off + p->reg_off);
+			mult = PARM_GET(p->width, p->shift, reg);
+		}
+	}
+
+	if (!div) {
+		div = 1;
+		p = &fixed_fact_conf->div_parm;
+		if (MESON_PARM_APPLICABLE(p)) {
+			reg = readl(clk_base + clk_conf->reg_off + p->reg_off);
+			mult = PARM_GET(p->width, p->shift, reg);
+		}
+	}
+
+	clk = clk_register_fixed_factor(NULL,
+			clk_conf->clk_name,
+			clk_conf->clks_parent[0],
+			clk_conf->flags,
+			mult, div);
+
+	return clk;
+}
+
+static struct clk __init *meson_clk_register_fixed_rate(struct clk_conf *clk_conf,
+							void __iomem *clk_base)
+{
+	struct clk *clk;
+	struct fixed_rate_conf *fixed_rate_conf;
+	struct parm *r;
+	unsigned long rate;
+	u32 reg;
+
+	fixed_rate_conf = &clk_conf->conf.fixed_rate;
+	rate = fixed_rate_conf->rate;
+
+	if (!rate) {
+		r = &fixed_rate_conf->rate_parm;
+		reg = readl(clk_base + clk_conf->reg_off + r->reg_off);
+		rate = PARM_GET(r->width, r->shift, reg);
+	}
+
+	rate *= 1000000;
+
+	clk = clk_register_fixed_rate(NULL,
+			clk_conf->clk_name,
+			clk_conf->num_parents
+				? clk_conf->clks_parent[0] : NULL,
+			clk_conf->flags, rate);
+
+	return clk;
+}
+
+void __init meson_clk_register_clks(struct clk_conf *clk_confs,
+				    unsigned int nr_confs,
+				    void __iomem *clk_base)
+{
+	unsigned int i;
+	struct clk *clk = NULL;
+
+	for (i = 0; i < nr_confs; i++) {
+		struct clk_conf *clk_conf = &clk_confs[i];
+
+		switch (clk_conf->clk_type) {
+		case clk_fixed_rate:
+			clk = meson_clk_register_fixed_rate(clk_conf,
+							    clk_base);
+			break;
+		case clk_fixed_factor:
+			clk = meson_clk_register_fixed_factor(clk_conf,
+							      clk_base);
+			break;
+		case clk_composite:
+			clk = meson_clk_register_composite(clk_conf,
+							   clk_base);
+			break;
+		case clk_cpu:
+			clk = meson_clk_register_cpu(clk_conf, clk_base,
+						     &clk_lock);
+			break;
+		case clk_pll:
+			clk = meson_clk_register_pll(clk_conf, clk_base,
+						     &clk_lock);
+			break;
+		}
+
+		if (!clk) {
+			pr_err("%s: unknown clock type %d\n", __func__,
+			       clk_conf->clk_type);
+			continue;
+		}
+
+		if (IS_ERR(clk)) {
+			pr_warn("%s: Unable to create %s clock\n", __func__,
+				clk_conf->clk_name);
+			continue;
+		}
+
+		meson_clk_add_lookup(clk, clk_conf->clk_id);
+	}
+}
diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
new file mode 100644
index 0000000..f189228
--- /dev/null
+++ b/drivers/clk/meson/clkc.h
@@ -0,0 +1,187 @@
+/*
+ * Copyright (c) 2015 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo@endlessm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __CLKC_H
+#define __CLKC_H
+
+#include <linux/clk.h>
+
+#define PMASK(width)			((1U << (width)) - 1)
+#define SETPMASK(width, shift)		(PMASK(width) << (shift))
+#define CLRPMASK(width, shift)		(~(SETPMASK(width, shift)))
+
+#define PARM_GET(width, shift, reg)					\
+	(((reg) & SETPMASK(width, shift)) >> (shift))
+#define PARM_SET(width, shift, reg, val)				\
+	(((reg) & CLRPMASK(width, shift)) | (val << (shift)))
+
+#define MESON_PARM_APPLICABLE(p)		(!!((p)->width))
+
+struct parm {
+	u16	reg_off;
+	u8	shift;
+	u8	width;
+};
+#define PARM(_r, _s, _w)						\
+	{								\
+		.reg_off	= (_r),					\
+		.shift		= (_s),					\
+		.width		= (_w),					\
+	}								\
+
+struct pll_rate_table {
+	unsigned long	rate;
+	u16		m;
+	u16		n;
+	u16		od;
+};
+#define PLL_RATE(_r, _m, _n, _od)					\
+	{								\
+		.rate		= (_r),					\
+		.m		= (_m),					\
+		.n		= (_n),					\
+		.od		= (_od),				\
+	}								\
+
+struct pll_conf {
+	struct pll_rate_table	*rate_table;
+	struct parm		m;
+	struct parm		n;
+	struct parm		od;
+};
+
+struct fixed_fact_conf {
+	unsigned int	div;
+	unsigned int	mult;
+	struct parm	div_parm;
+	struct parm	mult_parm;
+};
+
+struct fixed_rate_conf {
+	unsigned long	rate;
+	struct parm	rate_parm;
+};
+
+struct composite_conf {
+	struct parm		mux_parm;
+	struct parm		div_parm;
+	struct parm		gate_parm;
+	struct clk_div_table	*div_table;
+	u32			*mux_table;
+	u8			mux_flags;
+	u8			div_flags;
+	u8			gate_flags;
+};
+
+#define PNAME(x) static const char *x[] __initconst
+
+enum clk_type {
+	clk_fixed_factor,
+	clk_fixed_rate,
+	clk_composite,
+	clk_cpu,
+	clk_pll,
+};
+
+struct clk_conf {
+	u16				reg_off;
+	enum clk_type			clk_type;
+	unsigned int			clk_id;
+	const char			*clk_name;
+	const char			**clks_parent;
+	int				num_parents;
+	unsigned long			flags;
+	union {
+		struct fixed_fact_conf	fixed_fact;
+		struct fixed_rate_conf	fixed_rate;
+		struct composite_conf	*composite;
+		struct pll_conf		*pll;
+	} conf;
+};
+
+#define FIXED_RATE_P(_ro, _ci, _cn, _f, _c)				\
+	{								\
+		.reg_off			= (_ro),		\
+		.clk_type			= clk_fixed_rate,	\
+		.clk_id				= (_ci),		\
+		.clk_name			= (_cn),		\
+		.flags				= (_f),			\
+		.conf.fixed_rate.rate_parm	= _c,			\
+	}								\
+
+#define FIXED_RATE(_ci, _cn, _f, _r)					\
+	{								\
+		.clk_type			= clk_fixed_rate,	\
+		.clk_id				= (_ci),		\
+		.clk_name			= (_cn),		\
+		.flags				= (_f),			\
+		.conf.fixed_rate.rate		= (_r),			\
+	}								\
+
+#define PLL(_ro, _ci, _cn, _cp, _f, _c)					\
+	{								\
+		.reg_off			= (_ro),		\
+		.clk_type			= clk_pll,		\
+		.clk_id				= (_ci),		\
+		.clk_name			= (_cn),		\
+		.clks_parent			= (_cp),		\
+		.num_parents			= ARRAY_SIZE(_cp),	\
+		.flags				= (_f),			\
+		.conf.pll			= (_c),			\
+	}								\
+
+#define FIXED_FACTOR_DIV(_ci, _cn, _cp, _f, _d)				\
+	{								\
+		.clk_type			= clk_fixed_factor,	\
+		.clk_id				= (_ci),		\
+		.clk_name			= (_cn),		\
+		.clks_parent			= (_cp),		\
+		.num_parents			= ARRAY_SIZE(_cp),	\
+		.conf.fixed_fact.div		= (_d),			\
+	}								\
+
+#define CPU(_ro, _ci, _cn, _cp)						\
+	{								\
+		.reg_off			= (_ro),		\
+		.clk_type			= clk_cpu,		\
+		.clk_id				= (_ci),		\
+		.clk_name			= (_cn),		\
+		.clks_parent			= (_cp),		\
+		.num_parents			= ARRAY_SIZE(_cp),	\
+	}								\
+
+#define COMPOSITE(_ro, _ci, _cn, _cp, _f, _c)				\
+	{								\
+		.reg_off			= (_ro),		\
+		.clk_type			= clk_composite,	\
+		.clk_id				= (_ci),		\
+		.clk_name			= (_cn),		\
+		.clks_parent			= (_cp),		\
+		.num_parents			= ARRAY_SIZE(_cp),	\
+		.flags				= (_f),			\
+		.conf.composite			= (_c),			\
+	}								\
+
+struct clk **meson_clk_init(struct device_node *np, unsigned long nr_clks);
+void meson_clk_register_clks(struct clk_conf *clk_confs,
+			     unsigned int nr_confs, void __iomem *clk_base);
+struct clk *meson_clk_register_cpu(struct clk_conf *clk_conf,
+				   void __iomem *reg_base, spinlock_t *lock);
+struct clk *meson_clk_register_pll(struct clk_conf *clk_conf,
+				   void __iomem *reg_base, spinlock_t *lock);
+
+#endif /* __CLKC_H */
diff --git a/include/dt-bindings/clock/meson8b-clkc.h b/include/dt-bindings/clock/meson8b-clkc.h
new file mode 100644
index 0000000..9986c38
--- /dev/null
+++ b/include/dt-bindings/clock/meson8b-clkc.h
@@ -0,0 +1,20 @@
+/*
+ * Meson8b clock tree IDs
+ */
+
+#define CLKID_UNUSED		0
+#define CLKID_XTAL		1
+#define CLKID_PLL_FIXED		2
+#define CLKID_PLL_VID		3
+#define CLKID_PLL_SYS		4
+#define CLKID_FCLK_DIV2		5
+#define CLKID_FCLK_DIV3		6
+#define CLKID_FCLK_DIV4		7
+#define CLKID_FCLK_DIV5		8
+#define CLKID_FCLK_DIV7		9
+#define CLKID_CLK81		10
+#define CLKID_MALI		11
+#define CLKID_CPUCLK		12
+#define CLKID_ZERO		13
+
+#define CLK_NR_CLKS		(CLKID_ZERO + 1)
-- 
1.9.1

[PATCH v2 1/3] clk: meson: Add support for Meson clock controller

[Stephen Boyd]

On 05/16, Carlo Caione wrote:
> diff --git a/drivers/clk/meson/clk-cpu.c b/drivers/clk/meson/clk-cpu.c
> new file mode 100644
> index 0000000..88f4606
> --- /dev/null
> +++ b/drivers/clk/meson/clk-cpu.c
> @@ -0,0 +1,291 @@
> +/*
> + * Copyright (c) 2015 Endless Mobile, Inc.
> + * Author: Carlo Caione <carlo@endlessm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/*
> + * CPU clock path:
> + *
> + *                           +-[/N]-----|3|
> + *             MUX2  +--[/2]-+----------|2| MUX1
> + * [sys_pll]---|1|   |--[/3]------------|1|-|1|
> + *             | |---+------------------|0| | |----- [a5_clk]
> + *          +--|0|                          | |
> + * [xtal]---+-------------------------------|0|
> + *
> + *
> + *
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>

Is this include used?

> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>

Is this include used?

> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>

Is this include used?

Missing

#include <linux/spinlock.h>

?

> +
> +#define MESON_CPU_CLK_CNTL1		0x00
> +#define MESON_CPU_CLK_CNTL		0x40
> +
> +#define MESON_CPU_CLK_MUX1		BIT(7)
> +#define MESON_CPU_CLK_MUX2		BIT(0)
> +
> +#define MESON_N_WIDTH			9
> +#define MESON_N_SHIFT			20
> +#define MESON_SEL_WIDTH			2
> +#define MESON_SEL_SHIFT			2
> +
> +#define MULT_ROUND_UP(r, m) ((r) * (m) + (m) - 1)
> +
> +#include "clkc.h"
> +
> +struct meson_clk_cpu {
> +	struct notifier_block	clk_nb;
> +	struct clk_hw		hw;
> +	void __iomem		*base;
> +	u16			reg_off;
> +	spinlock_t		*lock;
> +};
> +#define to_meson_clk_cpu_hw(_hw) container_of(_hw, struct meson_clk_cpu, hw)
> +#define to_meson_clk_cpu_nb(_nb) container_of(_nb, struct meson_clk_cpu, clk_nb)
> +
> +static bool is_valid_div(int i)
> +{
> +	if ((i % 2) && (i != 1) && (i != 3))
> +		return 0;
> +	return 1;
> +}
> +
> +static bool is_best_div(unsigned long rate, unsigned long now,
> +			unsigned long best)
> +{
> +	return abs(rate - now) < abs(rate - best);
> +}
> +
> +static int meson_clk_cpu_get_div(struct clk_hw *hw, unsigned long rate,
> +				 unsigned long *best_parent_rate)
> +{
> +	unsigned long maxdiv, parent_rate, now;
> +	unsigned long best = 0;
> +	unsigned long parent_rate_saved = *best_parent_rate;
> +	int i, bestdiv = 0;
> +
> +	maxdiv = 2 * PMASK(MESON_N_WIDTH);
> +	maxdiv = min(ULONG_MAX / rate, maxdiv);
> +
> +	for (i = 1; i <= maxdiv; i++) {
> +		if (!is_valid_div(i))
> +			continue;
> +		if (rate * i == parent_rate_saved) {
> +			*best_parent_rate = parent_rate_saved;
> +			return i;
> +		}
> +		parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
> +				MULT_ROUND_UP(rate, i));
> +		now = DIV_ROUND_UP(parent_rate, i);
> +		if (is_best_div(rate, now, best)) {
> +			bestdiv = i;
> +			best = now;
> +			*best_parent_rate = parent_rate;
> +		}
> +	}
> +
> +	return bestdiv;
> +}
> +
> +static long meson_clk_cpu_round_rate(struct clk_hw *hw, unsigned long rate,
> +				     unsigned long *prate)
> +{
> +	int div;
> +
> +	div = meson_clk_cpu_get_div(hw, rate, prate);
> +
> +	return DIV_ROUND_UP(*prate, div);
> +}

How much of this is the same as the generic divider code? Can you
use the divider_*() functions for this code?

> +
> +static int meson_clk_cpu_set_rate(struct clk_hw *hw, unsigned long rate,
> +				  unsigned long parent_rate)
> +{
> +	struct meson_clk_cpu *clk_cpu = to_meson_clk_cpu_hw(hw);
> +	unsigned int div, sel, N = 0;
> +	u32 reg;
> +
> +	div = DIV_ROUND_UP(parent_rate, rate);
> +
> +	if (div <= 3) {
> +		sel = div - 1;
> +	} else {
> +		sel = 3;
> +		N = div / 2;
> +	}
> +
> +	reg = readl(clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL1);
> +	reg = PARM_SET(MESON_N_WIDTH, MESON_N_SHIFT, reg, N);
> +	writel(reg, clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL1);
> +
> +	reg = readl(clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL);
> +	reg = PARM_SET(MESON_SEL_WIDTH, MESON_SEL_SHIFT, reg, sel);
> +	writel(reg, clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL);
> +
> +	return 0;
> +}
> +
> +static unsigned long meson_clk_cpu_recalc_rate(struct clk_hw *hw,
> +					       unsigned long parent_rate)
> +{
> +	struct meson_clk_cpu *clk_cpu = to_meson_clk_cpu_hw(hw);
> +	unsigned int N, sel;
> +	unsigned int div = 1;
> +	u32 reg;
> +
> +	reg = readl(clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL1);
> +	N = PARM_GET(MESON_N_WIDTH, MESON_N_SHIFT, reg);
> +
> +	reg = readl(clk_cpu->base + clk_cpu->reg_off + MESON_CPU_CLK_CNTL);
> +	sel = PARM_GET(MESON_SEL_WIDTH, MESON_SEL_SHIFT, reg);
> +
> +	if (sel < 3)
> +		div = sel + 1;
> +	else
> +		div = 2 * N;
> +
> +	return parent_rate / div;
> +}
> +
> +static int meson_clk_cpu_pre_rate_change(struct meson_clk_cpu *clk_cpu,
> +					 struct clk_notifier_data *ndata)
> +{
> +	u32 cpu_clk_cntl;
> +
> +	spin_lock(clk_cpu->lock);

We don't need irqsave? What is this locking against? I only see
notifiers using it and notifiers are synchronized already.

> +
> +	/* switch MUX1 to xtal */
> +	cpu_clk_cntl = readl(clk_cpu->base + clk_cpu->reg_off
> +				+ MESON_CPU_CLK_CNTL);
> +	cpu_clk_cntl &= ~MESON_CPU_CLK_MUX1;
> +	writel(cpu_clk_cntl, clk_cpu->base + clk_cpu->reg_off
> +				+ MESON_CPU_CLK_CNTL);
> +	udelay(100);
> +
> +	/* switch MUX2 to sys-pll */
> +	cpu_clk_cntl |= MESON_CPU_CLK_MUX2;
> +	writel(cpu_clk_cntl, clk_cpu->base + clk_cpu->reg_off
> +				+ MESON_CPU_CLK_CNTL);
> +
> +	spin_unlock(clk_cpu->lock);
> +
> +	return 0;
> +}
> +
> +static int meson_clk_cpu_post_rate_change(struct meson_clk_cpu *clk_cpu,
> +					  struct clk_notifier_data *ndata)
> +{
> +	u32 cpu_clk_cntl;
> +
> +	spin_lock(clk_cpu->lock);
> +
> +	/* switch MUX1 to divisors' output */
> +	cpu_clk_cntl = readl(clk_cpu->base + clk_cpu->reg_off
> +				+ MESON_CPU_CLK_CNTL);
> +	cpu_clk_cntl |= MESON_CPU_CLK_MUX1;
> +	writel(cpu_clk_cntl, clk_cpu->base + clk_cpu->reg_off
> +				+ MESON_CPU_CLK_CNTL);
> +	udelay(100);
> +
> +	spin_unlock(clk_cpu->lock);
> +
> +	return 0;
> +}
> +
> +/*
> + * This clock notifier is called when the frequency of the of the parent
> + * PLL clock is to be changed. We use the xtal input as temporary parent
> + * while the PLL frequency is stabilized.
> + */
> +static int meson_clk_cpu_notifier_cb(struct notifier_block *nb,
> +				     unsigned long event, void *data)
> +{
> +	struct clk_notifier_data *ndata = data;
> +	struct meson_clk_cpu *clk_cpu = to_meson_clk_cpu_nb(nb);
> +	int ret = 0;
> +
> +	if (event == PRE_RATE_CHANGE)
> +		ret = meson_clk_cpu_pre_rate_change(clk_cpu, ndata);
> +	else if (event == POST_RATE_CHANGE)
> +		ret = meson_clk_cpu_post_rate_change(clk_cpu, ndata);
> +
> +	return notifier_from_errno(ret);
> +}
> +
> +static const struct clk_ops meson_clk_cpu_ops = {
> +	.recalc_rate	= meson_clk_cpu_recalc_rate,
> +	.round_rate	= meson_clk_cpu_round_rate,
> +	.set_rate	= meson_clk_cpu_set_rate,
> +};
> +
> +struct clk *meson_clk_register_cpu(struct clk_conf *clk_conf,
> +				   void __iomem *reg_base,
> +				   spinlock_t *lock)
> +{
> +	struct clk *clk;
> +	struct clk *pclk;
> +	struct meson_clk_cpu *clk_cpu;
> +	struct clk_init_data init;
> +	int ret;
> +
> +	clk_cpu = kzalloc(sizeof(*clk_cpu), GFP_KERNEL);
> +	if (!clk_cpu)
> +		return ERR_PTR(-ENOMEM);
> +
> +	clk_cpu->lock = lock;
> +	clk_cpu->base = reg_base;
> +	clk_cpu->reg_off = clk_conf->reg_off;
> +	clk_cpu->clk_nb.notifier_call = meson_clk_cpu_notifier_cb;
> +
> +	init.name = clk_conf->clk_name;
> +	init.ops = &meson_clk_cpu_ops;
> +	init.flags = clk_conf->flags | CLK_GET_RATE_NOCACHE;
> +	init.flags |= CLK_SET_RATE_PARENT;
> +	init.parent_names = clk_conf->clks_parent;
> +	init.num_parents = 1;
> +
> +	clk_cpu->hw.init = &init;
> +
> +	pclk = __clk_lookup(clk_conf->clks_parent[0]);
> +	if (!pclk) {
> +		pr_err("%s: could not lookup parent clock %s\n",
> +				__func__, clk_conf->clks_parent[0]);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	ret = clk_notifier_register(pclk, &clk_cpu->clk_nb);
> +	if (ret) {
> +		pr_err("%s: failed to register clock notifier for %s\n",
> +				__func__, clk_conf->clk_name);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	clk = clk_register(NULL, &clk_cpu->hw);
> +	if (IS_ERR(clk)) {
> +		clk_notifier_unregister(pclk, &clk_cpu->clk_nb);
> +		kfree(clk_cpu);
> +	}
> +
> +	return clk;
> +}
> +
> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
> new file mode 100644
> index 0000000..662d694
> --- /dev/null
> +++ b/drivers/clk/meson/clk-pll.c
> @@ -0,0 +1,231 @@
> +/*
> + * Copyright (c) 2015 Endless Mobile, Inc.
> + * Author: Carlo Caione <carlo@endlessm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/*
> + * In the most basic form, a Meson PLL is composed as follows:
> + *
> + *                     PLL
> + *      +------------------------------+
> + *      |                              |
> + * in -----[ /N ]---[ *M ]---[ >>OD ]----->> out
> + *      |         ^        ^           |
> + *      +------------------------------+
> + *                |        |
> + *               FREF     VCO
> + *
> + * out = (in * M / N) >> OD
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/clkdev.h>

Is this include used?

> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +
> +#include "clkc.h"
> +
> +#define MESON_PLL_RESET				BIT(29)
> +#define MESON_PLL_LOCK				BIT(31)
> +
> +struct meson_clk_pll {
> +	struct clk_hw	hw;
> +	void __iomem	*base;
> +	struct pll_conf	*conf;
> +	unsigned int	rate_count;
> +	spinlock_t	*lock;
> +};
> +#define to_meson_clk_pll(_hw) container_of(_hw, struct meson_clk_pll, hw)
> +
> +static unsigned long meson_clk_pll_recalc_rate(struct clk_hw *hw,
> +						unsigned long parent_rate)
> +{
> +	struct meson_clk_pll *pll = to_meson_clk_pll(hw);
> +	struct parm *p;
> +	unsigned long parent_rate_mhz = parent_rate / 1000000;
> +	unsigned long rate_mhz;
> +	u16 n, m, od;
> +	u32 reg;
> +
> +	p = &pll->conf->n;
> +	reg = readl(pll->base + p->reg_off);
> +	n = PARM_GET(p->width, p->shift, reg);
> +
> +	p = &pll->conf->m;
> +	reg = readl(pll->base + p->reg_off);
> +	m = PARM_GET(p->width, p->shift, reg);
> +
> +	p = &pll->conf->od;
> +	reg = readl(pll->base + p->reg_off);
> +	od = PARM_GET(p->width, p->shift, reg);
> +
> +	rate_mhz = (parent_rate_mhz * m / n) >> od;
> +
> +	return rate_mhz * 1000000;
> +}
> +
> +static long meson_clk_pll_round_rate(struct clk_hw *hw, unsigned long rate,
> +				      unsigned long *parent_rate)
> +{
> +	struct meson_clk_pll *pll = to_meson_clk_pll(hw);
> +	struct pll_rate_table *rate_table = pll->conf->rate_table;
> +	int i;
> +
> +	for (i = 0; i < pll->rate_count; i++) {
> +		if (rate <= rate_table[i].rate)
> +			return rate_table[i].rate;
> +	}
> +
> +	/* else return the smallest value */
> +	return rate_table[0].rate;
> +}
> +
> +static struct pll_rate_table *meson_clk_get_pll_settings(struct meson_clk_pll *pll,
> +							 unsigned long rate)
> +{
> +	struct pll_rate_table *rate_table = pll->conf->rate_table;
> +	int i;
> +
> +	for (i = 0; i < pll->rate_count; i++) {
> +		if (rate == rate_table[i].rate)
> +			return &rate_table[i];
> +	}
> +	return NULL;
> +}
> +
> +static int meson_clk_pll_wait_lock(struct meson_clk_pll *pll,
> +				   struct parm *p_n)
> +{
> +	int delay = 24000000;
> +	u32 reg;
> +
> +	while (delay > 0) {
> +		reg = readl(pll->base + p_n->reg_off);
> +
> +		if (reg & MESON_PLL_LOCK)
> +			return 0;
> +		delay--;

Do we somehow know that a delay-- takes so much time? It would
seem better to have a udelay() here. Otherwise we're left to the
speed of the CPU to execute these instructions.

> +	}
> +	return -ETIMEDOUT;
> +}
> +
> +static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
> +				  unsigned long parent_rate)
> +{
> +	struct meson_clk_pll *pll = to_meson_clk_pll(hw);
> +	struct parm *p;
> +	struct pll_rate_table *rate_set;
> +	unsigned long old_rate;
> +	int ret = 0;
> +	u32 reg;
> +
> +	if (parent_rate == 0 || rate == 0)
> +		return -EINVAL;
> +
> +	old_rate = rate;
> +
> +	rate_set = meson_clk_get_pll_settings(pll, rate);
> +	if (!rate_set)
> +		return -EINVAL;
> +
> +	/* PLL reset */
> +	p = &pll->conf->n;
> +	reg = readl(pll->base + p->reg_off);
> +	writel(reg | MESON_PLL_RESET, pll->base + p->reg_off);
> +
> +	reg = PARM_SET(p->width, p->shift, reg, rate_set->n);
> +	writel(reg, pll->base + p->reg_off);
> +
> +	p = &pll->conf->m;
> +	reg = readl(pll->base + p->reg_off);
> +	reg = PARM_SET(p->width, p->shift, reg, rate_set->m);
> +	writel(reg, pll->base + p->reg_off);
> +
> +	p = &pll->conf->od;
> +	reg = readl(pll->base + p->reg_off);
> +	reg = PARM_SET(p->width, p->shift, reg, rate_set->od);
> +	writel(reg, pll->base + p->reg_off);
> +
> +	p = &pll->conf->n;
> +	ret = meson_clk_pll_wait_lock(pll, p);
> +	if (ret) {
> +		pr_warn("%s: pll did not lock, trying to restore old rate %lu\n",
> +			__func__, old_rate);
> +		meson_clk_pll_set_rate(hw, old_rate, parent_rate);
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct clk_ops meson_clk_pll_ops = {
> +	.recalc_rate	= meson_clk_pll_recalc_rate,
> +	.round_rate	= meson_clk_pll_round_rate,
> +	.set_rate	= meson_clk_pll_set_rate,
> +};
> +
> +static const struct clk_ops meson_clk_pll_ro_ops = {
> +	.recalc_rate	= meson_clk_pll_recalc_rate,
> +};
> +
> +struct clk *meson_clk_register_pll(struct clk_conf *clk_conf,
> +				   void __iomem *reg_base,
> +				   spinlock_t *lock)
> +{
> +	struct clk *clk;
> +	struct meson_clk_pll *clk_pll;
> +	struct clk_init_data init;
> +
> +	clk_pll = kzalloc(sizeof(*clk_pll), GFP_KERNEL);
> +	if (!clk_pll)
> +		return ERR_PTR(-ENOMEM);
> +
> +	clk_pll->base = reg_base + clk_conf->reg_off;
> +	clk_pll->lock = lock;
> +	clk_pll->conf = clk_conf->conf.pll;
> +
> +	init.name = clk_conf->clk_name;
> +	init.flags = clk_conf->flags | CLK_GET_RATE_NOCACHE;
> +
> +	/* We usually don't touch PLLs */
> +	init.flags |= CLK_IGNORE_UNUSED;

Is this even important if the ops don't have an enable/disable
(read-only ops)?

> +
> +	init.parent_names = &clk_conf->clks_parent[0];
> +	init.num_parents = 1;
> +	init.ops = &meson_clk_pll_ro_ops;
> +
> +	/* If no rate_table is specified we assume the PLL is read-only */
> +	if (clk_pll->conf->rate_table) {
> +		int len;
> +
> +		for (len = 0; clk_pll->conf->rate_table[len].rate != 0; )
> +			len++;
> +
> +		 clk_pll->rate_count = len;
> +		 init.ops = &meson_clk_pll_ops;
> +	}
> +
> +	clk_pll->hw.init = &init;
> +
> +	clk = clk_register(NULL, &clk_pll->hw);
> +	if (IS_ERR(clk))
> +		kfree(clk_pll);
> +
> +	return clk;
> +}
> diff --git a/drivers/clk/meson/clkc.c b/drivers/clk/meson/clkc.c
> new file mode 100644
> index 0000000..8805772
> --- /dev/null
> +++ b/drivers/clk/meson/clkc.c
> @@ -0,0 +1,234 @@
> +/*
> + * Copyright (c) 2015 Endless Mobile, Inc.
> + * Author: Carlo Caione <carlo@endlessm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/slab.h>
> +
> +#include "clkc.h"
> +
> +static DEFINE_SPINLOCK(clk_lock);
> +
> +static struct clk **clks;
> +static struct clk_onecell_data clk_data;
> +
> +struct clk __init **meson_clk_init(struct device_node *np,

This should be 

struct clk ** __init meson_clk_init(struct ...)

> +				   unsigned long nr_clks)
> +{
> +	clks = kcalloc(nr_clks, sizeof(struct clk *), GFP_KERNEL);

sizeof(**clks) ?

> +	if (!clks) {
> +		pr_err("%s: could not allocate clock lookup table\n", __func__);

No need for error messages on allocation failures. Please run
checkpatch.

> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	clk_data.clks = clks;
> +	clk_data.clk_num = nr_clks;
> +	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
> +
> +	return clks;
> +}
> +
> +static void meson_clk_add_lookup(struct clk *clk, unsigned int id)
> +{
> +	if (clks && id)
> +		clks[id] = clk;
> +}
> +
> +static struct clk __init *meson_clk_register_composite(struct clk_conf *clk_conf,
> +						       void __iomem *clk_base)
> +{
> +	struct clk *clk;
> +	struct clk_mux *mux = NULL;
> +	struct clk_divider *div = NULL;
> +	struct clk_gate *gate = NULL;
> +	const struct clk_ops *mux_ops = NULL;
> +	struct composite_conf *composite_conf;
> +
> +	composite_conf = clk_conf->conf.composite;
> +
> +	if (clk_conf->num_parents > 1) {
> +		mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> +		if (!mux)
> +			return ERR_PTR(-ENOMEM);
> +
> +		mux->reg = clk_base + clk_conf->reg_off
> +				+ composite_conf->mux_parm.reg_off;
> +		mux->shift = composite_conf->mux_parm.shift;
> +		mux->mask = BIT(composite_conf->mux_parm.width) - 1;
> +		mux->flags = composite_conf->mux_flags;
> +		mux->lock = &clk_lock;
> +		mux->table = composite_conf->mux_table;
> +		mux_ops = (composite_conf->mux_flags & CLK_MUX_READ_ONLY) ?
> +			  &clk_mux_ro_ops : &clk_mux_ops;
> +	}
> +
> +	if (MESON_PARM_APPLICABLE(&composite_conf->div_parm)) {
> +		div = kzalloc(sizeof(*div), GFP_KERNEL);
> +		if (!div)
> +			return ERR_PTR(-ENOMEM);

Did we just leak memory if we had a mux?

> +
> +		div->reg = clk_base + clk_conf->reg_off
> +				+ composite_conf->div_parm.reg_off;
> +		div->shift = composite_conf->div_parm.shift;
> +		div->width = composite_conf->div_parm.width;
> +		div->lock = &clk_lock;
> +		div->flags = composite_conf->div_flags;
> +		div->table = composite_conf->div_table;
> +	}
> +
> +	if (MESON_PARM_APPLICABLE(&composite_conf->gate_parm)) {
> +		gate = kzalloc(sizeof(*gate), GFP_KERNEL);
> +		if (!gate)
> +			return ERR_PTR(-ENOMEM);

ditto

> +
> +		gate->reg = clk_base + clk_conf->reg_off
> +				+ composite_conf->div_parm.reg_off;
> +		gate->bit_idx = composite_conf->gate_parm.shift;
> +		gate->flags = composite_conf->gate_flags;
> +		gate->lock = &clk_lock;
> +	}
> +
> +	clk = clk_register_composite(NULL, clk_conf->clk_name,
> +				    clk_conf->clks_parent,
> +				    clk_conf->num_parents,
> +				    mux ? &mux->hw : NULL, mux_ops,
> +				    div ? &div->hw : NULL, &clk_divider_ops,
> +				    gate ? &gate->hw : NULL, &clk_gate_ops,
> +				    clk_conf->flags);
> +
> +	return clk;

Why not just return clk_register_composite() then?

> +}
> +
> +static struct clk __init *meson_clk_register_fixed_factor(struct clk_conf *clk_conf,
> +							  void __iomem *clk_base)
> +{
> +	struct clk *clk;
> +	struct fixed_fact_conf *fixed_fact_conf;
> +	struct parm *p;
> +	unsigned int mult, div;
> +	u32 reg;
> +
> +	fixed_fact_conf = &clk_conf->conf.fixed_fact;
> +
> +	mult = clk_conf->conf.fixed_fact.mult;
> +	div = clk_conf->conf.fixed_fact.div;
> +
> +	if (!mult) {
> +		mult = 1;
> +		p = &fixed_fact_conf->mult_parm;
> +		if (MESON_PARM_APPLICABLE(p)) {
> +			reg = readl(clk_base + clk_conf->reg_off + p->reg_off);
> +			mult = PARM_GET(p->width, p->shift, reg);
> +		}
> +	}
> +
> +	if (!div) {
> +		div = 1;
> +		p = &fixed_fact_conf->div_parm;
> +		if (MESON_PARM_APPLICABLE(p)) {
> +			reg = readl(clk_base + clk_conf->reg_off + p->reg_off);
> +			mult = PARM_GET(p->width, p->shift, reg);
> +		}
> +	}
> +
> +	clk = clk_register_fixed_factor(NULL,
> +			clk_conf->clk_name,
> +			clk_conf->clks_parent[0],
> +			clk_conf->flags,
> +			mult, div);
> +
> +	return clk;
> +}
> +
> +static struct clk __init *meson_clk_register_fixed_rate(struct clk_conf *clk_conf,
> +							void __iomem *clk_base)
> +{
> +	struct clk *clk;
> +	struct fixed_rate_conf *fixed_rate_conf;
> +	struct parm *r;
> +	unsigned long rate;
> +	u32 reg;
> +
> +	fixed_rate_conf = &clk_conf->conf.fixed_rate;
> +	rate = fixed_rate_conf->rate;
> +
> +	if (!rate) {
> +		r = &fixed_rate_conf->rate_parm;
> +		reg = readl(clk_base + clk_conf->reg_off + r->reg_off);
> +		rate = PARM_GET(r->width, r->shift, reg);
> +	}
> +
> +	rate *= 1000000;
> +
> +	clk = clk_register_fixed_rate(NULL,
> +			clk_conf->clk_name,
> +			clk_conf->num_parents
> +				? clk_conf->clks_parent[0] : NULL,
> +			clk_conf->flags, rate);
> +
> +	return clk;
> +}
> +
> +void __init meson_clk_register_clks(struct clk_conf *clk_confs,
> +				    unsigned int nr_confs,

size_t?

> +				    void __iomem *clk_base)
> +{
> +	unsigned int i;
> +	struct clk *clk = NULL;
> +
> +	for (i = 0; i < nr_confs; i++) {
> +		struct clk_conf *clk_conf = &clk_confs[i];
> +
> +		switch (clk_conf->clk_type) {
> +		case clk_fixed_rate:
> +			clk = meson_clk_register_fixed_rate(clk_conf,
> +							    clk_base);
> +			break;
> +		case clk_fixed_factor:
> +			clk = meson_clk_register_fixed_factor(clk_conf,
> +							      clk_base);
> +			break;
> +		case clk_composite:
> +			clk = meson_clk_register_composite(clk_conf,
> +							   clk_base);
> +			break;
> +		case clk_cpu:
> +			clk = meson_clk_register_cpu(clk_conf, clk_base,
> +						     &clk_lock);
> +			break;
> +		case clk_pll:
> +			clk = meson_clk_register_pll(clk_conf, clk_base,
> +						     &clk_lock);
> +			break;

default:
	clk = NULL;
	break;

> +		}
> +
> +		if (!clk) {
> +			pr_err("%s: unknown clock type %d\n", __func__,
> +			       clk_conf->clk_type);
> +			continue;
> +		}
> +
> +		if (IS_ERR(clk)) {
> +			pr_warn("%s: Unable to create %s clock\n", __func__,
> +				clk_conf->clk_name);
> +			continue;
> +		}
> +
> +		meson_clk_add_lookup(clk, clk_conf->clk_id);
> +	}
> +}
> diff --git a/drivers/clk/meson/clkc.h b/drivers/clk/meson/clkc.h
> new file mode 100644
> index 0000000..f189228
> --- /dev/null
> +++ b/drivers/clk/meson/clkc.h
> @@ -0,0 +1,187 @@
> +/*
> + * Copyright (c) 2015 Endless Mobile, Inc.
> + * Author: Carlo Caione <carlo@endlessm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef __CLKC_H
> +#define __CLKC_H
> +
> +#include <linux/clk.h>

What is this include for?

> +
> +#define PMASK(width)			((1U << (width)) - 1)
> +#define SETPMASK(width, shift)		(PMASK(width) << (shift))
> +#define CLRPMASK(width, shift)		(~(SETPMASK(width, shift)))
> +

We have GENMASK for these sorts of things, please use it.

> +#define PARM_GET(width, shift, reg)					\
> +	(((reg) & SETPMASK(width, shift)) >> (shift))
> +#define PARM_SET(width, shift, reg, val)				\
> +	(((reg) & CLRPMASK(width, shift)) | (val << (shift)))
> +
> +#define MESON_PARM_APPLICABLE(p)		(!!((p)->width))
> +
> +struct parm {
> +	u16	reg_off;
> +	u8	shift;
> +	u8	width;
> +};
> +#define PARM(_r, _s, _w)						\
> +	{								\
> +		.reg_off	= (_r),					\
> +		.shift		= (_s),					\
> +		.width		= (_w),					\
> +	}								\
> +
> +struct pll_rate_table {
> +	unsigned long	rate;
> +	u16		m;
> +	u16		n;
> +	u16		od;
> +};
> +#define PLL_RATE(_r, _m, _n, _od)					\
> +	{								\
> +		.rate		= (_r),					\
> +		.m		= (_m),					\
> +		.n		= (_n),					\
> +		.od		= (_od),				\
> +	}								\
> +
> +struct pll_conf {
> +	struct pll_rate_table	*rate_table;
> +	struct parm		m;
> +	struct parm		n;
> +	struct parm		od;
> +};
> +
> +struct fixed_fact_conf {
> +	unsigned int	div;
> +	unsigned int	mult;
> +	struct parm	div_parm;
> +	struct parm	mult_parm;
> +};
> +
> +struct fixed_rate_conf {
> +	unsigned long	rate;
> +	struct parm	rate_parm;
> +};
> +
> +struct composite_conf {
> +	struct parm		mux_parm;
> +	struct parm		div_parm;
> +	struct parm		gate_parm;
> +	struct clk_div_table	*div_table;
> +	u32			*mux_table;
> +	u8			mux_flags;
> +	u8			div_flags;
> +	u8			gate_flags;
> +};
> +
> +#define PNAME(x) static const char *x[] __initconst

Is this used?

> +
> +enum clk_type {
> +	clk_fixed_factor,
> +	clk_fixed_rate,
> +	clk_composite,
> +	clk_cpu,
> +	clk_pll,
> +};

Please use upper case for enums

> +
> +struct clk_conf {
> +	u16				reg_off;
> +	enum clk_type			clk_type;
> +	unsigned int			clk_id;
> +	const char			*clk_name;
> +	const char			**clks_parent;
> +	int				num_parents;
> +	unsigned long			flags;
> +	union {
> +		struct fixed_fact_conf	fixed_fact;
> +		struct fixed_rate_conf	fixed_rate;
> +		struct composite_conf	*composite;
> +		struct pll_conf		*pll;
> +	} conf;
> +};
> +
> +#define FIXED_RATE_P(_ro, _ci, _cn, _f, _c)				\
> +	{								\
> +		.reg_off			= (_ro),		\
> +		.clk_type			= clk_fixed_rate,	\
> +		.clk_id				= (_ci),		\
> +		.clk_name			= (_cn),		\
> +		.flags				= (_f),			\
> +		.conf.fixed_rate.rate_parm	= _c,			\
> +	}								\
> +
> +#define FIXED_RATE(_ci, _cn, _f, _r)					\
> +	{								\
> +		.clk_type			= clk_fixed_rate,	\
> +		.clk_id				= (_ci),		\
> +		.clk_name			= (_cn),		\
> +		.flags				= (_f),			\
> +		.conf.fixed_rate.rate		= (_r),			\
> +	}								\
> +
> +#define PLL(_ro, _ci, _cn, _cp, _f, _c)					\
> +	{								\
> +		.reg_off			= (_ro),		\
> +		.clk_type			= clk_pll,		\
> +		.clk_id				= (_ci),		\
> +		.clk_name			= (_cn),		\
> +		.clks_parent			= (_cp),		\
> +		.num_parents			= ARRAY_SIZE(_cp),	\
> +		.flags				= (_f),			\
> +		.conf.pll			= (_c),			\
> +	}								\
> +
> +#define FIXED_FACTOR_DIV(_ci, _cn, _cp, _f, _d)				\
> +	{								\
> +		.clk_type			= clk_fixed_factor,	\
> +		.clk_id				= (_ci),		\
> +		.clk_name			= (_cn),		\
> +		.clks_parent			= (_cp),		\
> +		.num_parents			= ARRAY_SIZE(_cp),	\
> +		.conf.fixed_fact.div		= (_d),			\
> +	}								\
> +
> +#define CPU(_ro, _ci, _cn, _cp)						\
> +	{								\
> +		.reg_off			= (_ro),		\
> +		.clk_type			= clk_cpu,		\
> +		.clk_id				= (_ci),		\
> +		.clk_name			= (_cn),		\
> +		.clks_parent			= (_cp),		\
> +		.num_parents			= ARRAY_SIZE(_cp),	\
> +	}								\
> +
> +#define COMPOSITE(_ro, _ci, _cn, _cp, _f, _c)				\
> +	{								\
> +		.reg_off			= (_ro),		\
> +		.clk_type			= clk_composite,	\
> +		.clk_id				= (_ci),		\
> +		.clk_name			= (_cn),		\
> +		.clks_parent			= (_cp),		\
> +		.num_parents			= ARRAY_SIZE(_cp),	\
> +		.flags				= (_f),			\
> +		.conf.composite			= (_c),			\
> +	}								\

I couldn't find any usage of these defines. If they're used later
in the series, please add them when they're used.

> +
> +
> +#endif /* __CLKC_H */
> diff --git a/include/dt-bindings/clock/meson8b-clkc.h b/include/dt-bindings/clock/meson8b-clkc.h
> new file mode 100644
> index 0000000..9986c38
> --- /dev/null
> +++ b/include/dt-bindings/clock/meson8b-clkc.h
> @@ -0,0 +1,20 @@
> +/*
> + * Meson8b clock tree IDs
> + */

Does this need an ifdef guard?

> +
> +#define CLKID_UNUSED		0
> +#define CLKID_XTAL		1
> +#define CLKID_PLL_FIXED		2
> +#define CLKID_PLL_VID		3
> +#define CLKID_PLL_SYS		4
> +#define CLKID_FCLK_DIV2		5
> +#define CLKID_FCLK_DIV3		6
> +#define CLKID_FCLK_DIV4		7
> +#define CLKID_FCLK_DIV5		8
> +#define CLKID_FCLK_DIV7		9
> +#define CLKID_CLK81		10
> +#define CLKID_MALI		11
> +#define CLKID_CPUCLK		12
> +#define CLKID_ZERO		13
> +
> +#define CLK_NR_CLKS		(CLKID_ZERO + 1)
> -- 
> 1.9.1
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 1/3] clk: meson: Add support for Meson clock controller

[Carlo Caione]

On Thu, May 28, 2015 at 11:55 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>
> Is this include used?
>
>> +#include <linux/delay.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>
> Is this include used?
>
>> +#include <linux/of_address.h>
>> +#include <linux/slab.h>
>> +#include <linux/string.h>
>
> Is this include used?
>
> Missing
>
> #include <linux/spinlock.h>
>
> ?

I'll fix all the headers in v3.

>> +static int meson_clk_cpu_get_div(struct clk_hw *hw, unsigned long rate,
>> +                              unsigned long *best_parent_rate)
>> +{
>> +     unsigned long maxdiv, parent_rate, now;
>> +     unsigned long best = 0;
>> +     unsigned long parent_rate_saved = *best_parent_rate;
>> +     int i, bestdiv = 0;
>> +
>> +     maxdiv = 2 * PMASK(MESON_N_WIDTH);
>> +     maxdiv = min(ULONG_MAX / rate, maxdiv);
>> +
>> +     for (i = 1; i <= maxdiv; i++) {
>> +             if (!is_valid_div(i))
>> +                     continue;
>> +             if (rate * i == parent_rate_saved) {
>> +                     *best_parent_rate = parent_rate_saved;
>> +                     return i;
>> +             }
>> +             parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
>> +                             MULT_ROUND_UP(rate, i));
>> +             now = DIV_ROUND_UP(parent_rate, i);
>> +             if (is_best_div(rate, now, best)) {
>> +                     bestdiv = i;
>> +                     best = now;
>> +                     *best_parent_rate = parent_rate;
>> +             }
>> +     }
>> +
>> +     return bestdiv;
>> +}
>> +
>> +static long meson_clk_cpu_round_rate(struct clk_hw *hw, unsigned long rate,
>> +                                  unsigned long *prate)
>> +{
>> +     int div;
>> +
>> +     div = meson_clk_cpu_get_div(hw, rate, prate);
>> +
>> +     return DIV_ROUND_UP(*prate, div);
>> +}
>
> How much of this is the same as the generic divider code? Can you
> use the divider_*() functions for this code?

Yes. When I submitted the v1 the divider_*() functions weren't exported yet.

>> +static int meson_clk_cpu_pre_rate_change(struct meson_clk_cpu *clk_cpu,
>> +                                      struct clk_notifier_data *ndata)
>> +{
>> +     u32 cpu_clk_cntl;
>> +
>> +     spin_lock(clk_cpu->lock);
>
> We don't need irqsave? What is this locking against? I only see
> notifiers using it and notifiers are synchronized already.

Didn't notice that. I'll fix it.

>> +
>> +#include <linux/clk-provider.h>
>> +#include <linux/clkdev.h>
>
> Is this include used?

It isn't.

>> +
>> +static int meson_clk_pll_wait_lock(struct meson_clk_pll *pll,
>> +                                struct parm *p_n)
>> +{
>> +     int delay = 24000000;
>> +     u32 reg;
>> +
>> +     while (delay > 0) {
>> +             reg = readl(pll->base + p_n->reg_off);
>> +
>> +             if (reg & MESON_PLL_LOCK)
>> +                     return 0;
>> +             delay--;
>
> Do we somehow know that a delay-- takes so much time? It would
> seem better to have a udelay() here. Otherwise we're left to the
> speed of the CPU to execute these instructions.

Actually this is not a delay cycle but we exit as soon as the
MESON_PLL_LOCK bit is flipped so in theory we want to cycle as fast as
possible.

>> +
>> +     /* We usually don't touch PLLs */
>> +     init.flags |= CLK_IGNORE_UNUSED;
>
> Is this even important if the ops don't have an enable/disable
> (read-only ops)?

Yeah, not really important.

>> +
>> +struct clk __init **meson_clk_init(struct device_node *np,
>
> This should be
>
> struct clk ** __init meson_clk_init(struct ...)

Agree

>> +                                unsigned long nr_clks)
>> +{
>> +     clks = kcalloc(nr_clks, sizeof(struct clk *), GFP_KERNEL);
>
> sizeof(**clks) ?

yeah ( sizeof(*clks) )

>> +     if (!clks) {
>> +             pr_err("%s: could not allocate clock lookup table\n", __func__);
>
> No need for error messages on allocation failures. Please run
> checkpatch.

I did. Probably missed that.

>> +     if (MESON_PARM_APPLICABLE(&composite_conf->div_parm)) {
>> +             div = kzalloc(sizeof(*div), GFP_KERNEL);
>> +             if (!div)
>> +                     return ERR_PTR(-ENOMEM);
>
> Did we just leak memory if we had a mux?
>
>> +
>> +             div->reg = clk_base + clk_conf->reg_off
>> +                             + composite_conf->div_parm.reg_off;
>> +             div->shift = composite_conf->div_parm.shift;
>> +             div->width = composite_conf->div_parm.width;
>> +             div->lock = &clk_lock;
>> +             div->flags = composite_conf->div_flags;
>> +             div->table = composite_conf->div_table;
>> +     }
>> +
>> +     if (MESON_PARM_APPLICABLE(&composite_conf->gate_parm)) {
>> +             gate = kzalloc(sizeof(*gate), GFP_KERNEL);
>> +             if (!gate)
>> +                     return ERR_PTR(-ENOMEM);
>
> ditto

Error path in v3.

>> +
>> +     clk = clk_register_composite(NULL, clk_conf->clk_name,
>> +                                 clk_conf->clks_parent,
>> +                                 clk_conf->num_parents,
>> +                                 mux ? &mux->hw : NULL, mux_ops,
>> +                                 div ? &div->hw : NULL, &clk_divider_ops,
>> +                                 gate ? &gate->hw : NULL, &clk_gate_ops,
>> +                                 clk_conf->flags);
>> +
>> +     return clk;
>
> Why not just return clk_register_composite() then?

Because it is missing the error checking on clk. I'll fix it.

>> +
>> +void __init meson_clk_register_clks(struct clk_conf *clk_confs,
>> +                                 unsigned int nr_confs,
>
> size_t?

Ok

>> +             case clk_pll:
>> +                     clk = meson_clk_register_pll(clk_conf, clk_base,
>> +                                                  &clk_lock);
>> +                     break;
>
> default:
>         clk = NULL;
>         break;

Agree

>> +
>> +#include <linux/clk.h>
>
> What is this include for?

Nothing, I guess?

>> +
>> +#define PMASK(width)                 ((1U << (width)) - 1)
>> +#define SETPMASK(width, shift)               (PMASK(width) << (shift))
>> +#define CLRPMASK(width, shift)               (~(SETPMASK(width, shift)))
>> +
>
> We have GENMASK for these sorts of things, please use it.

Oh, nice to have that.

>> +
>> +#define PNAME(x) static const char *x[] __initconst
>
> Is this used?

Yep, it is in meson8b-clkc.c

>> +
>> +enum clk_type {
>> +     clk_fixed_factor,
>> +     clk_fixed_rate,
>> +     clk_composite,
>> +     clk_cpu,
>> +     clk_pll,
>> +};
>
> Please use upper case for enums

Agree

>> +
>> +#define FIXED_RATE_P(_ro, _ci, _cn, _f, _c)                          \
>> +     {                                                               \
>> +             .reg_off                        = (_ro),                \
>> +             .clk_type                       = clk_fixed_rate,       \
>> +             .clk_id                         = (_ci),                \
>> +             .clk_name                       = (_cn),                \
>> +             .flags                          = (_f),                 \
>> +             .conf.fixed_rate.rate_parm      = _c,                   \
>> +     }                                                               \
>> +
>> +#define FIXED_RATE(_ci, _cn, _f, _r)                                 \
>> +     {                                                               \
>> +             .clk_type                       = clk_fixed_rate,       \
>> +             .clk_id                         = (_ci),                \
>> +             .clk_name                       = (_cn),                \
>> +             .flags                          = (_f),                 \
>> +             .conf.fixed_rate.rate           = (_r),                 \
>> +     }                                                               \
>> +
>> +#define PLL(_ro, _ci, _cn, _cp, _f, _c)                                      \
>> +     {                                                               \
>> +             .reg_off                        = (_ro),                \
>> +             .clk_type                       = clk_pll,              \
>> +             .clk_id                         = (_ci),                \
>> +             .clk_name                       = (_cn),                \
>> +             .clks_parent                    = (_cp),                \
>> +             .num_parents                    = ARRAY_SIZE(_cp),      \
>> +             .flags                          = (_f),                 \
>> +             .conf.pll                       = (_c),                 \
>> +     }                                                               \
>> +
>> +#define FIXED_FACTOR_DIV(_ci, _cn, _cp, _f, _d)                              \
>> +     {                                                               \
>> +             .clk_type                       = clk_fixed_factor,     \
>> +             .clk_id                         = (_ci),                \
>> +             .clk_name                       = (_cn),                \
>> +             .clks_parent                    = (_cp),                \
>> +             .num_parents                    = ARRAY_SIZE(_cp),      \
>> +             .conf.fixed_fact.div            = (_d),                 \
>> +     }                                                               \
>> +
>> +#define CPU(_ro, _ci, _cn, _cp)                                              \
>> +     {                                                               \
>> +             .reg_off                        = (_ro),                \
>> +             .clk_type                       = clk_cpu,              \
>> +             .clk_id                         = (_ci),                \
>> +             .clk_name                       = (_cn),                \
>> +             .clks_parent                    = (_cp),                \
>> +             .num_parents                    = ARRAY_SIZE(_cp),      \
>> +     }                                                               \
>> +
>> +#define COMPOSITE(_ro, _ci, _cn, _cp, _f, _c)                                \
>> +     {                                                               \
>> +             .reg_off                        = (_ro),                \
>> +             .clk_type                       = clk_composite,        \
>> +             .clk_id                         = (_ci),                \
>> +             .clk_name                       = (_cn),                \
>> +             .clks_parent                    = (_cp),                \
>> +             .num_parents                    = ARRAY_SIZE(_cp),      \
>> +             .flags                          = (_f),                 \
>> +             .conf.composite                 = (_c),                 \
>> +     }                                                               \
>
> I couldn't find any usage of these defines. If they're used later
> in the series, please add them when they're used.

These defines are used later in the Meson8b specific file and I think
they belong to this patch since they define all the basic clock types
we can find on all the Meson SoCs.

>> +
>> +#endif /* __CLKC_H */
>> diff --git a/include/dt-bindings/clock/meson8b-clkc.h b/include/dt-bindings/clock/meson8b-clkc.h
>> new file mode 100644
>> index 0000000..9986c38
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/meson8b-clkc.h
>> @@ -0,0 +1,20 @@
>> +/*
>> + * Meson8b clock tree IDs
>> + */
>
> Does this need an ifdef guard?

Yep.

Thank you for your review.

-- 
Carlo Caione

[PATCH v2 2/3] clk: meson: Document bindings for Meson8b clock controller

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

Add documentation for  the clock controller.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 .../bindings/clock/amlogic,meson8b-clkc.txt        | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt

diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
new file mode 100644
index 0000000..2b7b3fa
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
@@ -0,0 +1,40 @@
+* Amlogic Meson8b Clock and Reset Unit
+
+The Amlogic Meson8b clock controller generates and supplies clock to various
+controllers within the SoC.
+
+Required Properties:
+
+- compatible: should be "amlogic,meson8b-clkc"
+- reg: it must be composed by two tuples:
+	0) physical base address of the xtal register and length of memory
+	   mapped region.
+	1) physical base address of the clock controller and length of memory
+	   mapped region.
+
+- #clock-cells: should be 1.
+
+Each clock is assigned an identifier and client nodes can use this identifier
+to specify the clock which they consume. All available clocks are defined as
+preprocessor macros in the dt-bindings/clock/meson8b-clkc.h header and can be
+used in device tree sources.
+
+Example: Clock controller node:
+
+	clkc: clock-controller at c1104000 {
+		#clock-cells = <1>;
+		compatible = "amlogic,meson8b-clkc";
+		reg = <0xc1108000 0x4>, <0xc1104000 0x460>;
+	};
+
+
+Example: UART controller node that consumes the clock generated by the clock
+  controller:
+
+	uart_AO: serial at c81004c0 {
+		compatible = "amlogic,meson-uart";
+		reg = <0xc81004c0 0x14>;
+		interrupts = <0 90 1>;
+		clocks = <&clkc CLKID_CLK81>;
+		status = "disabled";
+	};
-- 
1.9.1

[PATCH v2 2/3] clk: meson: Document bindings for Meson8b clock controller

[Stephen Boyd]

On 05/16, Carlo Caione wrote:
> From: Carlo Caione <carlo@endlessm.com>
> 
> Add documentation for  the clock controller.
> 
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
>  .../bindings/clock/amlogic,meson8b-clkc.txt        | 40 ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
> new file mode 100644
> index 0000000..2b7b3fa
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/amlogic,meson8b-clkc.txt
> @@ -0,0 +1,40 @@
> +* Amlogic Meson8b Clock and Reset Unit
> +
> +The Amlogic Meson8b clock controller generates and supplies clock to various
> +controllers within the SoC.
> +
> +Required Properties:
> +
> +- compatible: should be "amlogic,meson8b-clkc"
> +- reg: it must be composed by two tuples:
> +	0) physical base address of the xtal register and length of memory
> +	   mapped region.
> +	1) physical base address of the clock controller and length of memory
> +	   mapped region.
> +
> +- #clock-cells: should be 1.
> +
> +Each clock is assigned an identifier and client nodes can use this identifier
> +to specify the clock which they consume. All available clocks are defined as
> +preprocessor macros in the dt-bindings/clock/meson8b-clkc.h header and can be
> +used in device tree sources.
> +
> +Example: Clock controller node:
> +
> +	clkc: clock-controller at c1104000 {

The address after the '@' should be the first reg property.

> +		#clock-cells = <1>;
> +		compatible = "amlogic,meson8b-clkc";
> +		reg = <0xc1108000 0x4>, <0xc1104000 0x460>;

And a reg property of length 0x4 looks like it may be part of
some other "device". Should the xtal register be part of a syscon
that this node has a phandle to?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 2/3] clk: meson: Document bindings for Meson8b clock controller

[Carlo Caione]

On Thu, May 28, 2015 at 11:57 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> +
>> +Example: Clock controller node:
>> +
>> +     clkc: clock-controller at c1104000 {
>
> The address after the '@' should be the first reg property.
>
>> +             #clock-cells = <1>;
>> +             compatible = "amlogic,meson8b-clkc";
>> +             reg = <0xc1108000 0x4>, <0xc1104000 0x460>;
>
> And a reg property of length 0x4 looks like it may be part of
> some other "device". Should the xtal register be part of a syscon
> that this node has a phandle to?

Probably it does. But I don't have any documentation about this
device. Also the registers mapped before and after this specific
register in the same memory region seem to not be used at all by the
code in the Amlogic SDK. So I guess I can safely map that specific
register for XTAL.

-- 
Carlo Caione

[PATCH v2 3/3] clk: meson8b: Add support for Meson8b clocks

[Carlo Caione]

From: Carlo Caione <carlo@endlessm.com>

This patch adds support for the basic clocks found on the Amlogic
Meson8b SoCs.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 drivers/clk/meson/Makefile       |   1 +
 drivers/clk/meson/meson8b-clkc.c | 182 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 183 insertions(+)
 create mode 100644 drivers/clk/meson/meson8b-clkc.c

diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index 66c6d8d..6d45531 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -3,3 +3,4 @@
 #
 
 obj-y += clkc.o clk-pll.o clk-cpu.o
+obj-y += meson8b-clkc.o
diff --git a/drivers/clk/meson/meson8b-clkc.c b/drivers/clk/meson/meson8b-clkc.c
new file mode 100644
index 0000000..b8be5c7
--- /dev/null
+++ b/drivers/clk/meson/meson8b-clkc.c
@@ -0,0 +1,182 @@
+/*
+ * Copyright (c) 2015 Endless Mobile, Inc.
+ * Author: Carlo Caione <carlo@endlessm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <dt-bindings/clock/meson8b-clkc.h>
+
+#include "clkc.h"
+
+#define MESON8B_REG_CTL0_ADDR		0x0000
+#define MESON8B_REG_SYS_CPU_CNTL1	0x015c
+#define MESON8B_REG_HHI_MPEG		0x0174
+#define MESON8B_REG_MALI		0x01b0
+#define MESON8B_REG_PLL_FIXED		0x0280
+#define MESON8B_REG_PLL_SYS		0x0300
+#define MESON8B_REG_PLL_VID		0x0320
+
+static struct pll_rate_table sys_pll_rate_table[] = {
+	PLL_RATE(312000000, 52, 1, 2),
+	PLL_RATE(336000000, 56, 1, 2),
+	PLL_RATE(360000000, 60, 1, 2),
+	PLL_RATE(384000000, 64, 1, 2),
+	PLL_RATE(408000000, 68, 1, 2),
+	PLL_RATE(432000000, 72, 1, 2),
+	PLL_RATE(456000000, 76, 1, 2),
+	PLL_RATE(480000000, 80, 1, 2),
+	PLL_RATE(504000000, 84, 1, 2),
+	PLL_RATE(528000000, 88, 1, 2),
+	PLL_RATE(552000000, 92, 1, 2),
+	PLL_RATE(576000000, 96, 1, 2),
+	PLL_RATE(600000000, 50, 1, 1),
+	PLL_RATE(624000000, 52, 1, 1),
+	PLL_RATE(648000000, 54, 1, 1),
+	PLL_RATE(672000000, 56, 1, 1),
+	PLL_RATE(696000000, 58, 1, 1),
+	PLL_RATE(720000000, 60, 1, 1),
+	PLL_RATE(744000000, 62, 1, 1),
+	PLL_RATE(768000000, 64, 1, 1),
+	PLL_RATE(792000000, 66, 1, 1),
+	PLL_RATE(816000000, 68, 1, 1),
+	PLL_RATE(840000000, 70, 1, 1),
+	PLL_RATE(864000000, 72, 1, 1),
+	PLL_RATE(888000000, 74, 1, 1),
+	PLL_RATE(912000000, 76, 1, 1),
+	PLL_RATE(936000000, 78, 1, 1),
+	PLL_RATE(960000000, 80, 1, 1),
+	PLL_RATE(984000000, 82, 1, 1),
+	PLL_RATE(1008000000, 84, 1, 1),
+	PLL_RATE(1032000000, 86, 1, 1),
+	PLL_RATE(1056000000, 88, 1, 1),
+	PLL_RATE(1080000000, 90, 1, 1),
+	PLL_RATE(1104000000, 92, 1, 1),
+	PLL_RATE(1128000000, 94, 1, 1),
+	PLL_RATE(1152000000, 96, 1, 1),
+	PLL_RATE(1176000000, 98, 1, 1),
+	PLL_RATE(1200000000, 50, 1, 0),
+	PLL_RATE(1224000000, 51, 1, 0),
+	PLL_RATE(1248000000, 52, 1, 0),
+	PLL_RATE(1272000000, 53, 1, 0),
+	PLL_RATE(1296000000, 54, 1, 0),
+	PLL_RATE(1320000000, 55, 1, 0),
+	PLL_RATE(1344000000, 56, 1, 0),
+	PLL_RATE(1368000000, 57, 1, 0),
+	PLL_RATE(1392000000, 58, 1, 0),
+	PLL_RATE(1416000000, 59, 1, 0),
+	PLL_RATE(1440000000, 60, 1, 0),
+	PLL_RATE(1464000000, 61, 1, 0),
+	PLL_RATE(1488000000, 62, 1, 0),
+	PLL_RATE(1512000000, 63, 1, 0),
+	PLL_RATE(1536000000, 64, 1, 0),
+	{ /* sentinel */ },
+};
+
+PNAME(p_xtal)		= { "xtal" };
+PNAME(p_fclk_div)	= { "fixed_pll" };
+PNAME(p_cpu_clk)	= { "sys_pll" };
+PNAME(p_clk81)		= { "fclk_div3", "fclk_div4", "fclk_div5" };
+PNAME(p_mali)		= { "fclk_div3", "fclk_div4", "fclk_div5",
+			    "fclk_div7", "zero" };
+
+static u32 mux_table_clk81[]	= { 6, 5, 7 };
+static u32 mux_table_mali[]	= { 6, 5, 7, 4, 0 };
+
+static struct pll_conf pll_confs = {
+	.m		= PARM(0x00, 0,  9),
+	.n		= PARM(0x00, 9,  5),
+	.od		= PARM(0x00, 16, 2),
+};
+
+static struct pll_conf sys_pll_conf = {
+	.m		= PARM(0x00, 0,  9),
+	.n		= PARM(0x00, 9,  5),
+	.od		= PARM(0x00, 16, 2),
+	.rate_table	= sys_pll_rate_table,
+};
+
+static struct composite_conf clk81_conf __initdata = {
+	.mux_table		= mux_table_clk81,
+	.mux_flags		= CLK_MUX_READ_ONLY,
+	.mux_parm		= PARM(0x00, 12, 3),
+	.div_parm		= PARM(0x00, 0, 7),
+	.gate_parm		= PARM(0x00, 7, 1),
+};
+
+static struct composite_conf mali_conf __initdata = {
+	.mux_table		= mux_table_mali,
+	.mux_parm		= PARM(0x00, 9, 3),
+	.div_parm		= PARM(0x00, 0, 7),
+	.gate_parm		= PARM(0x00, 8, 1),
+};
+
+static struct clk_conf meson8b_xtal_conf __initdata =
+	FIXED_RATE_P(MESON8B_REG_CTL0_ADDR, CLKID_XTAL, "xtal",
+		     CLK_IS_ROOT, PARM(0x00, 4, 7));
+
+static struct clk_conf meson8b_clk_confs[] __initdata = {
+	FIXED_RATE(CLKID_ZERO, "zero", CLK_IS_ROOT, 0),
+	PLL(MESON8B_REG_PLL_FIXED, CLKID_PLL_FIXED, "fixed_pll",
+	    p_xtal, 0, &pll_confs),
+	PLL(MESON8B_REG_PLL_VID, CLKID_PLL_VID, "vid_pll",
+	    p_xtal, 0, &pll_confs),
+	PLL(MESON8B_REG_PLL_SYS, CLKID_PLL_SYS, "sys_pll",
+	    p_xtal, 0, &sys_pll_conf),
+	FIXED_FACTOR_DIV(CLKID_FCLK_DIV2, "fclk_div2", p_fclk_div, 0, 2),
+	FIXED_FACTOR_DIV(CLKID_FCLK_DIV3, "fclk_div3", p_fclk_div, 0, 3),
+	FIXED_FACTOR_DIV(CLKID_FCLK_DIV4, "fclk_div4", p_fclk_div, 0, 4),
+	FIXED_FACTOR_DIV(CLKID_FCLK_DIV5, "fclk_div5", p_fclk_div, 0, 5),
+	FIXED_FACTOR_DIV(CLKID_FCLK_DIV7, "fclk_div7", p_fclk_div, 0, 7),
+	CPU(MESON8B_REG_SYS_CPU_CNTL1, CLKID_CPUCLK, "a5_clk", p_cpu_clk),
+	COMPOSITE(MESON8B_REG_HHI_MPEG, CLKID_CLK81, "clk81", p_clk81,
+		  CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED, &clk81_conf),
+	COMPOSITE(MESON8B_REG_MALI, CLKID_MALI, "mali", p_mali,
+		  CLK_IGNORE_UNUSED, &mali_conf),
+};
+
+static void __init meson8b_clkc_init(struct device_node *np)
+{
+	void __iomem *clk_base;
+
+	if (!meson_clk_init(np, CLK_NR_CLKS))
+		return;
+
+	/* XTAL */
+	clk_base = of_iomap(np, 0);
+	if (!clk_base) {
+		pr_err("%s: Unable to map xtal base\n", __func__);
+		return;
+	}
+
+	meson_clk_register_clks(&meson8b_xtal_conf, 1, clk_base);
+	iounmap(clk_base);
+
+	/*  Generic clocks and PLLs */
+	clk_base = of_iomap(np, 1);
+	if (!clk_base) {
+		pr_err("%s: Unable to map clk base\n", __func__);
+		return;
+	}
+
+	meson_clk_register_clks(meson8b_clk_confs,
+				ARRAY_SIZE(meson8b_clk_confs),
+				clk_base);
+}
+CLK_OF_DECLARE(meson8b_clock, "amlogic,meson8b-clkc", meson8b_clkc_init);
-- 
1.9.1

[PATCH v2 3/3] clk: meson8b: Add support for Meson8b clocks

[Stephen Boyd]

On 05/16, Carlo Caione wrote:
> diff --git a/drivers/clk/meson/meson8b-clkc.c b/drivers/clk/meson/meson8b-clkc.c
> new file mode 100644
> index 0000000..b8be5c7
> --- /dev/null
> +++ b/drivers/clk/meson/meson8b-clkc.c
> @@ -0,0 +1,182 @@
> +/*
> + * Copyright (c) 2015 Endless Mobile, Inc.
> + * Author: Carlo Caione <carlo@endlessm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <linux/clk.h>

Do you need this include?

> +#include <linux/clk-provider.h>
> +#include <linux/kernel.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <dt-bindings/clock/meson8b-clkc.h>
> +
> +#include "clkc.h"
> +
> +#define MESON8B_REG_CTL0_ADDR		0x0000
> +#define MESON8B_REG_SYS_CPU_CNTL1	0x015c
> +#define MESON8B_REG_HHI_MPEG		0x0174
> +#define MESON8B_REG_MALI		0x01b0
> +#define MESON8B_REG_PLL_FIXED		0x0280
> +#define MESON8B_REG_PLL_SYS		0x0300
> +#define MESON8B_REG_PLL_VID		0x0320
> +
> +static struct pll_rate_table sys_pll_rate_table[] = {

const?

> +	PLL_RATE(312000000, 52, 1, 2),
> +	PLL_RATE(336000000, 56, 1, 2),
> +	PLL_RATE(360000000, 60, 1, 2),
> +	PLL_RATE(384000000, 64, 1, 2),
> +	PLL_RATE(408000000, 68, 1, 2),
> +	PLL_RATE(432000000, 72, 1, 2),
> +	PLL_RATE(456000000, 76, 1, 2),
> +	PLL_RATE(480000000, 80, 1, 2),
> +	PLL_RATE(504000000, 84, 1, 2),
> +	PLL_RATE(528000000, 88, 1, 2),
> +	PLL_RATE(552000000, 92, 1, 2),
> +	PLL_RATE(576000000, 96, 1, 2),
> +	PLL_RATE(600000000, 50, 1, 1),
> +	PLL_RATE(624000000, 52, 1, 1),
> +	PLL_RATE(648000000, 54, 1, 1),
> +	PLL_RATE(672000000, 56, 1, 1),
> +	PLL_RATE(696000000, 58, 1, 1),
> +	PLL_RATE(720000000, 60, 1, 1),
> +	PLL_RATE(744000000, 62, 1, 1),
> +	PLL_RATE(768000000, 64, 1, 1),
> +	PLL_RATE(792000000, 66, 1, 1),
> +	PLL_RATE(816000000, 68, 1, 1),
> +	PLL_RATE(840000000, 70, 1, 1),
> +	PLL_RATE(864000000, 72, 1, 1),
> +	PLL_RATE(888000000, 74, 1, 1),
> +	PLL_RATE(912000000, 76, 1, 1),
> +	PLL_RATE(936000000, 78, 1, 1),
> +	PLL_RATE(960000000, 80, 1, 1),
> +	PLL_RATE(984000000, 82, 1, 1),
> +	PLL_RATE(1008000000, 84, 1, 1),
> +	PLL_RATE(1032000000, 86, 1, 1),
> +	PLL_RATE(1056000000, 88, 1, 1),
> +	PLL_RATE(1080000000, 90, 1, 1),
> +	PLL_RATE(1104000000, 92, 1, 1),
> +	PLL_RATE(1128000000, 94, 1, 1),
> +	PLL_RATE(1152000000, 96, 1, 1),
> +	PLL_RATE(1176000000, 98, 1, 1),
> +	PLL_RATE(1200000000, 50, 1, 0),
> +	PLL_RATE(1224000000, 51, 1, 0),
> +	PLL_RATE(1248000000, 52, 1, 0),
> +	PLL_RATE(1272000000, 53, 1, 0),
> +	PLL_RATE(1296000000, 54, 1, 0),
> +	PLL_RATE(1320000000, 55, 1, 0),
> +	PLL_RATE(1344000000, 56, 1, 0),
> +	PLL_RATE(1368000000, 57, 1, 0),
> +	PLL_RATE(1392000000, 58, 1, 0),
> +	PLL_RATE(1416000000, 59, 1, 0),
> +	PLL_RATE(1440000000, 60, 1, 0),
> +	PLL_RATE(1464000000, 61, 1, 0),
> +	PLL_RATE(1488000000, 62, 1, 0),
> +	PLL_RATE(1512000000, 63, 1, 0),
> +	PLL_RATE(1536000000, 64, 1, 0),
> +	{ /* sentinel */ },
> +};
> +
> +PNAME(p_xtal)		= { "xtal" };
> +PNAME(p_fclk_div)	= { "fixed_pll" };
> +PNAME(p_cpu_clk)	= { "sys_pll" };
> +PNAME(p_clk81)		= { "fclk_div3", "fclk_div4", "fclk_div5" };
> +PNAME(p_mali)		= { "fclk_div3", "fclk_div4", "fclk_div5",
> +			    "fclk_div7", "zero" };
> +
> +static u32 mux_table_clk81[]	= { 6, 5, 7 };
> +static u32 mux_table_mali[]	= { 6, 5, 7, 4, 0 };
> +
> +static struct pll_conf pll_confs = {
> +	.m		= PARM(0x00, 0,  9),
> +	.n		= PARM(0x00, 9,  5),
> +	.od		= PARM(0x00, 16, 2),
> +};
> +
> +static struct pll_conf sys_pll_conf = {
> +	.m		= PARM(0x00, 0,  9),
> +	.n		= PARM(0x00, 9,  5),
> +	.od		= PARM(0x00, 16, 2),
> +	.rate_table	= sys_pll_rate_table,
> +};
> +
> +static struct composite_conf clk81_conf __initdata = {

Can this be __initconstdata?

> +	.mux_table		= mux_table_clk81,
> +	.mux_flags		= CLK_MUX_READ_ONLY,
> +	.mux_parm		= PARM(0x00, 12, 3),
> +	.div_parm		= PARM(0x00, 0, 7),
> +	.gate_parm		= PARM(0x00, 7, 1),
> +};
> +
> +static struct composite_conf mali_conf __initdata = {
> +	.mux_table		= mux_table_mali,
> +	.mux_parm		= PARM(0x00, 9, 3),
> +	.div_parm		= PARM(0x00, 0, 7),
> +	.gate_parm		= PARM(0x00, 8, 1),
> +};
> +
> +static struct clk_conf meson8b_xtal_conf __initdata =
> +	FIXED_RATE_P(MESON8B_REG_CTL0_ADDR, CLKID_XTAL, "xtal",
> +		     CLK_IS_ROOT, PARM(0x00, 4, 7));
> +
> +static struct clk_conf meson8b_clk_confs[] __initdata = {

const?

> +	FIXED_RATE(CLKID_ZERO, "zero", CLK_IS_ROOT, 0),
> +	PLL(MESON8B_REG_PLL_FIXED, CLKID_PLL_FIXED, "fixed_pll",
> +	    p_xtal, 0, &pll_confs),
> +	PLL(MESON8B_REG_PLL_VID, CLKID_PLL_VID, "vid_pll",
> +	    p_xtal, 0, &pll_confs),
> +	PLL(MESON8B_REG_PLL_SYS, CLKID_PLL_SYS, "sys_pll",
> +	    p_xtal, 0, &sys_pll_conf),
> +	FIXED_FACTOR_DIV(CLKID_FCLK_DIV2, "fclk_div2", p_fclk_div, 0, 2),
> +	FIXED_FACTOR_DIV(CLKID_FCLK_DIV3, "fclk_div3", p_fclk_div, 0, 3),
> +	FIXED_FACTOR_DIV(CLKID_FCLK_DIV4, "fclk_div4", p_fclk_div, 0, 4),
> +	FIXED_FACTOR_DIV(CLKID_FCLK_DIV5, "fclk_div5", p_fclk_div, 0, 5),
> +	FIXED_FACTOR_DIV(CLKID_FCLK_DIV7, "fclk_div7", p_fclk_div, 0, 7),
> +	CPU(MESON8B_REG_SYS_CPU_CNTL1, CLKID_CPUCLK, "a5_clk", p_cpu_clk),
> +	COMPOSITE(MESON8B_REG_HHI_MPEG, CLKID_CLK81, "clk81", p_clk81,
> +		  CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED, &clk81_conf),
> +	COMPOSITE(MESON8B_REG_MALI, CLKID_MALI, "mali", p_mali,
> +		  CLK_IGNORE_UNUSED, &mali_conf),

Ah here we see the defines used.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 3/3] clk: meson8b: Add support for Meson8b clocks

[Carlo Caione]

On Fri, May 29, 2015 at 12:01 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> + */
>> +
>> +#include <linux/clk.h>
>
> Do you need this include?

I guess I have to revise all the includes :)

>> +#include <linux/clk-provider.h>
>> +#include <linux/kernel.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/slab.h>
>> +#include <dt-bindings/clock/meson8b-clkc.h>
>> +
>> +#include "clkc.h"
>> +
>> +#define MESON8B_REG_CTL0_ADDR                0x0000
>> +#define MESON8B_REG_SYS_CPU_CNTL1    0x015c
>> +#define MESON8B_REG_HHI_MPEG         0x0174
>> +#define MESON8B_REG_MALI             0x01b0
>> +#define MESON8B_REG_PLL_FIXED                0x0280
>> +#define MESON8B_REG_PLL_SYS          0x0300
>> +#define MESON8B_REG_PLL_VID          0x0320
>> +
>> +static struct pll_rate_table sys_pll_rate_table[] = {
>
> const?

Ok

>> +
>> +static struct composite_conf clk81_conf __initdata = {
>
> Can this be __initconstdata?

Yes, probably better static const ... __initconst

>> +     .mux_table              = mux_table_clk81,
>> +     .mux_flags              = CLK_MUX_READ_ONLY,
>> +     .mux_parm               = PARM(0x00, 12, 3),
>> +     .div_parm               = PARM(0x00, 0, 7),
>> +     .gate_parm              = PARM(0x00, 7, 1),
>> +};
>> +
>> +static struct composite_conf mali_conf __initdata = {
>> +     .mux_table              = mux_table_mali,
>> +     .mux_parm               = PARM(0x00, 9, 3),
>> +     .div_parm               = PARM(0x00, 0, 7),
>> +     .gate_parm              = PARM(0x00, 8, 1),
>> +};
>> +
>> +static struct clk_conf meson8b_xtal_conf __initdata =
>> +     FIXED_RATE_P(MESON8B_REG_CTL0_ADDR, CLKID_XTAL, "xtal",
>> +                  CLK_IS_ROOT, PARM(0x00, 4, 7));
>> +
>> +static struct clk_conf meson8b_clk_confs[] __initdata = {
>
> const?

Agree

>> +     FIXED_RATE(CLKID_ZERO, "zero", CLK_IS_ROOT, 0),
>> +     PLL(MESON8B_REG_PLL_FIXED, CLKID_PLL_FIXED, "fixed_pll",
>> +         p_xtal, 0, &pll_confs),
>> +     PLL(MESON8B_REG_PLL_VID, CLKID_PLL_VID, "vid_pll",
>> +         p_xtal, 0, &pll_confs),
>> +     PLL(MESON8B_REG_PLL_SYS, CLKID_PLL_SYS, "sys_pll",
>> +         p_xtal, 0, &sys_pll_conf),
>> +     FIXED_FACTOR_DIV(CLKID_FCLK_DIV2, "fclk_div2", p_fclk_div, 0, 2),
>> +     FIXED_FACTOR_DIV(CLKID_FCLK_DIV3, "fclk_div3", p_fclk_div, 0, 3),
>> +     FIXED_FACTOR_DIV(CLKID_FCLK_DIV4, "fclk_div4", p_fclk_div, 0, 4),
>> +     FIXED_FACTOR_DIV(CLKID_FCLK_DIV5, "fclk_div5", p_fclk_div, 0, 5),
>> +     FIXED_FACTOR_DIV(CLKID_FCLK_DIV7, "fclk_div7", p_fclk_div, 0, 7),
>> +     CPU(MESON8B_REG_SYS_CPU_CNTL1, CLKID_CPUCLK, "a5_clk", p_cpu_clk),
>> +     COMPOSITE(MESON8B_REG_HHI_MPEG, CLKID_CLK81, "clk81", p_clk81,
>> +               CLK_SET_RATE_NO_REPARENT | CLK_IGNORE_UNUSED, &clk81_conf),
>> +     COMPOSITE(MESON8B_REG_MALI, CLKID_MALI, "mali", p_mali,
>> +               CLK_IGNORE_UNUSED, &mali_conf),
>
> Ah here we see the defines used.

Thank you!

-- 
Carlo Caione

[PATCH v2 0/3] clk: meson: Add clock controller

[Carlo Caione]

On Sat, May 16, 2015 at 12:48 PM, Carlo Caione <carlo@caione.org> wrote:
> From: Carlo Caione <carlo@endlessm.com>
>
> This patchset introduces preliminary support for the clock controller
> found on Amlogic MesonX SoCs and enables it for the Meson8b.
>
> DTS patches with support for Meson8b SoCs will follow.
>
> Changelog:
>
> v1->v2:
>    * fix double CLK_IGNORE_UNUSED in clk-pll.c
>    * fix comment in meson8b-clkc.h

Gentle ping

-- 
Carlo Caione