[PATCH v2 0/7] clk: sunxi: Add support for the Audio PLL

[PATCH v2 0/7] clk: sunxi: Add support for the Audio PLL

[Maxime Ripard]

Hi,

This serie adds support for the PLL2 aka the Audio PLL on the
Allwinner A10 and the later SoCs.

This is the first stepping stone to get the audio support merged.

This serie is built on top of a generic clk-factor driver to handle
clock that multiply their parent clock rate (mostly PLL's), in order
to provide the driver for the PLL2 base clock, and then adds the
drivers for the clock that derive from the Audio PLL.

Thanks!
Maxime

Changes from v1:
  - Removed a bogus of_iomap in the mod1 clock driver
  - Wrote the clk-factor driver
  - Converted the PLL2 clock to that driver

Emilio López (5):
  clk: sunxi: codec clock support
  clk: sunxi: mod1 clock support
  ARM: sunxi: Add PLL2 support
  ARM: sunxi: Add codec clock support
  ARM: sun7i: Add mod1 clock nodes

Maxime Ripard (2):
  clk: Add a basic factor clock
  clk: sunxi: Add a driver for the PLL2

 arch/arm/boot/dts/sun4i-a10.dtsi           |  18 +++
 arch/arm/boot/dts/sun5i.dtsi               |  18 +++
 arch/arm/boot/dts/sun7i-a20.dtsi           |  73 +++++++++++
 drivers/clk/Makefile                       |   1 +
 drivers/clk/clk-factor.c                   | 176 +++++++++++++++++++++++++++
 drivers/clk/sunxi/Makefile                 |   3 +
 drivers/clk/sunxi/clk-a10-codec.c          |  44 +++++++
 drivers/clk/sunxi/clk-a10-mod1.c           |  84 +++++++++++++
 drivers/clk/sunxi/clk-a10-pll2.c           | 189 +++++++++++++++++++++++++++++
 include/dt-bindings/clock/sun4i-a10-pll2.h |  53 ++++++++
 include/linux/clk-provider.h               |  41 +++++++
 11 files changed, 700 insertions(+)
 create mode 100644 drivers/clk/clk-factor.c
 create mode 100644 drivers/clk/sunxi/clk-a10-codec.c
 create mode 100644 drivers/clk/sunxi/clk-a10-mod1.c
 create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
 create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h

-- 
2.4.1

[PATCH v2 0/7] clk: sunxi: Add support for the Audio PLL

[Maxime Ripard]

Hi,

This serie adds support for the PLL2 aka the Audio PLL on the
Allwinner A10 and the later SoCs.

This is the first stepping stone to get the audio support merged.

This serie is built on top of a generic clk-factor driver to handle
clock that multiply their parent clock rate (mostly PLL's), in order
to provide the driver for the PLL2 base clock, and then adds the
drivers for the clock that derive from the Audio PLL.

Thanks!
Maxime

Changes from v1:
  - Removed a bogus of_iomap in the mod1 clock driver
  - Wrote the clk-factor driver
  - Converted the PLL2 clock to that driver

Emilio L?pez (5):
  clk: sunxi: codec clock support
  clk: sunxi: mod1 clock support
  ARM: sunxi: Add PLL2 support
  ARM: sunxi: Add codec clock support
  ARM: sun7i: Add mod1 clock nodes

Maxime Ripard (2):
  clk: Add a basic factor clock
  clk: sunxi: Add a driver for the PLL2

 arch/arm/boot/dts/sun4i-a10.dtsi           |  18 +++
 arch/arm/boot/dts/sun5i.dtsi               |  18 +++
 arch/arm/boot/dts/sun7i-a20.dtsi           |  73 +++++++++++
 drivers/clk/Makefile                       |   1 +
 drivers/clk/clk-factor.c                   | 176 +++++++++++++++++++++++++++
 drivers/clk/sunxi/Makefile                 |   3 +
 drivers/clk/sunxi/clk-a10-codec.c          |  44 +++++++
 drivers/clk/sunxi/clk-a10-mod1.c           |  84 +++++++++++++
 drivers/clk/sunxi/clk-a10-pll2.c           | 189 +++++++++++++++++++++++++++++
 include/dt-bindings/clock/sun4i-a10-pll2.h |  53 ++++++++
 include/linux/clk-provider.h               |  41 +++++++
 11 files changed, 700 insertions(+)
 create mode 100644 drivers/clk/clk-factor.c
 create mode 100644 drivers/clk/sunxi/clk-a10-codec.c
 create mode 100644 drivers/clk/sunxi/clk-a10-mod1.c
 create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
 create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h

-- 
2.4.1

[PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

Some clocks are using a factor component, however, unlike their mux, gate
or divider counterpart, these factors don't have a basic clock
implementation.

This leads to code duplication across platforms that want to use that kind
of clocks, and the impossibility to use the composite clocks with such a
clock without defining your own rate operations.

Create such a driver in order to remove these issues, and hopefully factor
the implementations, reducing code size across platforms and consolidating
the various implementations.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/Makefile         |   1 +
 drivers/clk/clk-factor.c     | 176 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h |  41 ++++++++++
 3 files changed, 218 insertions(+)
 create mode 100644 drivers/clk/clk-factor.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 3d00c25382c5..d38b71d15ffa 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_HAVE_CLK)		+= clk-devres.o
 obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
 obj-$(CONFIG_COMMON_CLK)	+= clk.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-divider.o
+obj-$(CONFIG_COMMON_CLK)	+= clk-factor.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-factor.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-rate.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-gate.o
diff --git a/drivers/clk/clk-factor.c b/drivers/clk/clk-factor.c
new file mode 100644
index 000000000000..17f83852cdb6
--- /dev/null
+++ b/drivers/clk/clk-factor.c
@@ -0,0 +1,176 @@
+/*
+ * Copyright (C) 2015 Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/of.h>
+
+#define to_clk_factor(_hw) container_of(_hw, struct clk_factor, hw)
+
+static unsigned long __get_mult(struct clk_factor *factor,
+				unsigned long rate,
+				unsigned long parent_rate)
+{
+	if (factor->flags & CLK_FACTOR_ROUND_CLOSEST)
+		return DIV_ROUND_CLOSEST(rate, parent_rate);
+
+	return rate / parent_rate;
+}
+
+static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	struct clk_factor *factor = to_clk_factor(hw);
+	unsigned long val;
+	
+	val = clk_readl(factor->reg) >> factor->shift;
+	val &= GENMASK(factor->width, 0);
+
+	if (!val && factor->flags & CLK_FACTOR_ZERO_BYPASS)
+		val = 1;
+	
+	return parent_rate * val;
+}
+
+static bool __is_best_rate(unsigned long rate, unsigned long new,
+			   unsigned long best, unsigned long flags)
+{
+	if (flags & CLK_FACTOR_ROUND_CLOSEST)
+		return abs(rate - new) < abs(rate - best);
+
+	return new >= rate && new < best;
+}
+
+static unsigned long clk_factor_bestmult(struct clk_hw *hw, unsigned long rate,
+					 unsigned long *best_parent_rate,
+					 u8 width, unsigned long flags)
+{
+	unsigned long orig_parent_rate = *best_parent_rate;
+	unsigned long parent_rate, current_rate, best_rate = ~0;
+	unsigned int i, bestmult = 0;
+
+	if (!(__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
+		return rate / *best_parent_rate;
+
+	for (i = 1; i < ((1 << width) - 1); i++) {
+		if (rate * i == orig_parent_rate) {
+			/*
+			 * This is the best case for us if we have a
+			 * perfect match without changing the parent
+			 * rate.
+			 */
+			*best_parent_rate = orig_parent_rate;
+			return i;
+		}
+
+		parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
+					       rate / i);
+		current_rate = parent_rate * i;
+
+		if (__is_best_rate(rate, current_rate, best_rate, flags)) {
+			bestmult = i;
+			best_rate = current_rate;
+			*best_parent_rate = parent_rate;
+		}
+	}
+
+	return bestmult;
+}
+
+static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
+				  unsigned long *parent_rate)
+{
+	struct clk_factor *factor = to_clk_factor(hw);
+	unsigned long mult = clk_factor_bestmult(hw, rate, parent_rate,
+						 factor->width, factor->flags);
+
+	return *parent_rate * mult;
+}
+
+static int clk_factor_set_rate(struct clk_hw *hw, unsigned long rate,
+			       unsigned long parent_rate)
+{
+	struct clk_factor *factor = to_clk_factor(hw);
+	unsigned long mult = __get_mult(factor, rate, parent_rate);
+	unsigned long uninitialized_var(flags);
+	struct clk *clk = hw->clk;
+	unsigned long val;
+
+	if (factor->lock)
+		spin_lock_irqsave(factor->lock, flags);
+
+	val = clk_readl(factor->reg);
+	val &= ~GENMASK(factor->width + factor->shift, factor->shift);
+	val |= mult << factor->shift;
+	clk_writel(val, factor->reg);
+
+	if (factor->lock)
+		spin_unlock_irqrestore(factor->lock, flags);
+
+	return 0;
+}
+
+const struct clk_ops clk_factor_ops = {
+	.recalc_rate	= clk_factor_recalc_rate,
+	.round_rate	= clk_factor_round_rate,
+	.set_rate	= clk_factor_set_rate,
+};
+EXPORT_SYMBOL_GPL(clk_factor_ops);
+
+struct clk *clk_register_factor(struct device *dev, const char *name,
+				const char *parent_name, unsigned long flags,
+				void __iomem *reg, u8 shift, u8 width,
+				u8 clk_factor_flags, spinlock_t *lock)
+{
+	struct clk_init_data init;
+	struct clk_factor *factor;
+	struct clk *clk;
+
+	factor = kmalloc(sizeof(*factor), GFP_KERNEL);
+	if (!factor) {
+		pr_err("%s: could not allocate fixed factor clk\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	init.name = name;
+	init.ops = &clk_factor_ops;
+	init.flags = flags | CLK_IS_BASIC;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	factor->reg = reg;
+	factor->shift = shift;
+	factor->width = width;
+	factor->flags = clk_factor_flags;
+	factor->lock = lock;
+	factor->hw.init = &init;
+	
+	clk = clk_register(dev, &factor->hw);
+	if (IS_ERR(clk))
+		kfree(factor);
+
+	return clk;
+}
+EXPORT_SYMBOL_GPL(clk_register_factor);
+
+void clk_unregister_factor(struct clk *clk)
+{
+	struct clk_factor *factor;
+	struct clk_hw *hw;
+
+	hw = __clk_get_hw(clk);
+	if (!hw)
+		return;
+
+	factor = to_clk_factor(hw);
+
+	clk_unregister(clk);
+	kfree(factor);
+}
+EXPORT_SYMBOL_GPL(clk_unregister_factor);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index df695313f975..543918c378f5 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -493,6 +493,47 @@ struct clk *clk_register_fractional_divider(struct device *dev,
 		void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
 		u8 clk_divider_flags, spinlock_t *lock);
 
+/**
+ * struct clk_factor - adjustable factor clock
+ *
+ * @hw:		handle between common and hardware-specific interfaces
+ * @reg:	register containing the factor
+ * @shift:	shift to the factor bit field
+ * @width:	width of the factor bit field
+ * @lock:	register lock
+ *
+ * Clock with an adjustable factor affecting its output frequency.
+ * Implements .recalc_rate, .set_rate and .round_rate
+ *
+ * Flags:
+ * CLK_FACTOR_ZERO_BYPASS - By default, the factor is the value read
+ *	from the register, with 0 being a valid value effectively
+ *	zeroing the output clock rate. If CLK_FACTOR_ZERO_BYPASS is
+ *	set, then a null factor will be considered as a bypass,
+ *	leaving the parent rate unmodified.
+ * CLK_FACTOR_ROUND_CLOSEST - Makes the best calculated divider to be
+ *	rounded to the closest integer instead of the down one.
+ */
+struct clk_factor {
+	struct clk_hw	hw;
+	void __iomem	*reg;
+	u8		shift;
+	u8		width;
+	u8		flags;
+	spinlock_t	*lock;
+};
+
+#define CLK_FACTOR_ZERO_BYPASS		BIT(0)
+#define CLK_FACTOR_ROUND_CLOSEST	BIT(1)
+
+extern const struct clk_ops clk_factor_ops;
+
+struct clk *clk_register_factor(struct device *dev, const char *name,
+				const char *parent_name, unsigned long flags,
+				void __iomem *reg, u8 shift, u8 width,
+				u8 clk_factor_flags, spinlock_t *lock);
+void clk_unregister_factor(struct clk *clk);
+
 /***
  * struct clk_composite - aggregate clock of mux, divider and gate clocks
  *
-- 
2.4.1

[PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

Some clocks are using a factor component, however, unlike their mux, gate
or divider counterpart, these factors don't have a basic clock
implementation.

This leads to code duplication across platforms that want to use that kind
of clocks, and the impossibility to use the composite clocks with such a
clock without defining your own rate operations.

Create such a driver in order to remove these issues, and hopefully factor
the implementations, reducing code size across platforms and consolidating
the various implementations.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/Makefile         |   1 +
 drivers/clk/clk-factor.c     | 176 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h |  41 ++++++++++
 3 files changed, 218 insertions(+)
 create mode 100644 drivers/clk/clk-factor.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 3d00c25382c5..d38b71d15ffa 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_HAVE_CLK)		+= clk-devres.o
 obj-$(CONFIG_CLKDEV_LOOKUP)	+= clkdev.o
 obj-$(CONFIG_COMMON_CLK)	+= clk.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-divider.o
+obj-$(CONFIG_COMMON_CLK)	+= clk-factor.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-factor.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-rate.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-gate.o
diff --git a/drivers/clk/clk-factor.c b/drivers/clk/clk-factor.c
new file mode 100644
index 000000000000..17f83852cdb6
--- /dev/null
+++ b/drivers/clk/clk-factor.c
@@ -0,0 +1,176 @@
+/*
+ * Copyright (C) 2015 Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/of.h>
+
+#define to_clk_factor(_hw) container_of(_hw, struct clk_factor, hw)
+
+static unsigned long __get_mult(struct clk_factor *factor,
+				unsigned long rate,
+				unsigned long parent_rate)
+{
+	if (factor->flags & CLK_FACTOR_ROUND_CLOSEST)
+		return DIV_ROUND_CLOSEST(rate, parent_rate);
+
+	return rate / parent_rate;
+}
+
+static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
+					    unsigned long parent_rate)
+{
+	struct clk_factor *factor = to_clk_factor(hw);
+	unsigned long val;
+	
+	val = clk_readl(factor->reg) >> factor->shift;
+	val &= GENMASK(factor->width, 0);
+
+	if (!val && factor->flags & CLK_FACTOR_ZERO_BYPASS)
+		val = 1;
+	
+	return parent_rate * val;
+}
+
+static bool __is_best_rate(unsigned long rate, unsigned long new,
+			   unsigned long best, unsigned long flags)
+{
+	if (flags & CLK_FACTOR_ROUND_CLOSEST)
+		return abs(rate - new) < abs(rate - best);
+
+	return new >= rate && new < best;
+}
+
+static unsigned long clk_factor_bestmult(struct clk_hw *hw, unsigned long rate,
+					 unsigned long *best_parent_rate,
+					 u8 width, unsigned long flags)
+{
+	unsigned long orig_parent_rate = *best_parent_rate;
+	unsigned long parent_rate, current_rate, best_rate = ~0;
+	unsigned int i, bestmult = 0;
+
+	if (!(__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
+		return rate / *best_parent_rate;
+
+	for (i = 1; i < ((1 << width) - 1); i++) {
+		if (rate * i == orig_parent_rate) {
+			/*
+			 * This is the best case for us if we have a
+			 * perfect match without changing the parent
+			 * rate.
+			 */
+			*best_parent_rate = orig_parent_rate;
+			return i;
+		}
+
+		parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
+					       rate / i);
+		current_rate = parent_rate * i;
+
+		if (__is_best_rate(rate, current_rate, best_rate, flags)) {
+			bestmult = i;
+			best_rate = current_rate;
+			*best_parent_rate = parent_rate;
+		}
+	}
+
+	return bestmult;
+}
+
+static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
+				  unsigned long *parent_rate)
+{
+	struct clk_factor *factor = to_clk_factor(hw);
+	unsigned long mult = clk_factor_bestmult(hw, rate, parent_rate,
+						 factor->width, factor->flags);
+
+	return *parent_rate * mult;
+}
+
+static int clk_factor_set_rate(struct clk_hw *hw, unsigned long rate,
+			       unsigned long parent_rate)
+{
+	struct clk_factor *factor = to_clk_factor(hw);
+	unsigned long mult = __get_mult(factor, rate, parent_rate);
+	unsigned long uninitialized_var(flags);
+	struct clk *clk = hw->clk;
+	unsigned long val;
+
+	if (factor->lock)
+		spin_lock_irqsave(factor->lock, flags);
+
+	val = clk_readl(factor->reg);
+	val &= ~GENMASK(factor->width + factor->shift, factor->shift);
+	val |= mult << factor->shift;
+	clk_writel(val, factor->reg);
+
+	if (factor->lock)
+		spin_unlock_irqrestore(factor->lock, flags);
+
+	return 0;
+}
+
+const struct clk_ops clk_factor_ops = {
+	.recalc_rate	= clk_factor_recalc_rate,
+	.round_rate	= clk_factor_round_rate,
+	.set_rate	= clk_factor_set_rate,
+};
+EXPORT_SYMBOL_GPL(clk_factor_ops);
+
+struct clk *clk_register_factor(struct device *dev, const char *name,
+				const char *parent_name, unsigned long flags,
+				void __iomem *reg, u8 shift, u8 width,
+				u8 clk_factor_flags, spinlock_t *lock)
+{
+	struct clk_init_data init;
+	struct clk_factor *factor;
+	struct clk *clk;
+
+	factor = kmalloc(sizeof(*factor), GFP_KERNEL);
+	if (!factor) {
+		pr_err("%s: could not allocate fixed factor clk\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	init.name = name;
+	init.ops = &clk_factor_ops;
+	init.flags = flags | CLK_IS_BASIC;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	factor->reg = reg;
+	factor->shift = shift;
+	factor->width = width;
+	factor->flags = clk_factor_flags;
+	factor->lock = lock;
+	factor->hw.init = &init;
+	
+	clk = clk_register(dev, &factor->hw);
+	if (IS_ERR(clk))
+		kfree(factor);
+
+	return clk;
+}
+EXPORT_SYMBOL_GPL(clk_register_factor);
+
+void clk_unregister_factor(struct clk *clk)
+{
+	struct clk_factor *factor;
+	struct clk_hw *hw;
+
+	hw = __clk_get_hw(clk);
+	if (!hw)
+		return;
+
+	factor = to_clk_factor(hw);
+
+	clk_unregister(clk);
+	kfree(factor);
+}
+EXPORT_SYMBOL_GPL(clk_unregister_factor);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index df695313f975..543918c378f5 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -493,6 +493,47 @@ struct clk *clk_register_fractional_divider(struct device *dev,
 		void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
 		u8 clk_divider_flags, spinlock_t *lock);
 
+/**
+ * struct clk_factor - adjustable factor clock
+ *
+ * @hw:		handle between common and hardware-specific interfaces
+ * @reg:	register containing the factor
+ * @shift:	shift to the factor bit field
+ * @width:	width of the factor bit field
+ * @lock:	register lock
+ *
+ * Clock with an adjustable factor affecting its output frequency.
+ * Implements .recalc_rate, .set_rate and .round_rate
+ *
+ * Flags:
+ * CLK_FACTOR_ZERO_BYPASS - By default, the factor is the value read
+ *	from the register, with 0 being a valid value effectively
+ *	zeroing the output clock rate. If CLK_FACTOR_ZERO_BYPASS is
+ *	set, then a null factor will be considered as a bypass,
+ *	leaving the parent rate unmodified.
+ * CLK_FACTOR_ROUND_CLOSEST - Makes the best calculated divider to be
+ *	rounded to the closest integer instead of the down one.
+ */
+struct clk_factor {
+	struct clk_hw	hw;
+	void __iomem	*reg;
+	u8		shift;
+	u8		width;
+	u8		flags;
+	spinlock_t	*lock;
+};
+
+#define CLK_FACTOR_ZERO_BYPASS		BIT(0)
+#define CLK_FACTOR_ROUND_CLOSEST	BIT(1)
+
+extern const struct clk_ops clk_factor_ops;
+
+struct clk *clk_register_factor(struct device *dev, const char *name,
+				const char *parent_name, unsigned long flags,
+				void __iomem *reg, u8 shift, u8 width,
+				u8 clk_factor_flags, spinlock_t *lock);
+void clk_unregister_factor(struct clk *clk);
+
 /***
  * struct clk_composite - aggregate clock of mux, divider and gate clocks
  *
-- 
2.4.1

[PATCH v2 2/7] clk: sunxi: Add a driver for the PLL2

[Maxime Ripard]

The PLL2 on the A10 and later SoCs is the clock used for all the audio
related operations.

This clock has a somewhat complex output tree, with three outputs (2X, 4X
and 8X) with a fixed divider from the base clock, and an output (1X) with a
post divider.

However, we can simplify things since the 1X divider can be fixed, and we
end up by having a base clock not exposed to any device (or at least
directly, since the 4X output doesn't have any divider), and 4 fixed
divider clocks that will be exposed.

This clock seems to have been introduced, at least in this form, in the
revision B of the A10, but we don't have any information on the clock used
on the revision A.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/Makefile                 |   1 +
 drivers/clk/sunxi/clk-a10-pll2.c           | 189 +++++++++++++++++++++++++++++
 include/dt-bindings/clock/sun4i-a10-pll2.h |  53 ++++++++
 3 files changed, 243 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
 create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 058f273d6154..eb36c38d4120 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -4,6 +4,7 @@
 
 obj-y += clk-sunxi.o clk-factors.o
 obj-y += clk-a10-hosc.o
+obj-y += clk-a10-pll2.o
 obj-y += clk-a20-gmac.o
 obj-y += clk-mod0.o
 obj-y += clk-sun8i-mbus.o
diff --git a/drivers/clk/sunxi/clk-a10-pll2.c b/drivers/clk/sunxi/clk-a10-pll2.c
new file mode 100644
index 000000000000..8ffc3e0309cf
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-pll2.c
@@ -0,0 +1,189 @@
+/*
+ * Copyright 2013 Emilio López
+ * Emilio López <emilio@elopez.com.ar>
+ *
+ * Copyright 2015 Maxime Ripard
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
+
+#include "clk-factors.h"
+
+#define SUN4I_PLL2_ENABLE		31
+
+#define SUN4I_PLL2_PRE_DIV_SHIFT	0
+#define SUN4I_PLL2_PRE_DIV_WIDTH	5
+#define SUN4I_PLL2_PRE_DIV_MASK		GENMASK(SUN4I_PLL2_PRE_DIV_WIDTH, 0)
+
+#define SUN4I_PLL2_N_SHIFT		8
+#define SUN4I_PLL2_N_WIDTH		7
+#define SUN4I_PLL2_N_MASK		GENMASK(SUN4I_PLL2_N_WIDTH, 0)
+
+#define SUN4I_PLL2_POST_DIV_SHIFT	26
+#define SUN4I_PLL2_POST_DIV_WIDTH	4
+#define SUN4I_PLL2_POST_DIV_MASK	GENMASK(SUN4I_PLL2_POST_DIV_WIDTH, 0)
+
+#define SUN4I_PLL2_POST_DIV_VALUE	4
+
+#define SUN4I_PLL2_OUTPUTS		4
+
+static DEFINE_SPINLOCK(sun4i_a10_pll2_lock);
+
+static void __init sun4i_pll2_setup(struct device_node *node)
+{
+	const char *clk_name = node->name, *parent;
+	struct clk **clks, *base_clk, *prediv_clk;
+	struct clk_onecell_data *clk_data;
+	struct clk_factor *factor;
+	struct clk_gate *gate;
+	void __iomem *reg;
+	u32 val;
+
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (IS_ERR(reg))
+		return;
+
+	clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
+	if (!clk_data)
+		goto err_unmap;
+
+	clks = kcalloc(SUN4I_PLL2_OUTPUTS, sizeof(struct clk *), GFP_KERNEL);
+	if (!clks)
+		goto err_free_data;
+
+	parent = of_clk_get_parent_name(node, 0);
+	prediv_clk = clk_register_divider(NULL, "pll2-prediv",
+					  parent, 0, reg,
+					  SUN4I_PLL2_PRE_DIV_SHIFT,
+					  SUN4I_PLL2_PRE_DIV_WIDTH,
+					  CLK_DIVIDER_ONE_BASED |
+					  CLK_DIVIDER_ALLOW_ZERO,
+					  &sun4i_a10_pll2_lock);
+	if (!prediv_clk) {
+		pr_err("Couldn't register the prediv clock\n");
+		goto err_free_array;
+	}
+
+	/* Setup the gate part of the PLL2 */
+	gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
+	if (!gate)
+		goto err_unregister_prediv;
+
+	gate->reg = reg;
+	gate->bit_idx = SUN4I_PLL2_ENABLE;
+	gate->lock = &sun4i_a10_pll2_lock;
+
+	/* Setup the factor part of the PLL2 */
+	factor = kzalloc(sizeof(struct clk_factor), GFP_KERNEL);
+	if (!factor)
+		goto err_free_gate;
+
+	factor->reg = reg;
+	factor->shift = SUN4I_PLL2_N_SHIFT;
+	factor->width = 7;
+	factor->flags = CLK_FACTOR_ZERO_BYPASS | CLK_FACTOR_ROUND_CLOSEST;
+	factor->lock = &sun4i_a10_pll2_lock;
+
+	parent = __clk_get_name(prediv_clk);
+	base_clk = clk_register_composite(NULL, "pll2-base",
+					  &parent, 1,
+					  NULL, NULL,
+					  &factor->hw, &clk_factor_ops,
+					  &gate->hw, &clk_gate_ops,
+					  CLK_SET_RATE_PARENT);
+	if (!base_clk) {
+		pr_err("Couldn't register the base factor clock\n");
+		goto err_free_factor;
+	}
+
+	parent = __clk_get_name(base_clk);
+
+	/*
+	 * PLL2-1x
+	 *
+	 * This is supposed to have a post divider, but we won't need
+	 * to use it, we just need to initialise it to 4, and use a
+	 * fixed divider.
+	 */
+	val = readl(reg);
+	val &= ~(SUN4I_PLL2_POST_DIV_MASK << SUN4I_PLL2_POST_DIV_SHIFT);
+	val |= SUN4I_PLL2_POST_DIV_VALUE << SUN4I_PLL2_POST_DIV_SHIFT;
+	writel(val, reg);
+
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_1X, &clk_name);
+	clks[SUN4I_A10_PLL2_1X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    1,
+							    SUN4I_PLL2_POST_DIV_VALUE);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_1X]));
+
+	/*
+	 * PLL2-2x
+	 *
+	 * This clock doesn't use the post divider, and really is just
+	 * a fixed divider from the PLL2 base clock.
+	 */
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_2X, &clk_name);
+	clks[SUN4I_A10_PLL2_2X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    1, 2);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_2X]));
+
+	/* PLL2-4x */
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_4X, &clk_name);
+	clks[SUN4I_A10_PLL2_4X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    1, 1);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_4X]));
+
+	/* PLL2-8x */
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_8X, &clk_name);
+	clks[SUN4I_A10_PLL2_8X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    2, 1);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_8X]));
+
+	clk_data->clks = clks;
+	clk_data->clk_num = SUN4I_PLL2_OUTPUTS;
+	of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+
+	return;
+
+err_free_factor:
+	kfree(factor);
+err_free_gate:
+	kfree(gate);
+err_unregister_prediv:
+	clk_unregister_divider(prediv_clk);
+err_free_array:
+	kfree(clks);
+err_free_data:
+	kfree(clk_data);
+err_unmap:
+	iounmap(reg);
+}
+CLK_OF_DECLARE(sun4i_pll2, "allwinner,sun4i-a10-b-pll2-clk", sun4i_pll2_setup);
diff --git a/include/dt-bindings/clock/sun4i-a10-pll2.h b/include/dt-bindings/clock/sun4i-a10-pll2.h
new file mode 100644
index 000000000000..071c8112d531
--- /dev/null
+++ b/include/dt-bindings/clock/sun4i-a10-pll2.h
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2015 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_
+#define __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_
+
+#define SUN4I_A10_PLL2_1X	0
+#define SUN4I_A10_PLL2_2X	1
+#define SUN4I_A10_PLL2_4X	2
+#define SUN4I_A10_PLL2_8X	3
+
+#endif /* __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_ */
-- 
2.4.1

