[PATCH 0/3] clk: Add regmap support for clk mulitplexer

[PATCH 0/3] clk: Add regmap support for clk mulitplexer

[Matthias Brugger]

This patch set adds regmap support for the simple clock multiplexer.
Regmap use, apart from a pointer to the regmap struct needs an
offset value to know where in the regmap it has to read/write.
We add both fields to struct clk_mux.

The driver will distinguish between a clock which is based on regmap or not
through a flag specified for the mux.
The approach does not break the existing clock framework API but adds two
new functions for registering regmap clocks. Unregistering the clocks is
independent of the use of regmap or not, so that no new function was
implemented.

As an example user of the regmap clock multiplexer, it was implemented on
the mt8135. When accepted it will also be applied to the other Mediatek SoCs.
Other possible user are Qualcomm SoCs which up to now implement their own
regmap based clock multiplexer.

Any comments welcome.

Matthias Brugger (3):
  clk: mux: Add regmap support for simple mux
  clk: mediatek: Add support for clk-mux using regmap
  clk: mediatek: Use regmap clk-mux for mt8135

 drivers/clk/clk-mux.c             | 103 ++++++++++++++++++++++++++++++++++----
 drivers/clk/mediatek/clk-mt8135.c |  21 +++-----
 drivers/clk/mediatek/clk-mtk.c    |  37 ++++++++++++++
 drivers/clk/mediatek/clk-mtk.h    |  26 ++++++++++
 include/linux/clk-provider.h      |  40 +++++++++++++++
 5 files changed, 202 insertions(+), 25 deletions(-)

-- 
1.9.1

[PATCH 1/3] clk: mux: Add regmap support for simple mux

[Matthias Brugger]

Some devices like SoCs from Mediatek need to use the clock muxes
through a regmap interface.
This patch adds regmap support for simple the simple multiplexer
clock code.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
---
 drivers/clk/clk-mux.c        | 103 ++++++++++++++++++++++++++++++++++++++-----
 include/linux/clk-provider.h |  40 +++++++++++++++++
 2 files changed, 132 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index 1fa2a8d..c7c692a 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -29,6 +29,26 @@
 
 #define to_clk_mux(_hw) container_of(_hw, struct clk_mux, hw)
 
