[PATCH 0/7] ARM: berlin: refactor the clock

[PATCH 0/7] ARM: berlin: refactor the clock

[Antoine Tenart]

Hi,

Marvell Berlin SoCs have a chip control register set providing several
individual registers dealing with various controllers (pinctrl, reset,
clk). This chip controller is described by a single DT node since the
individual registers are spread among the chip control register bank.

Marvell Berlin also have a system control register set providing several
individual registers for pinctrl or adc.

A series was sent[1] to correctly handle these two nodes, by introducing
a Berlin mfd controller driver. The series converted the existing
pin-controller and reset drivers to take the changes into account.

Because the Berlin clock driver was still working with the above
modification, Berlin clock reworks were not included into the previous
series. Also the clock reworks change some of the clock framework
functions, so having a dedicated series helps.

This series aims to convert the Berlin clock driver to use the proper
way of dealing with the chip and system controller on Berlin SoCs. To do
this changes were done in the clk-mux and clk-gate helpers so they can
handle regmaps alongside iomem addresses.

This series has been tested on the Marvell Berlin BG2Q DMP.

This series is based on v3.19 and [1].

Thanks!

Antoine

[1] https://lkml.org/lkml/2015/2/11/367


Antoine Tenart (7):
  clk: convert clock mux to accept regmap
  clk: convert clock gate to accept regmap
  clk: berlin: use regmap
  Documentation: bindings: move the Berlin clock documentation
  ARM: berlin: rework the clock node for BG2
  ARM: berlin: rework the clock node for BG2CD
  ARM: berlin: rework the clock node for BG2Q

 .../devicetree/bindings/arm/marvell,berlin.txt     |  35 -------
 .../devicetree/bindings/clock/marvell,berlin.txt   |  31 ++++++
 arch/arm/boot/dts/berlin2.dtsi                     |  43 ++++----
 arch/arm/boot/dts/berlin2cd.dtsi                   |  39 +++----
 arch/arm/boot/dts/berlin2q.dtsi                    |  53 +++++-----
 drivers/clk/berlin/berlin2-avpll.c                 |  72 +++++++------
 drivers/clk/berlin/berlin2-avpll.h                 |  13 ++-
 drivers/clk/berlin/berlin2-div.c                   |  82 +++++----------
 drivers/clk/berlin/berlin2-div.h                   |   4 +-
 drivers/clk/berlin/berlin2-pll.c                   |  16 +--
 drivers/clk/berlin/berlin2-pll.h                   |   7 +-
 drivers/clk/berlin/bg2.c                           | 112 +++++++++++----------
 drivers/clk/berlin/bg2q.c                          |  64 +++++++-----
 drivers/clk/clk-gate.c                             |  94 +++++++++++++----
 drivers/clk/clk-mux.c                              |  72 +++++++++++--
 include/linux/clk-provider.h                       |  51 ++++++++--
 16 files changed, 467 insertions(+), 321 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/marvell,berlin.txt

-- 
2.3.0

[PATCH 1/7] clk: convert clock mux to accept regmap

[Antoine Tenart]

Rework the clk_mux helpers to either use an iomem base address or a
regmap.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/clk/clk-mux.c        | 72 +++++++++++++++++++++++++++++++++++++-------
 include/linux/clk-provider.h | 39 +++++++++++++++++++-----
 2 files changed, 93 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 6e1ecf94bf58..1cd0ecd35ce3 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -42,7 +42,11 @@ static u8 clk_mux_get_parent(struct clk_hw *hw)
 	 * OTOH, pmd_trace_clk_mux_ck uses a separate bit for each clock, so
 	 * val = 0x4 really means "bit 2, index starts at bit 0"
 	 */
-	val = clk_readl(mux->reg) >> mux->shift;
+	if (mux->reg_type == CLK_REG_TYPE_IOMEM)
+		val = clk_readl(mux->reg.iomem) >> mux->shift;
+	else
+		regmap_read(mux->reg.regmap, mux->reg.offset, &val);
+
 	val &= mux->mask;
 
 	if (mux->table) {
@@ -83,20 +87,24 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
 			index++;
 	}
 
-	if (mux->lock)
-		spin_lock_irqsave(mux->lock, flags);
+	if (mux->reg_type == CLK_REG_TYPE_IOMEM && mux->reg.lock)
+		spin_lock_irqsave(mux->reg.lock, flags);
 
 	if (mux->flags & CLK_MUX_HIWORD_MASK) {
 		val = mux->mask << (mux->shift + 16);
 	} else {
-		val = clk_readl(mux->reg);
+		val = clk_readl(mux->reg.iomem);
 		val &= ~(mux->mask << mux->shift);
 	}
 	val |= index << mux->shift;
-	clk_writel(val, mux->reg);
 
-	if (mux->lock)
-		spin_unlock_irqrestore(mux->lock, flags);
+	if (mux->reg_type == CLK_REG_TYPE_IOMEM)
+		clk_writel(val, mux->reg.iomem);
+	else
+		regmap_write(mux->reg.regmap, mux->reg.offset, val);
+
+	if (mux->reg_type == CLK_REG_TYPE_IOMEM && mux->reg.lock)
+		spin_unlock_irqrestore(mux->reg.lock, flags);
 
 	return 0;
 }
@@ -113,10 +121,10 @@ const struct clk_ops clk_mux_ro_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_mux_ro_ops);
 
-struct clk *clk_register_mux_table(struct device *dev, const char *name,
+struct clk *__clk_register_mux_table(struct device *dev, const char *name,
 		const char **parent_names, u8 num_parents, unsigned long flags,
-		void __iomem *reg, u8 shift, u32 mask,
-		u8 clk_mux_flags, u32 *table, spinlock_t *lock)
+		union clk_reg reg, enum clk_reg_type reg_type, u8 shift,
+		u32 mask, u8 clk_mux_flags, u32 *table)
 {
 	struct clk_mux *mux;
 	struct clk *clk;
@@ -149,10 +157,10 @@ struct clk *clk_register_mux_table(struct device *dev, const char *name,
 
 	/* struct clk_mux assignments */
 	mux->reg = reg;
+	mux->reg_type = reg_type;
 	mux->shift = shift;
 	mux->mask = mask;
 	mux->flags = clk_mux_flags;
-	mux->lock = lock;
 	mux->table = table;
 	mux->hw.init = &init;
 
@@ -163,8 +171,37 @@ struct clk *clk_register_mux_table(struct device *dev, const char *name,
 
 	return clk;
 }
+
+struct clk *clk_register_mux_table(struct device *dev, const char *name,
+		const char **parent_names, u8 num_parents, unsigned long flags,
+		void __iomem *reg, u8 shift, u32 mask,
+		u8 clk_mux_flags, u32 *table, spinlock_t *lock)
+{
+	union clk_reg clk_reg;
+
+	clk_reg.iomem = reg;
+	clk_reg.lock = lock;
+	return __clk_register_mux_table(dev, name, parent_names, num_parents,
+					flags, clk_reg, CLK_REG_TYPE_IOMEM,
+					shift, mask, clk_mux_flags, table);
+}
 EXPORT_SYMBOL_GPL(clk_register_mux_table);
 
+struct clk *clk_register_mux_table_regmap(struct device *dev, const char *name,
+		const char **parent_names, u8 num_parents, unsigned long flags,
+		struct regmap *regmap, unsigned int offset, u8 shift, u32 mask,
+		u8 clk_mux_flags, u32 *table)
+{
+	union clk_reg clk_reg;
+
+	clk_reg.regmap = regmap;
+	clk_reg.offset = offset;
+	return __clk_register_mux_table(dev, name, parent_names, num_parents,
+					flags, clk_reg, CLK_REG_TYPE_REGMAP,
+					shift, mask, clk_mux_flags, table);
+}
+EXPORT_SYMBOL_GPL(clk_register_mux_table_regmap);
+
 struct clk *clk_register_mux(struct device *dev, const char *name,
 		const char **parent_names, u8 num_parents, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
@@ -177,3 +214,16 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 				      NULL, lock);
 }
 EXPORT_SYMBOL_GPL(clk_register_mux);
+
+struct clk *clk_register_mux_regmap(struct device *dev, const char *name,
+		const char **parent_names, u8 num_parents, unsigned long flags,
+		struct regmap *regmap, unsigned int offset, u8 shift, u8 width,
+		u8 clk_mux_flags)
+{
+	u32 mask = BIT(width) - 1;
+
+	return clk_register_mux_table_regmap(dev, name, parent_names,
+				      num_parents, flags, regmap, offset, shift,
+				      mask, clk_mux_flags, NULL);
+}
+EXPORT_SYMBOL_GPL(clk_register_mux_regmap);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index d936409520f8..3d2b9ab2130d 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -14,6 +14,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/regmap.h>
 
 #ifdef CONFIG_COMMON_CLK
 
@@ -258,6 +259,22 @@ struct clk *clk_register_fixed_rate_with_accuracy(struct device *dev,
 
 void of_fixed_clk_setup(struct device_node *np);
 
+union clk_reg {
+	struct {
+		void __iomem	*iomem;
+		spinlock_t	*lock;
+	};
+	struct {
+		struct regmap	*regmap;
+		unsigned int	offset;
+	};
+};
+
+enum clk_reg_type {
+	CLK_REG_TYPE_IOMEM,
+	CLK_REG_TYPE_REGMAP,
+};
+
 /**
  * struct clk_gate - gating clock
  *
@@ -384,13 +401,13 @@ struct clk *clk_register_divider_table(struct device *dev, const char *name,
  *	indicate changing mux bits.
  */
 struct clk_mux {
-	struct clk_hw	hw;
-	void __iomem	*reg;
-	u32		*table;
-	u32		mask;
-	u8		shift;
-	u8		flags;
-	spinlock_t	*lock;
+	struct clk_hw		hw;
+	union clk_reg		reg;
+	enum clk_reg_type	reg_type;
+	u32			*table;
+	u32			mask;
+	u8			shift;
+	u8			flags;
 };
 
 #define CLK_MUX_INDEX_ONE		BIT(0)
@@ -405,11 +422,19 @@ struct clk *clk_register_mux(struct device *dev, const char *name,
 		const char **parent_names, u8 num_parents, unsigned long flags,
 		void __iomem *reg, u8 shift, u8 width,
 		u8 clk_mux_flags, spinlock_t *lock);
+struct clk *clk_register_mux_regmap(struct device *dev, const char *name,
+		const char **parent_names, u8 num_parents, unsigned long flags,
+		struct regmap *regmap, unsigned int offset, u8 shift, u8 width,
+		u8 clk_mux_flags);
 
 struct clk *clk_register_mux_table(struct device *dev, const char *name,
 		const char **parent_names, u8 num_parents, unsigned long flags,
 		void __iomem *reg, u8 shift, u32 mask,
 		u8 clk_mux_flags, u32 *table, spinlock_t *lock);
+struct clk *clk_register_mux_table_regmap(struct device *dev, const char *name,
+		const char **parent_names, u8 num_parents, unsigned long flags,
+		struct regmap *regmap, unsigned int offset, u8 shift, u32 mask,
+		u8 clk_mux_flags, u32 *table);
 
 void of_fixed_factor_clk_setup(struct device_node *node);
 
-- 
2.3.0

[PATCH 2/7] clk: convert clock gate to accept regmap

[Antoine Tenart]

Rework the clk_gate helpers to either use an iomem base address or a
regmap.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/clk/clk-gate.c       | 94 +++++++++++++++++++++++++++++++++-----------
 include/linux/clk-provider.h | 12 ++++--
 2 files changed, 80 insertions(+), 26 deletions(-)

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 51fd87fb7ba6..6cc27ebf6573 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -11,6 +11,7 @@
 
 #include <linux/clk-provider.h>
 #include <linux/module.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/io.h>
 #include <linux/err.h>
@@ -50,15 +51,18 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
 
 	set ^= enable;
 
-	if (gate->lock)
-		spin_lock_irqsave(gate->lock, flags);
+	if (gate->reg_type == CLK_REG_TYPE_IOMEM && gate->reg.lock)
+		spin_lock_irqsave(gate->reg.lock, flags);
 
 	if (gate->flags & CLK_GATE_HIWORD_MASK) {
 		reg = BIT(gate->bit_idx + 16);
 		if (set)
 			reg |= BIT(gate->bit_idx);
 	} else {
-		reg = clk_readl(gate->reg);
+		if (gate->reg_type == CLK_REG_TYPE_IOMEM)
+			reg = clk_readl(gate->reg.iomem);
+		else
+			regmap_read(gate->reg.regmap, gate->reg.offset, &reg);
 
 		if (set)
 			reg |= BIT(gate->bit_idx);
@@ -66,10 +70,13 @@ static void clk_gate_endisable(struct clk_hw *hw, int enable)
 			reg &= ~BIT(gate->bit_idx);
 	}
 
-	clk_writel(reg, gate->reg);
+	if (gate->reg_type == CLK_REG_TYPE_IOMEM)
+		clk_writel(reg, gate->reg.iomem);
+	else
+		regmap_write(gate->reg.regmap, gate->reg.offset, reg);
 
-	if (gate->lock)
-		spin_unlock_irqrestore(gate->lock, flags);
+	if (gate->reg_type == CLK_REG_TYPE_IOMEM && gate->reg.lock)
+		spin_unlock_irqrestore(gate->reg.lock, flags);
 }
 
 static int clk_gate_enable(struct clk_hw *hw)
@@ -89,7 +96,10 @@ static int clk_gate_is_enabled(struct clk_hw *hw)
 	u32 reg;
 	struct clk_gate *gate = to_clk_gate(hw);
 
-	reg = clk_readl(gate->reg);
+	if (gate->reg_type == CLK_REG_TYPE_IOMEM)
+		reg = clk_readl(gate->reg.iomem);
+	else
+		regmap_read(gate->reg.regmap, gate->reg.offset, &reg);
 
 	/* if a set bit disables this clk, flip it before masking */
 	if (gate->flags & CLK_GATE_SET_TO_DISABLE)
@@ -107,21 +117,10 @@ const struct clk_ops clk_gate_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_gate_ops);
 