[PATCH v2 2/7] clk: sunxi: Add a driver for the PLL2

[Maxime Ripard]

The PLL2 on the A10 and later SoCs is the clock used for all the audio
related operations.

This clock has a somewhat complex output tree, with three outputs (2X, 4X
and 8X) with a fixed divider from the base clock, and an output (1X) with a
post divider.

However, we can simplify things since the 1X divider can be fixed, and we
end up by having a base clock not exposed to any device (or at least
directly, since the 4X output doesn't have any divider), and 4 fixed
divider clocks that will be exposed.

This clock seems to have been introduced, at least in this form, in the
revision B of the A10, but we don't have any information on the clock used
on the revision A.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/Makefile                 |   1 +
 drivers/clk/sunxi/clk-a10-pll2.c           | 189 +++++++++++++++++++++++++++++
 include/dt-bindings/clock/sun4i-a10-pll2.h |  53 ++++++++
 3 files changed, 243 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-pll2.c
 create mode 100644 include/dt-bindings/clock/sun4i-a10-pll2.h

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 058f273d6154..eb36c38d4120 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -4,6 +4,7 @@
 
 obj-y += clk-sunxi.o clk-factors.o
 obj-y += clk-a10-hosc.o
+obj-y += clk-a10-pll2.o
 obj-y += clk-a20-gmac.o
 obj-y += clk-mod0.o
 obj-y += clk-sun8i-mbus.o
diff --git a/drivers/clk/sunxi/clk-a10-pll2.c b/drivers/clk/sunxi/clk-a10-pll2.c
new file mode 100644
index 000000000000..8ffc3e0309cf
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-pll2.c
@@ -0,0 +1,189 @@
+/*
+ * Copyright 2013 Emilio L?pez
+ * Emilio L?pez <emilio@elopez.com.ar>
+ *
+ * Copyright 2015 Maxime Ripard
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
+
+#include "clk-factors.h"
+
+#define SUN4I_PLL2_ENABLE		31
+
+#define SUN4I_PLL2_PRE_DIV_SHIFT	0
+#define SUN4I_PLL2_PRE_DIV_WIDTH	5
+#define SUN4I_PLL2_PRE_DIV_MASK		GENMASK(SUN4I_PLL2_PRE_DIV_WIDTH, 0)
+
+#define SUN4I_PLL2_N_SHIFT		8
+#define SUN4I_PLL2_N_WIDTH		7
+#define SUN4I_PLL2_N_MASK		GENMASK(SUN4I_PLL2_N_WIDTH, 0)
+
+#define SUN4I_PLL2_POST_DIV_SHIFT	26
+#define SUN4I_PLL2_POST_DIV_WIDTH	4
+#define SUN4I_PLL2_POST_DIV_MASK	GENMASK(SUN4I_PLL2_POST_DIV_WIDTH, 0)
+
+#define SUN4I_PLL2_POST_DIV_VALUE	4
+
+#define SUN4I_PLL2_OUTPUTS		4
+
+static DEFINE_SPINLOCK(sun4i_a10_pll2_lock);
+
+static void __init sun4i_pll2_setup(struct device_node *node)
+{
+	const char *clk_name = node->name, *parent;
+	struct clk **clks, *base_clk, *prediv_clk;
+	struct clk_onecell_data *clk_data;
+	struct clk_factor *factor;
+	struct clk_gate *gate;
+	void __iomem *reg;
+	u32 val;
+
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (IS_ERR(reg))
+		return;
+
+	clk_data = kzalloc(sizeof(*clk_data), GFP_KERNEL);
+	if (!clk_data)
+		goto err_unmap;
+
+	clks = kcalloc(SUN4I_PLL2_OUTPUTS, sizeof(struct clk *), GFP_KERNEL);
+	if (!clks)
+		goto err_free_data;
+
+	parent = of_clk_get_parent_name(node, 0);
+	prediv_clk = clk_register_divider(NULL, "pll2-prediv",
+					  parent, 0, reg,
+					  SUN4I_PLL2_PRE_DIV_SHIFT,
+					  SUN4I_PLL2_PRE_DIV_WIDTH,
+					  CLK_DIVIDER_ONE_BASED |
+					  CLK_DIVIDER_ALLOW_ZERO,
+					  &sun4i_a10_pll2_lock);
+	if (!prediv_clk) {
+		pr_err("Couldn't register the prediv clock\n");
+		goto err_free_array;
+	}
+
+	/* Setup the gate part of the PLL2 */
+	gate = kzalloc(sizeof(struct clk_gate), GFP_KERNEL);
+	if (!gate)
+		goto err_unregister_prediv;
+
+	gate->reg = reg;
+	gate->bit_idx = SUN4I_PLL2_ENABLE;
+	gate->lock = &sun4i_a10_pll2_lock;
+
+	/* Setup the factor part of the PLL2 */
+	factor = kzalloc(sizeof(struct clk_factor), GFP_KERNEL);
+	if (!factor)
+		goto err_free_gate;
+
+	factor->reg = reg;
+	factor->shift = SUN4I_PLL2_N_SHIFT;
+	factor->width = 7;
+	factor->flags = CLK_FACTOR_ZERO_BYPASS | CLK_FACTOR_ROUND_CLOSEST;
+	factor->lock = &sun4i_a10_pll2_lock;
+
+	parent = __clk_get_name(prediv_clk);
+	base_clk = clk_register_composite(NULL, "pll2-base",
+					  &parent, 1,
+					  NULL, NULL,
+					  &factor->hw, &clk_factor_ops,
+					  &gate->hw, &clk_gate_ops,
+					  CLK_SET_RATE_PARENT);
+	if (!base_clk) {
+		pr_err("Couldn't register the base factor clock\n");
+		goto err_free_factor;
+	}
+
+	parent = __clk_get_name(base_clk);
+
+	/*
+	 * PLL2-1x
+	 *
+	 * This is supposed to have a post divider, but we won't need
+	 * to use it, we just need to initialise it to 4, and use a
+	 * fixed divider.
+	 */
+	val = readl(reg);
+	val &= ~(SUN4I_PLL2_POST_DIV_MASK << SUN4I_PLL2_POST_DIV_SHIFT);
+	val |= SUN4I_PLL2_POST_DIV_VALUE << SUN4I_PLL2_POST_DIV_SHIFT;
+	writel(val, reg);
+
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_1X, &clk_name);
+	clks[SUN4I_A10_PLL2_1X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    1,
+							    SUN4I_PLL2_POST_DIV_VALUE);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_1X]));
+
+	/*
+	 * PLL2-2x
+	 *
+	 * This clock doesn't use the post divider, and really is just
+	 * a fixed divider from the PLL2 base clock.
+	 */
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_2X, &clk_name);
+	clks[SUN4I_A10_PLL2_2X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    1, 2);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_2X]));
+
+	/* PLL2-4x */
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_4X, &clk_name);
+	clks[SUN4I_A10_PLL2_4X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    1, 1);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_4X]));
+
+	/* PLL2-8x */
+	of_property_read_string_index(node, "clock-output-names",
+				      SUN4I_A10_PLL2_8X, &clk_name);
+	clks[SUN4I_A10_PLL2_8X] = clk_register_fixed_factor(NULL, clk_name,
+							    parent,
+							    CLK_SET_RATE_PARENT,
+							    2, 1);
+	WARN_ON(IS_ERR(clks[SUN4I_A10_PLL2_8X]));
+
+	clk_data->clks = clks;
+	clk_data->clk_num = SUN4I_PLL2_OUTPUTS;
+	of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+
+	return;
+
+err_free_factor:
+	kfree(factor);
+err_free_gate:
+	kfree(gate);
+err_unregister_prediv:
+	clk_unregister_divider(prediv_clk);
+err_free_array:
+	kfree(clks);
+err_free_data:
+	kfree(clk_data);
+err_unmap:
+	iounmap(reg);
+}
+CLK_OF_DECLARE(sun4i_pll2, "allwinner,sun4i-a10-b-pll2-clk", sun4i_pll2_setup);
diff --git a/include/dt-bindings/clock/sun4i-a10-pll2.h b/include/dt-bindings/clock/sun4i-a10-pll2.h
new file mode 100644
index 000000000000..071c8112d531
--- /dev/null
+++ b/include/dt-bindings/clock/sun4i-a10-pll2.h
@@ -0,0 +1,53 @@
+/*
+ * Copyright 2015 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file is free software; you can redistribute it and/or
+ *     modify it under the terms of the GNU General Public License as
+ *     published by the Free Software Foundation; either version 2 of the
+ *     License, or (at your option) any later version.
+ *
+ *     This file is distributed in the hope that it will be useful,
+ *     but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *     GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ *     obtaining a copy of this software and associated documentation
+ *     files (the "Software"), to deal in the Software without
+ *     restriction, including without limitation the rights to use,
+ *     copy, modify, merge, publish, distribute, sublicense, and/or
+ *     sell copies of the Software, and to permit persons to whom the
+ *     Software is furnished to do so, subject to the following
+ *     conditions:
+ *
+ *     The above copyright notice and this permission notice shall be
+ *     included in all copies or substantial portions of the Software.
+ *
+ *     THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ *     EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ *     OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ *     NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ *     HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ *     WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ *     FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ *     OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_
+#define __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_
+
+#define SUN4I_A10_PLL2_1X	0
+#define SUN4I_A10_PLL2_2X	1
+#define SUN4I_A10_PLL2_4X	2
+#define SUN4I_A10_PLL2_8X	3
+
+#endif /* __DT_BINDINGS_CLOCK_SUN4I_A10_PLL2_H_ */
-- 
2.4.1

