[PATCH v6 0/5] clk: sunxi: Add support for the Audio PLL

[PATCH v6 0/5] 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 last part of the audio codec support, it's been around for
quite a while now, and I expect it to be merged in 4.4.

This serie is built on top of a generic clk-multiplier 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 v5:
  - Fixed the multiplier calculation
  - Added Chen-Yu tags

Changes from v4:
  - Fixed the genmask calls
  - Fixed the number of parents registration in mod1
  - Added Chen-Yu's Reviewed-by

Changes from v3:
  - Fixed the header issues: removed unused ones, added some others
  - Added the __acquire / __release call when a spinlock is not
    defined

Changes from v2:
  - Renamed clk-factor to clk-multiplier
  - Added an exception for the A13 clock

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 (2):
  clk: sunxi: codec clock support
  clk: sunxi: mod1 clock support

Maxime Ripard (3):
  clk: Add a basic multiplier clock
  clk: sunxi: Add a driver for the PLL2
  clk: sunxi: pll2: Add A13 support

 drivers/clk/Makefile                       |   1 +
 drivers/clk/clk-multiplier.c               | 181 ++++++++++++++++++++++++
 drivers/clk/sunxi/Makefile                 |   3 +
 drivers/clk/sunxi/clk-a10-codec.c          |  44 ++++++
 drivers/clk/sunxi/clk-a10-mod1.c           |  81 +++++++++++
 drivers/clk/sunxi/clk-a10-pll2.c           | 216 +++++++++++++++++++++++++++++
 include/dt-bindings/clock/sun4i-a10-pll2.h |  53 +++++++
 include/linux/clk-provider.h               |  42 ++++++
 8 files changed, 621 insertions(+)
 create mode 100644 drivers/clk/clk-multiplier.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.6.2

[PATCH v6 1/5] clk: Add a basic multiplier clock

[Maxime Ripard]

Some clocks are using a multiplier 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>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/Makefile         |   1 +
 drivers/clk/clk-multiplier.c | 181 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h |  42 ++++++++++
 3 files changed, 224 insertions(+)
 create mode 100644 drivers/clk/clk-multiplier.c

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index d08b3e5985be..0b2101039508 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_COMMON_CLK)	+= clk-divider.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-factor.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fixed-rate.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-gate.o
+obj-$(CONFIG_COMMON_CLK)	+= clk-multiplier.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-mux.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-composite.o
 obj-$(CONFIG_COMMON_CLK)	+= clk-fractional-divider.o