+static void clk_mux_writel(struct clk_mux *mux, u32 val)
+{
+	if (mux->flags && CLK_MUX_USE_REGMAP)
+		regmap_write(mux->regmap, mux->offset, val);
+	else
+		clk_writel(val, mux->reg);
+}
+
+static u32 clk_mux_readl(struct clk_mux *mux)
+{
+	u32 val;
+
+	if (mux->flags && CLK_MUX_USE_REGMAP)
+		regmap_read(mux->regmap, mux->offset, &val);
+	else
+		val = clk_readl(mux->reg);
+
+	return val;
+}
+
 static u8 clk_mux_get_parent(struct clk_hw *hw)
 {
 	struct clk_mux *mux = to_clk_mux(hw);
@@ -42,7 +62,10 @@ 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;
+
+	val = clk_mux_readl(mux);
+
+	val >>= mux->shift;
 	val &= mux->mask;
 
 	if (mux->table) {
@@ -89,11 +112,11 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
 	if (mux->flags & CLK_MUX_HIWORD_MASK) {
 		val = mux->mask << (mux->shift + 16);
 	} else {
-		val = clk_readl(mux->reg);
+		val = clk_mux_readl(mux);
 		val &= ~(mux->mask << mux->shift);
 	}
 	val |= index << mux->shift;
-	clk_writel(val, mux->reg);
+	clk_mux_writel(mux, val);
 
 	if (mux->lock)
 		spin_unlock_irqrestore(mux->lock, flags);
@@ -113,9 +136,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 * const *parent_names, u8 num_parents, unsigned long flags,
-		void __iomem *reg, u8 shift, u32 mask,
+		void __iomem *reg, struct regmap *regmap,
+		u32 offset, u8 shift, u32 mask,
 		u8 clk_mux_flags, u32 *table, spinlock_t *lock)
 {
 	struct clk_mux *mux;
@@ -149,6 +173,8 @@ struct clk *clk_register_mux_table(struct device *dev, const char *name,
 
 	/* struct clk_mux assignments */
 	mux->reg = reg;
+	mux->regmap = regmap;
+	mux->offset = offset;
 	mux->shift = shift;
 	mux->mask = mask;
 	mux->flags = clk_mux_flags;
@@ -163,18 +189,40 @@ 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 * const *parent_names, u8 num_parents,
+		unsigned long flags, void __iomem *reg, u8 shift, u32 mask,
+		u8 clk_mux_flags, u32 *table, spinlock_t *lock)
+{
+	return __clk_register_mux_table(dev, name, parent_names, num_parents,
+						flags, reg, NULL, 0,
+						shift, mask, clk_mux_flags,
+						table, lock);
+}
 EXPORT_SYMBOL_GPL(clk_register_mux_table);
 
-struct clk *clk_register_mux(struct device *dev, const char *name,
-		const char * const *parent_names, u8 num_parents, unsigned long flags,
-		void __iomem *reg, u8 shift, u8 width,
+struct clk *__clk_register_mux(struct device *dev, const char *name,
+		const char * const *parent_names, u8 num_parents,
+		unsigned long flags, void __iomem *reg, struct regmap *regmap,
+		u32 offset, u8 shift, u8 width,
 		u8 clk_mux_flags, spinlock_t *lock)
 {
 	u32 mask = BIT(width) - 1;
 
-	return clk_register_mux_table(dev, name, parent_names, num_parents,
-				      flags, reg, shift, mask, clk_mux_flags,
-				      NULL, lock);
+	return __clk_register_mux_table(dev, name, parent_names, num_parents,
+					flags, reg, regmap, offset, shift, mask,
+					clk_mux_flags, NULL, lock);
+}
+
+struct clk *clk_register_mux(struct device *dev, const char *name,
+		const char * const *parent_names, u8 num_parents,
+		unsigned long flags, void __iomem *reg, u8 shift, u8 width,
+		u8 clk_mux_flags, spinlock_t *lock)
+{
+	return __clk_register_mux(dev, name, parent_names, num_parents, flags,
+					reg, NULL, 0, shift, width,
+					clk_mux_flags, lock);
 }
 EXPORT_SYMBOL_GPL(clk_register_mux);
 
@@ -193,3 +241,36 @@ void clk_unregister_mux(struct clk *clk)
 	kfree(mux);
 }
 EXPORT_SYMBOL_GPL(clk_unregister_mux);
+
+#ifdef CONFIG_REGMAP
+
+struct clk *clk_regm_register_mux(struct device *dev, const char *name,
+		const char * const *parent_names, u8 num_parents,
+		unsigned long flags, struct regmap *regmap,
+		u32 offset, u8 shift, u8 width,
+		u8 clk_mux_flags, spinlock_t *lock)
+{
+	clk_mux_flags |= CLK_MUX_USE_REGMAP;
+
+	return __clk_register_mux(dev, name, parent_names, num_parents, flags,
+					NULL, regmap, offset, shift, width,
+					clk_mux_flags, lock);
+}
+EXPORT_SYMBOL_GPL(clk_regm_register_mux);
+
+struct clk *clk_regm_register_mux_table(struct device *dev, const char *name,
+		const char * const *parent_names, u8 num_parents,
+		unsigned long flags, struct regmap *regmap,
+		u32 offset, u8 shift, u32 mask,
+		u8 clk_mux_flags, u32 *table, spinlock_t *lock)
+{
+	clk_mux_flags |= CLK_MUX_USE_REGMAP;
+
+	return __clk_register_mux_table(dev, name, parent_names, num_parents,
+					flags, NULL, regmap, offset,
+					shift, mask, clk_mux_flags,
+					table, lock);
+}
+EXPORT_SYMBOL_GPL(clk_regm_register_mux_table);
+
+#endif
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index ec609e5..7fd56ca 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
 
@@ -409,7 +410,9 @@ void clk_unregister_divider(struct clk *clk);
 struct clk_mux {
 	struct clk_hw	hw;
 	void __iomem	*reg;
+	struct regmap	*regmap;
 	u32		*table;
+	u32		offset;
 	u32		mask;
 	u8		shift;
 	u8		flags;
@@ -421,6 +424,7 @@ struct clk_mux {
 #define CLK_MUX_HIWORD_MASK		BIT(2)
 #define CLK_MUX_READ_ONLY		BIT(3) /* mux can't be changed */
 #define CLK_MUX_ROUND_CLOSEST		BIT(4)
+#define CLK_MUX_USE_REGMAP		BIT(5)
 
 extern const struct clk_ops clk_mux_ops;
 extern const struct clk_ops clk_mux_ro_ops;
@@ -437,6 +441,42 @@ struct clk *clk_register_mux_table(struct device *dev, const char *name,
 
 void clk_unregister_mux(struct clk *clk);
 
+#ifdef CONFIG_REGMAP
+
+struct clk *clk_regm_register_mux_table(struct device *dev, const char *name,
+		const char * const *parent_names, u8 num_parents,
+		unsigned long flags, struct regmap *regmap,
+		u32 offset, u8 shift, u32 mask,
+		u8 clk_mux_flags, u32 *table, spinlock_t *lock);
+
+struct clk *clk_regm_register_mux(struct device *dev, const char *name,
+		const char * const *parent_names, u8 num_parents,
+		unsigned long flags, struct regmap *regmap,
+		u32 offset, u8 shift, u8 width,
+		u8 clk_mux_flags, spinlock_t *lock);
+
+#else
+
+struct clk *clk_regm_register_mux_table(struct device *dev, const char *name,
+		const char * const *parent_names, u8 num_parents,
+		unsigned long flags, struct regmap *reg,
+		u32 offset, u8 shift, u32 mask,
+		u8 clk_mux_flags, u32 *table, spinlock_t *lock)
+{
+	return NULL;
+}
+
+struct clk *clk_regm_register_mux(struct device *dev, const char *name,
+		const char * const *parent_names, u8 num_parents,
+		unsigned long flags, struct regmap *reg,
+		u32 offset, u8 shift, u8 width,
+		u8 clk_mux_flags, spinlock_t *lock)
+{
+	return NULL;
+}
+
+#endif
+
 void of_fixed_factor_clk_setup(struct device_node *node);
 
 /**
-- 
1.9.1

[PATCH 2/3] clk: mediatek: Add support for clk-mux using regmap

[Matthias Brugger]

This patches adds support for the mediatek clocks to be able to register
and use a clk-mux wich relies on regmap.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
---
 drivers/clk/mediatek/clk-mtk.c | 37 +++++++++++++++++++++++++++++++++++++
 drivers/clk/mediatek/clk-mtk.h | 26 ++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 18444ae..cf953a8 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -111,6 +111,43 @@ int mtk_clk_register_gates(struct device_node *node, const struct mtk_gate *clks
 	return 0;
 }
 
+int mtk_clk_regm_register_mux(struct device_node *node,
+		const struct mtk_regm_mux *clks, int num,
+		struct clk_onecell_data *clk_data)
+{
+	int i;
+	struct clk *clk;
+	struct regmap *regmap;
+
+	if (!clk_data)
+		return -ENOMEM;
+
+	regmap = syscon_node_to_regmap(node);
+	if (IS_ERR(regmap)) {
+		pr_err("Cannot find regmap for %s: %ld\n", node->full_name,
+				PTR_ERR(regmap));
+		return PTR_ERR(regmap);
+	}
+
+	for (i = 0; i < num; i++) {
+		const struct mtk_regm_mux *mux = &clks[i];
+
+		clk = clk_regm_register_mux(NULL, mux->name, mux->parent_names,
+					mux->num_parents, 0, regmap,
+					mux->mux_offset, mux->mux_shift,
+					mux->mux_width, mux->flags, NULL);
+		if (IS_ERR(clk)) {
+			pr_err("Failed to register clk mux %s: %ld\n",
+					mux->name, PTR_ERR(clk));
+			continue;
+		}
+
+		clk_data->clks[mux->id] = clk;
+	}
+
+	return 0;
+}
+
 struct clk *mtk_clk_register_composite(const struct mtk_composite *mc,
 		void __iomem *base, spinlock_t *lock)
 {
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index 61035b9..3f41725 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -156,6 +156,32 @@ void __init mtk_clk_register_plls(struct device_node *node,
 		const struct mtk_pll_data *plls, int num_plls,
 		struct clk_onecell_data *clk_data);
 
+struct mtk_regm_mux {
+	int id;
+	const char *name;
+	const char * const *parent_names;
+	u8 num_parents;
+	unsigned flags;
+	uint32_t mux_offset;
+	uint32_t mux_shift;
+	uint32_t mux_width;
+};
+
+int __init mtk_clk_regm_register_mux(struct device_node *node,
+		const struct mtk_regm_mux *clks, int num,
+		struct clk_onecell_data *clk_data);
+
+#define MUX_REGMAP(_id, _name, _parents, _offset, _shift, _width) {	\
+		.id = _id,						\
+		.name = _name,						\
+		.parent_names = _parents,				\
+		.num_parents = ARRAY_SIZE(_parents),			\
+		.flags = CLK_SET_RATE_PARENT,				\
+		.mux_offset = _offset,					\
+		.mux_shift = _shift,					\
+		.mux_width = _width,					\
+	}
+
 #ifdef CONFIG_RESET_CONTROLLER
 void mtk_register_reset_controller(struct device_node *np,
 			unsigned int num_regs, int regofs);
-- 
1.9.1

[PATCH 3/3] clk: mediatek: Use regmap clk-mux for mt8135

[Matthias Brugger]

The pericfg controller is used by various device drivers, so that it
is implemented via a regmap. In the actual clk implementation for
mt8135, some clk-mux use the traditional register approach which
acceses the register via iomem.
This patch changes the use from iomem to the needed regmap.

Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
---
 drivers/clk/mediatek/clk-mt8135.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8135.c b/drivers/clk/mediatek/clk-mt8135.c
index a63435b..bfb56c6 100644
--- a/drivers/clk/mediatek/clk-mt8135.c
+++ b/drivers/clk/mediatek/clk-mt8135.c
@@ -513,11 +513,11 @@ static const char * const uart_ck_sel_parents[] __initconst = {
 	"uart_sel",
 };
 
-static const struct mtk_composite peri_clks[] __initconst = {
-	MUX(CLK_PERI_UART0_SEL, "uart0_ck_sel", uart_ck_sel_parents, 0x40c, 0, 1),
-	MUX(CLK_PERI_UART1_SEL, "uart1_ck_sel", uart_ck_sel_parents, 0x40c, 1, 1),
-	MUX(CLK_PERI_UART2_SEL, "uart2_ck_sel", uart_ck_sel_parents, 0x40c, 2, 1),
-	MUX(CLK_PERI_UART3_SEL, "uart3_ck_sel", uart_ck_sel_parents, 0x40c, 3, 1),
+static const struct mtk_regm_mux peri_clks[] __initconst = {
+	MUX_REGMAP(CLK_PERI_UART0_SEL, "uart0_ck_sel", uart_ck_sel_parents, 0x40c, 0, 1),
+	MUX_REGMAP(CLK_PERI_UART1_SEL, "uart1_ck_sel", uart_ck_sel_parents, 0x40c, 1, 1),
+	MUX_REGMAP(CLK_PERI_UART2_SEL, "uart2_ck_sel", uart_ck_sel_parents, 0x40c, 2, 1),
+	MUX_REGMAP(CLK_PERI_UART3_SEL, "uart3_ck_sel", uart_ck_sel_parents, 0x40c, 3, 1),
 };
 
 static void __init mtk_topckgen_init(struct device_node *node)
@@ -573,20 +573,13 @@ static void __init mtk_pericfg_init(struct device_node *node)
 {
 	struct clk_onecell_data *clk_data;
 	int r;
-	void __iomem *base;
-
-	base = of_iomap(node, 0);
-	if (!base) {
-		pr_err("%s(): ioremap failed\n", __func__);
-		return;
-	}
 
 	clk_data = mtk_alloc_clk_data(CLK_PERI_NR_CLK);
 
 	mtk_clk_register_gates(node, peri_gates, ARRAY_SIZE(peri_gates),
 						clk_data);
-	mtk_clk_register_composites(peri_clks, ARRAY_SIZE(peri_clks), base,
-			&mt8135_clk_lock, clk_data);
+	mtk_clk_regm_register_mux(node, peri_clks, ARRAY_SIZE(peri_clks),
+			clk_data);
 
 	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
 	if (r)
-- 
1.9.1

[PATCH 0/3] clk: Add regmap support for clk mulitplexer

[Sascha Hauer]

On Thu, May 28, 2015 at 08:41:45PM +0200, Matthias Brugger wrote:
> This patch set adds regmap support for the simple clock multiplexer.
> Regmap use, apart from a pointer to the regmap struct needs an
> offset value to know where in the regmap it has to read/write.
> We add both fields to struct clk_mux.
> 
> The driver will distinguish between a clock which is based on regmap or not
> through a flag specified for the mux.
> The approach does not break the existing clock framework API but adds two
> new functions for registering regmap clocks. Unregistering the clocks is
> independent of the use of regmap or not, so that no new function was
> implemented.
> 
> As an example user of the regmap clock multiplexer, it was implemented on
> the mt8135. When accepted it will also be applied to the other Mediatek SoCs.
> Other possible user are Qualcomm SoCs which up to now implement their own
> regmap based clock multiplexer.

I'm all in for providing regmap support for the basic clk providers.
>From digging through the archives I found out that the last try failed,
so I hesitated to implement this for Mediatek. I hope it's time to
revisit this. Using regmap would make the basic clock providers much
more versatilely usable.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/3] clk: mux: Add regmap support for simple mux

[Sascha Hauer]

On Thu, May 28, 2015 at 08:41:46PM +0200, Matthias Brugger wrote:
> Some devices like SoCs from Mediatek need to use the clock muxes
> through a regmap interface.
> This patch adds regmap support for simple the simple multiplexer
> clock code.
> 
> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
> ---
>  drivers/clk/clk-mux.c        | 103 ++++++++++++++++++++++++++++++++++++++-----
>  include/linux/clk-provider.h |  40 +++++++++++++++++
>  2 files changed, 132 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> index 1fa2a8d..c7c692a 100644
> --- a/drivers/clk/clk-mux.c
> +++ b/drivers/clk/clk-mux.c
> @@ -29,6 +29,26 @@
>  
>  #define to_clk_mux(_hw) container_of(_hw, struct clk_mux, hw)
>  
> +static void clk_mux_writel(struct clk_mux *mux, u32 val)
> +{
> +	if (mux->flags && CLK_MUX_USE_REGMAP)
> +		regmap_write(mux->regmap, mux->offset, val);
> +	else
> +		clk_writel(val, mux->reg);
> +}
> +
> +static u32 clk_mux_readl(struct clk_mux *mux)
> +{
> +	u32 val;
> +
> +	if (mux->flags && CLK_MUX_USE_REGMAP)
> +		regmap_read(mux->regmap, mux->offset, &val);
> +	else
> +		val = clk_readl(mux->reg);
> +
> +	return val;
> +}
> +
>  static u8 clk_mux_get_parent(struct clk_hw *hw)
>  {
>  	struct clk_mux *mux = to_clk_mux(hw);
> @@ -42,7 +62,10 @@ 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;
> +
> +	val = clk_mux_readl(mux);
> +
> +	val >>= mux->shift;
>  	val &= mux->mask;
>  
>  	if (mux->table) {
> @@ -89,11 +112,11 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>  	if (mux->flags & CLK_MUX_HIWORD_MASK) {
>  		val = mux->mask << (mux->shift + 16);
>  	} else {
> -		val = clk_readl(mux->reg);
> +		val = clk_mux_readl(mux);
>  		val &= ~(mux->mask << mux->shift);
>  	}
>  	val |= index << mux->shift;
> -	clk_writel(val, mux->reg);
> +	clk_mux_writel(mux, val);

With regmap the register lock is inside the regmap, you must hold it
during read/modify/write operations which means you have to use
regmap_update_bits rather than regmap_read followed by regmap_write.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 1/3] clk: mux: Add regmap support for simple mux

[Joachim Eastwood]

Hi Matthias,

On 28 May 2015 at 20:41, Matthias Brugger <matthias.bgg@gmail.com> wrote:
> Some devices like SoCs from Mediatek need to use the clock muxes
> through a regmap interface.
> This patch adds regmap support for simple the simple multiplexer
> clock code.

Nice to see regmap support. This something I would also like to see
added to the clk framework.
And if the approach is agreed upon I would like to see support on
gates and dividers also.

> +static void clk_mux_writel(struct clk_mux *mux, u32 val)
> +{
> +       if (mux->flags && CLK_MUX_USE_REGMAP)

Assume you mean &.

> +               regmap_write(mux->regmap, mux->offset, val);
> +       else
> +               clk_writel(val, mux->reg);
> +}
> +
> +static u32 clk_mux_readl(struct clk_mux *mux)
> +{
> +       u32 val;
> +
> +       if (mux->flags && CLK_MUX_USE_REGMAP)

And here.

> +               regmap_read(mux->regmap, mux->offset, &val);
> +       else
> +               val = clk_readl(mux->reg);
> +
> +       return val;
> +}
> +
[...]
> @@ -409,7 +410,9 @@ void clk_unregister_divider(struct clk *clk);
>  struct clk_mux {
>         struct clk_hw   hw;

>         void __iomem    *reg;
> +       struct regmap   *regmap;

Since it not possible to have both iomem and regmap at the same time.
Could this be put in a union or does that make registration more
difficult?

>         u32             *table;
> +       u32             offset;
>         u32             mask;
>         u8              shift;
>         u8              flags;


regards,
Joachim Eastwood

[PATCH 1/3] clk: mux: Add regmap support for simple mux

[Matthias Brugger]

2015-05-29 8:40 GMT+02:00 Sascha Hauer <s.hauer@pengutronix.de>:
> On Thu, May 28, 2015 at 08:41:46PM +0200, Matthias Brugger wrote:
>> Some devices like SoCs from Mediatek need to use the clock muxes
>> through a regmap interface.
>> This patch adds regmap support for simple the simple multiplexer
>> clock code.
>>
>> Signed-off-by: Matthias Brugger <matthias.bgg@gmail.com>
>> ---
>>  drivers/clk/clk-mux.c        | 103 ++++++++++++++++++++++++++++++++++++++-----
>>  include/linux/clk-provider.h |  40 +++++++++++++++++
>>  2 files changed, 132 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
>> index 1fa2a8d..c7c692a 100644
>> --- a/drivers/clk/clk-mux.c
>> +++ b/drivers/clk/clk-mux.c
>> @@ -29,6 +29,26 @@
>>
>>  #define to_clk_mux(_hw) container_of(_hw, struct clk_mux, hw)
>>
>> +static void clk_mux_writel(struct clk_mux *mux, u32 val)
>> +{
>> +     if (mux->flags && CLK_MUX_USE_REGMAP)
>> +             regmap_write(mux->regmap, mux->offset, val);
>> +     else
>> +             clk_writel(val, mux->reg);
>> +}
>> +
>> +static u32 clk_mux_readl(struct clk_mux *mux)
>> +{
>> +     u32 val;
>> +
>> +     if (mux->flags && CLK_MUX_USE_REGMAP)
>> +             regmap_read(mux->regmap, mux->offset, &val);
>> +     else
>> +             val = clk_readl(mux->reg);
>> +
>> +     return val;
>> +}
>> +
>>  static u8 clk_mux_get_parent(struct clk_hw *hw)
>>  {
>>       struct clk_mux *mux = to_clk_mux(hw);
>> @@ -42,7 +62,10 @@ 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;
>> +
>> +     val = clk_mux_readl(mux);
>> +
>> +     val >>= mux->shift;
>>       val &= mux->mask;
>>
>>       if (mux->table) {
>> @@ -89,11 +112,11 @@ static int clk_mux_set_parent(struct clk_hw *hw, u8 index)
>>       if (mux->flags & CLK_MUX_HIWORD_MASK) {
>>               val = mux->mask << (mux->shift + 16);
>>       } else {
>> -             val = clk_readl(mux->reg);
>> +             val = clk_mux_readl(mux);
>>               val &= ~(mux->mask << mux->shift);
>>       }
>>       val |= index << mux->shift;
>> -     clk_writel(val, mux->reg);
>> +     clk_mux_writel(mux, val);
>
> With regmap the register lock is inside the regmap, you must hold it
> during read/modify/write operations which means you have to use
> regmap_update_bits rather than regmap_read followed by regmap_write.

Fixed in next version.
Thanks for reviewing.

-- 
motzblog.wordpress.com

[PATCH 1/3] clk: mux: Add regmap support for simple mux

[Matthias Brugger]

2015-05-29 8:57 GMT+02:00 Joachim  Eastwood <manabian@gmail.com>:
> Hi Matthias,
>
> On 28 May 2015 at 20:41, Matthias Brugger <matthias.bgg@gmail.com> wrote:
>> Some devices like SoCs from Mediatek need to use the clock muxes
>> through a regmap interface.
>> This patch adds regmap support for simple the simple multiplexer
>> clock code.
>
> Nice to see regmap support. This something I would also like to see
> added to the clk framework.
> And if the approach is agreed upon I would like to see support on
> gates and dividers also.

I totally agree.

>
>> +static void clk_mux_writel(struct clk_mux *mux, u32 val)
>> +{
>> +       if (mux->flags && CLK_MUX_USE_REGMAP)
>
> Assume you mean &.

Sure, it's just luck that the patch didn't end up with a hanging
system when I tested it yesterday.
Fixed in next version.

>
>> +               regmap_write(mux->regmap, mux->offset, val);
>> +       else
>> +               clk_writel(val, mux->reg);
>> +}
>> +
>> +static u32 clk_mux_readl(struct clk_mux *mux)
>> +{
>> +       u32 val;
>> +
>> +       if (mux->flags && CLK_MUX_USE_REGMAP)
>
> And here.

Ditto.

>
>> +               regmap_read(mux->regmap, mux->offset, &val);
>> +       else
>> +               val = clk_readl(mux->reg);
>> +
>> +       return val;
>> +}
>> +
> [...]
>> @@ -409,7 +410,9 @@ void clk_unregister_divider(struct clk *clk);
>>  struct clk_mux {
>>         struct clk_hw   hw;
>
>>         void __iomem    *reg;
>> +       struct regmap   *regmap;
>
> Since it not possible to have both iomem and regmap at the same time.
> Could this be put in a union or does that make registration more
> difficult?

It just adds a flag check to the registration.
I will add it in the next version.

Thanks for reviewing.
Matthias


-- 
motzblog.wordpress.com