[PATCH v2 3/7] clk: sunxi: codec clock support

[Maxime Ripard]

From: Emilio López <emilio@elopez.com.ar>

The codec clock on sun4i, sun5i and sun7i is a simple gate with PLL2 as
parent. Add a driver for such a clock.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/Makefile        |  1 +
 drivers/clk/sunxi/clk-a10-codec.c | 45 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-codec.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index eb36c38d4120..6fa845e13067 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -3,6 +3,7 @@
 #
 
 obj-y += clk-sunxi.o clk-factors.o
+obj-y += clk-a10-codec.o
 obj-y += clk-a10-hosc.o
 obj-y += clk-a10-pll2.o
 obj-y += clk-a20-gmac.o
diff --git a/drivers/clk/sunxi/clk-a10-codec.c b/drivers/clk/sunxi/clk-a10-codec.c
new file mode 100644
index 000000000000..aaeccf8cde39
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-codec.c
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2013 Emilio López
+ *
+ * Emilio López <emilio@elopez.com.ar>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#define SUN4I_CODEC_GATE	31
+
+static void __init sun4i_codec_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	const char *clk_name = node->name, *parent_name;
+	void __iomem *reg;
+
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (IS_ERR(reg))
+		return;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	clk = clk_register_gate(NULL, clk_name, parent_name,
+				CLK_SET_RATE_PARENT, reg,
+				SUN4I_CODEC_GATE, 0, NULL);
+
+	if (!IS_ERR(clk))
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(sun4i_codec, "allwinner,sun4i-a10-codec-clk",
+	       sun4i_codec_clk_setup);
-- 
2.4.1

[PATCH v2 3/7] clk: sunxi: codec clock support

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

The codec clock on sun4i, sun5i and sun7i is a simple gate with PLL2 as
parent. Add a driver for such a clock.

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/Makefile        |  1 +
 drivers/clk/sunxi/clk-a10-codec.c | 45 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-codec.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index eb36c38d4120..6fa845e13067 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -3,6 +3,7 @@
 #
 
 obj-y += clk-sunxi.o clk-factors.o
+obj-y += clk-a10-codec.o
 obj-y += clk-a10-hosc.o
 obj-y += clk-a10-pll2.o
 obj-y += clk-a20-gmac.o
diff --git a/drivers/clk/sunxi/clk-a10-codec.c b/drivers/clk/sunxi/clk-a10-codec.c
new file mode 100644
index 000000000000..aaeccf8cde39
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-codec.c
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2013 Emilio L?pez
+ *
+ * Emilio L?pez <emilio@elopez.com.ar>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+#define SUN4I_CODEC_GATE	31
+
+static void __init sun4i_codec_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	const char *clk_name = node->name, *parent_name;
+	void __iomem *reg;
+
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (IS_ERR(reg))
+		return;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+	parent_name = of_clk_get_parent_name(node, 0);
+
+	clk = clk_register_gate(NULL, clk_name, parent_name,
+				CLK_SET_RATE_PARENT, reg,
+				SUN4I_CODEC_GATE, 0, NULL);
+
+	if (!IS_ERR(clk))
+		of_clk_add_provider(node, of_clk_src_simple_get, clk);
+}
+CLK_OF_DECLARE(sun4i_codec, "allwinner,sun4i-a10-codec-clk",
+	       sun4i_codec_clk_setup);
-- 
2.4.1

[PATCH v2 4/7] clk: sunxi: mod1 clock support

[Maxime Ripard]

From: Emilio López <emilio@elopez.com.ar>

The module 1 type of clocks consist of a gate and a mux and are used on
the audio blocks to mux and gate the PLL2 outputs for AC97, IIS or
SPDIF. This commit adds support for them on the sunxi clock driver.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/Makefile       |  1 +
 drivers/clk/sunxi/clk-a10-mod1.c | 84 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-mod1.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 6fa845e13067..960eeabc375f 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -5,6 +5,7 @@
 obj-y += clk-sunxi.o clk-factors.o
 obj-y += clk-a10-codec.o
 obj-y += clk-a10-hosc.o
+obj-y += clk-a10-mod1.o
 obj-y += clk-a10-pll2.o
 obj-y += clk-a20-gmac.o
 obj-y += clk-mod0.o