-/**
- * clk_register_gate - register a gate clock with the clock framework
- * @dev: device that is registering this clock
- * @name: name of this clock
- * @parent_name: name of this clock's parent
- * @flags: framework-specific flags for this clock
- * @reg: register address to control gating of this clock
- * @bit_idx: which bit in the register controls gating of this clock
- * @clk_gate_flags: gate-specific flags for this clock
- * @lock: shared register lock for this clock
- */
-struct clk *clk_register_gate(struct device *dev, const char *name,
+struct clk *__clk_register_gate(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
-		void __iomem *reg, u8 bit_idx,
-		u8 clk_gate_flags, spinlock_t *lock)
+		union clk_reg reg, enum clk_reg_type type, u8 bit_idx,
+		u8 clk_gate_flags)
 {
 	struct clk_gate *gate;
 	struct clk *clk;
@@ -149,9 +148,9 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
 
 	/* struct clk_gate assignments */
 	gate->reg = reg;
+	gate->reg_type = type;
 	gate->bit_idx = bit_idx;
 	gate->flags = clk_gate_flags;
-	gate->lock = lock;
 	gate->hw.init = &init;
 
 	clk = clk_register(dev, &gate->hw);
@@ -161,4 +160,55 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
 
 	return clk;
 }
+
+/**
+ * clk_register_gate - register a gate clock with the clock framework
+ * @dev: device that is registering this clock
+ * @name: name of this clock
+ * @parent_name: name of this clock's parent
+ * @flags: framework-specific flags for this clock
+ * @reg: register address to control gating of this clock
+ * @bit_idx: which bit in the register controls gating of this clock
+ * @clk_gate_flags: gate-specific flags for this clock
+ * @lock: shared register lock for this clock
+ */
+struct clk *clk_register_gate(struct device *dev, const char *name,
+		const char *parent_name, unsigned long flags,
+		void __iomem *reg, u8 bit_idx,
+		u8 clk_gate_flags, spinlock_t *lock)
+{
+	union clk_reg clk_reg;
+
+	clk_reg.iomem = reg;
+	clk_reg.lock = lock;
+
+	return __clk_register_gate(dev, name, parent_name, flags, clk_reg,
+				   CLK_REG_TYPE_IOMEM, bit_idx, clk_gate_flags);
+}
 EXPORT_SYMBOL_GPL(clk_register_gate);
+
+/**
+ * clk_register_gate_regmap - register a gate clock with the clock framework
+ * @dev: device that is registering this clock
+ * @name: name of this clock
+ * @parent_name: name of this clock's parent
+ * @flags: framework-specific flags for this clock
+ * @reg: regmap to control the gating of this clock
+ * @bit_idx: which bit in the register controls gating of this clock
+ * @clk_gate_flags: gate-specific flags for this clock
+ * @lock: shared register lock for this clock
+ */
+struct clk *clk_register_gate_regmap(struct device *dev, const char *name,
+		const char *parent_name, unsigned long flags,
+		struct regmap *reg, unsigned int offset, u8 bit_idx,
+		u8 clk_gate_flags)
+{
+	union clk_reg clk_reg;
+
+	clk_reg.regmap = reg;
+	clk_reg.offset = offset;
+	return __clk_register_gate(dev, name, parent_name, flags, clk_reg,
+				   CLK_REG_TYPE_REGMAP, bit_idx,
+				   clk_gate_flags);
+}
+EXPORT_SYMBOL_GPL(clk_register_gate_regmap);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 3d2b9ab2130d..b3f7018960fb 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -297,10 +297,10 @@ enum clk_reg_type {
  */
 struct clk_gate {
 	struct clk_hw hw;
-	void __iomem	*reg;
-	u8		bit_idx;
-	u8		flags;
-	spinlock_t	*lock;
+	union clk_reg		reg;
+	enum clk_reg_type	reg_type;
+	u8			bit_idx;
+	u8			flags;
 };
 
 #define CLK_GATE_SET_TO_DISABLE		BIT(0)
@@ -311,6 +311,10 @@ struct clk *clk_register_gate(struct device *dev, const char *name,
 		const char *parent_name, unsigned long flags,
 		void __iomem *reg, u8 bit_idx,
 		u8 clk_gate_flags, spinlock_t *lock);
+struct clk *clk_register_gate_regmap(struct device *dev, const char *name,
+		const char *parent_name, unsigned long flags,
+		struct regmap *reg, unsigned int offset, u8 bit_idx,
+		u8 clk_gate_flags);
 
 struct clk_div_table {
 	unsigned int	val;
-- 
2.3.0

[PATCH 3/7] clk: berlin: use regmap

[Antoine Tenart]

The Berlin clock driver was sharing a DT node with the pin controller
and the reset driver. All these device are now a sub-node of the chip
and system controllers and a regmap is available thanks to syscon to
access the registers safely.

Rework the Berlin clock driver to use the regmap provided by syscon.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 drivers/clk/berlin/berlin2-avpll.c |  72 +++++++++++++-----------
 drivers/clk/berlin/berlin2-avpll.h |  13 +++--
 drivers/clk/berlin/berlin2-div.c   |  82 +++++++++------------------
 drivers/clk/berlin/berlin2-div.h   |   4 +-
 drivers/clk/berlin/berlin2-pll.c   |  16 ++++--
 drivers/clk/berlin/berlin2-pll.h   |   7 ++-
 drivers/clk/berlin/bg2.c           | 112 +++++++++++++++++++------------------
 drivers/clk/berlin/bg2q.c          |  64 +++++++++++++--------
 8 files changed, 188 insertions(+), 182 deletions(-)

diff --git a/drivers/clk/berlin/berlin2-avpll.c b/drivers/clk/berlin/berlin2-avpll.c
index fd0f26c38465..7244afb2251f 100644
--- a/drivers/clk/berlin/berlin2-avpll.c
+++ b/drivers/clk/berlin/berlin2-avpll.c
@@ -21,6 +21,7 @@
 #include <linux/kernel.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
 #include "berlin2-avpll.h"
@@ -115,7 +116,8 @@
 
 struct berlin2_avpll_vco {
 	struct clk_hw hw;
-	void __iomem *base;
+	struct regmap *regmap;
+	unsigned int offset;
 	u8 flags;
 };
 
@@ -126,7 +128,7 @@ static int berlin2_avpll_vco_is_enabled(struct clk_hw *hw)
 	struct berlin2_avpll_vco *vco = to_avpll_vco(hw);
 	u32 reg;
 
-	reg = readl_relaxed(vco->base + VCO_CTRL0);
+	regmap_read(vco->regmap, vco->offset + VCO_CTRL0, &reg);
 	if (vco->flags & BERLIN2_AVPLL_BIT_QUIRK)
 		reg >>= 4;
 
@@ -138,12 +140,12 @@ static int berlin2_avpll_vco_enable(struct clk_hw *hw)
 	struct berlin2_avpll_vco *vco = to_avpll_vco(hw);
 	u32 reg;
 
-	reg = readl_relaxed(vco->base + VCO_CTRL0);
+	regmap_read(vco->regmap, vco->offset + VCO_CTRL0, &reg);
 	if (vco->flags & BERLIN2_AVPLL_BIT_QUIRK)
 		reg |= VCO_POWERUP << 4;
 	else
 		reg |= VCO_POWERUP;
-	writel_relaxed(reg, vco->base + VCO_CTRL0);
+	regmap_write(vco->regmap, vco->offset + VCO_CTRL0, reg);
 
 	return 0;
 }
@@ -153,12 +155,12 @@ static void berlin2_avpll_vco_disable(struct clk_hw *hw)
 	struct berlin2_avpll_vco *vco = to_avpll_vco(hw);
 	u32 reg;
 
-	reg = readl_relaxed(vco->base + VCO_CTRL0);
+	regmap_read(vco->regmap, vco->offset + VCO_CTRL0, &reg);
 	if (vco->flags & BERLIN2_AVPLL_BIT_QUIRK)
 		reg &= ~(VCO_POWERUP << 4);
 	else
 		reg &= ~VCO_POWERUP;
-	writel_relaxed(reg, vco->base + VCO_CTRL0);
+	regmap_write(vco->regmap, vco->offset + VCO_CTRL0, reg);
 }
 
 static u8 vco_refdiv[] = { 1, 2, 4, 3 };
@@ -171,7 +173,7 @@ berlin2_avpll_vco_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	u64 freq = parent_rate;
 
 	/* AVPLL VCO frequency: Fvco = (Fref / refdiv) * fbdiv */
-	reg = readl_relaxed(vco->base + VCO_CTRL1);
+	regmap_read(vco->regmap, vco->offset + VCO_CTRL1, &reg);
 	refdiv = (reg & VCO_REFDIV_MASK) >> VCO_REFDIV_SHIFT;
 	refdiv = vco_refdiv[refdiv];
 	fbdiv = (reg & VCO_FBDIV_MASK) >> VCO_FBDIV_SHIFT;
@@ -188,9 +190,10 @@ static const struct clk_ops berlin2_avpll_vco_ops = {
 	.recalc_rate	= berlin2_avpll_vco_recalc_rate,
 };
 
