Skip to contentSkip to conversation

clk: sunxi: Add support for the Audio PLL

PATCH Superseded Maxime Ripard 2 participants 15 messages Sep 29, 2015 – Oct 7, 2015

Subsystems (4)

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v3 cover clk: sunxi: Add support for the Audio PLL

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.

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 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               | 176 +++++++++++++++++++++++
 drivers/clk/sunxi/Makefile                 |   3 +
 drivers/clk/sunxi/clk-a10-codec.c          |  45 ++++++
 drivers/clk/sunxi/clk-a10-mod1.c           |  84 +++++++++++
 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, 620 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
Signature · 1 line
2.5.3
#1 lore raw

PATCH v3 1/5 clk: Add a basic multiplier clock

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.

---
 drivers/clk/Makefile         |   1 +
 drivers/clk/clk-multiplier.c | 176 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/clk-provider.h |  42 +++++++++++
 3 files changed, 219 insertions(+)
 create mode 100644 drivers/clk/clk-multiplier.c
drivers/clk/Makefile +1 -0
index d08b3e5985be..0b2101039508 100644
@@ -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
drivers/clk/clk-multiplier.c new file +176 -0

176 lines not on this page · raw message

include/linux/clk-provider.h +42 -0

48 lines not on this page · raw message

Signature · 1 line
2.5.3
#2 lore raw

PATCH v3 1/5 clk: Add a basic multiplier clock

Quoted 6 lines · Maxime Ripard
drivers/clk/clk-multiplier.c new file +0 -0
new file mode 100644
index 000000000000..61097e365d55

drivers/clk/clk-multiplier.c line 8