diff --git a/drivers/clk/sunxi/clk-a10-mod1.c b/drivers/clk/sunxi/clk-a10-mod1.c
new file mode 100644
index 000000000000..490f72432a1e
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-mod1.c
@@ -0,0 +1,84 @@
+/*
+ * Copyright 2013 Emilio López
+ *
+ * Emilio López <emilio@elopez.com.ar>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+static DEFINE_SPINLOCK(mod1_lock);
+
+#define SUN4I_MOD1_ENABLE	31
+#define SUN4I_MOD1_MUX		16
+#define SUN4I_MOD1_MUX_WIDTH	2
+#define SUN4I_MOD1_MAX_PARENTS	4
+
+static void __init sun4i_mod1_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	struct clk_mux *mux;
+	struct clk_gate *gate;
+	const char *parents[4];
+	const char *clk_name = node->name;
+	void __iomem *reg;
+	int i = 0;
+
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (IS_ERR(reg))
+		return;
+
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		goto err_unmap;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		goto err_free_mux;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	while (i < SUN4I_MOD1_MAX_PARENTS &&
+	       (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
+		i++;
+
+	gate->reg = reg;
+	gate->bit_idx = SUN4I_MOD1_ENABLE;
+	gate->lock = &mod1_lock;
+	mux->reg = reg;
+	mux->shift = SUN4I_MOD1_MUX;
+	mux->mask = BIT(SUN4I_MOD1_MUX_WIDTH) - 1;
+	mux->lock = &mod1_lock;
+
+	clk = clk_register_composite(NULL, clk_name, parents, i,
+				     &mux->hw, &clk_mux_ops,
+				     NULL, NULL,
+				     &gate->hw, &clk_gate_ops, 0);
+	if (IS_ERR(clk))
+		goto err_free_gate;
+
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+	return;
+
+err_free_gate:
+	kfree(gate);
+err_free_mux:
+	kfree(mux);
+err_unmap:
+	iounmap(reg);
+}
+CLK_OF_DECLARE(sun4i_mod1, "allwinner,sun4i-a10-mod1-clk",
+	       sun4i_mod1_clk_setup);
-- 
2.4.1

[PATCH v2 4/7] clk: sunxi: mod1 clock support

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

The module 1 type of clocks consist of a gate and a mux and are used on
the audio blocks to mux and gate the PLL2 outputs for AC97, IIS or
SPDIF. This commit adds support for them on the sunxi clock driver.

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/Makefile       |  1 +
 drivers/clk/sunxi/clk-a10-mod1.c | 84 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-mod1.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 6fa845e13067..960eeabc375f 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -5,6 +5,7 @@
 obj-y += clk-sunxi.o clk-factors.o
 obj-y += clk-a10-codec.o
 obj-y += clk-a10-hosc.o
+obj-y += clk-a10-mod1.o
 obj-y += clk-a10-pll2.o
 obj-y += clk-a20-gmac.o
 obj-y += clk-mod0.o
diff --git a/drivers/clk/sunxi/clk-a10-mod1.c b/drivers/clk/sunxi/clk-a10-mod1.c
new file mode 100644
index 000000000000..490f72432a1e
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-mod1.c
@@ -0,0 +1,84 @@
+/*
+ * Copyright 2013 Emilio L?pez
+ *
+ * Emilio L?pez <emilio@elopez.com.ar>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+
+static DEFINE_SPINLOCK(mod1_lock);
+
+#define SUN4I_MOD1_ENABLE	31
+#define SUN4I_MOD1_MUX		16
+#define SUN4I_MOD1_MUX_WIDTH	2
+#define SUN4I_MOD1_MAX_PARENTS	4
+
+static void __init sun4i_mod1_clk_setup(struct device_node *node)
+{
+	struct clk *clk;
+	struct clk_mux *mux;
+	struct clk_gate *gate;
+	const char *parents[4];
+	const char *clk_name = node->name;
+	void __iomem *reg;
+	int i = 0;
+
+	reg = of_io_request_and_map(node, 0, of_node_full_name(node));
+	if (IS_ERR(reg))
+		return;
+
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		goto err_unmap;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		goto err_free_mux;
+
+	of_property_read_string(node, "clock-output-names", &clk_name);
+
+	while (i < SUN4I_MOD1_MAX_PARENTS &&
+	       (parents[i] = of_clk_get_parent_name(node, i)) != NULL)
+		i++;
+
+	gate->reg = reg;
+	gate->bit_idx = SUN4I_MOD1_ENABLE;
+	gate->lock = &mod1_lock;
+	mux->reg = reg;
+	mux->shift = SUN4I_MOD1_MUX;
+	mux->mask = BIT(SUN4I_MOD1_MUX_WIDTH) - 1;
+	mux->lock = &mod1_lock;
+
+	clk = clk_register_composite(NULL, clk_name, parents, i,
+				     &mux->hw, &clk_mux_ops,
+				     NULL, NULL,
+				     &gate->hw, &clk_gate_ops, 0);
+	if (IS_ERR(clk))
+		goto err_free_gate;
+
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+	return;
+
+err_free_gate:
+	kfree(gate);
+err_free_mux:
+	kfree(mux);
+err_unmap:
+	iounmap(reg);
+}
+CLK_OF_DECLARE(sun4i_mod1, "allwinner,sun4i-a10-mod1-clk",
+	       sun4i_mod1_clk_setup);
-- 
2.4.1

[PATCH v2 5/7] ARM: sunxi: Add PLL2 support

[Maxime Ripard]

From: Emilio López <emilio@elopez.com.ar>

This commit adds the PLL2 definition to the sun4i, sun5i and sun7i device
trees. PLL2 is used to clock audio devices.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 9 +++++++++
 arch/arm/boot/dts/sun5i.dtsi     | 9 +++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi | 9 +++++++++
 3 files changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 73b6143e3e91..235e5204742f 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -195,6 +195,15 @@
 			clock-output-names = "pll1";
 		};
 
+		pll2: clk@01c20008 {
+			#clock-cells = <1>;
+			compatible = "allwinner,sun4i-a10-b-pll2-clk";
+			reg = <0x01c20008 0x8>;
+			clocks = <&osc24M>;
+			clock-output-names = "pll2-1x", "pll2-2x",
+					     "pll2-4x", "pll2-8x";
+		};
+
 		pll4: clk@01c20018 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-pll1-clk";
diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi
index 5f662435a935..2f553a92a6dc 100644
--- a/arch/arm/boot/dts/sun5i.dtsi
+++ b/arch/arm/boot/dts/sun5i.dtsi
@@ -102,6 +102,15 @@
 			clock-output-names = "pll1";
 		};
 
+		pll2: clk@01c20008 {
+			#clock-cells = <1>;
+			compatible = "allwinner,sun4i-a10-b-pll2-clk";
+			reg = <0x01c20008 0x8>;
+			clocks = <&osc24M>;
+			clock-output-names = "pll2-1x", "pll2-2x",
+					     "pll2-4x", "pll2-8x";
+		};
+
 		pll4: clk@01c20018 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-pll1-clk";
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index e04cdf9d53f9..0141cc9876a7 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -199,6 +199,15 @@
 			clock-output-names = "pll1";
 		};
 
+		pll2: clk@01c20008 {
+			#clock-cells = <1>;
+			compatible = "allwinner,sun4i-a10-b-pll2-clk";
+			reg = <0x01c20008 0x8>;
+			clocks = <&osc24M>;
+			clock-output-names = "pll2-1x", "pll2-2x",
+					     "pll2-4x", "pll2-8x";
+		};
+
 		pll4: clk@01c20018 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun7i-a20-pll4-clk";
-- 
2.4.1

[PATCH v2 5/7] ARM: sunxi: Add PLL2 support

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

This commit adds the PLL2 definition to the sun4i, sun5i and sun7i device
trees. PLL2 is used to clock audio devices.

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun4i-a10.dtsi | 9 +++++++++
 arch/arm/boot/dts/sun5i.dtsi     | 9 +++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi | 9 +++++++++
 3 files changed, 27 insertions(+)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 73b6143e3e91..235e5204742f 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -195,6 +195,15 @@
 			clock-output-names = "pll1";
 		};
 
+		pll2: clk at 01c20008 {
+			#clock-cells = <1>;
+			compatible = "allwinner,sun4i-a10-b-pll2-clk";
+			reg = <0x01c20008 0x8>;
+			clocks = <&osc24M>;
+			clock-output-names = "pll2-1x", "pll2-2x",
+					     "pll2-4x", "pll2-8x";
+		};
+
 		pll4: clk at 01c20018 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-pll1-clk";
diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi
index 5f662435a935..2f553a92a6dc 100644
--- a/arch/arm/boot/dts/sun5i.dtsi
+++ b/arch/arm/boot/dts/sun5i.dtsi
@@ -102,6 +102,15 @@
 			clock-output-names = "pll1";
 		};
 
+		pll2: clk at 01c20008 {
+			#clock-cells = <1>;
+			compatible = "allwinner,sun4i-a10-b-pll2-clk";
+			reg = <0x01c20008 0x8>;
+			clocks = <&osc24M>;
+			clock-output-names = "pll2-1x", "pll2-2x",
+					     "pll2-4x", "pll2-8x";
+		};
+
 		pll4: clk at 01c20018 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-pll1-clk";
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index e04cdf9d53f9..0141cc9876a7 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -199,6 +199,15 @@
 			clock-output-names = "pll1";
 		};
 
+		pll2: clk at 01c20008 {
+			#clock-cells = <1>;
+			compatible = "allwinner,sun4i-a10-b-pll2-clk";
+			reg = <0x01c20008 0x8>;
+			clocks = <&osc24M>;
+			clock-output-names = "pll2-1x", "pll2-2x",
+					     "pll2-4x", "pll2-8x";
+		};
+
 		pll4: clk at 01c20018 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun7i-a20-pll4-clk";
-- 
2.4.1

[PATCH v2 6/7] ARM: sunxi: Add codec clock support

[Maxime Ripard]

From: Emilio López <emilio@elopez.com.ar>

This commit adds the codec clock definition to the sun4i, sun5i and
sun7i device trees. The codec clock is used in the analog codec block.

Signed-off-by: Emilio López <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun4i-a10.dtsi  | 9 +++++++++
 arch/arm/boot/dts/sun5i.dtsi      | 9 +++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi  | 9 +++++++++
 drivers/clk/sunxi/clk-a10-codec.c | 1 -
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 235e5204742f..dfa8cd2d3ba8 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -45,6 +45,7 @@
 
 #include <dt-bindings/thermal/thermal.h>
 
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
 
@@ -455,6 +456,14 @@
 			clocks = <&osc24M>, <&pll6 1>, <&pll5 1>;
 			clock-output-names = "spi3";
 		};
+
+		codec_clk: clk@01c20140 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-codec-clk";
+			reg = <0x01c20140 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "codec";
+		};
 	};
 
 	/*
diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi
index 2f553a92a6dc..ab5184d2094d 100644
--- a/arch/arm/boot/dts/sun5i.dtsi
+++ b/arch/arm/boot/dts/sun5i.dtsi
@@ -44,6 +44,7 @@
 
 #include "skeleton.dtsi"
 
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
 
@@ -293,6 +294,14 @@
 			clock-output-names = "usb_ohci0", "usb_phy";
 		};
 
+		codec_clk: clk@01c20140 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-codec-clk";
+			reg = <0x01c20140 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "codec";
+		};
+
 		mbus_clk: clk@01c2015c {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun5i-a13-mbus-clk";
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 0141cc9876a7..b5ecfa5fa0eb 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -47,6 +47,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/thermal/thermal.h>
 
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
 
@@ -469,6 +470,14 @@
 			clock-output-names = "spi3";
 		};
 
+		codec_clk: clk@01c20140 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-codec-clk";
+			reg = <0x01c20140 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "codec";
+		};
+
 		mbus_clk: clk@01c2015c {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun5i-a13-mbus-clk";
diff --git a/drivers/clk/sunxi/clk-a10-codec.c b/drivers/clk/sunxi/clk-a10-codec.c
index aaeccf8cde39..ac321d6a0df5 100644
--- a/drivers/clk/sunxi/clk-a10-codec.c
+++ b/drivers/clk/sunxi/clk-a10-codec.c
@@ -15,7 +15,6 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-- 
2.4.1

[PATCH v2 6/7] ARM: sunxi: Add codec clock support

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

This commit adds the codec clock definition to the sun4i, sun5i and
sun7i device trees. The codec clock is used in the analog codec block.

Signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
 arch/arm/boot/dts/sun4i-a10.dtsi  | 9 +++++++++
 arch/arm/boot/dts/sun5i.dtsi      | 9 +++++++++
 arch/arm/boot/dts/sun7i-a20.dtsi  | 9 +++++++++
 drivers/clk/sunxi/clk-a10-codec.c | 1 -
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 235e5204742f..dfa8cd2d3ba8 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -45,6 +45,7 @@
 
 #include <dt-bindings/thermal/thermal.h>
 
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
 
@@ -455,6 +456,14 @@
 			clocks = <&osc24M>, <&pll6 1>, <&pll5 1>;
 			clock-output-names = "spi3";
 		};
+
+		codec_clk: clk at 01c20140 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-codec-clk";
+			reg = <0x01c20140 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "codec";
+		};
 	};
 
 	/*
diff --git a/arch/arm/boot/dts/sun5i.dtsi b/arch/arm/boot/dts/sun5i.dtsi
index 2f553a92a6dc..ab5184d2094d 100644
--- a/arch/arm/boot/dts/sun5i.dtsi
+++ b/arch/arm/boot/dts/sun5i.dtsi
@@ -44,6 +44,7 @@
 
 #include "skeleton.dtsi"
 
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
 
@@ -293,6 +294,14 @@
 			clock-output-names = "usb_ohci0", "usb_phy";
 		};
 
+		codec_clk: clk at 01c20140 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-codec-clk";
+			reg = <0x01c20140 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "codec";
+		};
+
 		mbus_clk: clk at 01c2015c {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun5i-a13-mbus-clk";
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 0141cc9876a7..b5ecfa5fa0eb 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -47,6 +47,7 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/thermal/thermal.h>
 
+#include <dt-bindings/clock/sun4i-a10-pll2.h>
 #include <dt-bindings/dma/sun4i-a10.h>
 #include <dt-bindings/pinctrl/sun4i-a10.h>
 
@@ -469,6 +470,14 @@
 			clock-output-names = "spi3";
 		};
 
+		codec_clk: clk at 01c20140 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-codec-clk";
+			reg = <0x01c20140 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "codec";
+		};
+
 		mbus_clk: clk at 01c2015c {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun5i-a13-mbus-clk";
diff --git a/drivers/clk/sunxi/clk-a10-codec.c b/drivers/clk/sunxi/clk-a10-codec.c
index aaeccf8cde39..ac321d6a0df5 100644
--- a/drivers/clk/sunxi/clk-a10-codec.c
+++ b/drivers/clk/sunxi/clk-a10-codec.c
@@ -15,7 +15,6 @@
  */
 
 #include <linux/clk-provider.h>
-#include <linux/clkdev.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 
-- 
2.4.1

[PATCH v2 7/7] ARM: sun7i: Add mod1 clock nodes

[Maxime Ripard]

From: Emilio López <emilio@elopez.com.ar>

This commit adds all the mod1 clocks available on A20 to its device
tree. This list was created by looking at the A20 user manual.

Not-signed-off-by: Emilio López <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 55 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index b5ecfa5fa0eb..9b85e9a9ca61 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -452,6 +452,39 @@
 			clock-output-names = "ir1";
 		};
 
+		iis0_clk: clk@01c200b8 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200b8 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_8X>;
+			clock-output-names = "iis0";
+		};
+
+		ac97_clk: clk@01c200bc {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200bc 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_8X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "ac97";
+		};
+
+		spdif_clk: clk@01c200c0 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200c0 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_8X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "spdif";
+		};
+
 		usb_clk: clk@01c200cc {
 			#clock-cells = <1>;
 			#reset-cells = <1>;
@@ -470,6 +503,28 @@
 			clock-output-names = "spi3";
 		};
 
+		iis1_clk: clk@01c200d8 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200d8 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_8X>;
+			clock-output-names = "iis1";
+		};
+
+		iis2_clk: clk@01c200dc {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200dc 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_8X>;
+			clock-output-names = "iis2";
+		};
+
 		codec_clk: clk@01c20140 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-codec-clk";
-- 
2.4.1

[PATCH v2 7/7] ARM: sun7i: Add mod1 clock nodes

[Maxime Ripard]

From: Emilio L?pez <emilio@elopez.com.ar>

This commit adds all the mod1 clocks available on A20 to its device
tree. This list was created by looking at the A20 user manual.

Not-signed-off-by: Emilio L?pez <emilio@elopez.com.ar>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 55 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index b5ecfa5fa0eb..9b85e9a9ca61 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -452,6 +452,39 @@
 			clock-output-names = "ir1";
 		};
 
+		iis0_clk: clk at 01c200b8 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200b8 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_8X>;
+			clock-output-names = "iis0";
+		};
+
+		ac97_clk: clk at 01c200bc {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200bc 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_8X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "ac97";
+		};
+
+		spdif_clk: clk at 01c200c0 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200c0 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_8X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_1X>;
+			clock-output-names = "spdif";
+		};
+
 		usb_clk: clk at 01c200cc {
 			#clock-cells = <1>;
 			#reset-cells = <1>;
@@ -470,6 +503,28 @@
 			clock-output-names = "spi3";
 		};
 