diff --git a/drivers/clk/clk-multiplier.c b/drivers/clk/clk-multiplier.c
new file mode 100644
index 000000000000..43ec269fcbff
--- /dev/null
+++ b/drivers/clk/clk-multiplier.c
@@ -0,0 +1,181 @@
+/*
+ * 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/bitops.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+
+#define to_clk_multiplier(_hw) container_of(_hw, struct clk_multiplier, hw)
+
+static unsigned long __get_mult(struct clk_multiplier *mult,
+				unsigned long rate,
+				unsigned long parent_rate)
+{
+	if (mult->flags & CLK_MULTIPLIER_ROUND_CLOSEST)
+		return DIV_ROUND_CLOSEST(rate, parent_rate);
+
+	return rate / parent_rate;
+}
+
+static unsigned long clk_multiplier_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
+{
+	struct clk_multiplier *mult = to_clk_multiplier(hw);
+	unsigned long val;
+
+	val = clk_readl(mult->reg) >> mult->shift;
+	val &= GENMASK(mult->width - 1, 0);
+
+	if (!val && mult->flags & CLK_MULTIPLIER_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_MULTIPLIER_ROUND_CLOSEST)
+		return abs(rate - new) < abs(rate - best);
+
+	return new >= rate && new < best;
+}
+
+static unsigned long __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_hw_get_flags(hw) & CLK_SET_RATE_PARENT))
+		return rate / *best_parent_rate;
+
+	for (i = 1; i < ((1 << width) - 1); i++) {
+		if (rate == orig_parent_rate * i) {
+			/*
+			 * 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_hw_round_rate(clk_hw_get_parent(hw),
+						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_multiplier_round_rate(struct clk_hw *hw, unsigned long rate,
+				  unsigned long *parent_rate)
+{
+	struct clk_multiplier *mult = to_clk_multiplier(hw);
+	unsigned long factor = __bestmult(hw, rate, parent_rate,
+					  mult->width, mult->flags);
+
+	return *parent_rate * factor;
+}
+
+static int clk_multiplier_set_rate(struct clk_hw *hw, unsigned long rate,
+			       unsigned long parent_rate)
+{
+	struct clk_multiplier *mult = to_clk_multiplier(hw);
+	unsigned long factor = __get_mult(mult, rate, parent_rate);
+	unsigned long flags = 0;
+	unsigned long val;
+
+	if (mult->lock)
+		spin_lock_irqsave(mult->lock, flags);
+	else
+		__acquire(mult->lock);
+
+	val = clk_readl(mult->reg);
+	val &= ~GENMASK(mult->width + mult->shift - 1, mult->shift);
+	val |= factor << mult->shift;
+	clk_writel(val, mult->reg);
+
+	if (mult->lock)
+		spin_unlock_irqrestore(mult->lock, flags);
+	else
+		__release(mult->lock);
+
+	return 0;
+}
+
+const struct clk_ops clk_multiplier_ops = {
+	.recalc_rate	= clk_multiplier_recalc_rate,
+	.round_rate	= clk_multiplier_round_rate,
+	.set_rate	= clk_multiplier_set_rate,
+};
+EXPORT_SYMBOL_GPL(clk_multiplier_ops);
+
+struct clk *clk_register_multiplier(struct device *dev, const char *name,
+				    const char *parent_name,
+				    unsigned long flags,
+				    void __iomem *reg, u8 shift, u8 width,
+				    u8 clk_mult_flags, spinlock_t *lock)
+{
+	struct clk_init_data init;
+	struct clk_multiplier *mult;
+	struct clk *clk;
+
+	mult = kmalloc(sizeof(*mult), GFP_KERNEL);
+	if (!mult)
+		return ERR_PTR(-ENOMEM);
+
+	init.name = name;
+	init.ops = &clk_multiplier_ops;
+	init.flags = flags | CLK_IS_BASIC;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	mult->reg = reg;
+	mult->shift = shift;
+	mult->width = width;
+	mult->flags = clk_mult_flags;
+	mult->lock = lock;
+	mult->hw.init = &init;
+
+	clk = clk_register(dev, &mult->hw);
+	if (IS_ERR(clk))
+		kfree(mult);
+
+	return clk;
+}
+EXPORT_SYMBOL_GPL(clk_register_multiplier);
+
+void clk_unregister_multiplier(struct clk *clk)
+{
+	struct clk_multiplier *mult;
+	struct clk_hw *hw;
+
+	hw = __clk_get_hw(clk);
+	if (!hw)
+		return;
+
+	mult = to_clk_multiplier(hw);
+
+	clk_unregister(clk);
+	kfree(mult);
+}
+EXPORT_SYMBOL_GPL(clk_unregister_multiplier);
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 3ecc07d0da77..6a7dfe33a317 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -518,6 +518,48 @@ 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_multiplier - adjustable multiplier clock
+ *
+ * @hw:		handle between common and hardware-specific interfaces
+ * @reg:	register containing the multiplier
+ * @shift:	shift to the multiplier bit field
+ * @width:	width of the multiplier bit field
+ * @lock:	register lock
+ *
+ * Clock with an adjustable multiplier affecting its output frequency.
+ * Implements .recalc_rate, .set_rate and .round_rate
+ *
+ * Flags:
+ * CLK_MULTIPLIER_ZERO_BYPASS - By default, the multiplier is the value read
+ *	from the register, with 0 being a valid value effectively
+ *	zeroing the output clock rate. If CLK_MULTIPLIER_ZERO_BYPASS is
+ *	set, then a null multiplier will be considered as a bypass,
+ *	leaving the parent rate unmodified.
+ * CLK_MULTIPLIER_ROUND_CLOSEST - Makes the best calculated divider to be
+ *	rounded to the closest integer instead of the down one.
+ */
+struct clk_multiplier {
+	struct clk_hw	hw;
+	void __iomem	*reg;
+	u8		shift;
+	u8		width;
+	u8		flags;
+	spinlock_t	*lock;
+};
+
+#define CLK_MULTIPLIER_ZERO_BYPASS		BIT(0)
+#define CLK_MULTIPLIER_ROUND_CLOSEST	BIT(1)
+
+extern const struct clk_ops clk_multiplier_ops;
+
+struct clk *clk_register_multiplier(struct device *dev, const char *name,
+				    const char *parent_name,
+				    unsigned long flags,
+				    void __iomem *reg, u8 shift, u8 width,
+				    u8 clk_mult_flags, spinlock_t *lock);
+void clk_unregister_multiplier(struct clk *clk);
+
 /***
  * struct clk_composite - aggregate clock of mux, divider and gate clocks
  *
-- 
2.6.2

[PATCH v6 2/5] 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>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/Makefile                 |   1 +
 drivers/clk/sunxi/clk-a10-pll2.c           | 188 +++++++++++++++++++++++++++++
 include/dt-bindings/clock/sun4i-a10-pll2.h |  53 ++++++++
 3 files changed, 242 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 f5a35b82cc1a..c658a18ba7cb 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-simple-gates.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..a57742a8576d
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-pll2.c
@@ -0,0 +1,188 @@
+/*
+ * 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>
+
+#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 - 1, 0)
+
+#define SUN4I_PLL2_N_SHIFT		8
+#define SUN4I_PLL2_N_WIDTH		7
+#define SUN4I_PLL2_N_MASK		GENMASK(SUN4I_PLL2_N_WIDTH - 1, 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 - 1, 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_multiplier *mult;
+	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 multiplier part of the PLL2 */
+	mult = kzalloc(sizeof(struct clk_multiplier), GFP_KERNEL);
+	if (!mult)
+		goto err_free_gate;
+
+	mult->reg = reg;
+	mult->shift = SUN4I_PLL2_N_SHIFT;
+	mult->width = 7;
+	mult->flags = CLK_MULTIPLIER_ZERO_BYPASS |
+			CLK_MULTIPLIER_ROUND_CLOSEST;
+	mult->lock = &sun4i_a10_pll2_lock;
+
+	parent = __clk_get_name(prediv_clk);
+	base_clk = clk_register_composite(NULL, "pll2-base",
+					  &parent, 1,
+					  NULL, NULL,
+					  &mult->hw, &clk_multiplier_ops,
+					  &gate->hw, &clk_gate_ops,
+					  CLK_SET_RATE_PARENT);
+	if (!base_clk) {
+		pr_err("Couldn't register the base multiplier clock\n");
+		goto err_free_multiplier;
+	}
+
+	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_multiplier:
+	kfree(mult);
+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-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.6.2