Quoted 8 lines
drivers/clk/clk-multiplier.c new file +8 -0
@@ -0,0 +1,176 @@
+/*
+ * Copyright (C) 2015 Maxime Ripard <[email protected]>
+ *
+ * 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>

Maybe export.h is more appropriate?

drivers/clk/clk-multiplier.c line 21

Quoted 13 lines
drivers/clk/clk-multiplier.c new file +13 -0
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/of.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);

We should include kernel.h for this macro.

drivers/clk/clk-multiplier.c line 33

Quoted 12 lines
drivers/clk/clk-multiplier.c new file +12 -0
+
+	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, 0);

and bitops.h for GENMASK

drivers/clk/clk-multiplier.c line 44

Quoted 11 lines
drivers/clk/clk-multiplier.c new file +11 -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)

Is the only difference in this function vs the divider one that flag? Maybe we should make this function generic to the framework and pass a flag indicating closest or not.

Quoted 4 lines
drivers/clk/clk-multiplier.c new file +4 -0
+		return abs(rate - new) < abs(rate - best);
+
+	return new >= rate && new < best;
+}

drivers/clk/clk-multiplier.c line 58

Quoted 10 lines
drivers/clk/clk-multiplier.c new file +10 -0
+
+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_get_flags(hw->clk) & CLK_SET_RATE_PARENT))

Please use clk_hw_get_flags. I'd *really* like to kill __clk_get_flags() but we still have one user in omap code.

Quoted 23 lines
drivers/clk/clk-multiplier.c new file +23 -0
+		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_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;
+		}
+	}

drivers/clk/clk-multiplier.c line 101

Quoted 10 lines
drivers/clk/clk-multiplier.c new file +10 -0
+
+	return bestmult;
+}
+
+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 uninitialized_var(flags);

hmmm ok. We set it to 0 in other drivers.

drivers/clk/clk-multiplier.c line 105

Quoted 4 lines
drivers/clk/clk-multiplier.c new file +4 -0
+	unsigned long val;
+
+	if (mult->lock)
+		spin_lock_irqsave(mult->lock, flags);

This needs the same "trick" that we did in the generic clock types to avoid sparse warnings.

Quoted 15 lines
drivers/clk/clk-multiplier.c new file +15 -0
+
+	val = clk_readl(mult->reg);
+	val &= ~GENMASK(mult->width + mult->shift, mult->shift);
+	val |= factor << mult->shift;
+	clk_writel(val, mult->reg);
+
+	if (mult->lock)
+		spin_unlock_irqrestore(mult->lock, flags);
+
+	return 0;
+}
+
+struct clk *clk_register_multiplier(struct device *dev, const char *name,
+				    const char *parent_name,
+				    unsigned long flags,

drivers/clk/clk-multiplier.c line 137

Quoted 10 lines
drivers/clk/clk-multiplier.c new file +10 -0
+				    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) {
+		pr_err("%s: could not allocate multiplier clk\n", __func__);

We don't need allocation error messages.

Quoted 2 lines
drivers/clk/clk-multiplier.c new file +2 -0
+		return ERR_PTR(-ENOMEM);
+	}
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
#3 lore raw

PATCH v3 1/5 clk: Add a basic multiplier clock

Hi,

Quoted 17 lines · Stephen Boyd
Quoted 6 lines · Maxime Ripard
drivers/clk/clk-multiplier.c new file +0 -0
new file mode 100644
index 000000000000..61097e365d55

drivers/clk/clk-multiplier.c line 8

Quoted 8 lines
drivers/clk/clk-multiplier.c new file +8 -0
@@ -0,0 +1,176 @@
+/*
+ * Copyright (C) 2015 Maxime Ripard <[email protected]>
+ *
+ * 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>

Maybe export.h is more appropriate?

Indeed.

Quoted 7 lines
Quoted 7 lines
drivers/clk/clk-multiplier.c new file +7 -0
+#include <linux/clk-provider.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/of.h>
+
+#define to_clk_multiplier(_hw) container_of(_hw, struct clk_multiplier, hw)
+
Quoted 8 lines

drivers/clk/clk-multiplier.c line 21

Quoted 6 lines
drivers/clk/clk-multiplier.c new file +6 -0
+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);

We should include kernel.h for this macro.

Ack.

Quoted 6 lines
Quoted 6 lines
drivers/clk/clk-multiplier.c new file +6 -0
+
+	return rate / parent_rate;
+}
+
+static unsigned long clk_multiplier_recalc_rate(struct clk_hw *hw,
+						unsigned long parent_rate)
Quoted 8 lines

drivers/clk/clk-multiplier.c line 33

Quoted 6 lines
drivers/clk/clk-multiplier.c new file +6 -0
+{
+	struct clk_multiplier *mult = to_clk_multiplier(hw);
+	unsigned long val;
+	
+	val = clk_readl(mult->reg) >> mult->shift;
+	val &= GENMASK(mult->width, 0);

and bitops.h for GENMASK

Ack.

Quoted 7 lines
Quoted 7 lines
drivers/clk/clk-multiplier.c new file +7 -0
+
+	if (!val && mult->flags & CLK_MULTIPLIER_ZERO_BYPASS)
+		val = 1;
+	
+	return parent_rate * val;
+}
+
Quoted 8 lines

drivers/clk/clk-multiplier.c line 44

Quoted 4 lines
drivers/clk/clk-multiplier.c new file +4 -0
+static bool __is_best_rate(unsigned long rate, unsigned long new,
+			   unsigned long best, unsigned long flags)
+{
+	if (flags & CLK_MULTIPLIER_ROUND_CLOSEST)

Is the only difference in this function vs the divider one that flag? Maybe we should make this function generic to the framework and pass a flag indicating closest or not.

Actually, the logic is also reversed.

The divider driver will always try to find some rate that is higher than the one we already have, without going above than the one requested.

Here, we're tring to be lower than the best rate, without going below the requested rate.

Quoted 10 lines
Quoted 10 lines
drivers/clk/clk-multiplier.c new file +9 -0
+		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)
+{
Quoted 8 lines

drivers/clk/clk-multiplier.c line 58

Quoted 5 lines
drivers/clk/clk-multiplier.c new file +5 -0
+	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))

Please use clk_hw_get_flags. I'd *really* like to kill __clk_get_flags() but we still have one user in omap code.

Ack.

Quoted 28 lines
Quoted 28 lines
drivers/clk/clk-multiplier.c new file +27 -0
+		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_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;
+}
+
Quoted 8 lines

drivers/clk/clk-multiplier.c line 101

Quoted 6 lines
drivers/clk/clk-multiplier.c new file +6 -0
+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 uninitialized_var(flags);

hmmm ok. We set it to 0 in other drivers.

I'll change it then.

Quoted 8 lines

drivers/clk/clk-multiplier.c line 105

Quoted 5 lines
drivers/clk/clk-multiplier.c new file +4 -0
+	unsigned long val;
+
+	if (mult->lock)
+		spin_lock_irqsave(mult->lock, flags);

This needs the same "trick" that we did in the generic clock types to avoid sparse warnings.

The __acquire call ?

Quoted 19 lines
Quoted 19 lines
drivers/clk/clk-multiplier.c new file +19 -0
+
+	val = clk_readl(mult->reg);
+	val &= ~GENMASK(mult->width + mult->shift, mult->shift);
+	val |= factor << mult->shift;
+	clk_writel(val, mult->reg);
+
+	if (mult->lock)
+		spin_unlock_irqrestore(mult->lock, flags);
+
+	return 0;
+}
+
+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;
Quoted 8 lines

drivers/clk/clk-multiplier.c line 137

Quoted 6 lines
drivers/clk/clk-multiplier.c new file +6 -0
+	struct clk_multiplier *mult;
+	struct clk *clk;
+
+	mult = kmalloc(sizeof(*mult), GFP_KERNEL);
+	if (!mult) {
+		pr_err("%s: could not allocate multiplier clk\n", __func__);

We don't need allocation error messages.

Ack.

Thanks!
Maxime

Signature · 10 lines
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/20151005/6be879ec/attachment-0001.sig>
#4 lore raw

PATCH v3 1/5 clk: Add a basic multiplier clock

Quoted 20 lines · Maxime Ripard

Hi,

Quoted 16 lines · Stephen Boyd

drivers/clk/clk-multiplier.c line 44

Quoted 11 lines · Maxime Ripard
drivers/clk/clk-multiplier.c new file +11 -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)

Is the only difference in this function vs the divider one that flag? Maybe we should make this function generic to the framework and pass a flag indicating closest or not.

Quoted 8 lines

Actually, the logic is also reversed.

The divider driver will always try to find some rate that is higher than the one we already have, without going above than the one requested.

Here, we're tring to be lower than the best rate, without going below the requested rate.

So then a tri-state flag that indicates, closest, less than, greater than?

Quoted 11 lines
Quoted 9 lines

drivers/clk/clk-multiplier.c line 105

Quoted 6 lines
drivers/clk/clk-multiplier.c new file +4 -0
+	unsigned long val;
+
+	if (mult->lock)
+		spin_lock_irqsave(mult->lock, flags);

This needs the same "trick" that we did in the generic clock types to avoid sparse warnings.

The __acquire call ?

Yes.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
#5 lore raw

PATCH v3 1/5 clk: Add a basic multiplier clock

Quoted 27 lines · Stephen Boyd
Quoted 26 lines · Maxime Ripard

Hi,

Quoted 16 lines · Stephen Boyd

drivers/clk/clk-multiplier.c line 44

Quoted 11 lines · Maxime Ripard
drivers/clk/clk-multiplier.c new file +11 -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)

Is the only difference in this function vs the divider one that flag? Maybe we should make this function generic to the framework and pass a flag indicating closest or not.

Actually, the logic is also reversed.

The divider driver will always try to find some rate that is higher than the one we already have, without going above than the one requested.

Quoted 5 lines
Quoted 2 lines

Here, we're tring to be lower than the best rate, without going below the requested rate.

So then a tri-state flag that indicates, closest, less than, greater than?

Still, the computation itself is different, and the only consolidation we could possibly do is by not duplicating the ROUND_CLOSEST. We would end up with two different code pathes in the same function, which I feel would make it unnecessarily complex.

Quoted 11 lines
Quoted 11 lines
Quoted 10 lines

drivers/clk/clk-multiplier.c line 105

Quoted 7 lines
drivers/clk/clk-multiplier.c new file +4 -0
+	unsigned long val;
+
+	if (mult->lock)
+		spin_lock_irqsave(mult->lock, flags);

This needs the same "trick" that we did in the generic clock types to avoid sparse warnings.

Quoted 3 lines
Quoted 1 line

The __acquire call ?

Yes.

Ok.

Thanks!
Maxime

Signature · 10 lines
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/20151007/46995fa7/attachment.sig>
#6 lore raw

PATCH v3 1/5 clk: Add a basic multiplier clock

Quoted 12 lines · Maxime Ripard
Quoted 11 lines · Stephen Boyd
Quoted 9 lines · Maxime Ripard

Actually, the logic is also reversed.

The divider driver will always try to find some rate that is higher than the one we already have, without going above than the one requested.

Here, we're tring to be lower than the best rate, without going below the requested rate.

Quoted 7 lines
Quoted 2 lines

So then a tri-state flag that indicates, closest, less than, greater than?

Still, the computation itself is different, and the only consolidation we could possibly do is by not duplicating the ROUND_CLOSEST. We would end up with two different code pathes in the same function, which I feel would make it unnecessarily complex.

Ok. No worries.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
#7 lore raw

PATCH v3 2/5 clk: sunxi: Add a driver for the PLL2

In reply to 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.

---
 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
drivers/clk/sunxi/Makefile +1 -0
index f5a35b82cc1a..c658a18ba7cb 100644
@@ -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
drivers/clk/sunxi/clk-a10-pll2.c new file +188 -0

188 lines not on this page · raw message

include/dt-bindings/clock/sun4i-a10-pll2.h new file +53 -0

53 lines not on this page · raw message

Signature · 1 line
2.5.3
#8 lore raw

PATCH v3 3/5 clk: sunxi: pll2: Add A13 support

In reply to 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.

---
 drivers/clk/sunxi/clk-a10-pll2.c | 38 +++++++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 5 deletions(-)
drivers/clk/sunxi/clk-a10-pll2.c +33 -5
index b877cc6c3622..bae9235f7528 100644
@@ -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);
Signature · 1 line
2.5.3
#9 lore raw

PATCH v3 4/5 clk: sunxi: codec clock support

In reply to Maxime Ripard

From: Emilio L?pez <[email protected]>

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

---
 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
drivers/clk/sunxi/Makefile +1 -0
index c658a18ba7cb..70a449a419e6 100644
@@ -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
drivers/clk/sunxi/clk-a10-codec.c new file +45 -0

45 lines not on this page · raw message

Signature · 1 line
2.5.3
#10 lore raw

PATCH v3 4/5 clk: sunxi: codec clock support

Quoted 14 lines · Maxime Ripard
drivers/clk/sunxi/clk-a10-codec.c new file +8 -0
new file mode 100644
index 000000000000..aaeccf8cde39
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2013 Emilio L?pez
+ *
+ * Emilio L?pez <[email protected]>
+ *
+ * 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

drivers/clk/sunxi/clk-a10-codec.c line 18

Quoted 10 lines
drivers/clk/sunxi/clk-a10-codec.c new file +10 -0
+ * (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>

Is this include used?

Quoted 2 lines
drivers/clk/sunxi/clk-a10-codec.c new file +2 -0
+#include <linux/of.h>
+#include <linux/of_address.h>
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
#11 lore raw

PATCH v3 4/5 clk: sunxi: codec clock support

Quoted 19 lines · Stephen Boyd
Quoted 18 lines · Maxime Ripard
drivers/clk/sunxi/clk-a10-codec.c new file +12 -0
new file mode 100644
index 000000000000..aaeccf8cde39
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2013 Emilio L?pez
+ *
+ * Emilio L?pez <[email protected]>
+ *
+ * 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
Quoted 8 lines

drivers/clk/sunxi/clk-a10-codec.c line 18

Quoted 6 lines
drivers/clk/sunxi/clk-a10-codec.c new file +6 -0
+ * 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>

Is this include used?

It isn't, I've removed it.

Thanks!
Maxime

Signature · 10 lines
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/20151005/6a238a62/attachment.sig>
#12 lore raw

PATCH v3 5/5 clk: sunxi: mod1 clock support

In reply to Maxime Ripard

From: Emilio L?pez <[email protected]>

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.

---
 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
drivers/clk/sunxi/Makefile +1 -0
index 70a449a419e6..cb4c299214ce 100644
@@ -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
drivers/clk/sunxi/clk-a10-mod1.c new file +84 -0

84 lines not on this page · raw message

Signature · 1 line
2.5.3
#13 lore raw

PATCH v3 5/5 clk: sunxi: mod1 clock support

drivers/clk/sunxi/clk-a10-mod1.c line 18

Quoted 10 lines · Maxime Ripard

+ * (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.
+ */

drivers/clk/sunxi/clk-a10-mod1.c new file +3 -0
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>

Is this include used?

drivers/clk/sunxi/clk-a10-mod1.c line 54

Quoted 7 lines
drivers/clk/sunxi/clk-a10-mod1.c new file +7 -0
+	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)

Can we use of_clk_parent_fill() here?

Quoted 2 lines
(quoted diff) +2 -0
+		i++;
+
Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
#14 lore raw

PATCH v3 5/5 clk: sunxi: mod1 clock support

Quoted 13 lines · Stephen Boyd

drivers/clk/sunxi/clk-a10-mod1.c line 18

Quoted 10 lines · Maxime Ripard

+ * (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.
+ */

drivers/clk/sunxi/clk-a10-mod1.c new file +3 -0
+
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>

Is this include used?

Fixed.

Quoted 9 lines

drivers/clk/sunxi/clk-a10-mod1.c line 54

Quoted 7 lines
drivers/clk/sunxi/clk-a10-mod1.c new file +7 -0
+	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)

Can we use of_clk_parent_fill() here?

We can, I switched to it.

Thanks!
Maxime

Signature · 10 lines
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/20151005/2ca90072/attachment.sig>