+		iis1_clk: clk at 01c200d8 {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200d8 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_8X>;
+			clock-output-names = "iis1";
+		};
+
+		iis2_clk: clk at 01c200dc {
+			#clock-cells = <0>;
+			compatible = "allwinner,sun4i-a10-mod1-clk";
+			reg = <0x01c200dc 0x4>;
+			clocks = <&pll2 SUN4I_A10_PLL2_1X>,
+				 <&pll2 SUN4I_A10_PLL2_2X>,
+				 <&pll2 SUN4I_A10_PLL2_4X>,
+				 <&pll2 SUN4I_A10_PLL2_8X>;
+			clock-output-names = "iis2";
+		};
+
 		codec_clk: clk at 01c20140 {
 			#clock-cells = <0>;
 			compatible = "allwinner,sun4i-a10-codec-clk";
-- 
2.4.1

Re: [PATCH v2 1/7] clk: Add a basic factor clock

[Chen-Yu Tsai]

Hi,

On Fri, May 22, 2015 at 4:54 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Some clocks are using a factor component, however, unlike their mux, gate
> or divider counterpart, these factors don't have a basic clock
> implementation.

I think "multiplier" would be a better name here, considering it is the
counterpart of "divider". "factor" implies you can multiply and/or divide
the clock rate.

Unless of course you plan to add that? :)

ChenYu