-struct clk * __init berlin2_avpll_vco_register(void __iomem *base,
-			       const char *name, const char *parent_name,
-			       u8 vco_flags, unsigned long flags)
+struct clk * __init berlin2_avpll_vco_register(struct regmap *regmap,
+			       unsigned int offset, const char *name,
+			       const char *parent_name, u8 vco_flags,
+			       unsigned long flags)
 {
 	struct berlin2_avpll_vco *vco;
 	struct clk_init_data init;
@@ -199,7 +202,8 @@ struct clk * __init berlin2_avpll_vco_register(void __iomem *base,
 	if (!vco)
 		return ERR_PTR(-ENOMEM);
 
-	vco->base = base;
+	vco->regmap = regmap;
+	vco->offset = offset;
 	vco->flags = vco_flags;
 	vco->hw.init = &init;
 	init.name = name;
@@ -213,7 +217,8 @@ struct clk * __init berlin2_avpll_vco_register(void __iomem *base,
 
 struct berlin2_avpll_channel {
 	struct clk_hw hw;
-	void __iomem *base;
+	struct regmap *regmap;
+	unsigned int offset;
 	u8 flags;
 	u8 index;
 };
@@ -228,7 +233,7 @@ static int berlin2_avpll_channel_is_enabled(struct clk_hw *hw)
 	if (ch->index == 7)
 		return 1;
 
-	reg = readl_relaxed(ch->base + VCO_CTRL10);
+	regmap_read(ch->regmap, ch->offset + VCO_CTRL10, &reg);
 	reg &= VCO_POWERUP_CH1 << ch->index;
 
 	return !!reg;
@@ -239,9 +244,9 @@ static int berlin2_avpll_channel_enable(struct clk_hw *hw)
 	struct berlin2_avpll_channel *ch = to_avpll_channel(hw);
 	u32 reg;
 
-	reg = readl_relaxed(ch->base + VCO_CTRL10);
+	regmap_read(ch->regmap, ch->offset + VCO_CTRL10, &reg);
 	reg |= VCO_POWERUP_CH1 << ch->index;
-	writel_relaxed(reg, ch->base + VCO_CTRL10);
+	regmap_write(ch->regmap, ch->offset + VCO_CTRL10, reg);
 
 	return 0;
 }
@@ -251,9 +256,9 @@ static void berlin2_avpll_channel_disable(struct clk_hw *hw)
 	struct berlin2_avpll_channel *ch = to_avpll_channel(hw);
 	u32 reg;
 
-	reg = readl_relaxed(ch->base + VCO_CTRL10);
+	regmap_read(ch->regmap, ch->offset + VCO_CTRL10, &reg);
 	reg &= ~(VCO_POWERUP_CH1 << ch->index);
-	writel_relaxed(reg, ch->base + VCO_CTRL10);
+	regmap_write(ch->regmap, ch->offset + VCO_CTRL10, reg);
 }
 
 static const u8 div_hdmi[] = { 1, 2, 4, 6 };
@@ -266,7 +271,7 @@ berlin2_avpll_channel_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	u32 reg, div_av2, div_av3, divider = 1;
 	u64 freq = parent_rate;
 
-	reg = readl_relaxed(ch->base + VCO_CTRL30);
+	regmap_read(ch->regmap, ch->offset + VCO_CTRL30, &reg);
 	if ((reg & (VCO_DPLL_CH1_ENABLE << ch->index)) == 0)
 		goto skip_div;
 
@@ -275,13 +280,13 @@ berlin2_avpll_channel_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	 *    (sync1 * div_hdmi * div_av1 * div_av2 * div_av3)
 	 */
 
-	reg = readl_relaxed(ch->base + VCO_SYNC1n(ch->index));
+	regmap_read(ch->regmap, ch->offset + VCO_SYNC1n(ch->index), &reg);
 	/* BG2/BG2CDs SYNC1 reg on AVPLL_B channel 1 is shifted by 4 */
 	if (ch->flags & BERLIN2_AVPLL_BIT_QUIRK && ch->index == 0)
 		reg >>= 4;
 	divider = reg & VCO_SYNC1_MASK;
 
-	reg = readl_relaxed(ch->base + VCO_SYNC2n(ch->index));
+	regmap_read(ch->regmap, ch->offset + VCO_SYNC2n(ch->index), &reg);
 	freq *= reg & VCO_SYNC2_MASK;
 
 	/* Channel 8 has no dividers */
@@ -292,7 +297,8 @@ berlin2_avpll_channel_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	 * HDMI divider start at VCO_CTRL11, bit 7; MSB is enable, lower 2 bit
 	 * determine divider.
 	 */
-	reg = readl_relaxed(ch->base + VCO_CTRL11) >> 7;
+	regmap_read(ch->regmap, ch->offset + VCO_CTRL11, &reg);
+	reg >>= 7;
 	reg = (reg >> (ch->index * 3));
 	if (reg & BIT(2))
 		divider *= div_hdmi[reg & 0x3];
@@ -302,10 +308,10 @@ berlin2_avpll_channel_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	 * determine divider.
 	 */
 	if (ch->index == 0) {
-		reg = readl_relaxed(ch->base + VCO_CTRL11);
+		regmap_read(ch->regmap, ch->offset + VCO_CTRL11, &reg);
 		reg >>= 28;
 	} else {
-		reg = readl_relaxed(ch->base + VCO_CTRL12);
+		regmap_read(ch->regmap, ch->offset + VCO_CTRL12, &reg);
 		reg >>= (ch->index-1) * 3;
 	}
 	if (reg & BIT(2))
@@ -316,13 +322,13 @@ berlin2_avpll_channel_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	 * zero is not a valid value.
 	 */
 	if (ch->index < 2) {
-		reg = readl_relaxed(ch->base + VCO_CTRL12);
+		regmap_read(ch->regmap, ch->offset + VCO_CTRL12, &reg);
 		reg >>= 18 + (ch->index * 7);
 	} else if (ch->index < 7) {
-		reg = readl_relaxed(ch->base + VCO_CTRL13);
+		regmap_read(ch->regmap, ch->offset + VCO_CTRL13, &reg);
 		reg >>= (ch->index - 2) * 7;
 	} else {
-		reg = readl_relaxed(ch->base + VCO_CTRL14);
+		regmap_read(ch->regmap, ch->offset + VCO_CTRL14, &reg);
 	}
 	div_av2 = reg & 0x7f;
 	if (div_av2)
@@ -334,10 +340,10 @@ berlin2_avpll_channel_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	 * are allowed. AV3 != 0 divides by AV2/2, AV3=0 is bypass.
 	 */
 	if (ch->index < 6) {
-		reg = readl_relaxed(ch->base + VCO_CTRL14);
+		regmap_read(ch->regmap, ch->offset + VCO_CTRL14, &reg);
 		reg >>= 7 + (ch->index * 4);
 	} else {
-		reg = readl_relaxed(ch->base + VCO_CTRL15);
+		regmap_read(ch->regmap, ch->offset + VCO_CTRL15, &reg);
 	}
 	div_av3 = reg & 0xf;
 	if (div_av2 && div_av3)
@@ -364,9 +370,10 @@ static const struct clk_ops berlin2_avpll_channel_ops = {
  */
 static const u8 quirk_index[] __initconst = { 0, 6, 5, 4, 3, 2, 1, 7 };
 
-struct clk * __init berlin2_avpll_channel_register(void __iomem *base,
-			   const char *name, u8 index, const char *parent_name,
-			   u8 ch_flags, unsigned long flags)
+struct clk * __init berlin2_avpll_channel_register(struct regmap *regmap,
+			   unsigned int offset, const char *name, u8 index,
+			   const char *parent_name, u8 ch_flags,
+			   unsigned long flags)
 {
 	struct berlin2_avpll_channel *ch;
 	struct clk_init_data init;
@@ -375,7 +382,8 @@ struct clk * __init berlin2_avpll_channel_register(void __iomem *base,
 	if (!ch)
 		return ERR_PTR(-ENOMEM);
 
-	ch->base = base;
+	ch->regmap = regmap;
+	ch->offset = offset;
 	if (ch_flags & BERLIN2_AVPLL_SCRAMBLE_QUIRK)
 		ch->index = quirk_index[index];
 	else
diff --git a/drivers/clk/berlin/berlin2-avpll.h b/drivers/clk/berlin/berlin2-avpll.h
index a37f5068d299..cdc2c5bd4706 100644
--- a/drivers/clk/berlin/berlin2-avpll.h
+++ b/drivers/clk/berlin/berlin2-avpll.h
@@ -19,18 +19,21 @@
 #ifndef __BERLIN2_AVPLL_H
 #define __BERLIN2_AVPLL_H
 
+#include <linux/regmap.h>
+
 struct clk;
 
 #define BERLIN2_AVPLL_BIT_QUIRK		BIT(0)
 #define BERLIN2_AVPLL_SCRAMBLE_QUIRK	BIT(1)
 
 struct clk * __init
-berlin2_avpll_vco_register(void __iomem *base, const char *name,
-	   const char *parent_name, u8 vco_flags, unsigned long flags);
+berlin2_avpll_vco_register(struct regmap *regmap, unsigned int offset,
+	   const char *name, const char *parent_name, u8 vco_flags,
+	   unsigned long flags);
 
 struct clk * __init
-berlin2_avpll_channel_register(void __iomem *base, const char *name,
-		       u8 index, const char *parent_name, u8 ch_flags,
-		       unsigned long flags);
+berlin2_avpll_channel_register(struct regmap *regmap, unsigned int offset,
+		       const char *name, u8 index, const char *parent_name,
+		       u8 ch_flags, unsigned long flags);
 
 #endif /* __BERLIN2_AVPLL_H */
diff --git a/drivers/clk/berlin/berlin2-div.c b/drivers/clk/berlin/berlin2-div.c
index 81ff97f8aa0b..ff708fe831d0 100644
--- a/drivers/clk/berlin/berlin2-div.c
+++ b/drivers/clk/berlin/berlin2-div.c
@@ -20,6 +20,7 @@
 #include <linux/clk-provider.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 
@@ -63,9 +64,8 @@
 
 struct berlin2_div {
 	struct clk_hw hw;
-	void __iomem *base;
+	struct regmap *regmap;
 	struct berlin2_div_map map;
-	spinlock_t *lock;
 };
 
 #define to_berlin2_div(hw) container_of(hw, struct berlin2_div, hw)
@@ -78,15 +78,9 @@ static int berlin2_div_is_enabled(struct clk_hw *hw)
 	struct berlin2_div_map *map = &div->map;
 	u32 reg;
 
-	if (div->lock)
-		spin_lock(div->lock);
-
-	reg = readl_relaxed(div->base + map->gate_offs);
+	regmap_read(div->regmap, map->gate_offs, &reg);
 	reg >>= map->gate_shift;
 
-	if (div->lock)
-		spin_unlock(div->lock);
-
 	return (reg & 0x1);
 }
 
@@ -96,15 +90,9 @@ static int berlin2_div_enable(struct clk_hw *hw)
 	struct berlin2_div_map *map = &div->map;
 	u32 reg;
 
-	if (div->lock)
-		spin_lock(div->lock);
-
-	reg = readl_relaxed(div->base + map->gate_offs);
+	regmap_read(div->regmap, map->gate_offs, &reg);
 	reg |= BIT(map->gate_shift);
-	writel_relaxed(reg, div->base + map->gate_offs);
-
-	if (div->lock)
-		spin_unlock(div->lock);
+	regmap_write(div->regmap, map->gate_offs, reg);
 
 	return 0;
 }
@@ -115,15 +103,9 @@ static void berlin2_div_disable(struct clk_hw *hw)
 	struct berlin2_div_map *map = &div->map;
 	u32 reg;
 
-	if (div->lock)
-		spin_lock(div->lock);
-
-	reg = readl_relaxed(div->base + map->gate_offs);
+	regmap_read(div->regmap, map->gate_offs, &reg);
 	reg &= ~BIT(map->gate_shift);
-	writel_relaxed(reg, div->base + map->gate_offs);
-
-	if (div->lock)
-		spin_unlock(div->lock);
+	regmap_write(div->regmap, map->gate_offs, reg);
 }
 
 static int berlin2_div_set_parent(struct clk_hw *hw, u8 index)
@@ -132,28 +114,24 @@ static int berlin2_div_set_parent(struct clk_hw *hw, u8 index)
 	struct berlin2_div_map *map = &div->map;
 	u32 reg;
 
-	if (div->lock)
-		spin_lock(div->lock);
-
 	/* index == 0 is PLL_SWITCH */
-	reg = readl_relaxed(div->base + map->pll_switch_offs);
+	regmap_read(div->regmap, map->pll_switch_offs, &reg);
 	if (index == 0)
 		reg &= ~BIT(map->pll_switch_shift);
 	else
 		reg |= BIT(map->pll_switch_shift);
-	writel_relaxed(reg, div->base + map->pll_switch_offs);
+	regmap_write(div->regmap, map->pll_switch_offs, reg);
 
 	/* index > 0 is PLL_SELECT */
 	if (index > 0) {
-		reg = readl_relaxed(div->base + map->pll_select_offs);
+		regmap_read(div->regmap, map->pll_switch_offs,
+			    &reg);
 		reg &= ~(PLL_SELECT_MASK << map->pll_select_shift);
 		reg |= (index - 1) << map->pll_select_shift;
-		writel_relaxed(reg, div->base + map->pll_select_offs);
+		regmap_write(div->regmap, map->pll_switch_offs,
+			     reg);
 	}
 
-	if (div->lock)
-		spin_unlock(div->lock);
-
 	return 0;
 }
 