[PATCH v6 3/5] clk: sunxi: pll2: Add A13 support

[Maxime Ripard]

The A13, unlike the A10 and A20, doesn't use a pass-through exception for
the 0 value in the pre and post dividers, but increments all the values
written in the register by one.

Add an exception for both these cases to handle them nicely.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/clk-a10-pll2.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/sunxi/clk-a10-pll2.c b/drivers/clk/sunxi/clk-a10-pll2.c
index a57742a8576d..5484c31ec568 100644
--- a/drivers/clk/sunxi/clk-a10-pll2.c
+++ b/drivers/clk/sunxi/clk-a10-pll2.c
@@ -41,9 +41,15 @@
 
 #define SUN4I_PLL2_OUTPUTS		4
 
+struct sun4i_pll2_data {
+	u32	post_div_offset;
+	u32	pre_div_flags;
+};
+
 static DEFINE_SPINLOCK(sun4i_a10_pll2_lock);
 
-static void __init sun4i_pll2_setup(struct device_node *node)
+static void __init sun4i_pll2_setup(struct device_node *node,
+				    struct sun4i_pll2_data *data)
 {
 	const char *clk_name = node->name, *parent;
 	struct clk **clks, *base_clk, *prediv_clk;
@@ -70,8 +76,7 @@ static void __init sun4i_pll2_setup(struct device_node *node)
 					  parent, 0, reg,
 					  SUN4I_PLL2_PRE_DIV_SHIFT,
 					  SUN4I_PLL2_PRE_DIV_WIDTH,
-					  CLK_DIVIDER_ONE_BASED |
-					  CLK_DIVIDER_ALLOW_ZERO,
+					  data->pre_div_flags,
 					  &sun4i_a10_pll2_lock);
 	if (!prediv_clk) {
 		pr_err("Couldn't register the prediv clock\n");
@@ -122,7 +127,7 @@ static void __init sun4i_pll2_setup(struct device_node *node)
 	 */
 	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;
+	val |= (SUN4I_PLL2_POST_DIV_VALUE - data->post_div_offset) << SUN4I_PLL2_POST_DIV_SHIFT;
 	writel(val, reg);
 
 	of_property_read_string_index(node, "clock-output-names",
@@ -185,4 +190,27 @@ err_free_data:
 err_unmap:
 	iounmap(reg);
 }
-CLK_OF_DECLARE(sun4i_pll2, "allwinner,sun4i-a10-pll2-clk", sun4i_pll2_setup);
+
+static struct sun4i_pll2_data sun4i_a10_pll2_data = {
+	.pre_div_flags	= CLK_DIVIDER_ONE_BASED | CLK_DIVIDER_ALLOW_ZERO,
+};
+
+static void __init sun4i_a10_pll2_setup(struct device_node *node)
+{
+	sun4i_pll2_setup(node, &sun4i_a10_pll2_data);
+}
+
+CLK_OF_DECLARE(sun4i_a10_pll2, "allwinner,sun4i-a10-pll2-clk",
+	       sun4i_a10_pll2_setup);
+
+static struct sun4i_pll2_data sun5i_a13_pll2_data = {
+	.post_div_offset	= 1,
+};
+
+static void __init sun5i_a13_pll2_setup(struct device_node *node)
+{
+	sun4i_pll2_setup(node, &sun5i_a13_pll2_data);
+}
+
+CLK_OF_DECLARE(sun5i_a13_pll2, "allwinner,sun5i-a13-pll2-clk",
+	       sun5i_a13_pll2_setup);
-- 
2.6.2

[PATCH v6 4/5] 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 | 44 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-codec.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index c658a18ba7cb..70a449a419e6 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..ac321d6a0df5
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-codec.c
@@ -0,0 +1,44 @@
+/*
+ * 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/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.6.2

[PATCH v6 5/5] 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>
Reviewed-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/clk/sunxi/Makefile       |  1 +
 drivers/clk/sunxi/clk-a10-mod1.c | 81 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-a10-mod1.c

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 70a449a419e6..cb4c299214ce 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..e9d870de165c
--- /dev/null
+++ b/drivers/clk/sunxi/clk-a10-mod1.c
@@ -0,0 +1,81 @@
+/*
+ * 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/of.h>
+#include <linux/of_address.h>
+#include <linux/slab.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;
+
+	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);
+	i = of_clk_parent_fill(node, parents, SUN4I_MOD1_MAX_PARENTS);
+
+	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.6.2

[PATCH v6 1/5] clk: Add a basic multiplier clock

[Michael Turquette]

Hi Maxime,

Quoting Maxime Ripard (2015-10-20 00:36:45)
> +struct clk *clk_register_multiplier(struct device *dev, const char *name,
> +                                   const char *parent_name,
> +                                   unsigned long flags,
> +                                   void __iomem *reg, u8 shift, u8 width,
> +                                   u8 clk_mult_flags, spinlock_t *lock)
> +{

Patch looks good in general. However this is a good opportunity to stop
the madness around the registration functions in these basic clock
types.

clk_register is really all that we need since we've had struct
clk_init_data for a while. Initializing a multiplier should be as simple
as:

	struct clk_multiplier clk_foo = {
		.hw.init = &(struct clk_init_data){
			.name = "foo",
			.parent_names = (const char *[]){
				"bar",
			},
			.num_parents = 1;
			.ops = &clk_multiplier_ops,
		},
		.reg = 0xd34db33f,
		.shift = 1,
		.width = 2,
	};

	clk_register(dev, &clk_foo.hw);

This is nice since it turns these basic clocks into even more of a
library and less of a poor mans driver.

(I really hope the above works. I did not test it)

Is it possible you can convert to using this method, and if it is
correct for you then just remove clk_multiplier_register altogether? (In
fact you might not use the registration function at all since you use
the composite clock...)

Regards,
Mike

[PATCH v6 1/5] clk: Add a basic multiplier clock

[Maxime Ripard]

Hi Mike,

On Tue, Oct 20, 2015 at 06:43:43AM -0700, Michael Turquette wrote:
> Hi Maxime,
> 
> Quoting Maxime Ripard (2015-10-20 00:36:45)
> > +struct clk *clk_register_multiplier(struct device *dev, const char *name,
> > +                                   const char *parent_name,
> > +                                   unsigned long flags,
> > +                                   void __iomem *reg, u8 shift, u8 width,
> > +                                   u8 clk_mult_flags, spinlock_t *lock)
> > +{
> 
> Patch looks good in general. However this is a good opportunity to stop
> the madness around the registration functions in these basic clock
> types.
> 
> clk_register is really all that we need since we've had struct
> clk_init_data for a while. Initializing a multiplier should be as simple
> as:
> 
> 	struct clk_multiplier clk_foo = {
> 		.hw.init = &(struct clk_init_data){
> 			.name = "foo",
> 			.parent_names = (const char *[]){
> 				"bar",
> 			},
> 			.num_parents = 1;
> 			.ops = &clk_multiplier_ops,
> 		},
> 		.reg = 0xd34db33f,
> 		.shift = 1,
> 		.width = 2,
> 	};
> 
> 	clk_register(dev, &clk_foo.hw);
> 
> This is nice since it turns these basic clocks into even more of a
> library and less of a poor mans driver.
> 
> (I really hope the above works. I did not test it)
> 
> Is it possible you can convert to using this method, and if it is
> correct for you then just remove clk_multiplier_register altogether? (In
> fact you might not use the registration function at all since you use
> the composite clock...)

This chunk of code has been here since v2, which has been first posted
in May, two and half kernel releases ago.

In the meantime, we had a full-blown DMA driver and a quite unusual
ASoC driver merged. For some reason, this is the only piece of the
audio support that is missing for us, while at the same time it's the
most trivial.

If that's the only issue you have with this patch, I'm fine with
sending a subsequent patch this week. But I'd be really unhappy with
sending yet another version for a single change, while you had 5
monthes to review it, and we discussed it several times on IRC and
face to face.

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/20151020/e18b1520/attachment.sig>

[PATCH v6 1/5] clk: Add a basic multiplier clock

[Michael Turquette]

Quoting Maxime Ripard (2015-10-20 07:40:47)
> Hi Mike,
> 
> On Tue, Oct 20, 2015 at 06:43:43AM -0700, Michael Turquette wrote:
> > Hi Maxime,
> > 
> > Quoting Maxime Ripard (2015-10-20 00:36:45)
> > > +struct clk *clk_register_multiplier(struct device *dev, const char *name,
> > > +                                   const char *parent_name,
> > > +                                   unsigned long flags,
> > > +                                   void __iomem *reg, u8 shift, u8 width,
> > > +                                   u8 clk_mult_flags, spinlock_t *lock)
> > > +{
> > 
> > Patch looks good in general. However this is a good opportunity to stop
> > the madness around the registration functions in these basic clock
> > types.
> > 
> > clk_register is really all that we need since we've had struct
> > clk_init_data for a while. Initializing a multiplier should be as simple
> > as:
> > 
> >       struct clk_multiplier clk_foo = {
> >               .hw.init = &(struct clk_init_data){
> >                       .name = "foo",
> >                       .parent_names = (const char *[]){
> >                               "bar",
> >                       },
> >                       .num_parents = 1;
> >                       .ops = &clk_multiplier_ops,
> >               },
> >               .reg = 0xd34db33f,
> >               .shift = 1,
> >               .width = 2,
> >       };
> > 
> >       clk_register(dev, &clk_foo.hw);
> > 
> > This is nice since it turns these basic clocks into even more of a
> > library and less of a poor mans driver.
> > 
> > (I really hope the above works. I did not test it)
> > 
> > Is it possible you can convert to using this method, and if it is
> > correct for you then just remove clk_multiplier_register altogether? (In
> > fact you might not use the registration function at all since you use
> > the composite clock...)
> 
> This chunk of code has been here since v2, which has been first posted
> in May, two and half kernel releases ago.
> 
> In the meantime, we had a full-blown DMA driver and a quite unusual
> ASoC driver merged. For some reason, this is the only piece of the
> audio support that is missing for us, while at the same time it's the
> most trivial.
> 
> If that's the only issue you have with this patch, I'm fine with
> sending a subsequent patch this week. But I'd be really unhappy with
> sending yet another version for a single change, while you had 5
> monthes to review it, and we discussed it several times on IRC and
> face to face.

The change can go in later. It's not a prerequisite. I had a feeling
you'd be grumpy about me asking but I thought I'd try anyways. I won't
even ask if you got sign-off from Jim on whether this works for his
platforms ;-)

The copy/paste nature of these basic clock types really sucks and it is
one of many reasons that I am hesitant to accept them and slow to merge
them...

Anyways it seems that you are not using the registration function at all
so I might just follow up with a patch to remove it.

I can pick these 5 patches directly, or do you plan to send a PR?

Regards,
Mike

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

[PATCH v6 1/5] clk: Add a basic multiplier clock

[Maxime Ripard]

On Tue, Oct 20, 2015 at 09:29:39AM -0700, Michael Turquette wrote:
> Quoting Maxime Ripard (2015-10-20 07:40:47)
> > Hi Mike,
> > 
> > On Tue, Oct 20, 2015 at 06:43:43AM -0700, Michael Turquette wrote:
> > > Hi Maxime,
> > > 
> > > Quoting Maxime Ripard (2015-10-20 00:36:45)
> > > > +struct clk *clk_register_multiplier(struct device *dev, const char *name,
> > > > +                                   const char *parent_name,
> > > > +                                   unsigned long flags,
> > > > +                                   void __iomem *reg, u8 shift, u8 width,
> > > > +                                   u8 clk_mult_flags, spinlock_t *lock)
> > > > +{
> > > 
> > > Patch looks good in general. However this is a good opportunity to stop
> > > the madness around the registration functions in these basic clock
> > > types.
> > > 
> > > clk_register is really all that we need since we've had struct
> > > clk_init_data for a while. Initializing a multiplier should be as simple
> > > as:
> > > 
> > >       struct clk_multiplier clk_foo = {
> > >               .hw.init = &(struct clk_init_data){
> > >                       .name = "foo",
> > >                       .parent_names = (const char *[]){
> > >                               "bar",
> > >                       },
> > >                       .num_parents = 1;
> > >                       .ops = &clk_multiplier_ops,
> > >               },
> > >               .reg = 0xd34db33f,
> > >               .shift = 1,
> > >               .width = 2,
> > >       };
> > > 
> > >       clk_register(dev, &clk_foo.hw);
> > > 
> > > This is nice since it turns these basic clocks into even more of a
> > > library and less of a poor mans driver.
> > > 
> > > (I really hope the above works. I did not test it)
> > > 
> > > Is it possible you can convert to using this method, and if it is
> > > correct for you then just remove clk_multiplier_register altogether? (In
> > > fact you might not use the registration function at all since you use
> > > the composite clock...)
> > 
> > This chunk of code has been here since v2, which has been first posted
> > in May, two and half kernel releases ago.
> > 
> > In the meantime, we had a full-blown DMA driver and a quite unusual
> > ASoC driver merged. For some reason, this is the only piece of the
> > audio support that is missing for us, while at the same time it's the
> > most trivial.
> > 
> > If that's the only issue you have with this patch, I'm fine with
> > sending a subsequent patch this week. But I'd be really unhappy with
> > sending yet another version for a single change, while you had 5
> > monthes to review it, and we discussed it several times on IRC and
> > face to face.
> 
> The change can go in later. It's not a prerequisite. I had a feeling
> you'd be grumpy about me asking but I thought I'd try anyways. I won't
> even ask if you got sign-off from Jim on whether this works for his
> platforms ;-)

I asked several times, he never replied... :/

> The copy/paste nature of these basic clock types really sucks and it is
> one of many reasons that I am hesitant to accept them and slow to merge
> them...

I guess we cover all cases now? So it shouldn't grow that much.

> Anyways it seems that you are not using the registration function at all
> so I might just follow up with a patch to remove it.
> 
> I can pick these 5 patches directly, or do you plan to send a PR?

I have a pull request coming for you with a single patch, I can apply
them on that branch and send you the PR later today if it's okay?

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/20151021/6d5be779/attachment.sig>

[PATCH v6 1/5] clk: Add a basic multiplier clock

[Michael Turquette]

Quoting Maxime Ripard (2015-10-21 07:53:35)
> On Tue, Oct 20, 2015 at 09:29:39AM -0700, Michael Turquette wrote:
> > Quoting Maxime Ripard (2015-10-20 07:40:47)
> > > Hi Mike,
> > > 
> > > On Tue, Oct 20, 2015 at 06:43:43AM -0700, Michael Turquette wrote:
> > > > Hi Maxime,
> > > > 
> > > > Quoting Maxime Ripard (2015-10-20 00:36:45)
> > > > > +struct clk *clk_register_multiplier(struct device *dev, const char *name,
> > > > > +                                   const char *parent_name,
> > > > > +                                   unsigned long flags,
> > > > > +                                   void __iomem *reg, u8 shift, u8 width,
> > > > > +                                   u8 clk_mult_flags, spinlock_t *lock)
> > > > > +{
> > > > 
> > > > Patch looks good in general. However this is a good opportunity to stop
> > > > the madness around the registration functions in these basic clock
> > > > types.
> > > > 
> > > > clk_register is really all that we need since we've had struct
> > > > clk_init_data for a while. Initializing a multiplier should be as simple
> > > > as:
> > > > 
> > > >       struct clk_multiplier clk_foo = {
> > > >               .hw.init = &(struct clk_init_data){
> > > >                       .name = "foo",
> > > >                       .parent_names = (const char *[]){
> > > >                               "bar",
> > > >                       },
> > > >                       .num_parents = 1;
> > > >                       .ops = &clk_multiplier_ops,
> > > >               },
> > > >               .reg = 0xd34db33f,
> > > >               .shift = 1,
> > > >               .width = 2,
> > > >       };
> > > > 
> > > >       clk_register(dev, &clk_foo.hw);
> > > > 
> > > > This is nice since it turns these basic clocks into even more of a
> > > > library and less of a poor mans driver.
> > > > 
> > > > (I really hope the above works. I did not test it)
> > > > 
> > > > Is it possible you can convert to using this method, and if it is
> > > > correct for you then just remove clk_multiplier_register altogether? (In
> > > > fact you might not use the registration function at all since you use
> > > > the composite clock...)
> > > 
> > > This chunk of code has been here since v2, which has been first posted
> > > in May, two and half kernel releases ago.
> > > 
> > > In the meantime, we had a full-blown DMA driver and a quite unusual
> > > ASoC driver merged. For some reason, this is the only piece of the
> > > audio support that is missing for us, while at the same time it's the
> > > most trivial.
> > > 
> > > If that's the only issue you have with this patch, I'm fine with
> > > sending a subsequent patch this week. But I'd be really unhappy with
> > > sending yet another version for a single change, while you had 5
> > > monthes to review it, and we discussed it several times on IRC and
> > > face to face.
> > 
> > The change can go in later. It's not a prerequisite. I had a feeling
> > you'd be grumpy about me asking but I thought I'd try anyways. I won't
> > even ask if you got sign-off from Jim on whether this works for his
> > platforms ;-)
> 
> I asked several times, he never replied... :/
> 
> > The copy/paste nature of these basic clock types really sucks and it is
> > one of many reasons that I am hesitant to accept them and slow to merge
> > them...
> 
> I guess we cover all cases now? So it shouldn't grow that much.
> 
> > Anyways it seems that you are not using the registration function at all
> > so I might just follow up with a patch to remove it.
> > 
> > I can pick these 5 patches directly, or do you plan to send a PR?
> 
> I have a pull request coming for you with a single patch, I can apply
> them on that branch and send you the PR later today if it's okay?

Sounds good to me.

Regards,
Mike

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