> This leads to code duplication across platforms that want to use that kind
> of clocks, and the impossibility to use the composite clocks with such a
> clock without defining your own rate operations.
>
> Create such a driver in order to remove these issues, and hopefully factor
> the implementations, reducing code size across platforms and consolidating
> the various implementations.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/Makefile         |   1 +
>  drivers/clk/clk-factor.c     | 176 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk-provider.h |  41 ++++++++++
>  3 files changed, 218 insertions(+)
>  create mode 100644 drivers/clk/clk-factor.c
>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 3d00c25382c5..d38b71d15ffa 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_HAVE_CLK)          += clk-devres.o
>  obj-$(CONFIG_CLKDEV_LOOKUP)    += clkdev.o
>  obj-$(CONFIG_COMMON_CLK)       += clk.o
>  obj-$(CONFIG_COMMON_CLK)       += clk-divider.o
> +obj-$(CONFIG_COMMON_CLK)       += clk-factor.o
>  obj-$(CONFIG_COMMON_CLK)       += clk-fixed-factor.o
>  obj-$(CONFIG_COMMON_CLK)       += clk-fixed-rate.o
>  obj-$(CONFIG_COMMON_CLK)       += clk-gate.o
> diff --git a/drivers/clk/clk-factor.c b/drivers/clk/clk-factor.c
> new file mode 100644
> index 000000000000..17f83852cdb6
> --- /dev/null
> +++ b/drivers/clk/clk-factor.c
> @@ -0,0 +1,176 @@
> +/*
> + * Copyright (C) 2015 Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/module.h>
> +#include <linux/clk-provider.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +
> +#define to_clk_factor(_hw) container_of(_hw, struct clk_factor, hw)
> +
> +static unsigned long __get_mult(struct clk_factor *factor,
> +                               unsigned long rate,
> +                               unsigned long parent_rate)
> +{
> +       if (factor->flags & CLK_FACTOR_ROUND_CLOSEST)
> +               return DIV_ROUND_CLOSEST(rate, parent_rate);
> +
> +       return rate / parent_rate;
> +}
> +
> +static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
> +                                           unsigned long parent_rate)
> +{
> +       struct clk_factor *factor = to_clk_factor(hw);
> +       unsigned long val;
> +
> +       val = clk_readl(factor->reg) >> factor->shift;
> +       val &= GENMASK(factor->width, 0);
> +
> +       if (!val && factor->flags & CLK_FACTOR_ZERO_BYPASS)
> +               val = 1;
> +
> +       return parent_rate * val;
> +}
> +
> +static bool __is_best_rate(unsigned long rate, unsigned long new,
> +                          unsigned long best, unsigned long flags)
> +{
> +       if (flags & CLK_FACTOR_ROUND_CLOSEST)
> +               return abs(rate - new) < abs(rate - best);
> +
> +       return new >= rate && new < best;
> +}
> +
> +static unsigned long clk_factor_bestmult(struct clk_hw *hw, unsigned long rate,
> +                                        unsigned long *best_parent_rate,
> +                                        u8 width, unsigned long flags)
> +{
> +       unsigned long orig_parent_rate = *best_parent_rate;
> +       unsigned long parent_rate, current_rate, best_rate = ~0;
> +       unsigned int i, bestmult = 0;
> +
> +       if (!(__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
> +               return rate / *best_parent_rate;
> +
> +       for (i = 1; i < ((1 << width) - 1); i++) {
> +               if (rate * i == orig_parent_rate) {
> +                       /*
> +                        * This is the best case for us if we have a
> +                        * perfect match without changing the parent
> +                        * rate.
> +                        */
> +                       *best_parent_rate = orig_parent_rate;
> +                       return i;
> +               }
> +
> +               parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
> +                                              rate / i);
> +               current_rate = parent_rate * i;
> +
> +               if (__is_best_rate(rate, current_rate, best_rate, flags)) {
> +                       bestmult = i;
> +                       best_rate = current_rate;
> +                       *best_parent_rate = parent_rate;
> +               }
> +       }
> +
> +       return bestmult;
> +}
> +
> +static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
> +                                 unsigned long *parent_rate)
> +{
> +       struct clk_factor *factor = to_clk_factor(hw);
> +       unsigned long mult = clk_factor_bestmult(hw, rate, parent_rate,
> +                                                factor->width, factor->flags);
> +
> +       return *parent_rate * mult;
> +}
> +
> +static int clk_factor_set_rate(struct clk_hw *hw, unsigned long rate,
> +                              unsigned long parent_rate)
> +{
> +       struct clk_factor *factor = to_clk_factor(hw);
> +       unsigned long mult = __get_mult(factor, rate, parent_rate);
> +       unsigned long uninitialized_var(flags);
> +       struct clk *clk = hw->clk;
> +       unsigned long val;
> +
> +       if (factor->lock)
> +               spin_lock_irqsave(factor->lock, flags);
> +
> +       val = clk_readl(factor->reg);
> +       val &= ~GENMASK(factor->width + factor->shift, factor->shift);
> +       val |= mult << factor->shift;
> +       clk_writel(val, factor->reg);
> +
> +       if (factor->lock)
> +               spin_unlock_irqrestore(factor->lock, flags);
> +
> +       return 0;
> +}
> +
> +const struct clk_ops clk_factor_ops = {
> +       .recalc_rate    = clk_factor_recalc_rate,
> +       .round_rate     = clk_factor_round_rate,
> +       .set_rate       = clk_factor_set_rate,
> +};
> +EXPORT_SYMBOL_GPL(clk_factor_ops);
> +
> +struct clk *clk_register_factor(struct device *dev, const char *name,
> +                               const char *parent_name, unsigned long flags,
> +                               void __iomem *reg, u8 shift, u8 width,
> +                               u8 clk_factor_flags, spinlock_t *lock)
> +{
> +       struct clk_init_data init;
> +       struct clk_factor *factor;
> +       struct clk *clk;
> +
> +       factor = kmalloc(sizeof(*factor), GFP_KERNEL);
> +       if (!factor) {
> +               pr_err("%s: could not allocate fixed factor clk\n", __func__);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       init.name = name;
> +       init.ops = &clk_factor_ops;
> +       init.flags = flags | CLK_IS_BASIC;
> +       init.parent_names = &parent_name;
> +       init.num_parents = 1;
> +
> +       factor->reg = reg;
> +       factor->shift = shift;
> +       factor->width = width;
> +       factor->flags = clk_factor_flags;
> +       factor->lock = lock;
> +       factor->hw.init = &init;
> +
> +       clk = clk_register(dev, &factor->hw);
> +       if (IS_ERR(clk))
> +               kfree(factor);
> +
> +       return clk;
> +}
> +EXPORT_SYMBOL_GPL(clk_register_factor);
> +
> +void clk_unregister_factor(struct clk *clk)
> +{
> +       struct clk_factor *factor;
> +       struct clk_hw *hw;
> +
> +       hw = __clk_get_hw(clk);
> +       if (!hw)
> +               return;
> +
> +       factor = to_clk_factor(hw);
> +
> +       clk_unregister(clk);
> +       kfree(factor);
> +}
> +EXPORT_SYMBOL_GPL(clk_unregister_factor);
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index df695313f975..543918c378f5 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -493,6 +493,47 @@ struct clk *clk_register_fractional_divider(struct device *dev,
>                 void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
>                 u8 clk_divider_flags, spinlock_t *lock);
>
> +/**
> + * struct clk_factor - adjustable factor clock
> + *
> + * @hw:                handle between common and hardware-specific interfaces
> + * @reg:       register containing the factor
> + * @shift:     shift to the factor bit field
> + * @width:     width of the factor bit field
> + * @lock:      register lock
> + *
> + * Clock with an adjustable factor affecting its output frequency.
> + * Implements .recalc_rate, .set_rate and .round_rate
> + *
> + * Flags:
> + * CLK_FACTOR_ZERO_BYPASS - By default, the factor is the value read
> + *     from the register, with 0 being a valid value effectively
> + *     zeroing the output clock rate. If CLK_FACTOR_ZERO_BYPASS is
> + *     set, then a null factor will be considered as a bypass,
> + *     leaving the parent rate unmodified.
> + * CLK_FACTOR_ROUND_CLOSEST - Makes the best calculated divider to be
> + *     rounded to the closest integer instead of the down one.
> + */
> +struct clk_factor {
> +       struct clk_hw   hw;
> +       void __iomem    *reg;
> +       u8              shift;
> +       u8              width;
> +       u8              flags;
> +       spinlock_t      *lock;
> +};
> +
> +#define CLK_FACTOR_ZERO_BYPASS         BIT(0)
> +#define CLK_FACTOR_ROUND_CLOSEST       BIT(1)
> +
> +extern const struct clk_ops clk_factor_ops;
> +
> +struct clk *clk_register_factor(struct device *dev, const char *name,
> +                               const char *parent_name, unsigned long flags,
> +                               void __iomem *reg, u8 shift, u8 width,
> +                               u8 clk_factor_flags, spinlock_t *lock);
> +void clk_unregister_factor(struct clk *clk);
> +
>  /***
>   * struct clk_composite - aggregate clock of mux, divider and gate clocks
>   *
> --
> 2.4.1
>

[PATCH v2 1/7] clk: Add a basic factor clock

[Chen-Yu Tsai]

Hi,

On Fri, May 22, 2015 at 4:54 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Some clocks are using a factor component, however, unlike their mux, gate
> or divider counterpart, these factors don't have a basic clock
> implementation.

I think "multiplier" would be a better name here, considering it is the
counterpart of "divider". "factor" implies you can multiply and/or divide
the clock rate.

Unless of course you plan to add that? :)

ChenYu

> This leads to code duplication across platforms that want to use that kind
> of clocks, and the impossibility to use the composite clocks with such a
> clock without defining your own rate operations.
>
> Create such a driver in order to remove these issues, and hopefully factor
> the implementations, reducing code size across platforms and consolidating
> the various implementations.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/Makefile         |   1 +
>  drivers/clk/clk-factor.c     | 176 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/clk-provider.h |  41 ++++++++++
>  3 files changed, 218 insertions(+)
>  create mode 100644 drivers/clk/clk-factor.c
>
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 3d00c25382c5..d38b71d15ffa 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -3,6 +3,7 @@ obj-$(CONFIG_HAVE_CLK)          += clk-devres.o
>  obj-$(CONFIG_CLKDEV_LOOKUP)    += clkdev.o
>  obj-$(CONFIG_COMMON_CLK)       += clk.o
>  obj-$(CONFIG_COMMON_CLK)       += clk-divider.o
> +obj-$(CONFIG_COMMON_CLK)       += clk-factor.o
>  obj-$(CONFIG_COMMON_CLK)       += clk-fixed-factor.o
>  obj-$(CONFIG_COMMON_CLK)       += clk-fixed-rate.o
>  obj-$(CONFIG_COMMON_CLK)       += clk-gate.o
> diff --git a/drivers/clk/clk-factor.c b/drivers/clk/clk-factor.c
> new file mode 100644
> index 000000000000..17f83852cdb6
> --- /dev/null
> +++ b/drivers/clk/clk-factor.c
> @@ -0,0 +1,176 @@
> +/*
> + * Copyright (C) 2015 Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +#include <linux/module.h>
> +#include <linux/clk-provider.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +
> +#define to_clk_factor(_hw) container_of(_hw, struct clk_factor, hw)
> +
> +static unsigned long __get_mult(struct clk_factor *factor,
> +                               unsigned long rate,
> +                               unsigned long parent_rate)
> +{
> +       if (factor->flags & CLK_FACTOR_ROUND_CLOSEST)
> +               return DIV_ROUND_CLOSEST(rate, parent_rate);
> +
> +       return rate / parent_rate;
> +}
> +
> +static unsigned long clk_factor_recalc_rate(struct clk_hw *hw,
> +                                           unsigned long parent_rate)
> +{
> +       struct clk_factor *factor = to_clk_factor(hw);
> +       unsigned long val;
> +
> +       val = clk_readl(factor->reg) >> factor->shift;
> +       val &= GENMASK(factor->width, 0);
> +
> +       if (!val && factor->flags & CLK_FACTOR_ZERO_BYPASS)
> +               val = 1;
> +
> +       return parent_rate * val;
> +}
> +
> +static bool __is_best_rate(unsigned long rate, unsigned long new,
> +                          unsigned long best, unsigned long flags)
> +{
> +       if (flags & CLK_FACTOR_ROUND_CLOSEST)
> +               return abs(rate - new) < abs(rate - best);
> +
> +       return new >= rate && new < best;
> +}
> +
> +static unsigned long clk_factor_bestmult(struct clk_hw *hw, unsigned long rate,
> +                                        unsigned long *best_parent_rate,
> +                                        u8 width, unsigned long flags)
> +{
> +       unsigned long orig_parent_rate = *best_parent_rate;
> +       unsigned long parent_rate, current_rate, best_rate = ~0;
> +       unsigned int i, bestmult = 0;
> +
> +       if (!(__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT))
> +               return rate / *best_parent_rate;
> +
> +       for (i = 1; i < ((1 << width) - 1); i++) {
> +               if (rate * i == orig_parent_rate) {
> +                       /*
> +                        * This is the best case for us if we have a
> +                        * perfect match without changing the parent
> +                        * rate.
> +                        */
> +                       *best_parent_rate = orig_parent_rate;
> +                       return i;
> +               }
> +
> +               parent_rate = __clk_round_rate(__clk_get_parent(hw->clk),
> +                                              rate / i);
> +               current_rate = parent_rate * i;
> +
> +               if (__is_best_rate(rate, current_rate, best_rate, flags)) {
> +                       bestmult = i;
> +                       best_rate = current_rate;
> +                       *best_parent_rate = parent_rate;
> +               }
> +       }
> +
> +       return bestmult;
> +}
> +
> +static long clk_factor_round_rate(struct clk_hw *hw, unsigned long rate,
> +                                 unsigned long *parent_rate)
> +{
> +       struct clk_factor *factor = to_clk_factor(hw);
> +       unsigned long mult = clk_factor_bestmult(hw, rate, parent_rate,
> +                                                factor->width, factor->flags);
> +
> +       return *parent_rate * mult;
> +}
> +
> +static int clk_factor_set_rate(struct clk_hw *hw, unsigned long rate,
> +                              unsigned long parent_rate)
> +{
> +       struct clk_factor *factor = to_clk_factor(hw);
> +       unsigned long mult = __get_mult(factor, rate, parent_rate);
> +       unsigned long uninitialized_var(flags);
> +       struct clk *clk = hw->clk;
> +       unsigned long val;
> +
> +       if (factor->lock)
> +               spin_lock_irqsave(factor->lock, flags);
> +
> +       val = clk_readl(factor->reg);
> +       val &= ~GENMASK(factor->width + factor->shift, factor->shift);
> +       val |= mult << factor->shift;
> +       clk_writel(val, factor->reg);
> +
> +       if (factor->lock)
> +               spin_unlock_irqrestore(factor->lock, flags);
> +
> +       return 0;
> +}
> +
> +const struct clk_ops clk_factor_ops = {
> +       .recalc_rate    = clk_factor_recalc_rate,
> +       .round_rate     = clk_factor_round_rate,
> +       .set_rate       = clk_factor_set_rate,
> +};
> +EXPORT_SYMBOL_GPL(clk_factor_ops);
> +
> +struct clk *clk_register_factor(struct device *dev, const char *name,
> +                               const char *parent_name, unsigned long flags,
> +                               void __iomem *reg, u8 shift, u8 width,
> +                               u8 clk_factor_flags, spinlock_t *lock)
> +{
> +       struct clk_init_data init;
> +       struct clk_factor *factor;
> +       struct clk *clk;
> +
> +       factor = kmalloc(sizeof(*factor), GFP_KERNEL);
> +       if (!factor) {
> +               pr_err("%s: could not allocate fixed factor clk\n", __func__);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       init.name = name;
> +       init.ops = &clk_factor_ops;
> +       init.flags = flags | CLK_IS_BASIC;
> +       init.parent_names = &parent_name;
> +       init.num_parents = 1;
> +
> +       factor->reg = reg;
> +       factor->shift = shift;
> +       factor->width = width;
> +       factor->flags = clk_factor_flags;
> +       factor->lock = lock;
> +       factor->hw.init = &init;
> +
> +       clk = clk_register(dev, &factor->hw);
> +       if (IS_ERR(clk))
> +               kfree(factor);
> +
> +       return clk;
> +}
> +EXPORT_SYMBOL_GPL(clk_register_factor);
> +
> +void clk_unregister_factor(struct clk *clk)
> +{
> +       struct clk_factor *factor;
> +       struct clk_hw *hw;
> +
> +       hw = __clk_get_hw(clk);
> +       if (!hw)
> +               return;
> +
> +       factor = to_clk_factor(hw);
> +
> +       clk_unregister(clk);
> +       kfree(factor);
> +}
> +EXPORT_SYMBOL_GPL(clk_unregister_factor);
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index df695313f975..543918c378f5 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -493,6 +493,47 @@ struct clk *clk_register_fractional_divider(struct device *dev,
>                 void __iomem *reg, u8 mshift, u8 mwidth, u8 nshift, u8 nwidth,
>                 u8 clk_divider_flags, spinlock_t *lock);
>
> +/**
> + * struct clk_factor - adjustable factor clock
> + *
> + * @hw:                handle between common and hardware-specific interfaces
> + * @reg:       register containing the factor
> + * @shift:     shift to the factor bit field
> + * @width:     width of the factor bit field
> + * @lock:      register lock
> + *
> + * Clock with an adjustable factor affecting its output frequency.
> + * Implements .recalc_rate, .set_rate and .round_rate
> + *
> + * Flags:
> + * CLK_FACTOR_ZERO_BYPASS - By default, the factor is the value read
> + *     from the register, with 0 being a valid value effectively
> + *     zeroing the output clock rate. If CLK_FACTOR_ZERO_BYPASS is
> + *     set, then a null factor will be considered as a bypass,
> + *     leaving the parent rate unmodified.
> + * CLK_FACTOR_ROUND_CLOSEST - Makes the best calculated divider to be
> + *     rounded to the closest integer instead of the down one.
> + */
> +struct clk_factor {
> +       struct clk_hw   hw;
> +       void __iomem    *reg;
> +       u8              shift;
> +       u8              width;
> +       u8              flags;
> +       spinlock_t      *lock;
> +};
> +
> +#define CLK_FACTOR_ZERO_BYPASS         BIT(0)
> +#define CLK_FACTOR_ROUND_CLOSEST       BIT(1)
> +
> +extern const struct clk_ops clk_factor_ops;
> +
> +struct clk *clk_register_factor(struct device *dev, const char *name,
> +                               const char *parent_name, unsigned long flags,
> +                               void __iomem *reg, u8 shift, u8 width,
> +                               u8 clk_factor_flags, spinlock_t *lock);
> +void clk_unregister_factor(struct clk *clk);
> +
>  /***
>   * struct clk_composite - aggregate clock of mux, divider and gate clocks
>   *
> --
> 2.4.1
>

Re: [PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

On Fri, May 22, 2015 at 12:35:35PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Fri, May 22, 2015 at 4:54 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Some clocks are using a factor component, however, unlike their mux, gate
> > or divider counterpart, these factors don't have a basic clock
> > implementation.
> 
> I think "multiplier" would be a better name here, considering it is the
> counterpart of "divider". "factor" implies you can multiply and/or divide
> the clock rate.

You're probably right, I didn't though of it that way :)

> Unless of course you plan to add that? :)

Hmmm, no, not really.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

On Fri, May 22, 2015 at 12:35:35PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Fri, May 22, 2015 at 4:54 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Some clocks are using a factor component, however, unlike their mux, gate
> > or divider counterpart, these factors don't have a basic clock
> > implementation.
> 
> I think "multiplier" would be a better name here, considering it is the
> counterpart of "divider". "factor" implies you can multiply and/or divide
> the clock rate.

You're probably right, I didn't though of it that way :)

> Unless of course you plan to add that? :)

Hmmm, no, not really.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150523/e78aa35e/attachment.sig>

Re: [PATCH v2 0/7] clk: sunxi: Add support for the Audio PLL

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 758 bytes --]

Hi Mike, Stephen,

On Thu, May 21, 2015 at 10:53:59PM +0200, Maxime Ripard wrote:
> Hi,
> 
> This serie adds support for the PLL2 aka the Audio PLL on the
> Allwinner A10 and the later SoCs.
> 
> This is the first stepping stone to get the audio support merged.
> 
> This serie is built on top of a generic clk-factor driver to handle
> clock that multiply their parent clock rate (mostly PLL's), in order
> to provide the driver for the PLL2 base clock, and then adds the
> drivers for the clock that derive from the Audio PLL.
> 
> Thanks!
> Maxime
> 

Any comments on that (especially the first patch)?

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 0/7] clk: sunxi: Add support for the Audio PLL

[Maxime Ripard]

Hi Mike, Stephen,

On Thu, May 21, 2015 at 10:53:59PM +0200, Maxime Ripard wrote:
> Hi,
> 
> This serie adds support for the PLL2 aka the Audio PLL on the
> Allwinner A10 and the later SoCs.
> 
> This is the first stepping stone to get the audio support merged.
> 
> This serie is built on top of a generic clk-factor driver to handle
> clock that multiply their parent clock rate (mostly PLL's), in order
> to provide the driver for the PLL2 base clock, and then adds the
> drivers for the clock that derive from the Audio PLL.
> 
> Thanks!
> Maxime
> 

Any comments on that (especially the first patch)?

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150604/b9d8f7be/attachment.sig>

Re: [PATCH v2 1/7] clk: Add a basic factor clock

[Michael Turquette]

Quoting Maxime Ripard (2015-05-23 00:49:30)
> On Fri, May 22, 2015 at 12:35:35PM +0800, Chen-Yu Tsai wrote:
> > Hi,
> > =

> > On Fri, May 22, 2015 at 4:54 AM, Maxime Ripard
> > <maxime.ripard@free-electrons.com> wrote:
> > > Some clocks are using a factor component, however, unlike their mux, =
gate
> > > or divider counterpart, these factors don't have a basic clock
> > > implementation.
> > =

> > I think "multiplier" would be a better name here, considering it is the
> > counterpart of "divider". "factor" implies you can multiply and/or divi=
de
> > the clock rate.
> =

> You're probably right, I didn't though of it that way :)

I also prefer multiplier over factor.

Jim Quinlan submitted a similar patch. See here:

http://www.spinics.net/lists/linux-clk/msg00691.html

I nacked that patch because Stephen and I are trying to figure out how
the basic clock types should work going forward. Code reuse is good, but
they are not very maintainable.

Since there are two potential users of this code, I should reconsider.
Can you two come up with a common implementation that works for both of
you?  Please do not put any CLK_OF_DECLARE stuff in there (there isn't
any now, but I wanted to be clear).

Regards,
Mike

> =

> > Unless of course you plan to add that? :)
> =

> Hmmm, no, not really.
> =

> Maxime
> =

> -- =

> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

[PATCH v2 1/7] clk: Add a basic factor clock

[Michael Turquette]

Quoting Maxime Ripard (2015-05-23 00:49:30)
> On Fri, May 22, 2015 at 12:35:35PM +0800, Chen-Yu Tsai wrote:
> > Hi,
> > 
> > On Fri, May 22, 2015 at 4:54 AM, Maxime Ripard
> > <maxime.ripard@free-electrons.com> wrote:
> > > Some clocks are using a factor component, however, unlike their mux, gate
> > > or divider counterpart, these factors don't have a basic clock
> > > implementation.
> > 
> > I think "multiplier" would be a better name here, considering it is the
> > counterpart of "divider". "factor" implies you can multiply and/or divide
> > the clock rate.
> 
> You're probably right, I didn't though of it that way :)

I also prefer multiplier over factor.

Jim Quinlan submitted a similar patch. See here:

http://www.spinics.net/lists/linux-clk/msg00691.html

I nacked that patch because Stephen and I are trying to figure out how
the basic clock types should work going forward. Code reuse is good, but
they are not very maintainable.

Since there are two potential users of this code, I should reconsider.
Can you two come up with a common implementation that works for both of
you?  Please do not put any CLK_OF_DECLARE stuff in there (there isn't
any now, but I wanted to be clear).

Regards,
Mike

> 
> > Unless of course you plan to add that? :)
> 
> Hmmm, no, not really.
> 
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Re: [PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3342 bytes --]

Hi Mike,

On Thu, Jul 23, 2015 at 05:00:01PM -0700, Michael Turquette wrote:
> Quoting Maxime Ripard (2015-05-23 00:49:30)
> > On Fri, May 22, 2015 at 12:35:35PM +0800, Chen-Yu Tsai wrote:
> > > Hi,
> > > 
> > > On Fri, May 22, 2015 at 4:54 AM, Maxime Ripard
> > > <maxime.ripard@free-electrons.com> wrote:
> > > > Some clocks are using a factor component, however, unlike their mux, gate
> > > > or divider counterpart, these factors don't have a basic clock
> > > > implementation.
> > > 
> > > I think "multiplier" would be a better name here, considering it is the
> > > counterpart of "divider". "factor" implies you can multiply and/or divide
> > > the clock rate.
> > 
> > You're probably right, I didn't though of it that way :)
> 
> I also prefer multiplier over factor.

Ok. Note that I re-submitted it some time this week and forgot to
change it, but I'm perfectly fine with the name.

> Jim Quinlan submitted a similar patch. See here:
> 
> http://www.spinics.net/lists/linux-clk/msg00691.html

It looks very similar indeed.

> I nacked that patch because Stephen and I are trying to figure out how
> the basic clock types should work going forward. Code reuse is good, but
> they are not very maintainable.

What are the issues with maintaining them? The only drawback I'm
seeing with introducing such a driver is that you can't really have a
clock that is both a divider and a multiplier, but that can be solved
by splitting it into two sub-clocks.

From a pure maintainance point of view, refusing it would mean that
each and every platform would have to come up with its own
implementation. For example, we do have clk-factors.c for sunxi that
does just that, and implies some cooperation from each clock driver
that have to provide some code to determine the various components of
the output formula. This can prove to be very challenging (and bug
prone) for clocks like the audio one we have where we have 1
multiplier and 2 dividers that needs some adjusting.

Splitting it into sub-clocks for each of these components would allow
to have less bugs, while keeping the whole thing very simple, and the
implementation on the driver side very trivial.

Overall, the clk-factors code we have (client side) is approximately a
thousand lines of code logic that could be replaced by (less) trivial
probing code for such a driver.

> Since there are two potential users of this code, I should reconsider.

Like I said, eventually, I'd like to leverage that code a lot more
than for the single clock alone, and I think it could benefit other
platforms too (like Jim has proven).

> Can you two come up with a common implementation that works for both of
> you?

Yes, sure. Jim, how do you want to go with this? Do you want to
resubmit your patch on top of 4.2-rc something so that I could give it
a try? Otherwise, I posted such a patch this week

https://lkml.kernel.org/r/1437235304-2208-1-git-send-email-maxime.ripard@free-electrons.com

Can you give it a try and your feedback?

> Please do not put any CLK_OF_DECLARE stuff in there (there isn't
> any now, but I wanted to be clear).

That's never been my intention ;)

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

Hi Mike,

On Thu, Jul 23, 2015 at 05:00:01PM -0700, Michael Turquette wrote:
> Quoting Maxime Ripard (2015-05-23 00:49:30)
> > On Fri, May 22, 2015 at 12:35:35PM +0800, Chen-Yu Tsai wrote:
> > > Hi,
> > > 
> > > On Fri, May 22, 2015 at 4:54 AM, Maxime Ripard
> > > <maxime.ripard@free-electrons.com> wrote:
> > > > Some clocks are using a factor component, however, unlike their mux, gate
> > > > or divider counterpart, these factors don't have a basic clock
> > > > implementation.
> > > 
> > > I think "multiplier" would be a better name here, considering it is the
> > > counterpart of "divider". "factor" implies you can multiply and/or divide
> > > the clock rate.
> > 
> > You're probably right, I didn't though of it that way :)
> 
> I also prefer multiplier over factor.

Ok. Note that I re-submitted it some time this week and forgot to
change it, but I'm perfectly fine with the name.

> Jim Quinlan submitted a similar patch. See here:
> 
> http://www.spinics.net/lists/linux-clk/msg00691.html

It looks very similar indeed.

> I nacked that patch because Stephen and I are trying to figure out how
> the basic clock types should work going forward. Code reuse is good, but
> they are not very maintainable.

What are the issues with maintaining them? The only drawback I'm
seeing with introducing such a driver is that you can't really have a
clock that is both a divider and a multiplier, but that can be solved
by splitting it into two sub-clocks.

>From a pure maintainance point of view, refusing it would mean that
each and every platform would have to come up with its own
implementation. For example, we do have clk-factors.c for sunxi that
does just that, and implies some cooperation from each clock driver
that have to provide some code to determine the various components of
the output formula. This can prove to be very challenging (and bug
prone) for clocks like the audio one we have where we have 1
multiplier and 2 dividers that needs some adjusting.

Splitting it into sub-clocks for each of these components would allow
to have less bugs, while keeping the whole thing very simple, and the
implementation on the driver side very trivial.

Overall, the clk-factors code we have (client side) is approximately a
thousand lines of code logic that could be replaced by (less) trivial
probing code for such a driver.

> Since there are two potential users of this code, I should reconsider.

Like I said, eventually, I'd like to leverage that code a lot more
than for the single clock alone, and I think it could benefit other
platforms too (like Jim has proven).

> Can you two come up with a common implementation that works for both of
> you?

Yes, sure. Jim, how do you want to go with this? Do you want to
resubmit your patch on top of 4.2-rc something so that I could give it
a try? Otherwise, I posted such a patch this week

https://lkml.kernel.org/r/1437235304-2208-1-git-send-email-maxime.ripard at free-electrons.com

Can you give it a try and your feedback?

> Please do not put any CLK_OF_DECLARE stuff in there (there isn't
> any now, but I wanted to be clear).

That's never been my intention ;)

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150724/5dffba06/attachment-0001.sig>

Re: [PATCH v2 1/7] clk: Add a basic factor clock

[Michael Turquette]

Quoting Maxime Ripard (2015-07-23 23:50:17)
> Hi Mike,
> 
> On Thu, Jul 23, 2015 at 05:00:01PM -0700, Michael Turquette wrote:
> > Quoting Maxime Ripard (2015-05-23 00:49:30)
> > > On Fri, May 22, 2015 at 12:35:35PM +0800, Chen-Yu Tsai wrote:
> > > > Hi,
> > > > 
> > > > On Fri, May 22, 2015 at 4:54 AM, Maxime Ripard
> > > > <maxime.ripard@free-electrons.com> wrote:
> > > > > Some clocks are using a factor component, however, unlike their mux, gate
> > > > > or divider counterpart, these factors don't have a basic clock
> > > > > implementation.
> > > > 
> > > > I think "multiplier" would be a better name here, considering it is the
> > > > counterpart of "divider". "factor" implies you can multiply and/or divide
> > > > the clock rate.
> > > 
> > > You're probably right, I didn't though of it that way :)
> > 
> > I also prefer multiplier over factor.
> 
> Ok. Note that I re-submitted it some time this week and forgot to
> change it, but I'm perfectly fine with the name.

Hi Maxime,

Yes, I saw your V2 after the fact.

> 
> > Jim Quinlan submitted a similar patch. See here:
> > 
> > http://www.spinics.net/lists/linux-clk/msg00691.html
> 
> It looks very similar indeed.
> 
> > I nacked that patch because Stephen and I are trying to figure out how
> > the basic clock types should work going forward. Code reuse is good, but
> > they are not very maintainable.
> 
> What are the issues with maintaining them? The only drawback I'm
> seeing with introducing such a driver is that you can't really have a
> clock that is both a divider and a multiplier, but that can be solved
> by splitting it into two sub-clocks.

There are a bunch of problems with the basic clock types. First is that
there is some feature creep every merge window that subtly breaks an
existing user (e.g. the round_rate stuff in clk-divider.c), and then
there are the growing number of flags to handle corner cases that are
one-off quirks for a single chip.

These make it harder to maintain, but it is possible.

The real problem with these basic clock types is that they are an
abstraction layer at the wrong level. Each clock type implements both
the policy of a given clock, as well as the machine-specific details.
For example clk-divider.c has made some assumptions in the past about
rounding the rate, or how to calculate the best divider; this is a
matter of policy and is useful on its own. But additionally that same
policy is glued to a specific implementation: memory-mapped register
controls for a clock divider.

The I/O accessor stuff needs to be addressed at some point.  Currently
the basic clock types assume specific patterns of access to
memory-mapped clock registers. There are lots of other clock controls
out there that talk to firmware, or over i2c, or whatever. The amount of
code that has to be copy/pasted for each different type of access is
100%; i.e. we do not have abstractions at the right level such as
.get_best_div(struct clk_hw *hw, unsigned long rate).

What I would like to see in time is a re-usable layer for clock policy
(e.g. common rules for how dividers or multipliers should behave), and
then have that sit on top of the machine-specific callbacks that
directly touch the hardware, such as the .get_best_div callback above.

For this reason I like to limit the number of new basic clock types. If
there is a single user then I'm inclined to have the author put it with
the machine-specific code. But in this case since there are two users, I
see the value in making a new basic clock type. It will just be more
code to update in the far away future when we get around to implementing
the changes above.

> 
> From a pure maintainance point of view, refusing it would mean that
> each and every platform would have to come up with its own
> implementation. For example, we do have clk-factors.c for sunxi that
> does just that, and implies some cooperation from each clock driver
> that have to provide some code to determine the various components of
> the output formula. This can prove to be very challenging (and bug
> prone) for clocks like the audio one we have where we have 1
> multiplier and 2 dividers that needs some adjusting.
> 
> Splitting it into sub-clocks for each of these components would allow
> to have less bugs, while keeping the whole thing very simple, and the
> implementation on the driver side very trivial.
> 
> Overall, the clk-factors code we have (client side) is approximately a
> thousand lines of code logic that could be replaced by (less) trivial
> probing code for such a driver.
> 
> > Since there are two potential users of this code, I should reconsider.
> 
> Like I said, eventually, I'd like to leverage that code a lot more
> than for the single clock alone, and I think it could benefit other
> platforms too (like Jim has proven).
> 
> > Can you two come up with a common implementation that works for both of
> > you?
> 
> Yes, sure. Jim, how do you want to go with this? Do you want to
> resubmit your patch on top of 4.2-rc something so that I could give it
> a try? Otherwise, I posted such a patch this week
> 
> https://lkml.kernel.org/r/1437235304-2208-1-git-send-email-maxime.ripard@free-electrons.com
> 
> Can you give it a try and your feedback?
> 
> > Please do not put any CLK_OF_DECLARE stuff in there (there isn't
> > any now, but I wanted to be clear).
> 
> That's never been my intention ;)

OK, sounds great.

Thanks,
Mike

> 
> Thanks!
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/7] clk: Add a basic factor clock

[Michael Turquette]

Quoting Maxime Ripard (2015-07-23 23:50:17)
> Hi Mike,
> 
> On Thu, Jul 23, 2015 at 05:00:01PM -0700, Michael Turquette wrote:
> > Quoting Maxime Ripard (2015-05-23 00:49:30)
> > > On Fri, May 22, 2015 at 12:35:35PM +0800, Chen-Yu Tsai wrote:
> > > > Hi,
> > > > 
> > > > On Fri, May 22, 2015 at 4:54 AM, Maxime Ripard
> > > > <maxime.ripard@free-electrons.com> wrote:
> > > > > Some clocks are using a factor component, however, unlike their mux, gate
> > > > > or divider counterpart, these factors don't have a basic clock
> > > > > implementation.
> > > > 
> > > > I think "multiplier" would be a better name here, considering it is the
> > > > counterpart of "divider". "factor" implies you can multiply and/or divide
> > > > the clock rate.
> > > 
> > > You're probably right, I didn't though of it that way :)
> > 
> > I also prefer multiplier over factor.
> 
> Ok. Note that I re-submitted it some time this week and forgot to
> change it, but I'm perfectly fine with the name.

Hi Maxime,

Yes, I saw your V2 after the fact.

> 
> > Jim Quinlan submitted a similar patch. See here:
> > 
> > http://www.spinics.net/lists/linux-clk/msg00691.html
> 
> It looks very similar indeed.
> 
> > I nacked that patch because Stephen and I are trying to figure out how
> > the basic clock types should work going forward. Code reuse is good, but
> > they are not very maintainable.
> 
> What are the issues with maintaining them? The only drawback I'm
> seeing with introducing such a driver is that you can't really have a
> clock that is both a divider and a multiplier, but that can be solved
> by splitting it into two sub-clocks.

There are a bunch of problems with the basic clock types. First is that
there is some feature creep every merge window that subtly breaks an
existing user (e.g. the round_rate stuff in clk-divider.c), and then
there are the growing number of flags to handle corner cases that are
one-off quirks for a single chip.

These make it harder to maintain, but it is possible.

The real problem with these basic clock types is that they are an
abstraction layer at the wrong level. Each clock type implements both
the policy of a given clock, as well as the machine-specific details.
For example clk-divider.c has made some assumptions in the past about
rounding the rate, or how to calculate the best divider; this is a
matter of policy and is useful on its own. But additionally that same
policy is glued to a specific implementation: memory-mapped register
controls for a clock divider.

The I/O accessor stuff needs to be addressed at some point.  Currently
the basic clock types assume specific patterns of access to
memory-mapped clock registers. There are lots of other clock controls
out there that talk to firmware, or over i2c, or whatever. The amount of
code that has to be copy/pasted for each different type of access is
100%; i.e. we do not have abstractions at the right level such as
.get_best_div(struct clk_hw *hw, unsigned long rate).

What I would like to see in time is a re-usable layer for clock policy
(e.g. common rules for how dividers or multipliers should behave), and
then have that sit on top of the machine-specific callbacks that
directly touch the hardware, such as the .get_best_div callback above.

For this reason I like to limit the number of new basic clock types. If
there is a single user then I'm inclined to have the author put it with
the machine-specific code. But in this case since there are two users, I
see the value in making a new basic clock type. It will just be more
code to update in the far away future when we get around to implementing
the changes above.

> 
> From a pure maintainance point of view, refusing it would mean that
> each and every platform would have to come up with its own
> implementation. For example, we do have clk-factors.c for sunxi that
> does just that, and implies some cooperation from each clock driver
> that have to provide some code to determine the various components of
> the output formula. This can prove to be very challenging (and bug
> prone) for clocks like the audio one we have where we have 1
> multiplier and 2 dividers that needs some adjusting.
> 
> Splitting it into sub-clocks for each of these components would allow
> to have less bugs, while keeping the whole thing very simple, and the
> implementation on the driver side very trivial.
> 
> Overall, the clk-factors code we have (client side) is approximately a
> thousand lines of code logic that could be replaced by (less) trivial
> probing code for such a driver.
> 
> > Since there are two potential users of this code, I should reconsider.
> 
> Like I said, eventually, I'd like to leverage that code a lot more
> than for the single clock alone, and I think it could benefit other
> platforms too (like Jim has proven).
> 
> > Can you two come up with a common implementation that works for both of
> > you?
> 
> Yes, sure. Jim, how do you want to go with this? Do you want to
> resubmit your patch on top of 4.2-rc something so that I could give it
> a try? Otherwise, I posted such a patch this week
> 
> https://lkml.kernel.org/r/1437235304-2208-1-git-send-email-maxime.ripard at free-electrons.com
> 
> Can you give it a try and your feedback?
> 
> > Please do not put any CLK_OF_DECLARE stuff in there (there isn't
> > any now, but I wanted to be clear).
> 
> That's never been my intention ;)

OK, sounds great.

Thanks,
Mike

> 
> Thanks!
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 3151 bytes --]

Hi Mike,

On Fri, Jul 24, 2015 at 11:26:19AM -0700, Michael Turquette wrote:
> > What are the issues with maintaining them? The only drawback I'm
> > seeing with introducing such a driver is that you can't really have a
> > clock that is both a divider and a multiplier, but that can be solved
> > by splitting it into two sub-clocks.
> 
> There are a bunch of problems with the basic clock types. First is that
> there is some feature creep every merge window that subtly breaks an
> existing user (e.g. the round_rate stuff in clk-divider.c), and then
> there are the growing number of flags to handle corner cases that are
> one-off quirks for a single chip.
> 
> These make it harder to maintain, but it is possible.

Ok. It's the downside of having common code I guess, everyone wants to
use it, and the clocks are just more subject to it than other drivers :/

> The real problem with these basic clock types is that they are an
> abstraction layer at the wrong level. Each clock type implements both
> the policy of a given clock, as well as the machine-specific details.
> For example clk-divider.c has made some assumptions in the past about
> rounding the rate, or how to calculate the best divider; this is a
> matter of policy and is useful on its own. But additionally that same
> policy is glued to a specific implementation: memory-mapped register
> controls for a clock divider.
> 
> The I/O accessor stuff needs to be addressed at some point.  Currently
> the basic clock types assume specific patterns of access to
> memory-mapped clock registers. There are lots of other clock controls
> out there that talk to firmware, or over i2c, or whatever. The amount of
> code that has to be copy/pasted for each different type of access is
> 100%; i.e. we do not have abstractions at the right level such as
> .get_best_div(struct clk_hw *hw, unsigned long rate).
> 
> What I would like to see in time is a re-usable layer for clock policy
> (e.g. common rules for how dividers or multipliers should behave), and
> then have that sit on top of the machine-specific callbacks that
> directly touch the hardware, such as the .get_best_div callback above.

Can't that be solved by moving to regmap using Matthias' patches, or
at least the IO method abstraction?

We would then have to only provide an additional callback then for
providers that have specific requirements about the divider
calculation.

So far, I haven't had any usecase where I needed anything but having
the as-close-as-possible rate, so I would expect to not provide
anything beside whether I'd like to round down or round up, but I do
understand that some other might have different requirements.

> For this reason I like to limit the number of new basic clock types. If
> there is a single user then I'm inclined to have the author put it with
> the machine-specific code. But in this case since there are two users, I
> see the value in making a new basic clock type.

That makes sense.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

Hi Mike,

On Fri, Jul 24, 2015 at 11:26:19AM -0700, Michael Turquette wrote:
> > What are the issues with maintaining them? The only drawback I'm
> > seeing with introducing such a driver is that you can't really have a
> > clock that is both a divider and a multiplier, but that can be solved
> > by splitting it into two sub-clocks.
> 
> There are a bunch of problems with the basic clock types. First is that
> there is some feature creep every merge window that subtly breaks an
> existing user (e.g. the round_rate stuff in clk-divider.c), and then
> there are the growing number of flags to handle corner cases that are
> one-off quirks for a single chip.
> 
> These make it harder to maintain, but it is possible.

Ok. It's the downside of having common code I guess, everyone wants to
use it, and the clocks are just more subject to it than other drivers :/

> The real problem with these basic clock types is that they are an
> abstraction layer at the wrong level. Each clock type implements both
> the policy of a given clock, as well as the machine-specific details.
> For example clk-divider.c has made some assumptions in the past about
> rounding the rate, or how to calculate the best divider; this is a
> matter of policy and is useful on its own. But additionally that same
> policy is glued to a specific implementation: memory-mapped register
> controls for a clock divider.
> 
> The I/O accessor stuff needs to be addressed at some point.  Currently
> the basic clock types assume specific patterns of access to
> memory-mapped clock registers. There are lots of other clock controls
> out there that talk to firmware, or over i2c, or whatever. The amount of
> code that has to be copy/pasted for each different type of access is
> 100%; i.e. we do not have abstractions at the right level such as
> .get_best_div(struct clk_hw *hw, unsigned long rate).
> 
> What I would like to see in time is a re-usable layer for clock policy
> (e.g. common rules for how dividers or multipliers should behave), and
> then have that sit on top of the machine-specific callbacks that
> directly touch the hardware, such as the .get_best_div callback above.

Can't that be solved by moving to regmap using Matthias' patches, or
at least the IO method abstraction?

We would then have to only provide an additional callback then for
providers that have specific requirements about the divider
calculation.

So far, I haven't had any usecase where I needed anything but having
the as-close-as-possible rate, so I would expect to not provide
anything beside whether I'd like to round down or round up, but I do
understand that some other might have different requirements.

> For this reason I like to limit the number of new basic clock types. If
> there is a single user then I'm inclined to have the author put it with
> the machine-specific code. But in this case since there are two users, I
> see the value in making a new basic clock type.

That makes sense.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150725/5ac50816/attachment.sig>

Re: [PATCH v2 1/7] clk: Add a basic factor clock

[Michael Turquette]

Quoting Maxime Ripard (2015-07-25 00:39:25)
> Hi Mike,
> =

> On Fri, Jul 24, 2015 at 11:26:19AM -0700, Michael Turquette wrote:
> > > What are the issues with maintaining them? The only drawback I'm
> > > seeing with introducing such a driver is that you can't really have a
> > > clock that is both a divider and a multiplier, but that can be solved
> > > by splitting it into two sub-clocks.
> > =

> > There are a bunch of problems with the basic clock types. First is that
> > there is some feature creep every merge window that subtly breaks an
> > existing user (e.g. the round_rate stuff in clk-divider.c), and then
> > there are the growing number of flags to handle corner cases that are
> > one-off quirks for a single chip.
> > =

> > These make it harder to maintain, but it is possible.
> =

> Ok. It's the downside of having common code I guess, everyone wants to
> use it, and the clocks are just more subject to it than other drivers :/
> =

> > The real problem with these basic clock types is that they are an
> > abstraction layer at the wrong level. Each clock type implements both
> > the policy of a given clock, as well as the machine-specific details.
> > For example clk-divider.c has made some assumptions in the past about
> > rounding the rate, or how to calculate the best divider; this is a
> > matter of policy and is useful on its own. But additionally that same
> > policy is glued to a specific implementation: memory-mapped register
> > controls for a clock divider.
> > =

> > The I/O accessor stuff needs to be addressed at some point.  Currently
> > the basic clock types assume specific patterns of access to
> > memory-mapped clock registers. There are lots of other clock controls
> > out there that talk to firmware, or over i2c, or whatever. The amount of
> > code that has to be copy/pasted for each different type of access is
> > 100%; i.e. we do not have abstractions at the right level such as
> > .get_best_div(struct clk_hw *hw, unsigned long rate).
> > =

> > What I would like to see in time is a re-usable layer for clock policy
> > (e.g. common rules for how dividers or multipliers should behave), and
> > then have that sit on top of the machine-specific callbacks that
> > directly touch the hardware, such as the .get_best_div callback above.
> =

> Can't that be solved by moving to regmap using Matthias' patches, or
> at least the IO method abstraction?

I need to look at those patches again, but I do not think they address
the problem I described above: abstraction at the wrong level (or more
specifically, combining two abstract layers into one).

I'll have more bandwidth to look at this problem after the next merge
window. The clk framework is slowly be rewritten in bits and pieces here
on the list (shh don't tell anybody) and this is on my radar and
Stephen's.

Regards,
Mike

> =

> We would then have to only provide an additional callback then for
> providers that have specific requirements about the divider
> calculation.
> =

> So far, I haven't had any usecase where I needed anything but having
> the as-close-as-possible rate, so I would expect to not provide
> anything beside whether I'd like to round down or round up, but I do
> understand that some other might have different requirements.
> =

> > For this reason I like to limit the number of new basic clock types. If
> > there is a single user then I'm inclined to have the author put it with
> > the machine-specific code. But in this case since there are two users, I
> > see the value in making a new basic clock type.
> =

> That makes sense.
> =

> Maxime
> =

> -- =

> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
> =

> =

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/7] clk: Add a basic factor clock

[Michael Turquette]

Quoting Maxime Ripard (2015-07-25 00:39:25)
> Hi Mike,
> 
> On Fri, Jul 24, 2015 at 11:26:19AM -0700, Michael Turquette wrote:
> > > What are the issues with maintaining them? The only drawback I'm
> > > seeing with introducing such a driver is that you can't really have a
> > > clock that is both a divider and a multiplier, but that can be solved
> > > by splitting it into two sub-clocks.
> > 
> > There are a bunch of problems with the basic clock types. First is that
> > there is some feature creep every merge window that subtly breaks an
> > existing user (e.g. the round_rate stuff in clk-divider.c), and then
> > there are the growing number of flags to handle corner cases that are
> > one-off quirks for a single chip.
> > 
> > These make it harder to maintain, but it is possible.
> 
> Ok. It's the downside of having common code I guess, everyone wants to
> use it, and the clocks are just more subject to it than other drivers :/
> 
> > The real problem with these basic clock types is that they are an
> > abstraction layer at the wrong level. Each clock type implements both
> > the policy of a given clock, as well as the machine-specific details.
> > For example clk-divider.c has made some assumptions in the past about
> > rounding the rate, or how to calculate the best divider; this is a
> > matter of policy and is useful on its own. But additionally that same
> > policy is glued to a specific implementation: memory-mapped register
> > controls for a clock divider.
> > 
> > The I/O accessor stuff needs to be addressed at some point.  Currently
> > the basic clock types assume specific patterns of access to
> > memory-mapped clock registers. There are lots of other clock controls
> > out there that talk to firmware, or over i2c, or whatever. The amount of
> > code that has to be copy/pasted for each different type of access is
> > 100%; i.e. we do not have abstractions at the right level such as
> > .get_best_div(struct clk_hw *hw, unsigned long rate).
> > 
> > What I would like to see in time is a re-usable layer for clock policy
> > (e.g. common rules for how dividers or multipliers should behave), and
> > then have that sit on top of the machine-specific callbacks that
> > directly touch the hardware, such as the .get_best_div callback above.
> 
> Can't that be solved by moving to regmap using Matthias' patches, or
> at least the IO method abstraction?

I need to look at those patches again, but I do not think they address
the problem I described above: abstraction at the wrong level (or more
specifically, combining two abstract layers into one).

I'll have more bandwidth to look at this problem after the next merge
window. The clk framework is slowly be rewritten in bits and pieces here
on the list (shh don't tell anybody) and this is on my radar and
Stephen's.

Regards,
Mike

> 
> We would then have to only provide an additional callback then for
> providers that have specific requirements about the divider
> calculation.
> 
> So far, I haven't had any usecase where I needed anything but having
> the as-close-as-possible rate, so I would expect to not provide
> anything beside whether I'd like to round down or round up, but I do
> understand that some other might have different requirements.
> 
> > For this reason I like to limit the number of new basic clock types. If
> > there is a single user then I'm inclined to have the author put it with
> > the machine-specific code. But in this case since there are two users, I
> > see the value in making a new basic clock type.
> 
> That makes sense.
> 
> Maxime
> 
> -- 
> Maxime Ripard, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2555 bytes --]

On Tue, Aug 11, 2015 at 02:30:41PM -0700, Michael Turquette wrote:
> > > The real problem with these basic clock types is that they are an
> > > abstraction layer at the wrong level. Each clock type implements both
> > > the policy of a given clock, as well as the machine-specific details.
> > > For example clk-divider.c has made some assumptions in the past about
> > > rounding the rate, or how to calculate the best divider; this is a
> > > matter of policy and is useful on its own. But additionally that same
> > > policy is glued to a specific implementation: memory-mapped register
> > > controls for a clock divider.
> > > 
> > > The I/O accessor stuff needs to be addressed at some point.  Currently
> > > the basic clock types assume specific patterns of access to
> > > memory-mapped clock registers. There are lots of other clock controls
> > > out there that talk to firmware, or over i2c, or whatever. The amount of
> > > code that has to be copy/pasted for each different type of access is
> > > 100%; i.e. we do not have abstractions at the right level such as
> > > .get_best_div(struct clk_hw *hw, unsigned long rate).
> > > 
> > > What I would like to see in time is a re-usable layer for clock policy
> > > (e.g. common rules for how dividers or multipliers should behave), and
> > > then have that sit on top of the machine-specific callbacks that
> > > directly touch the hardware, such as the .get_best_div callback above.
> > 
> > Can't that be solved by moving to regmap using Matthias' patches, or
> > at least the IO method abstraction?
> 
> I need to look at those patches again, but I do not think they address
> the problem I described above: abstraction at the wrong level (or more
> specifically, combining two abstract layers into one).

Well, maybe I'm missing something, but you end up with this patch with
the clock driver allocating the regmap, and passing it to the generic
part of the clock framework, that will only do regmap calls and not
care anymore about what kind of accesses it is (the point of regmap
being the abstraction).

Isn't it what you were looking for?

> I'll have more bandwidth to look at this problem after the next merge
> window. The clk framework is slowly be rewritten in bits and pieces here
> on the list (shh don't tell anybody) and this is on my radar and
> Stephen's.

Ok, i'll resend it after the merge window then.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

On Tue, Aug 11, 2015 at 02:30:41PM -0700, Michael Turquette wrote:
> > > The real problem with these basic clock types is that they are an
> > > abstraction layer at the wrong level. Each clock type implements both
> > > the policy of a given clock, as well as the machine-specific details.
> > > For example clk-divider.c has made some assumptions in the past about
> > > rounding the rate, or how to calculate the best divider; this is a
> > > matter of policy and is useful on its own. But additionally that same
> > > policy is glued to a specific implementation: memory-mapped register
> > > controls for a clock divider.
> > > 
> > > The I/O accessor stuff needs to be addressed at some point.  Currently
> > > the basic clock types assume specific patterns of access to
> > > memory-mapped clock registers. There are lots of other clock controls
> > > out there that talk to firmware, or over i2c, or whatever. The amount of
> > > code that has to be copy/pasted for each different type of access is
> > > 100%; i.e. we do not have abstractions at the right level such as
> > > .get_best_div(struct clk_hw *hw, unsigned long rate).
> > > 
> > > What I would like to see in time is a re-usable layer for clock policy
> > > (e.g. common rules for how dividers or multipliers should behave), and
> > > then have that sit on top of the machine-specific callbacks that
> > > directly touch the hardware, such as the .get_best_div callback above.
> > 
> > Can't that be solved by moving to regmap using Matthias' patches, or
> > at least the IO method abstraction?
> 
> I need to look at those patches again, but I do not think they address
> the problem I described above: abstraction at the wrong level (or more
> specifically, combining two abstract layers into one).

Well, maybe I'm missing something, but you end up with this patch with
the clock driver allocating the regmap, and passing it to the generic
part of the clock framework, that will only do regmap calls and not
care anymore about what kind of accesses it is (the point of regmap
being the abstraction).

Isn't it what you were looking for?

> I'll have more bandwidth to look at this problem after the next merge
> window. The clk framework is slowly be rewritten in bits and pieces here
> on the list (shh don't tell anybody) and this is on my radar and
> Stephen's.

Ok, i'll resend it after the merge window then.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150819/4a3c9aeb/attachment.sig>

Re: [PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

[-- Attachment #1.1: Type: text/plain, Size: 2446 bytes --]

Hi Jim,

On Fri, Jul 24, 2015 at 08:50:17AM +0200, Maxime Ripard wrote:
> > Jim Quinlan submitted a similar patch. See here:
> > 
> > http://www.spinics.net/lists/linux-clk/msg00691.html
> 
> It looks very similar indeed.
> 
> > I nacked that patch because Stephen and I are trying to figure out how
> > the basic clock types should work going forward. Code reuse is good, but
> > they are not very maintainable.
> 
> What are the issues with maintaining them? The only drawback I'm
> seeing with introducing such a driver is that you can't really have a
> clock that is both a divider and a multiplier, but that can be solved
> by splitting it into two sub-clocks.
> 
> From a pure maintainance point of view, refusing it would mean that
> each and every platform would have to come up with its own
> implementation. For example, we do have clk-factors.c for sunxi that
> does just that, and implies some cooperation from each clock driver
> that have to provide some code to determine the various components of
> the output formula. This can prove to be very challenging (and bug
> prone) for clocks like the audio one we have where we have 1
> multiplier and 2 dividers that needs some adjusting.
> 
> Splitting it into sub-clocks for each of these components would allow
> to have less bugs, while keeping the whole thing very simple, and the
> implementation on the driver side very trivial.
> 
> Overall, the clk-factors code we have (client side) is approximately a
> thousand lines of code logic that could be replaced by (less) trivial
> probing code for such a driver.
> 
> > Since there are two potential users of this code, I should reconsider.
> 
> Like I said, eventually, I'd like to leverage that code a lot more
> than for the single clock alone, and I think it could benefit other
> platforms too (like Jim has proven).
> 
> > Can you two come up with a common implementation that works for both of
> > you?
> 
> Yes, sure. Jim, how do you want to go with this? Do you want to
> resubmit your patch on top of 4.2-rc something so that I could give it
> a try? Otherwise, I posted such a patch this week
> 
> https://lkml.kernel.org/r/1437235304-2208-1-git-send-email-maxime.ripard@free-electrons.com
> 
> Can you give it a try and your feedback?

Ping?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/7] clk: Add a basic factor clock

[Maxime Ripard]

Hi Jim,

On Fri, Jul 24, 2015 at 08:50:17AM +0200, Maxime Ripard wrote:
> > Jim Quinlan submitted a similar patch. See here:
> > 
> > http://www.spinics.net/lists/linux-clk/msg00691.html
> 
> It looks very similar indeed.
> 
> > I nacked that patch because Stephen and I are trying to figure out how
> > the basic clock types should work going forward. Code reuse is good, but
> > they are not very maintainable.
> 
> What are the issues with maintaining them? The only drawback I'm
> seeing with introducing such a driver is that you can't really have a
> clock that is both a divider and a multiplier, but that can be solved
> by splitting it into two sub-clocks.
> 
> From a pure maintainance point of view, refusing it would mean that
> each and every platform would have to come up with its own
> implementation. For example, we do have clk-factors.c for sunxi that
> does just that, and implies some cooperation from each clock driver
> that have to provide some code to determine the various components of
> the output formula. This can prove to be very challenging (and bug
> prone) for clocks like the audio one we have where we have 1
> multiplier and 2 dividers that needs some adjusting.
> 
> Splitting it into sub-clocks for each of these components would allow
> to have less bugs, while keeping the whole thing very simple, and the
> implementation on the driver side very trivial.
> 
> Overall, the clk-factors code we have (client side) is approximately a
> thousand lines of code logic that could be replaced by (less) trivial
> probing code for such a driver.
> 
> > Since there are two potential users of this code, I should reconsider.
> 
> Like I said, eventually, I'd like to leverage that code a lot more
> than for the single clock alone, and I think it could benefit other
> platforms too (like Jim has proven).
> 
> > Can you two come up with a common implementation that works for both of
> > you?
> 
> Yes, sure. Jim, how do you want to go with this? Do you want to
> resubmit your patch on top of 4.2-rc something so that I could give it
> a try? Otherwise, I posted such a patch this week
> 
> https://lkml.kernel.org/r/1437235304-2208-1-git-send-email-maxime.ripard at free-electrons.com
> 
> Can you give it a try and your feedback?

Ping?

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150919/3fd0ab8c/attachment.sig>