@@ -164,22 +142,17 @@ static u8 berlin2_div_get_parent(struct clk_hw *hw)
 	u32 reg;
 	u8 index = 0;
 
-	if (div->lock)
-		spin_lock(div->lock);
-
 	/* PLL_SWITCH == 0 is index 0 */
-	reg = readl_relaxed(div->base + map->pll_switch_offs);
+	regmap_read(div->regmap, map->pll_switch_offs, &reg);
 	reg &= BIT(map->pll_switch_shift);
 	if (reg) {
-		reg = readl_relaxed(div->base + map->pll_select_offs);
+		regmap_read(div->regmap, map->pll_switch_offs,
+			    &reg);
 		reg >>= map->pll_select_shift;
 		reg &= PLL_SELECT_MASK;
 		index = 1 + reg;
 	}
 
-	if (div->lock)
-		spin_unlock(div->lock);
-
 	return index;
 }
 
@@ -190,13 +163,11 @@ static unsigned long berlin2_div_recalc_rate(struct clk_hw *hw,
 	struct berlin2_div_map *map = &div->map;
 	u32 divsw, div3sw, divider = 1;
 
-	if (div->lock)
-		spin_lock(div->lock);
+	regmap_read(div->regmap, map->div_switch_offs, &divsw);
+	divsw &= (1 << map->div_switch_shift);
 
-	divsw = readl_relaxed(div->base + map->div_switch_offs) &
-		(1 << map->div_switch_shift);
-	div3sw = readl_relaxed(div->base + map->div3_switch_offs) &
-		(1 << map->div3_switch_shift);
+	regmap_read(div->regmap, map->div3_switch_offs, &div3sw);
+	div3sw &= (1 << map->div3_switch_shift);
 
 	/* constant divide-by-3 (dominant) */
 	if (div3sw != 0) {
@@ -207,15 +178,13 @@ static unsigned long berlin2_div_recalc_rate(struct clk_hw *hw,
 	/* clock divider determined by DIV_SELECT */
 	} else {
 		u32 reg;
-		reg = readl_relaxed(div->base + map->div_select_offs);
+		regmap_read(div->regmap, map->div_select_offs,
+			    &reg);
 		reg >>= map->div_select_shift;
 		reg &= DIV_SELECT_MASK;
 		divider = clk_div[reg];
 	}
 
-	if (div->lock)
-		spin_unlock(div->lock);
-
 	return parent_rate / divider;
 }
 
@@ -236,9 +205,9 @@ static const struct clk_ops berlin2_div_mux_ops = {
 
 struct clk * __init
 berlin2_div_register(const struct berlin2_div_map *map,
-		     void __iomem *base, const char *name, u8 div_flags,
+		     struct regmap *regmap, const char *name, u8 div_flags,
 		     const char **parent_names, int num_parents,
-		     unsigned long flags, spinlock_t *lock)
+		     unsigned long flags)
 {
 	const struct clk_ops *mux_ops = &berlin2_div_mux_ops;
 	const struct clk_ops *rate_ops = &berlin2_div_rate_ops;
@@ -251,8 +220,7 @@ berlin2_div_register(const struct berlin2_div_map *map,
 
 	/* copy div_map to allow __initconst */
 	memcpy(&div->map, map, sizeof(*map));
-	div->base = base;
-	div->lock = lock;
+	div->regmap = regmap;
 
 	if ((div_flags & BERLIN2_DIV_HAS_GATE) == 0)
 		gate_ops = NULL;
diff --git a/drivers/clk/berlin/berlin2-div.h b/drivers/clk/berlin/berlin2-div.h
index 15e3384f3116..8b726561cd00 100644
--- a/drivers/clk/berlin/berlin2-div.h
+++ b/drivers/clk/berlin/berlin2-div.h
@@ -82,8 +82,8 @@ struct berlin2_div_data {
 
 struct clk * __init
 berlin2_div_register(const struct berlin2_div_map *map,
-	     void __iomem *base,  const char *name, u8 div_flags,
+	     struct regmap *regmap, const char *name, u8 div_flags,
 	     const char **parent_names, int num_parents,
-	     unsigned long flags,  spinlock_t *lock);
+	     unsigned long flags);
 
 #endif /* __BERLIN2_DIV_H */
diff --git a/drivers/clk/berlin/berlin2-pll.c b/drivers/clk/berlin/berlin2-pll.c
index bdc506b03824..96fbb643d37d 100644
--- a/drivers/clk/berlin/berlin2-pll.c
+++ b/drivers/clk/berlin/berlin2-pll.c
@@ -21,6 +21,7 @@
 #include <linux/kernel.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 #include <asm/div64.h>
 
@@ -36,7 +37,8 @@ struct berlin2_pll_map {
 
 struct berlin2_pll {
 	struct clk_hw hw;
-	void __iomem *base;
+	struct regmap *regmap;
+	unsigned int offset;
 	struct berlin2_pll_map map;
 };
 
@@ -64,7 +66,7 @@ berlin2_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 	u32 val, fbdiv, rfdiv, vcodivsel, vcodiv;
 	u64 rate = parent_rate;
 
-	val = readl_relaxed(pll->base + SPLL_CTRL0);
+	regmap_read(pll->regmap, SPLL_CTRL0, &val);
 	fbdiv = (val >> map->fbdiv_shift) & FBDIV_MASK;
 	rfdiv = (val >> map->rfdiv_shift) & RFDIV_MASK;
 	if (rfdiv == 0) {
@@ -72,7 +74,7 @@ berlin2_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
 		rfdiv = 1;
 	}
 
-	val = readl_relaxed(pll->base + SPLL_CTRL1);
+	regmap_read(pll->regmap, SPLL_CTRL1, &val);
 	vcodivsel = (val >> map->divsel_shift) & DIVSEL_MASK;
 	vcodiv = map->vcodiv[vcodivsel];
 	if (vcodiv == 0) {
@@ -93,8 +95,9 @@ static const struct clk_ops berlin2_pll_ops = {
 
 struct clk * __init
 berlin2_pll_register(const struct berlin2_pll_map *map,
-		     void __iomem *base, const char *name,
-		     const char *parent_name, unsigned long flags)
+		     struct regmap *regmap, unsigned int offset,
+		     const char *name, const char *parent_name,
+		     unsigned long flags)
 {
 	struct clk_init_data init;
 	struct berlin2_pll *pll;
@@ -105,7 +108,8 @@ berlin2_pll_register(const struct berlin2_pll_map *map,
 
 	/* copy pll_map to allow __initconst */
 	memcpy(&pll->map, map, sizeof(*map));
-	pll->base = base;
+	pll->regmap = regmap;
+	pll->offset = offset;
 	pll->hw.init = &init;
 	init.name = name;
 	init.ops = &berlin2_pll_ops;
diff --git a/drivers/clk/berlin/berlin2-pll.h b/drivers/clk/berlin/berlin2-pll.h
index 8831ce27ac1e..9e2f6b172e51 100644
--- a/drivers/clk/berlin/berlin2-pll.h
+++ b/drivers/clk/berlin/berlin2-pll.h
@@ -19,6 +19,8 @@
 #ifndef __BERLIN2_PLL_H
 #define __BERLIN2_PLL_H
 
+#include <linux/regmap.h>
+
 struct clk;
 
 struct berlin2_pll_map {
@@ -31,7 +33,8 @@ struct berlin2_pll_map {
 
 struct clk * __init
 berlin2_pll_register(const struct berlin2_pll_map *map,
-		     void __iomem *base, const char *name,
-		     const char *parent_name, unsigned long flags);
+		     struct regmap *regmap, unsigned int offset,
+		     const char *name, const char *parent_name,
+		     unsigned long flags);
 
 #endif /* __BERLIN2_PLL_H */
diff --git a/drivers/clk/berlin/bg2.c b/drivers/clk/berlin/bg2.c
index 515fb133495c..d6a379ac01a6 100644
--- a/drivers/clk/berlin/bg2.c
+++ b/drivers/clk/berlin/bg2.c
@@ -20,8 +20,10 @@
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
 #include <dt-bindings/clock/berlin2.h>
@@ -94,8 +96,6 @@
 #define	MAX_CLKS 41
 static struct clk *clks[MAX_CLKS];
 static struct clk_onecell_data clk_data;
-static DEFINE_SPINLOCK(lock);
-static void __iomem *gbase;
 
 enum {
 	REFCLK, VIDEO_EXT0,
@@ -502,14 +502,19 @@ static const struct berlin2_gate_data bg2_gates[] __initconst = {
 
 static void __init berlin2_clock_setup(struct device_node *np)
 {
+	struct device_node *parent_np = of_get_parent(np);
+	struct regmap *regmap;
 	const char *parent_names[9];
 	struct clk *clk;
 	u8 avpll_flags = 0;
 	int n;
 
-	gbase = of_iomap(np, 0);
-	if (!gbase)
+	regmap = syscon_node_to_regmap(parent_np);
+	of_node_put(parent_np);
+	if (IS_ERR(regmap)) {
+		pr_err("Unable to retrieve regmap: %ld\n", PTR_ERR(regmap));
 		return;
+	}
 
 	/* overwrite default clock names with DT provided ones */
 	clk = of_clk_get_by_name(np, clk_names[REFCLK]);
@@ -525,119 +530,119 @@ static void __init berlin2_clock_setup(struct device_node *np)
 	}
 
 	/* simple register PLLs */
-	clk = berlin2_pll_register(&bg2_pll_map, gbase + REG_SYSPLLCTL0,
+	clk = berlin2_pll_register(&bg2_pll_map, regmap, REG_SYSPLLCTL0,
 				   clk_names[SYSPLL], clk_names[REFCLK], 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
-	clk = berlin2_pll_register(&bg2_pll_map, gbase + REG_MEMPLLCTL0,
+	clk = berlin2_pll_register(&bg2_pll_map, regmap, REG_MEMPLLCTL0,
 				   clk_names[MEMPLL], clk_names[REFCLK], 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
-	clk = berlin2_pll_register(&bg2_pll_map, gbase + REG_CPUPLLCTL0,
+	clk = berlin2_pll_register(&bg2_pll_map, regmap, REG_CPUPLLCTL0,
 				   clk_names[CPUPLL], clk_names[REFCLK], 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
 	if (of_device_is_compatible(np, "marvell,berlin2-global-register"))
 		avpll_flags |= BERLIN2_AVPLL_SCRAMBLE_QUIRK;
 
 	/* audio/video VCOs */
-	clk = berlin2_avpll_vco_register(gbase + REG_AVPLLCTL0, "avpll_vcoA",
+	clk = berlin2_avpll_vco_register(regmap, REG_AVPLLCTL0, "avpll_vcoA",
 			 clk_names[REFCLK], avpll_flags, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
 	for (n = 0; n < 8; n++) {
-		clk = berlin2_avpll_channel_register(gbase + REG_AVPLLCTL0,
+		clk = berlin2_avpll_channel_register(regmap, REG_AVPLLCTL0,
 			     clk_names[AVPLL_A1 + n], n, "avpll_vcoA",
 			     avpll_flags, 0);
 		if (IS_ERR(clk))
-			goto bg2_fail;
+			return;
 	}
 
-	clk = berlin2_avpll_vco_register(gbase + REG_AVPLLCTL31, "avpll_vcoB",
+	clk = berlin2_avpll_vco_register(regmap, REG_AVPLLCTL31, "avpll_vcoB",
 				 clk_names[REFCLK], BERLIN2_AVPLL_BIT_QUIRK |
 				 avpll_flags, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
 	for (n = 0; n < 8; n++) {
-		clk = berlin2_avpll_channel_register(gbase + REG_AVPLLCTL31,
+		clk = berlin2_avpll_channel_register(regmap, REG_AVPLLCTL31,
 			     clk_names[AVPLL_B1 + n], n, "avpll_vcoB",
 			     BERLIN2_AVPLL_BIT_QUIRK | avpll_flags, 0);
 		if (IS_ERR(clk))
-			goto bg2_fail;
+			return;
 	}
 
 	/* reference clock bypass switches */
 	parent_names[0] = clk_names[SYSPLL];
 	parent_names[1] = clk_names[REFCLK];
-	clk = clk_register_mux(NULL, "syspll_byp", parent_names, 2,
-			       0, gbase + REG_CLKSWITCH0, 0, 1, 0, &lock);
+	clk = clk_register_mux_regmap(NULL, "syspll_byp", parent_names, 2,
+				      0, regmap, REG_CLKSWITCH0, 0, 1, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 	clk_names[SYSPLL] = __clk_get_name(clk);
 
 	parent_names[0] = clk_names[MEMPLL];
 	parent_names[1] = clk_names[REFCLK];
-	clk = clk_register_mux(NULL, "mempll_byp", parent_names, 2,
-			       0, gbase + REG_CLKSWITCH0, 1, 1, 0, &lock);
+	clk = clk_register_mux_regmap(NULL, "mempll_byp", parent_names, 2,
+				      0, regmap, REG_CLKSWITCH0, 1, 1, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 	clk_names[MEMPLL] = __clk_get_name(clk);
 
 	parent_names[0] = clk_names[CPUPLL];
 	parent_names[1] = clk_names[REFCLK];
-	clk = clk_register_mux(NULL, "cpupll_byp", parent_names, 2,
-			       0, gbase + REG_CLKSWITCH0, 2, 1, 0, &lock);
+	clk = clk_register_mux_regmap(NULL, "cpupll_byp", parent_names, 2,
+				      0, regmap, REG_CLKSWITCH0, 2, 1, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 	clk_names[CPUPLL] = __clk_get_name(clk);
 
 	/* clock muxes */
 	parent_names[0] = clk_names[AVPLL_B3];
 	parent_names[1] = clk_names[AVPLL_A3];
-	clk = clk_register_mux(NULL, clk_names[AUDIO1_PLL], parent_names, 2,
-			       0, gbase + REG_CLKSELECT2, 29, 1, 0, &lock);
+	clk = clk_register_mux_regmap(NULL, clk_names[AUDIO1_PLL], parent_names,
+				      2, 0, regmap, REG_CLKSELECT2, 29, 1, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
 	parent_names[0] = clk_names[VIDEO0_PLL];
 	parent_names[1] = clk_names[VIDEO_EXT0];
-	clk = clk_register_mux(NULL, clk_names[VIDEO0_IN], parent_names, 2,
-			       0, gbase + REG_CLKSELECT3, 4, 1, 0, &lock);
+	clk = clk_register_mux_regmap(NULL, clk_names[VIDEO0_IN], parent_names,
+				      2, 0, regmap, REG_CLKSELECT3, 4, 1, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
 	parent_names[0] = clk_names[VIDEO1_PLL];
 	parent_names[1] = clk_names[VIDEO_EXT0];
-	clk = clk_register_mux(NULL, clk_names[VIDEO1_IN], parent_names, 2,
-			       0, gbase + REG_CLKSELECT3, 6, 1, 0, &lock);
+	clk = clk_register_mux_regmap(NULL, clk_names[VIDEO1_IN], parent_names,
+				      2, 0, regmap, REG_CLKSELECT3, 6, 1, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
 	parent_names[0] = clk_names[AVPLL_A2];
 	parent_names[1] = clk_names[AVPLL_B2];
-	clk = clk_register_mux(NULL, clk_names[VIDEO1_PLL], parent_names, 2,
-			       0, gbase + REG_CLKSELECT3, 7, 1, 0, &lock);
+	clk = clk_register_mux_regmap(NULL, clk_names[VIDEO1_PLL], parent_names,
+				      2, 0,regmap, REG_CLKSELECT3, 7, 1, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
 	parent_names[0] = clk_names[VIDEO2_PLL];
 	parent_names[1] = clk_names[VIDEO_EXT0];
-	clk = clk_register_mux(NULL, clk_names[VIDEO2_IN], parent_names, 2,
-			       0, gbase + REG_CLKSELECT3, 9, 1, 0, &lock);
+	clk = clk_register_mux_regmap(NULL, clk_names[VIDEO2_IN], parent_names,
+				      2, 0, regmap, REG_CLKSELECT3, 9, 1, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
 	parent_names[0] = clk_names[AVPLL_B1];
 	parent_names[1] = clk_names[AVPLL_A5];
-	clk = clk_register_mux(NULL, clk_names[VIDEO2_PLL], parent_names, 2,
-			       0, gbase + REG_CLKSELECT3, 10, 1, 0, &lock);
+	clk = clk_register_mux_regmap(NULL, clk_names[VIDEO2_PLL], parent_names,
+				      2, 0, regmap, REG_CLKSELECT3, 10, 1, 0);
 	if (IS_ERR(clk))
-		goto bg2_fail;
+		return;
 
 	/* clock divider cells */
 	for (n = 0; n < ARRAY_SIZE(bg2_divs); n++) {
@@ -647,18 +652,18 @@ static void __init berlin2_clock_setup(struct device_node *np)
 		for (k = 0; k < dd->num_parents; k++)
 			parent_names[k] = clk_names[dd->parent_ids[k]];
 
-		clks[CLKID_SYS + n] = berlin2_div_register(&dd->map, gbase,
+		clks[CLKID_SYS + n] = berlin2_div_register(&dd->map, regmap,
 				dd->name, dd->div_flags, parent_names,
-				dd->num_parents, dd->flags, &lock);
+				dd->num_parents, dd->flags);
 	}
 
 	/* clock gate cells */
 	for (n = 0; n < ARRAY_SIZE(bg2_gates); n++) {
 		const struct berlin2_gate_data *gd = &bg2_gates[n];
 
-		clks[CLKID_GETH0 + n] = clk_register_gate(NULL, gd->name,
-			    gd->parent_name, gd->flags, gbase + REG_CLKENABLE,
-			    gd->bit_idx, 0, &lock);
+		clks[CLKID_GETH0 + n] = clk_register_gate_regmap(NULL, gd->name,
+			    gd->parent_name, gd->flags, regmap, REG_CLKENABLE,
+			    gd->bit_idx, 0);
 	}
 
 	/* twdclk is derived from cpu/3 */
@@ -672,7 +677,7 @@ static void __init berlin2_clock_setup(struct device_node *np)
 
 		pr_err("%s: Unable to register leaf clock %d\n",
 		       np->full_name, n);
-		goto bg2_fail;
+		return;
 	}
 
 	/* register clk-provider */
@@ -681,11 +686,8 @@ static void __init berlin2_clock_setup(struct device_node *np)
 	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
 
 	return;
-
-bg2_fail:
-	iounmap(gbase);
 }
 CLK_OF_DECLARE(berlin2_clock, "marvell,berlin2-chip-ctrl",
 	       berlin2_clock_setup);
-CLK_OF_DECLARE(berlin2cd_clock, "marvell,berlin2cd-chip-ctrl",
+CLK_OF_DECLARE(berlin2cd_clock, "marvell,berlin2-clk",
 	       berlin2_clock_setup);
diff --git a/drivers/clk/berlin/bg2q.c b/drivers/clk/berlin/bg2q.c
index 440ef81ab15c..25bedca9e350 100644
--- a/drivers/clk/berlin/bg2q.c
+++ b/drivers/clk/berlin/bg2q.c
@@ -19,9 +19,12 @@
 
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/device.h>
 #include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
+#include <linux/regmap.h>
 #include <linux/slab.h>
 
 #include <dt-bindings/clock/berlin2q.h>
@@ -46,11 +49,9 @@
 #define REG_SDIO1XIN_CLKCTL	0x015c
 
 #define	MAX_CLKS 27
+
 static struct clk *clks[MAX_CLKS];
 static struct clk_onecell_data clk_data;
-static DEFINE_SPINLOCK(lock);
-static void __iomem *gbase;
-static void __iomem *cpupll_base;
 
 enum {
 	REFCLK,
@@ -290,24 +291,45 @@ static const struct berlin2_gate_data bg2q_gates[] __initconst = {
 
 static void __init berlin2q_clock_setup(struct device_node *np)
 {
+	struct device_node *parent_np = of_get_parent(np);
 	const char *parent_names[9];
+	struct regmap *regmap, *cpupll_regmap;
+	struct regmap_config *rmconfig;
+	struct resource res;
 	struct clk *clk;
+	void __iomem *cpupll_base;
 	int n;
 
-	gbase = of_iomap(np, 0);
-	if (!gbase) {
-		pr_err("%s: Unable to map global base\n", np->full_name);
+	regmap = syscon_node_to_regmap(parent_np);
+	of_node_put(parent_np);
+	if (IS_ERR(regmap)) {
+		pr_err("Unable to retrieve regmap: %ld\n", PTR_ERR(regmap));
 		return;
 	}
 
+	rmconfig = kzalloc(sizeof(*rmconfig), GFP_KERNEL);
+	if (!rmconfig)
+		return;
+
+	if (of_address_to_resource(parent_np, 1, &res))
+		return;
+
 	/* BG2Q CPU PLL is not part of global registers */
-	cpupll_base = of_iomap(np, 1);
+	cpupll_base = ioremap(res.start, resource_size(&res));
 	if (!cpupll_base) {
 		pr_err("%s: Unable to map cpupll base\n", np->full_name);
-		iounmap(gbase);
 		return;
 	}
 
+	rmconfig->reg_bits = 32;
+	rmconfig->val_bits = 32;
+	rmconfig->reg_stride = 4;
+	rmconfig->max_register = resource_size(&res);
+
+	cpupll_regmap = regmap_init_mmio(NULL, cpupll_base, rmconfig);
+	if (IS_ERR(cpupll_regmap))
+		return;
+
 	/* overwrite default clock names with DT provided ones */
 	clk = of_clk_get_by_name(np, clk_names[REFCLK]);
 	if (!IS_ERR(clk)) {
@@ -316,15 +338,15 @@ static void __init berlin2q_clock_setup(struct device_node *np)
 	}
 
 	/* simple register PLLs */
-	clk = berlin2_pll_register(&bg2q_pll_map, gbase + REG_SYSPLLCTL0,
+	clk = berlin2_pll_register(&bg2q_pll_map, regmap, REG_SYSPLLCTL0,
 				   clk_names[SYSPLL], clk_names[REFCLK], 0);
 	if (IS_ERR(clk))
-		goto bg2q_fail;
+		return;
 
-	clk = berlin2_pll_register(&bg2q_pll_map, cpupll_base,
+	clk = berlin2_pll_register(&bg2q_pll_map, cpupll_regmap, 0,
 				   clk_names[CPUPLL], clk_names[REFCLK], 0);
 	if (IS_ERR(clk))
-		goto bg2q_fail;
+		return;
 
 	/* TODO: add BG2Q AVPLL */
 
@@ -341,18 +363,18 @@ static void __init berlin2q_clock_setup(struct device_node *np)
 		for (k = 0; k < dd->num_parents; k++)
 			parent_names[k] = clk_names[dd->parent_ids[k]];
 
-		clks[CLKID_SYS + n] = berlin2_div_register(&dd->map, gbase,
+		clks[CLKID_SYS + n] = berlin2_div_register(&dd->map, regmap,
 				dd->name, dd->div_flags, parent_names,
-				dd->num_parents, dd->flags, &lock);
+				dd->num_parents, dd->flags);
 	}
 
 	/* clock gate cells */
 	for (n = 0; n < ARRAY_SIZE(bg2q_gates); n++) {
 		const struct berlin2_gate_data *gd = &bg2q_gates[n];
 
-		clks[CLKID_GFX2DAXI + n] = clk_register_gate(NULL, gd->name,
-			    gd->parent_name, gd->flags, gbase + REG_CLKENABLE,
-			    gd->bit_idx, 0, &lock);
+		clks[CLKID_GFX2DAXI + n] = clk_register_gate_regmap(NULL,
+				gd->name, gd->parent_name, gd->flags,
+				regmap, REG_CLKENABLE, gd->bit_idx, 0);
 	}
 
 	/*
@@ -370,7 +392,7 @@ static void __init berlin2q_clock_setup(struct device_node *np)
 
 		pr_err("%s: Unable to register leaf clock %d\n",
 		       np->full_name, n);
-		goto bg2q_fail;
+		return;
 	}
 
 	/* register clk-provider */
@@ -379,10 +401,6 @@ static void __init berlin2q_clock_setup(struct device_node *np)
 	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
 
 	return;
-
-bg2q_fail:
-	iounmap(cpupll_base);
-	iounmap(gbase);
 }
-CLK_OF_DECLARE(berlin2q_clock, "marvell,berlin2q-chip-ctrl",
+CLK_OF_DECLARE(berlin2q_clock, "marvell,berlin2q-clk",
 	       berlin2q_clock_setup);
-- 
2.3.0

[PATCH 4/7] Documentation: bindings: move the Berlin clock documentation

[Antoine Tenart]

The Berlin clock documentation was part of the Marvell Berlin SoC
documentation because the Berlin clock configuration was inside the
chip controller. With the recent rework of the chip and system
controller handling (now an MFD driver registers all sub-devices of the
two soc and system controller nodes and each device has its own
sub-node), the documentation of the Berlin clock driver can be moved to
the generic clock documentation directory.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 .../devicetree/bindings/arm/marvell,berlin.txt     | 35 ----------------------
 .../devicetree/bindings/clock/marvell,berlin.txt   | 31 +++++++++++++++++++
 2 files changed, 31 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/marvell,berlin.txt

diff --git a/Documentation/devicetree/bindings/arm/marvell,berlin.txt b/Documentation/devicetree/bindings/arm/marvell,berlin.txt
index 080953daf9f5..7f9b3ccdf25b 100644
--- a/Documentation/devicetree/bindings/arm/marvell,berlin.txt
+++ b/Documentation/devicetree/bindings/arm/marvell,berlin.txt
@@ -39,38 +39,3 @@ cpu-ctrl@f7dd0000 {
 	compatible = "marvell,berlin-cpu-ctrl";
 	reg = <0xf7dd0000 0x10000>;
 };
-
-* Clock provider binding
-
-As clock related registers are spread among the chip control registers, the
-chip control node also provides the clocks. Marvell Berlin2 (BG2, BG2CD, BG2Q)
-SoCs share the same IP for PLLs and clocks, with some minor differences in
-features and register layout.
-
-Required properties:
-- #clock-cells: shall be set to 1
-- clocks: clock specifiers referencing the core clock input clocks
-- clock-names: array of strings describing the input clock specifiers above.
-    Allowed clock-names for the reference clocks are
-      "refclk" for the SoCs osciallator input on all SoCs,
-    and SoC-specific input clocks for
-      BG2/BG2CD: "video_ext0" for the external video clock input
-
-Clocks provided by core clocks shall be referenced by a clock specifier
-indexing one of the provided clocks. Refer to dt-bindings/clock/berlin<soc>.h
-for the corresponding index mapping.
-
-Example:
-
-chip: chip-control@ea0000 {
-	compatible = "marvell,berlin2-chip-ctrl";
-	#clock-cells = <1>;
-	reg = <0xea0000 0x400>;
-	clocks = <&refclk>, <&externaldev 0>;
-	clock-names = "refclk", "video_ext0";
-};
-
-sysctrl: system-controller@d000 {
-	compatible = "marvell,berlin2-system-ctrl";
-	reg = <0xd000 0x100>;
-};
diff --git a/Documentation/devicetree/bindings/clock/marvell,berlin.txt b/Documentation/devicetree/bindings/clock/marvell,berlin.txt
new file mode 100644
index 000000000000..c611c495f3ff
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/marvell,berlin.txt
@@ -0,0 +1,31 @@
+Device Tree Clock bindings for Marvell Berlin
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Clock related registers are spread among the chip control registers. Berlin
+clock node should be a sub-node of the chip controller node. Marvell Berlin2
+(BG2, BG2CD, BG2Q) SoCs share the same IP for PLLs and clocks, with some
+minor differences in features and register layout.
+
+Required properties:
+- compatible: must be "marvell,berlin2-clk" or "marvell,berlin2q-clk"
+- #clock-cells: must be 1
+- clocks: must be the input parent clock phandle
+- clock-names: name of the input parent clock
+	Allowed clock-names for the reference clocks are
+	"refclk" for the SoCs oscillator input on all SoCs,
+	and SoC-specific input clocks for
+	BG2/BG2CD: "video_ext0" for the external video clock input
+
+
+Example:
+
+chip_clk: clock {
+	compatible = "marvell,berlin2q-clk";
+
+	#clock-cells = <1>;
+	clocks = <&refclk>;
+	clock-names = "refclk";
+};
-- 
2.3.0

[PATCH 5/7] ARM: berlin: rework the clock node for BG2

[Antoine Tenart]

With the introduction of the Berlin mfd controller driver, all drivers
previously sharing the chip and system controller nodes now have their
own sub-node.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2.dtsi | 43 +++++++++++++++++++++++-------------------
 1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi
index 498ffd5b0e05..c18cf5fbbaa6 100644
--- a/arch/arm/boot/dts/berlin2.dtsi
+++ b/arch/arm/boot/dts/berlin2.dtsi
@@ -56,7 +56,7 @@
 		sdhci0: sdhci@ab0000 {
 			compatible = "mrvl,pxav3-mmc";
 			reg = <0xab0000 0x200>;
-			clocks = <&chip CLKID_SDIO0XIN>, <&chip CLKID_SDIO0>;
+			clocks = <&chip_clk CLKID_SDIO0XIN>, <&chip_clk CLKID_SDIO0>;
 			clock-names = "io", "core";
 			interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
@@ -65,7 +65,7 @@
 		sdhci1: sdhci@ab0800 {
 			compatible = "mrvl,pxav3-mmc";
 			reg = <0xab0800 0x200>;
-			clocks = <&chip CLKID_SDIO1XIN>, <&chip CLKID_SDIO1>;
+			clocks = <&chip_clk CLKID_SDIO1XIN>, <&chip_clk CLKID_SDIO1>;
 			clock-names = "io", "core";
 			interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
@@ -75,7 +75,7 @@
 			compatible = "mrvl,pxav3-mmc";
 			reg = <0xab1000 0x200>;
 			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_NFC_ECC>, <&chip CLKID_NFC>;
+			clocks = <&chip_clk CLKID_NFC_ECC>, <&chip_clk CLKID_NFC>;
 			clock-names = "io", "core";
 			pinctrl-0 = <&emmc_pmux>;
 			pinctrl-names = "default";
@@ -105,13 +105,13 @@
 			compatible = "arm,cortex-a9-twd-timer";
 			reg = <0xad0600 0x20>;
 			interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_TWD>;
+			clocks = <&chip_clk CLKID_TWD>;
 		};
 
 		eth1: ethernet@b90000 {
 			compatible = "marvell,pxa168-eth";
 			reg = <0xb90000 0x10000>;
-			clocks = <&chip CLKID_GETH1>;
+			clocks = <&chip_clk CLKID_GETH1>;
 			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
 			/* set by bootloader */
 			local-mac-address = [00 00 00 00 00 00];
@@ -134,7 +134,7 @@
 		eth0: ethernet@e50000 {
 			compatible = "marvell,pxa168-eth";
 			reg = <0xe50000 0x10000>;
-			clocks = <&chip CLKID_GETH0>;
+			clocks = <&chip_clk CLKID_GETH0>;
 			interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
 			/* set by bootloader */
 			local-mac-address = [00 00 00 00 00 00];
@@ -233,7 +233,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c00 0x14>;
 				interrupts = <8>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "okay";
 			};
@@ -242,7 +242,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c14 0x14>;
 				interrupts = <9>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "okay";
 			};
@@ -251,7 +251,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c28 0x14>;
 				interrupts = <10>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -260,7 +260,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c3c 0x14>;
 				interrupts = <11>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -269,7 +269,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c50 0x14>;
 				interrupts = <12>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -278,7 +278,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c64 0x14>;
 				interrupts = <13>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -287,7 +287,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c78 0x14>;
 				interrupts = <14>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -296,7 +296,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c8c 0x14>;
 				interrupts = <15>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -315,7 +315,7 @@
 			compatible = "marvell,berlin2-ahci", "generic-ahci";
 			reg = <0xe90000 0x1000>;
 			interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_SATA>;
+			clocks = <&chip_clk CLKID_SATA>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 
@@ -335,7 +335,7 @@
 		sata_phy: phy@e900a0 {
 			compatible = "marvell,berlin2-sata-phy";
 			reg = <0xe900a0 0x200>;
-			clocks = <&chip CLKID_SATA>;
+			clocks = <&chip_clk CLKID_SATA>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			#phy-cells = <1>;
@@ -353,9 +353,14 @@
 		chip: chip-controller@ea0000 {
 			compatible = "marvell,berlin2-chip-ctrl", "syscon";
 			reg = <0xea0000 0x400>;
-			#clock-cells = <1>;
-			clocks = <&refclk>;
-			clock-names = "refclk";
+
+			chip_clk: clock {
+				compatible = "marvell,berlin2-clk";
+
+				#clock-cells = <1>;
+				clocks = <&refclk>;
+				clock-names = "refclk";
+			};
 
 			soc_pinctrl: pin-controller {
 				compatible = "marvell,berlin2-soc-pinctrl";
-- 
2.3.0

[PATCH 6/7] ARM: berlin: rework the clock node for BG2CD

[Antoine Tenart]

With the introduction of the Berlin mfd controller driver, all drivers
previously sharing the chip and system controller nodes now have their
own sub-node.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2cd.dtsi | 39 ++++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/arch/arm/boot/dts/berlin2cd.dtsi b/arch/arm/boot/dts/berlin2cd.dtsi
index 314cde70a3ba..34d3ec957243 100644
--- a/arch/arm/boot/dts/berlin2cd.dtsi
+++ b/arch/arm/boot/dts/berlin2cd.dtsi
@@ -48,7 +48,7 @@
 		sdhci0: sdhci@ab0000 {
 			compatible = "mrvl,pxav3-mmc";
 			reg = <0xab0000 0x200>;
-			clocks = <&chip CLKID_SDIO0XIN>, <&chip CLKID_SDIO0>;
+			clocks = <&chip_clk CLKID_SDIO0XIN>, <&chip_clk CLKID_SDIO0>;
 			clock-names = "io", "core";
 			interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
@@ -72,7 +72,7 @@
 			compatible = "arm,cortex-a9-twd-timer";
 			reg = <0xad0600 0x20>;
 			interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_TWD>;
+			clocks = <&chip_clk CLKID_TWD>;
 		};
 
 		usb_phy0: usb-phy@b74000 {
@@ -94,7 +94,7 @@
 		eth1: ethernet@b90000 {
 			compatible = "marvell,pxa168-eth";
 			reg = <0xb90000 0x10000>;
-			clocks = <&chip CLKID_GETH1>;
+			clocks = <&chip_clk CLKID_GETH1>;
 			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
 			/* set by bootloader */
 			local-mac-address = [00 00 00 00 00 00];
@@ -112,7 +112,7 @@
 		eth0: ethernet@e50000 {
 			compatible = "marvell,pxa168-eth";
 			reg = <0xe50000 0x10000>;
-			clocks = <&chip CLKID_GETH0>;
+			clocks = <&chip_clk CLKID_GETH0>;
 			interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
 			/* set by bootloader */
 			local-mac-address = [00 00 00 00 00 00];
@@ -211,7 +211,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c00 0x14>;
 				interrupts = <8>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "okay";
 			};
@@ -220,7 +220,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c14 0x14>;
 				interrupts = <9>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "okay";
 			};
@@ -229,7 +229,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c28 0x14>;
 				interrupts = <10>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -238,7 +238,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c3c 0x14>;
 				interrupts = <11>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -247,7 +247,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c50 0x14>;
 				interrupts = <12>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -256,7 +256,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c64 0x14>;
 				interrupts = <13>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -265,7 +265,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c78 0x14>;
 				interrupts = <14>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -274,7 +274,7 @@
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c8c 0x14>;
 				interrupts = <15>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -292,9 +292,14 @@
 		chip: chip-controller@ea0000 {
 			compatible = "marvell,berlin2cd-chip-ctrl", "syscon";
 			reg = <0xea0000 0x400>;
-			#clock-cells = <1>;
-			clocks = <&refclk>;
-			clock-names = "refclk";
+
+			chip_clk: clock {
+				compatible = "marvell,berlin2-clk";
+
+				#clock-cells = <1>;
+				clocks = <&refclk>;
+				clock-names = "refclk";
+			};
 
 			soc_pinctrl: pin-controller {
 				compatible = "marvell,berlin2cd-soc-pinctrl";
@@ -315,7 +320,7 @@
 			compatible = "chipidea,usb2";
 			reg = <0xed0000 0x200>;
 			interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_USB0>;
+			clocks = <&chip_clk CLKID_USB0>;
 			phys = <&usb_phy0>;
 			phy-names = "usb-phy";
 			status = "disabled";
@@ -325,7 +330,7 @@
 			compatible = "chipidea,usb2";
 			reg = <0xee0000 0x200>;
 			interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_USB1>;
+			clocks = <&chip_clk CLKID_USB1>;
 			phys = <&usb_phy1>;
 			phy-names = "usb-phy";
 			status = "disabled";
-- 
2.3.0

[PATCH 7/7] ARM: berlin: rework the clock node for BG2Q

[Antoine Tenart]

With the introduction of the Berlin mfd controller driver, all drivers
previously sharing the chip and system controller nodes now have their
own sub-node.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 arch/arm/boot/dts/berlin2q.dtsi | 53 ++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index bd86d4121ec8..ca69dc26782f 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -66,7 +66,7 @@
 		sdhci0: sdhci@ab0000 {
 			compatible = "mrvl,pxav3-mmc";
 			reg = <0xab0000 0x200>;
-			clocks = <&chip CLKID_SDIO1XIN>;
+			clocks = <&chip_clk CLKID_SDIO1XIN>;
 			interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
 		};
@@ -74,7 +74,7 @@
 		sdhci1: sdhci@ab0800 {
 			compatible = "mrvl,pxav3-mmc";
 			reg = <0xab0800 0x200>;
-			clocks = <&chip CLKID_SDIO1XIN>;
+			clocks = <&chip_clk CLKID_SDIO1XIN>;
 			interrupts = <GIC_SPI 20 IRQ_TYPE_LEVEL_HIGH>;
 			status = "disabled";
 		};
@@ -83,7 +83,7 @@
 			compatible = "mrvl,pxav3-mmc";
 			reg = <0xab1000 0x200>;
 			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_NFC_ECC>, <&chip CLKID_NFC>;
+			clocks = <&chip_clk CLKID_NFC_ECC>, <&chip_clk CLKID_NFC>;
 			clock-names = "io", "core";
 			status = "disabled";
 		};
@@ -104,7 +104,7 @@
 		local-timer@ad0600 {
 			compatible = "arm,cortex-a9-twd-timer";
 			reg = <0xad0600 0x20>;
-			clocks = <&chip CLKID_TWD>;
+			clocks = <&chip_clk CLKID_TWD>;
 			interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH>;
 		};
 
@@ -127,7 +127,7 @@
 			compatible = "chipidea,usb2";
 			reg = <0xa30000 0x10000>;
 			interrupts = <GIC_SPI 52 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_USB2>;
+			clocks = <&chip_clk CLKID_USB2>;
 			phys = <&usb_phy2>;
 			phy-names = "usb-phy";
 			status = "disabled";
@@ -145,14 +145,14 @@
 			compatible = "marvell,berlin2-usb-phy";
 			reg = <0xb78000 0x128>;
 			#phy-cells = <0>;
-			resets = <&chip 0x104 13>;
+			resets = <&chip_rst 0x104 13>;
 			status = "disabled";
 		};
 
 		eth0: ethernet@b90000 {
 			compatible = "marvell,pxa168-eth";
 			reg = <0xb90000 0x10000>;
-			clocks = <&chip CLKID_GETH0>;
+			clocks = <&chip_clk CLKID_GETH0>;
 			interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
 			/* set by bootloader */
 			local-mac-address = [00 00 00 00 00 00];
@@ -259,7 +259,7 @@
 				reg = <0x1400 0x100>;
 				interrupt-parent = <&aic>;
 				interrupts = <4>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				pinctrl-0 = <&twsi0_pmux>;
 				pinctrl-names = "default";
 				status = "disabled";
@@ -272,7 +272,7 @@
 				reg = <0x1800 0x100>;
 				interrupt-parent = <&aic>;
 				interrupts = <5>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				pinctrl-0 = <&twsi1_pmux>;
 				pinctrl-names = "default";
 				status = "disabled";
@@ -281,7 +281,7 @@
 			timer0: timer@2c00 {
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c00 0x14>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				interrupts = <8>;
 			};
@@ -289,14 +289,14 @@
 			timer1: timer@2c14 {
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c14 0x14>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 			};
 
 			timer2: timer@2c28 {
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c28 0x14>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -304,7 +304,7 @@
 			timer3: timer@2c3c {
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c3c 0x14>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -312,7 +312,7 @@
 			timer4: timer@2c50 {
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c50 0x14>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -320,7 +320,7 @@
 			timer5: timer@2c64 {
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c64 0x14>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -328,7 +328,7 @@
 			timer6: timer@2c78 {
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c78 0x14>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -336,7 +336,7 @@
 			timer7: timer@2c8c {
 				compatible = "snps,dw-apb-timer";
 				reg = <0x2c8c 0x14>;
-				clocks = <&chip CLKID_CFG>;
+				clocks = <&chip_clk CLKID_CFG>;
 				clock-names = "timer";
 				status = "disabled";
 			};
@@ -354,9 +354,14 @@
 		chip: chip-controller@ea0000 {
 			compatible = "marvell,berlin2q-chip-ctrl", "syscon";
 			reg = <0xea0000 0x400>, <0xdd0170 0x10>;
-			#clock-cells = <1>;
-			clocks = <&refclk>;
-			clock-names = "refclk";
+
+			chip_clk: clock {
+				compatible = "marvell,berlin2q-clk";
+
+				#clock-cells = <1>;
+				clocks = <&refclk>;
+				clock-names = "refclk";
+			};
 
 			soc_pinctrl: pin-controller {
 				compatible = "marvell,berlin2q-soc-pinctrl";
@@ -382,7 +387,7 @@
 			compatible = "marvell,berlin2q-ahci", "generic-ahci";
 			reg = <0xe90000 0x1000>;
 			interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_SATA>;
+			clocks = <&chip_clk CLKID_SATA>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 
@@ -402,7 +407,7 @@
 		sata_phy: phy@e900a0 {
 			compatible = "marvell,berlin2q-sata-phy";
 			reg = <0xe900a0 0x200>;
-			clocks = <&chip CLKID_SATA>;
+			clocks = <&chip_clk CLKID_SATA>;
 			#address-cells = <1>;
 			#size-cells = <0>;
 			#phy-cells = <1>;
@@ -421,7 +426,7 @@
 			compatible = "chipidea,usb2";
 			reg = <0xed0000 0x10000>;
 			interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_USB0>;
+			clocks = <&chip_clk CLKID_USB0>;
 			phys = <&usb_phy0>;
 			phy-names = "usb-phy";
 			status = "disabled";
@@ -431,7 +436,7 @@
 			compatible = "chipidea,usb2";
 			reg = <0xee0000 0x10000>;
 			interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&chip CLKID_USB1>;
+			clocks = <&chip_clk CLKID_USB1>;
 			phys = <&usb_phy1>;
 			phy-names = "usb-phy";
 			status = "disabled";
-- 
2.3.0

Re: [PATCH 0/7] ARM: berlin: refactor the clock

[Andrew Lunn]

On Fri, Feb 13, 2015 at 05:42:54PM +0100, Antoine Tenart wrote:
> Hi,
> 
> Marvell Berlin SoCs have a chip control register set providing several
> individual registers dealing with various controllers (pinctrl, reset,
> clk). This chip controller is described by a single DT node since the
> individual registers are spread among the chip control register bank.
> 
> Marvell Berlin also have a system control register set providing several
> individual registers for pinctrl or adc.
> 
> A series was sent[1] to correctly handle these two nodes, by introducing
> a Berlin mfd controller driver. The series converted the existing
> pin-controller and reset drivers to take the changes into account.

Hi Antoine

Something which needs to be discussed for both this patchset and the
previous one, is backwards compatibility of the device tree.

As far as i can see, these changes are not backwards compatible.
Somebody trying to boot a new kernel with a old DT blob is going to
have trouble.

How do we want to handle this?

    Andrew

Re: [PATCH 0/7] ARM: berlin: refactor the clock

[Antoine Tenart]

Andrew,

On Fri, Feb 13, 2015 at 06:31:21PM +0100, Andrew Lunn wrote:
> On Fri, Feb 13, 2015 at 05:42:54PM +0100, Antoine Tenart wrote:
> > 
> > Marvell Berlin SoCs have a chip control register set providing several
> > individual registers dealing with various controllers (pinctrl, reset,
> > clk). This chip controller is described by a single DT node since the
> > individual registers are spread among the chip control register bank.
> > 
> > Marvell Berlin also have a system control register set providing several
> > individual registers for pinctrl or adc.
> > 
> > A series was sent[1] to correctly handle these two nodes, by introducing
> > a Berlin mfd controller driver. The series converted the existing
> > pin-controller and reset drivers to take the changes into account.
> 
> Something which needs to be discussed for both this patchset and the
> previous one, is backwards compatibility of the device tree.
> 
> As far as i can see, these changes are not backwards compatible.
> Somebody trying to boot a new kernel with a old DT blob is going to
> have trouble.

Big trouble :)

> 
> How do we want to handle this?

Keeping a backward compatibility here would make Berlin drivers hard to
maintain, with lots of quirks. This rework is needed because since we
started to push things on Berlin we discovered how the chip and the
system controller are organized and working. It would have been
difficult to guess the perfect device tree organization at first, but
the current way we handle it is not acceptable on the long term.

The support is quite new and can be considered as in development and not
stable yet as far as I'm concerned. We just begin to have enough
functionalities supported. In this case backward compatibility would be
the same as having dead code in Berlin drivers.

IMHO, we do not want to handle this. But I'll let others, and Sebastian,
answer and I'm open to the discussion if needed.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 0/7] ARM: berlin: refactor the clock

[Thomas Petazzoni]

Dear Andrew Lunn,

On Fri, 13 Feb 2015 18:31:21 +0100, Andrew Lunn wrote:

> Something which needs to be discussed for both this patchset and the
> previous one, is backwards compatibility of the device tree.
> 
> As far as i can see, these changes are not backwards compatible.
> Somebody trying to boot a new kernel with a old DT blob is going to
> have trouble.
> 
> How do we want to handle this?

I think one thing that should really be taken into account here is that
we have basically no usable datasheet for those SoCs. We only have very
partial datasheets, and only fairly a vendor kernel tree. This makes it
nearly impossible to have from the start up a clear picture of the
hardware layout and how it should be represented in the DT. Antoine,
and I guess Sebastian as well, are discovering progressively the layout
of the registers, their functionality, depending on the features that
are enabled.

The DT backward compatibility requirement essentially requires either:

 * A very clear picture of the hardware from the beginning, so that we
   can be pretty sure when designing the first DT binding, that it is
   going to be alright. This is clearly not something we have for the
   Marvell Berlin platforms.

 * Keep vast amount of legacy code to support old DTs, which nobody is
   every going to test, which means that such code is guaranteed to
   bitrot within 2 or 3 kernel releases, if not earlier.

Instead, since I believe at this point Marvell doesn't care about DT
backward compatibility for its platforms, could we instead declare the
DT bindings of this platform as "unstable", just like the AT91 guys did
for their DT bindings
(http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/arm/Atmel/README#n100) ?

Best regards,

Thomas Petazzoni
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 0/7] ARM: berlin: refactor the clock

[Sebastian Hesselbarth]

On 13.02.2015 19:19, Thomas Petazzoni wrote:
> On Fri, 13 Feb 2015 18:31:21 +0100, Andrew Lunn wrote:
>> Something which needs to be discussed for both this patchset and the
>> previous one, is backwards compatibility of the device tree.
>>
>> As far as i can see, these changes are not backwards compatible.
>> Somebody trying to boot a new kernel with a old DT blob is going to
>> have trouble.
>>
>> How do we want to handle this?
>
> I think one thing that should really be taken into account here is that
> we have basically no usable datasheet for those SoCs. We only have very
> partial datasheets, and only fairly a vendor kernel tree. This makes it
> nearly impossible to have from the start up a clear picture of the
> hardware layout and how it should be represented in the DT. Antoine,
> and I guess Sebastian as well, are discovering progressively the layout
> of the registers, their functionality, depending on the features that
> are enabled.

Yup, the situation with usable information on Berlin SoCs for me is
even worse. The only reliable source of information is the GPL'd vendor
kernels for two or three boards and some left-over includes describing
some of the register bits.

> The DT backward compatibility requirement essentially requires either:
>
>   * A very clear picture of the hardware from the beginning, so that we
>     can be pretty sure when designing the first DT binding, that it is
>     going to be alright. This is clearly not something we have for the
>     Marvell Berlin platforms.

It is not only the clear picture of the hardware but you'll have to
have the driver layout in mind, too. I know that DT shouldn't follow
any specific kernel or bootloader, but a lot of decisions that we make, 
e.g. where we cut registers into separate nodes, are constantly driven 
by some driver in mind.

>   * Keep vast amount of legacy code to support old DTs, which nobody is
>     every going to test, which means that such code is guaranteed to
>     bitrot within 2 or 3 kernel releases, if not earlier.

That is even more true for Berlin. Remember that any device available
comes with some secure boot chain thingy. Not many ppl take their board
apart immediately and even less if the risk to brick it is as high as
for the Berlin devices.

> Instead, since I believe at this point Marvell doesn't care about DT
> backward compatibility for its platforms, could we instead declare the

I doubt that it is even possible to have them close. All we can do is
work out a sane binding in mainline and wait for Marvell to pick it up.
They want a fully working kernel for their SoC _way_ before we even
made up our mind on the binding.

> DT bindings of this platform as "unstable", just like the AT91 guys did
> for their DT bindings
> (http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/arm/Atmel/README#n100) ?

Sounds like a plan.

Sebastian

Re: [PATCH 0/7] ARM: berlin: refactor the clock

[Andrew Lunn]

> >DT bindings of this platform as "unstable", just like the AT91 guys did
> >for their DT bindings
> >(http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/Documentation/arm/Atmel/README#n100) ?
> 
> Sounds like a plan.

So we all seem to agree that we should break backwards compatibility
with the current binding. And we should explicitly document that
things are currently unstable. I would also suggest putting a comment
about being unstable into the .dts files as well. The .dts files are
more visible to potential users than the binding documents.

     Andrew

Re: [PATCH 0/7] ARM: berlin: refactor the clock

[Jisheng Zhang]

Hi all,

On Fri, 13 Feb 2015 08:42:54 -0800
Antoine Tenart <antoine.tenart@free-electrons.com> wrote:

> Hi,
> 
> Marvell Berlin SoCs have a chip control register set providing several
> individual registers dealing with various controllers (pinctrl, reset,
> clk). This chip controller is described by a single DT node since the
> individual registers are spread among the chip control register bank.
> 
> Marvell Berlin also have a system control register set providing several
> individual registers for pinctrl or adc.

There's no chip control IP. The HW just put some HW registers into the so
called "chip control" address space, the registers in this space are mostly used for
"control" purpose, but some are not. Take the clk as an example, some clocks'
registers are put into the system control register space, some clocks' are
not.

So far, there are five type of clocks in Berlin (from the driver programmer's point
of view):

1. gate clocks. The clocks may share register. The clocks can only be gated or ungated.

2. individual clocks. The clock doesn't share registers with each other.
The gate/ungate, clock source selection, clock divider etc. bits of each clock are
put into one individual register. one register per clock.

3. group clocks. The gate/ungate bits of these clocks are grouped into one register,
the clock source selection, clock divider bits of these clocks are grouped into another
one or two register.

4. plls. The pll doesn't share registers with each other. For example, syspll,
cpupll etc. one or two register per pll

5. fixed clock. the oscillator used for reference clock.

In newer chips, there are no group clocks any more. So the driver code can be more
simpler and clean.

So I think we'd better to implement drivers without the "chip control" concept in
mind. The previous clock patches reflect what the HW really does.

https://lkml.org/lkml/2014/3/21/413

https://lkml.org/lkml/2014/4/24/624


The above is just my humble opinions and the current berlin clk driver is different
with the previous one I dunno how can we handle this situation now. I really need
help!

Thanks,
Jisheng

Re: [PATCH 0/7] ARM: berlin: refactor the clock

[Jisheng Zhang]

Hi all,

On Sun, 15 Feb 2015 19:37:38 -0800
Jisheng Zhang <jszhang@marvell.com> wrote:

> Hi all,
> 
> On Fri, 13 Feb 2015 08:42:54 -0800
> Antoine Tenart <antoine.tenart@free-electrons.com> wrote:
> 
> > Hi,
> > 
> > Marvell Berlin SoCs have a chip control register set providing several
> > individual registers dealing with various controllers (pinctrl, reset,
> > clk). This chip controller is described by a single DT node since the
> > individual registers are spread among the chip control register bank.
> > 
> > Marvell Berlin also have a system control register set providing several
> > individual registers for pinctrl or adc.
> 
> There's no chip control IP. The HW just put some HW registers into the so
> called "chip control" address space, the registers in this space are mostly
> used for "control" purpose, but some are not. Take the clk as an example,
> some clocks' registers are put into the system control register space, some
> clocks' are not.
> 
> So far, there are five type of clocks in Berlin (from the driver
> programmer's point of view):
> 
> 1. gate clocks. The clocks may share register. The clocks can only be gated
> or ungated.
> 
> 2. individual clocks. The clock doesn't share registers with each other.
> The gate/ungate, clock source selection, clock divider etc. bits of each
> clock are put into one individual register. one register per clock.
> 
> 3. group clocks. The gate/ungate bits of these clocks are grouped into one
> register, the clock source selection, clock divider bits of these clocks
> are grouped into another one or two register.
> 
> 4. plls. The pll doesn't share registers with each other. For example,
> syspll, cpupll etc. one or two register per pll
> 
> 5. fixed clock. the oscillator used for reference clock.
> 
> In newer chips, there are no group clocks any more. So the driver code can
> be more simpler and clean.
> 
> So I think we'd better to implement drivers without the "chip control"
> concept in mind. The previous clock patches reflect what the HW really does.
> 
> https://lkml.org/lkml/2014/3/21/413
> 
> https://lkml.org/lkml/2014/4/24/624

To be honest, these two patches are what mrvl used internally. And for newer
linux kernel version, we replaced the mainline clk driver with this mrvl specific
one.

> 
> 
> The above is just my humble opinions and the current berlin clk driver is
> different with the previous one I dunno how can we handle this situation
> now. I really need help!

Re: [PATCH 0/7] ARM: berlin: refactor the clock

[Sebastian Hesselbarth]

On 16.02.2015 04:37, Jisheng Zhang wrote:
> On Fri, 13 Feb 2015 08:42:54 -0800
> Antoine Tenart <antoine.tenart@free-electrons.com> wrote:
>> Marvell Berlin SoCs have a chip control register set providing several
>> individual registers dealing with various controllers (pinctrl, reset,
>> clk). This chip controller is described by a single DT node since the
>> individual registers are spread among the chip control register bank.
>>
>> Marvell Berlin also have a system control register set providing several
>> individual registers for pinctrl or adc.
>
> There's no chip control IP. The HW just put some HW registers into the so
> called "chip control" address space, the registers in this space are mostly used for
> "control" purpose, but some are not. Take the clk as an example, some clocks'
> registers are put into the system control register space, some clocks' are
> not.

Jisheng,

you are right, there is no specific IP for those registers. But as we
don't want these registers to be spread among our SoC nodes, we chose
to sum them all up into a single node.

Back when the clk driver was proposed, Mike requested to not expose
each of the clocks in DT - so we joined them basically into a single
node and let the driver do the rest.

Now, this patch set goes a little bit further and simply joins all of
the chip ctrl registers into a single node and just adds sub-nodes where
we need them (e.g. pinctrl).

[...]
> In newer chips, there are no group clocks any more. So the driver code can be more
> simpler and clean.
>
> So I think we'd better to implement drivers without the "chip control" concept in
> mind. The previous clock patches reflect what the HW really does.

I see no problem in what future SoCs do with register layout. It seems
that it will be fundamentally different anyway, so we might consider to
have a completely new driver for any SoC past BG2Q.

> The above is just my humble opinions and the current berlin clk driver is different
> with the previous one I dunno how can we handle this situation now. I really need
> help!

We appreciate you share your opinion!

How does having a single node  (and basically a single reg property
shared by regmap) block you from implementing support for your new SoC?

Also, you don't need to follow the chip-ctrl node concept for the new
SoC if it is too different. It is just that we kind of give up to chop
this register set into functional pieces in DT and think it will be
better dealt with in each of the drivers.

Sebastian