[PATCH F 00/12] OMAP clock, F of F: more clock cleanup

[PATCH F 00/12] OMAP clock, F of F: more clock cleanup

[Paul Walmsley]

This series is the sixth of six to bring the mainline kernel OMAP
clock code up-to-date with the linux-omap tree.  This series fixes
several bugs and cleans up some of the clock code.

Some patches have been "compressed" together, as requested by rmk.
Original commit IDs are in the patch descriptions.

Compile-tested on OSK5912 (OMAP1), H4 and 2430SDP (OMAP2), and
BeagleBoard (OMAP3).  Boot-tested on 2430SDP and BeagleBoard.

Applies on top of series E, posted earlier.


- Paul

---

Paul Walmsley (12):
      OMAP2/3 McBSP: add temporary clockdomain fix for McBSP virtual clocks
      OMAP2/3 clock: don't tinker with hardirqs when they are supposed to be disabled
      OMAP2/3 clock: omap2_clk_enable(): fix logic
      OMAP2/3 clock: omap2_clk_enable(): fix usecount decrement bug
      OMAP2/3 clock: omap2_clk_enable(): fix bugs in clockdomain handling
      OMAP2/3 clock: omap2_clk_enable(): refactor usecount check
      OMAP2/3 clock: every clock must have a clkdm
      OMAP clock: add OMAP chip family-specific clk_register() option
      OMAP clock: drop clk_get_usecount()
      OMAP2/3 clock: convert remaining MPU barriers into OCP barriers
      OMAP2xxx clock: consolidate DELAYED_APP clock commits; fix barrier
      OMAP2/3 clock: don't use a barrier after clk_disable()


 arch/arm/mach-omap2/clock.c             |  102 +++++++++++++++++++------------
 arch/arm/mach-omap2/clock.h             |    1 
 arch/arm/mach-omap2/clock24xx.c         |   15 +----
 arch/arm/mach-omap2/clock34xx.c         |    7 +-
 arch/arm/mach-omap2/clockdomains.h      |    2 -
 arch/arm/mach-omap2/mcbsp.c             |    5 ++
 arch/arm/plat-omap/clock.c              |   27 +++-----
 arch/arm/plat-omap/include/mach/clock.h |    2 -
 8 files changed, 85 insertions(+), 76 deletions(-)

   text    data     bss     dec     hex filename
3241435  165504  100880 3507819  35866b vmlinux.beagle.orig
3241418  165504  100880 3507802  35865a vmlinux.beagle

[PATCH F 01/12] OMAP2/3 clock: don't use a barrier after clk_disable()

[Paul Walmsley]

clk_disable() previously used an ARM barrier, wmb(), to try to ensure
that the hardware write completed before continuing.  There are some
problems with this approach.

The first problem is that wmb() only ensures that the write leaves the
ARM -- not that it actually reaches the endpoint device.  In this
case, the endpoint device - either the PRM, CM, or SCM - is three
interconnects away from the ARM, and the final interconnect is
low-speed.  And the OCP interconnects will post the write, who knows
how long that will take to complete.  So the wmb() is not really what
we want.

Worse, the wmb() is indiscriminate; it will cause the ARM to flush any
other unrelated buffered writes and wait for the local interconnect to
acknowledge them - potentially very expensive.

This first problem could be fixed by doing a readback of the same PRM/CM/SCM
register.  Since these devices use a single OCP thread, this will cause the
MPU to wait for the write to complete.

But the primary problem is a conceptual one: clk_disable() should not
need any kind of barrier.  clk_enable() needs one since device driver
code must not access a device until its clocks are known to be
enabled.  But clk_disable() has no such restriction.

Since blocking the MPU on a PRM/CM/SCM write can be a very
high-latency operation - several hundred MPU cycles - it's worth
avoiding this barrier if possible.

linux-omap source commit is f4aacad2c0ed1055622d5c1e910befece24ef0e2.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 07b6232..a1ccdcb 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -448,7 +448,7 @@ static void _omap2_clk_disable(struct clk *clk)
 	else
 		v &= ~(1 << clk->enable_bit);
 	_omap2_clk_write_reg(v, clk->enable_reg, clk);
-	wmb();
+	/* No OCP barrier needed here since it is a disable operation */
 }
 
 void omap2_clk_disable(struct clk *clk)

[PATCH F 02/12] OMAP2xxx clock: consolidate DELAYED_APP clock commits; fix barrier

[Paul Walmsley]

Consolidate the commit code for DELAYED_APP clocks into a subroutine,
_omap2xxx_clk_commit().  Also convert the MPU barrier wmb() into an
OCP barrier, since with an MPU barrier, we have no guarantee that the
write actually reached the endpoint device.

linux-omap source commit is 0f5bdb736515801b296125d16937a21ff7b3cfdc.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clock.c |   34 ++++++++++++++++++++++++----------
 1 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index a1ccdcb..25efa93 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -119,6 +119,28 @@ static void _omap2_clk_write_reg(u32 v, u16 reg_offset, struct clk *clk)
 		cm_write_mod_reg(v, clk->prcm_mod, reg_offset);
 }
 
+/**
+ * _omap2xxx_clk_commit - commit clock parent/rate changes in hardware
+ * @clk: struct clk *
+ *
+ * If @clk has the DELAYED_APP flag set, meaning that parent/rate changes
+ * don't take effect until the VALID_CONFIG bit is written, write the
+ * VALID_CONFIG bit and wait for the write to complete.  No return value.
+ */
+static void _omap2xxx_clk_commit(struct clk *clk)
+{
+	if (!cpu_is_omap24xx())
+		return;
+
+	if (!(clk->flags & DELAYED_APP))
+		return;
+
+	prm_write_mod_reg(OMAP24XX_VALID_CONFIG, OMAP24XX_GR_MOD,
+			  OMAP24XX_PRCM_CLKCFG_CTRL_OFFSET);
+	/* OCP barrier */
+	prm_read_mod_reg(OMAP24XX_GR_MOD, OMAP24XX_PRCM_CLKCFG_CTRL_OFFSET);
+}
+
 /*
  * _dpll_test_fint - test whether an Fint value is valid for the DPLL
  * @clk: DPLL struct clk to test
@@ -755,11 +777,7 @@ int omap2_clksel_set_rate(struct clk *clk, unsigned long rate)
 
 	clk->rate = clk->parent->rate / new_div;
 
-	if (clk->flags & DELAYED_APP && cpu_is_omap24xx()) {
-		prm_write_mod_reg(OMAP24XX_VALID_CONFIG,
-			OMAP24XX_GR_MOD, OMAP24XX_PRCM_CLKCFG_CTRL_OFFSET);
-		wmb();
-	}
+	_omap2xxx_clk_commit(clk);
 
 	return 0;
 }
@@ -833,11 +851,7 @@ int omap2_clk_set_parent(struct clk *clk, struct clk *new_parent)
 	_omap2_clk_write_reg(v, clk->clksel_reg, clk);
 	wmb();
 
-	if (clk->flags & DELAYED_APP && cpu_is_omap24xx()) {
-		prm_write_mod_reg(OMAP24XX_VALID_CONFIG,
-			OMAP24XX_GR_MOD, OMAP24XX_PRCM_CLKCFG_CTRL_OFFSET);
-		wmb();
-	}
+	_omap2xxx_clk_commit(clk);
 
 	if (clk->usecount > 0)
 		_omap2_clk_enable(clk);

[PATCH F 03/12] OMAP2/3 clock: convert remaining MPU barriers into OCP barriers

[Paul Walmsley]

Several parts of the OMAP2/3 clock code use wmb() to try to ensure
that the hardware write completes before continuing.  This approach is
problematic: wmb() only ensures that the write leaves the ARM.  It
does not ensure that the write actually reaches the endpoint device.
The endpoint device in this case - either the PRM, CM, or SCM - is
three interconnects away from the ARM - and the final interconnect is
low-speed.  And the OCP interconnects will post the write, and who
knows how long that will take to complete.  So the wmb() is not what
we want.  Worse, the wmb() is indiscriminate; it causes the ARM to
flush any other unrelated buffered writes and wait for the local
interconnect to acknowledge them - potentially very expensive.

Fix this by converting the wmb()s into readbacks of the same PRM/CM/SCM
register.  Since the PRM/CM/SCM devices use a single OCP thread, this
will cause the MPU to block while waiting for posted writes to that device
to complete.

linux-omap source commit is 260f5487848681b4d8ea7430a709a601bbcb21d1.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clock.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 25efa93..bbc6e7d 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -444,7 +444,7 @@ static int _omap2_clk_enable(struct clk *clk)
 	else
 		v |= (1 << clk->enable_bit);
 	_omap2_clk_write_reg(v, clk->enable_reg, clk);
-	wmb();
+	v = _omap2_clk_read_reg(clk->enable_reg, clk); /* OCP barrier */
 
 	omap2_clk_wait_ready(clk);
 
@@ -772,8 +772,7 @@ int omap2_clksel_set_rate(struct clk *clk, unsigned long rate)
 	v &= ~clk->clksel_mask;
 	v |= field_val << __ffs(clk->clksel_mask);
 	_omap2_clk_write_reg(v, clk->clksel_reg, clk);
-
-	wmb();
+	v = _omap2_clk_read_reg(clk->clksel_reg, clk); /* OCP barrier */
 
 	clk->rate = clk->parent->rate / new_div;
 
@@ -849,7 +848,7 @@ int omap2_clk_set_parent(struct clk *clk, struct clk *new_parent)
 	v &= ~clk->clksel_mask;
 	v |= field_val << __ffs(clk->clksel_mask);
 	_omap2_clk_write_reg(v, clk->clksel_reg, clk);
-	wmb();
+	v = _omap2_clk_read_reg(clk->clksel_reg, clk);    /* OCP barrier */
 
 	_omap2xxx_clk_commit(clk);
 

[PATCH F 04/12] OMAP clock: drop clk_get_usecount()

[Paul Walmsley]

This function is race-prone and mistakenly conveys the impression to
drivers that it is part of the clock interface.  Get rid of it: core
code that absolutely needs this can just check clk->usecount.  Drivers
should not use it at all.

linux-omap source commit is 5df9e4adc2f6a6d55aca53ee27b8baad18897c05.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/plat-omap/clock.c              |   16 ----------------
 arch/arm/plat-omap/include/mach/clock.h |    1 -
 2 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index 2a4819f..f4a89c5 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -260,22 +260,6 @@ out:
 }
 EXPORT_SYMBOL(clk_disable);
 
-int clk_get_usecount(struct clk *clk)
-{
-	unsigned long flags;
-	int ret = 0;
-
-	if (clk == NULL || IS_ERR(clk))
-		return 0;
-
-	spin_lock_irqsave(&clockfw_lock, flags);
-	ret = clk->usecount;
-	spin_unlock_irqrestore(&clockfw_lock, flags);
-
-	return ret;
-}
-EXPORT_SYMBOL(clk_get_usecount);
-
 unsigned long clk_get_rate(struct clk *clk)
 {
 	unsigned long flags;
diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
index 9d38a25..2a30268 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -142,7 +142,6 @@ extern void followparent_recalc(struct clk *clk, unsigned long parent_rate,
 				u8 rate_storage);
 extern void clk_allow_idle(struct clk *clk);
 extern void clk_deny_idle(struct clk *clk);
-extern int clk_get_usecount(struct clk *clk);
 extern void clk_enable_init_clocks(void);
 #ifdef CONFIG_CPU_FREQ
 extern void clk_init_cpufreq_table(struct cpufreq_frequency_table **table);

[PATCH F 05/12] OMAP clock: add OMAP chip family-specific clk_register() option

[Paul Walmsley]

Provide a mechanism for OMAP chip family (e.g., OMAP1, 2, 3) clock
code to define a clk_register function via struct clk_functions.  This
is currently only used for OMAP2/3, to handle clock->clockdomain
registration.

linux-omap source commit is 12081fce83c10221ccd1b282e3e2fbe56f742e21.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clock.c             |    6 ++++++
 arch/arm/mach-omap2/clock.h             |    1 +
 arch/arm/mach-omap2/clock24xx.c         |    3 +--
 arch/arm/mach-omap2/clock34xx.c         |    5 ++---
 arch/arm/plat-omap/clock.c              |   11 ++++++++++-
 arch/arm/plat-omap/include/mach/clock.h |    1 +
 6 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index bbc6e7d..c93a5aa 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -1087,3 +1087,9 @@ void omap2_clk_disable_unused(struct clk *clk)
 		_omap2_clk_disable(clk);
 }
 #endif
+
+int omap2_clk_register(struct clk *clk)
+{
+	omap2_init_clk_clkdm(clk);
+	return 0;
+}
diff --git a/arch/arm/mach-omap2/clock.h b/arch/arm/mach-omap2/clock.h
index a026ec9..f4d489f 100644
--- a/arch/arm/mach-omap2/clock.h
+++ b/arch/arm/mach-omap2/clock.h
@@ -37,6 +37,7 @@
 #define OMAP3XXX_EN_DPLL_LOCKED			0x7
 
 int omap2_clk_init(void);
+int omap2_clk_register(struct clk *clk);
 int omap2_clk_enable(struct clk *clk);
 void omap2_clk_disable(struct clk *clk);
 long omap2_clk_round_rate(struct clk *clk, unsigned long rate);
diff --git a/arch/arm/mach-omap2/clock24xx.c b/arch/arm/mach-omap2/clock24xx.c
index cd9fa0d..c24c937 100644
--- a/arch/arm/mach-omap2/clock24xx.c
+++ b/arch/arm/mach-omap2/clock24xx.c
@@ -451,6 +451,7 @@ void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)
 #endif
 
 static struct clk_functions omap2_clk_functions = {
+	.clk_register		= omap2_clk_register,
 	.clk_enable		= omap2_clk_enable,
 	.clk_disable		= omap2_clk_disable,
 	.clk_round_rate		= omap2_clk_round_rate,
@@ -579,13 +580,11 @@ int __init omap2_clk_init(void)
 
 		if ((*clkp)->flags & CLOCK_IN_OMAP242X && cpu_is_omap2420()) {
 			clk_register(*clkp);
-			omap2_init_clk_clkdm(*clkp);
 			continue;
 		}
 
 		if ((*clkp)->flags & CLOCK_IN_OMAP243X && cpu_is_omap2430()) {
 			clk_register(*clkp);
-			omap2_init_clk_clkdm(*clkp);
 			continue;
 		}
 	}
diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index 917664d..824144e 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -633,6 +633,7 @@ static void omap3_clkoutx2_recalc(struct clk *clk, unsigned long parent_rate,
 #if defined(CONFIG_ARCH_OMAP3)
 
 static struct clk_functions omap2_clk_functions = {
+	.clk_register		= omap2_clk_register,
 	.clk_enable		= omap2_clk_enable,
 	.clk_disable		= omap2_clk_disable,
 	.clk_round_rate		= omap2_clk_round_rate,
@@ -729,10 +730,8 @@ int __init omap2_clk_init(void)
 	for (clkp = onchip_34xx_clks;
 	     clkp < onchip_34xx_clks + ARRAY_SIZE(onchip_34xx_clks);
 	     clkp++) {
-		if ((*clkp)->flags & cpu_clkflg) {
+		if ((*clkp)->flags & cpu_clkflg)
 			clk_register(*clkp);
-			omap2_init_clk_clkdm(*clkp);
-		}
 	}
 
 	/* REVISIT: Not yet ready for OMAP3 */
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index f4a89c5..c1fad02 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -439,10 +439,17 @@ void recalculate_root_clocks(void)
 
 int clk_register(struct clk *clk)
 {
+	int ret;
+
 	if (clk == NULL || IS_ERR(clk))
 		return -EINVAL;
 
 	mutex_lock(&clocks_mutex);
+	if (arch_clock->clk_register) {
+		ret = arch_clock->clk_register(clk);
+		if (ret)
+			goto cr_out;
+	}
 	list_add(&clk->node, &clocks);
 	if (!clk->children.next)
 		INIT_LIST_HEAD(&clk->children);
@@ -450,9 +457,11 @@ int clk_register(struct clk *clk)
 		omap_clk_add_child(clk->parent, clk);
 	if (clk->init)
 		clk->init(clk);
+	ret = 0;
+cr_out:
 	mutex_unlock(&clocks_mutex);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(clk_register);
 
diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
index 2a30268..db57b71 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -117,6 +117,7 @@ struct clk {
 struct cpufreq_frequency_table;
 
 struct clk_functions {
+	int		(*clk_register)(struct clk *clk);
 	int		(*clk_enable)(struct clk *clk);
 	void		(*clk_disable)(struct clk *clk);
 	long		(*clk_round_rate)(struct clk *clk, unsigned long rate);

[PATCH F 06/12] OMAP2/3 clock: every clock must have a clkdm

[Paul Walmsley]

Every OMAP2/3 clock must now have an assigned clockdomain, so we can
remove the checks from clk_enable()/clk_disable().  Instead, verify
that the clockdomain is present only at clock init time via the
arch_clock clk_register() hook.

linux-omap source commit is 60b8b431e47d8c5b8c02a2e4fa9af388aae20790.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clock.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index c93a5aa..600b2f4 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -199,11 +199,6 @@ void omap2_init_clk_clkdm(struct clk *clk)
 {
 	struct clockdomain *clkdm;
 
-	if (!clk->clkdm.name) {
-		pr_err("clock: %s: missing clockdomain", clk->name);
-		return;
-	}
-
 	clkdm = clkdm_lookup(clk->clkdm.name);
 	if (clkdm) {
 		pr_debug("clock: associated clk %s to clkdm %s\n",
@@ -479,8 +474,7 @@ void omap2_clk_disable(struct clk *clk)
 		_omap2_clk_disable(clk);
 		if (clk->parent)
 			omap2_clk_disable(clk->parent);
-		if (clk->clkdm.ptr)
-			omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
+		omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
 
 	}
 }
@@ -498,14 +492,12 @@ int omap2_clk_enable(struct clk *clk)
 			return ret;
 		}
 
-		if (clk->clkdm.ptr)
-			omap2_clkdm_clk_enable(clk->clkdm.ptr, clk);
+		omap2_clkdm_clk_enable(clk->clkdm.ptr, clk);
 
 		ret = _omap2_clk_enable(clk);
 
 		if (ret != 0) {
-			if (clk->clkdm.ptr)
-				omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
+			omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
 
 			if (clk->parent) {
 				omap2_clk_disable(clk->parent);
@@ -1090,6 +1082,12 @@ void omap2_clk_disable_unused(struct clk *clk)
 
 int omap2_clk_register(struct clk *clk)
 {
+	if (!clk->clkdm.name) {
+		pr_debug("clock: %s: missing clockdomain", clk->name);
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
 	omap2_init_clk_clkdm(clk);
 	return 0;
 }

[PATCH F 07/12] OMAP2/3 clock: omap2_clk_enable(): refactor usecount check

[Paul Walmsley]

Clean up omap2_clk_enable() by moving most of the function body out of
the usecount check.  Should result in no functional change.

linux-omap source commit is 18d4ecba1c1afc52379d5395766f442948822fc0.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clock.c |   31 ++++++++++++++++---------------
 1 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 600b2f4..6ab9de3 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -483,26 +483,27 @@ int omap2_clk_enable(struct clk *clk)
 {
 	int ret = 0;
 
-	if (clk->usecount++ == 0) {
-		if (clk->parent)
-			ret = omap2_clk_enable(clk->parent);
+	if (++clk->usecount > 1)
+		return 0;
 
-		if (ret != 0) {
-			clk->usecount--;
-			return ret;
-		}
+	if (clk->parent)
+		ret = omap2_clk_enable(clk->parent);
+
+	if (ret != 0) {
+		clk->usecount--;
+		return ret;
+	}
 
-		omap2_clkdm_clk_enable(clk->clkdm.ptr, clk);
+	omap2_clkdm_clk_enable(clk->clkdm.ptr, clk);
 
-		ret = _omap2_clk_enable(clk);
+	ret = _omap2_clk_enable(clk);
 
-		if (ret != 0) {
-			omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
+	if (ret != 0) {
+		omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
 
-			if (clk->parent) {
-				omap2_clk_disable(clk->parent);
-				clk->usecount--;
-			}
+		if (clk->parent) {
+			omap2_clk_disable(clk->parent);
+			clk->usecount--;
 		}
 	}
 

[PATCH F 08/12] OMAP2/3 clock: omap2_clk_enable(): fix bugs in clockdomain handling

[Paul Walmsley]

omap2_clk_enable() should enable a clock's clockdomain before
attempting to enable its parent clock's clockdomain.  Similarly, in
the unlikely event that the parent clock enable fails, the clockdomain
should be disabled.

linux-omap source commit is 6d6e285e5a7912b1ea68fadac387304c914aaba8.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clock.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 6ab9de3..adbb928 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -486,16 +486,17 @@ int omap2_clk_enable(struct clk *clk)
 	if (++clk->usecount > 1)
 		return 0;
 
+	omap2_clkdm_clk_enable(clk->clkdm.ptr, clk);
+
 	if (clk->parent)
 		ret = omap2_clk_enable(clk->parent);
 
 	if (ret != 0) {
 		clk->usecount--;
+		omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
 		return ret;
 	}
 
-	omap2_clkdm_clk_enable(clk->clkdm.ptr, clk);
-
 	ret = _omap2_clk_enable(clk);
 
 	if (ret != 0) {

[PATCH F 09/12] OMAP2/3 clock: omap2_clk_enable(): fix usecount decrement bug

[Paul Walmsley]

If _omap2_clk_enable() fails, the clock's usecount must be decremented by
one no matter whether the clock has a parent or not.

linux-omap source commit is 75fc235fe0f671b56873a75994513df5e665b053.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clock.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index adbb928..5f17a2c 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -500,12 +500,10 @@ int omap2_clk_enable(struct clk *clk)
 	ret = _omap2_clk_enable(clk);
 
 	if (ret != 0) {
+		clk->usecount--;
 		omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
-
-		if (clk->parent) {
+		if (clk->parent)
 			omap2_clk_disable(clk->parent);
-			clk->usecount--;
-		}
 	}
 
 	return ret;

[PATCH F 10/12] OMAP2/3 clock: omap2_clk_enable(): fix logic

[Paul Walmsley]

Rearrange the parent clock enable status check code so it actually makes
sense.  No functional change.

linux-omap source commit is 8f0680aff352906b81c49b0450172ffa690b03bb.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clock.c |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/clock.c b/arch/arm/mach-omap2/clock.c
index 5f17a2c..72e3db8 100644
--- a/arch/arm/mach-omap2/clock.c
+++ b/arch/arm/mach-omap2/clock.c
@@ -481,20 +481,23 @@ void omap2_clk_disable(struct clk *clk)
 
 int omap2_clk_enable(struct clk *clk)
 {
-	int ret = 0;
+	int ret;
 
 	if (++clk->usecount > 1)
 		return 0;
 
 	omap2_clkdm_clk_enable(clk->clkdm.ptr, clk);
 
-	if (clk->parent)
-		ret = omap2_clk_enable(clk->parent);
+	if (clk->parent) {
+		int parent_ret;
 
-	if (ret != 0) {
-		clk->usecount--;
-		omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
-		return ret;
+		parent_ret = omap2_clk_enable(clk->parent);
+
+		if (parent_ret != 0) {
+			clk->usecount--;
+			omap2_clkdm_clk_disable(clk->clkdm.ptr, clk);
+			return parent_ret;
+		}
 	}
 
 	ret = _omap2_clk_enable(clk);

[PATCH F 11/12] OMAP2/3 clock: don't tinker with hardirqs when they are supposed to be disabled

[Paul Walmsley]

Clock rate change code executes inside a spinlock with hardirqs
disabled.  The only code that should be messing around with the
hardirq state should be the plat-omap/clock.c code.  In the
omap2_reprogram_dpllcore() case, this probably just wastes cycles, but
in the omap3_core_dpll_m2_set_rate() case, this is a nasty bug.

linux-omap source commit is b9b6208dadb5e0d8b290900a3ffa911673ca97ed.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clock24xx.c |   12 +++---------
 arch/arm/mach-omap2/clock34xx.c |    2 --
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/clock24xx.c b/arch/arm/mach-omap2/clock24xx.c
index c24c937..1947ffa 100644
--- a/arch/arm/mach-omap2/clock24xx.c
+++ b/arch/arm/mach-omap2/clock24xx.c
@@ -221,10 +221,7 @@ static int omap2_reprogram_dpllcore(struct clk *clk, unsigned long rate)
 	u32 bypass = 0;
 	struct prcm_config tmpset;
 	const struct dpll_data *dd;
-	unsigned long flags;
-	int ret = -EINVAL;
 
-	local_irq_save(flags);
 	cur_rate = omap2xxx_clk_get_core_rate(&dpll_ck, dpll_ck.parent->rate);
 	mult = cm_read_mod_reg(PLL_MOD, CM_CLKSEL2);
 	mult &= OMAP24XX_CORE_CLK_SRC_MASK;
@@ -236,7 +233,7 @@ static int omap2_reprogram_dpllcore(struct clk *clk, unsigned long rate)
 	} else if (rate != cur_rate) {
 		valid_rate = omap2_dpllcore_round_rate(rate);
 		if (valid_rate != rate)
-			goto dpll_exit;
+			return -EINVAL;
 
 		if (mult == 1)
 			low = curr_prcm_set->dpll_speed;
@@ -245,7 +242,7 @@ static int omap2_reprogram_dpllcore(struct clk *clk, unsigned long rate)
 
 		dd = clk->dpll_data;
 		if (!dd)
-			goto dpll_exit;
+			return -EINVAL;
 
 		tmpset.cm_clksel1_pll = cm_read_mod_reg(clk->prcm_mod,
 							dd->mult_div1_reg);
@@ -283,11 +280,8 @@ static int omap2_reprogram_dpllcore(struct clk *clk, unsigned long rate)
 		omap2xxx_sdrc_init_params(omap2xxx_sdrc_dll_is_unlocked());
 		omap2xxx_sdrc_reprogram(done_rate, 0);
 	}
-	ret = 0;
 
-dpll_exit:
-	local_irq_restore(flags);
-	return(ret);
+	return 0;
 }
 
 /**
diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index 824144e..f7ac5c1 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -491,10 +491,8 @@ static int omap3_core_dpll_m2_set_rate(struct clk *clk, unsigned long rate)
 	WARN_ON(new_div != 1 && new_div != 2);
 
 	/* REVISIT: Add SDRC_MR changing to this code also */
-	local_irq_disable();
 	omap3_configure_core_dpll(sp->rfr_ctrl, sp->actim_ctrla,
 				  sp->actim_ctrlb, new_div);
-	local_irq_enable();
 
 	return 0;
 }

[PATCH F 12/12] OMAP2/3 McBSP: add temporary clockdomain fix for McBSP virtual clocks

[Paul Walmsley]

The McBSP driver uses virtual clocks to handle enabling and disabling
its hardware clocks.  These virtual clocks have no associated
clockdomain. After commit 60b8b431e47d8c5b8c02a2e4fa9af388aae20790,
this prevents the McBSP clocks from registering correctly.
Resolve this for the short term by using virt_opp_clkdm for these clocks.
These McBSP virtual clocks should be removed, but such a fix would require
significant changes to the McBSP drivers that would require testing on
OMAP1, 2, and 3 platforms.

Tested on 2430SDP and 3430SDP GP ES2.1.

linux-omap source commit is 818862e11bad091dc635baedace58265a126b5c8.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Eero Nurkkala <ext-eero.nurkkala@nokia.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/clockdomains.h |    2 +-
 arch/arm/mach-omap2/mcbsp.c        |    5 +++++
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clockdomains.h b/arch/arm/mach-omap2/clockdomains.h
index 3d4eaca..b29035e 100644
--- a/arch/arm/mach-omap2/clockdomains.h
+++ b/arch/arm/mach-omap2/clockdomains.h
@@ -40,7 +40,7 @@ static struct clockdomain cm_clkdm = {
 static struct clockdomain virt_opp_clkdm = {
 	.name		= "virt_opp_clkdm",
 	.pwrdm		= { .name = "wkup_pwrdm" },
-	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP24XX),
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP24XX | CHIP_IS_OMAP3430),
 };
 
 /*
diff --git a/arch/arm/mach-omap2/mcbsp.c b/arch/arm/mach-omap2/mcbsp.c
index e20023c..1785d7a 100644
--- a/arch/arm/mach-omap2/mcbsp.c
+++ b/arch/arm/mach-omap2/mcbsp.c
@@ -79,6 +79,7 @@ static struct mcbsp_internal_clk omap_mcbsp_clks[] = {
 		.clk = {
 			.name 		= "mcbsp_clk",
 			.id		= 1,
+			.clkdm		= { .name = "virt_opp_clkdm" },
 			.enable		= omap_mcbsp_clk_enable,
 			.disable	= omap_mcbsp_clk_disable,
 		},
@@ -87,6 +88,7 @@ static struct mcbsp_internal_clk omap_mcbsp_clks[] = {
 		.clk = {
 			.name 		= "mcbsp_clk",
 			.id		= 2,
+			.clkdm		= { .name = "virt_opp_clkdm" },
 			.enable		= omap_mcbsp_clk_enable,
 			.disable	= omap_mcbsp_clk_disable,
 		},
@@ -95,6 +97,7 @@ static struct mcbsp_internal_clk omap_mcbsp_clks[] = {
 		.clk = {
 			.name		= "mcbsp_clk",
 			.id		= 3,
+			.clkdm		= { .name = "virt_opp_clkdm" },
 			.enable		= omap_mcbsp_clk_enable,
 			.disable	= omap_mcbsp_clk_disable,
 		},
@@ -103,6 +106,7 @@ static struct mcbsp_internal_clk omap_mcbsp_clks[] = {
 		.clk = {
 			.name		= "mcbsp_clk",
 			.id		= 4,
+			.clkdm		= { .name = "virt_opp_clkdm" },
 			.enable		= omap_mcbsp_clk_enable,
 			.disable	= omap_mcbsp_clk_disable,
 		},
@@ -111,6 +115,7 @@ static struct mcbsp_internal_clk omap_mcbsp_clks[] = {
 		.clk = {
 			.name		= "mcbsp_clk",
 			.id		= 5,
+			.clkdm		= { .name = "virt_opp_clkdm" },
 			.enable		= omap_mcbsp_clk_enable,
 			.disable	= omap_mcbsp_clk_disable,
 		},

Re: [PATCH F 10/12] OMAP2/3 clock: omap2_clk_enable(): fix logic

[Russell King - ARM Linux]

On Wed, Jan 28, 2009 at 12:35:28PM -0700, Paul Walmsley wrote:
> Rearrange the parent clock enable status check code so it actually makes
> sense.  No functional change.

I don't think there's anything gained from introducing the additional
'parent_ret' variable.

Re: [PATCH F 12/12] OMAP2/3 McBSP: add temporary clockdomain fix for McBSP virtual clocks

[Russell King - ARM Linux]

On Wed, Jan 28, 2009 at 12:35:33PM -0700, Paul Walmsley wrote:
> The McBSP driver uses virtual clocks to handle enabling and disabling
> its hardware clocks.  These virtual clocks have no associated
> clockdomain. After commit 60b8b431e47d8c5b8c02a2e4fa9af388aae20790,
> this prevents the McBSP clocks from registering correctly.
> Resolve this for the short term by using virt_opp_clkdm for these clocks.
> These McBSP virtual clocks should be removed, but such a fix would require
> significant changes to the McBSP drivers that would require testing on
> OMAP1, 2, and 3 platforms.

With either my clkdev patches or the spinlock fixing for mcbsp, this patch
seems to be redundant.

Re: [PATCH F 06/12] OMAP2/3 clock: every clock must have a clkdm

[Russell King - ARM Linux]

On Wed, Jan 28, 2009 at 12:35:15PM -0700, Paul Walmsley wrote:
> Every OMAP2/3 clock must now have an assigned clockdomain, so we can
> remove the checks from clk_enable()/clk_disable().  Instead, verify
> that the clockdomain is present only at clock init time via the
> arch_clock clk_register() hook.

I don't see the point of requiring all clocks to have a clockdomain field.
Given that we have virtual clocks, it is quite reasonable for some clocks
not to have a clock domain associated with them.

The overhead for avoiding this requirement isn't that great, and I feel
that there's little benefit from this change.

Re: [PATCH F 10/12] OMAP2/3 clock: omap2_clk_enable(): fix logic

[Paul Walmsley]

Hi Russell,

On Sat, 31 Jan 2009, Russell King - ARM Linux wrote:

> On Wed, Jan 28, 2009 at 12:35:28PM -0700, Paul Walmsley wrote:
> > Rearrange the parent clock enable status check code so it actually makes
> > sense.  No functional change.
> 
> I don't think there's anything gained from introducing the additional
> 'parent_ret' variable.

I thought it made the code slightly easier to understand; but, am not 
attached to it.  Please let me know if you'd like me to send a version of 
the patch without it,


- Paul

Re: [PATCH F 12/12] OMAP2/3 McBSP: add temporary clockdomain fix for McBSP virtual clocks

[Paul Walmsley]

Hi Russell,

On Sat, 31 Jan 2009, Russell King - ARM Linux wrote:

> On Wed, Jan 28, 2009 at 12:35:33PM -0700, Paul Walmsley wrote:
> > The McBSP driver uses virtual clocks to handle enabling and disabling
> > its hardware clocks.  These virtual clocks have no associated
> > clockdomain. After commit 60b8b431e47d8c5b8c02a2e4fa9af388aae20790,
> > this prevents the McBSP clocks from registering correctly.
> > Resolve this for the short term by using virt_opp_clkdm for these clocks.
> > These McBSP virtual clocks should be removed, but such a fix would require
> > significant changes to the McBSP drivers that would require testing on
> > OMAP1, 2, and 3 platforms.
> 
> With either my clkdev patches or the spinlock fixing for mcbsp, this patch
> seems to be redundant.

Indeed.  It was included so the patch set would work if your tree didn't 
have Stanley's McBSP patch applied; I don't think it's made it up to 
mainline.


- Paul

Re: [PATCH F 06/12] OMAP2/3 clock: every clock must have a clkdm

[Paul Walmsley]

Hello Russell,

On Sat, 31 Jan 2009, Russell King - ARM Linux wrote:

> On Wed, Jan 28, 2009 at 12:35:15PM -0700, Paul Walmsley wrote:
> > Every OMAP2/3 clock must now have an assigned clockdomain, so we can
> > remove the checks from clk_enable()/clk_disable().  Instead, verify
> > that the clockdomain is present only at clock init time via the
> > arch_clock clk_register() hook.
> 
> I don't see the point of requiring all clocks to have a clockdomain field.

For physical clocks, the idea is to match the OMAP2/3 hardware, in which 
nearly every clock is associated with a clockdomain.  The point for 
virtual clocks is to discourage the use of virtual clocks.

> Given that we have virtual clocks, it is quite reasonable for some clocks
> not to have a clock domain associated with them.

Virtual clocks should soon be eliminated from the OMAP clock tree.  ( 
"Virtual clocks" is here used to refer a clock that does not have a 
referent in the hardware; the usage of the term in the code is loose.)  
So far as I know, all of the OMAP virtual clocks have either turned out to 
be superfluous (and prone to spinlock recursion bugs), or to be better 
implemented outside of the clock framework (such as the virtual OPP 
clocks).  We've eliminated the former.  We should be able to eliminate the 
latter in a few months.

Only physical hardware clocks will then be left in the clock tree.  For 
OMAP2/3, it makes sense to keep those grouped into clockdomains, since 
that is how they are implemented in the hardware.

So I'd recommend keeping this requirement.


- Paul

RE: [PATCH F 06/12] OMAP2/3 clock: every clock must have a clkdm

[Woodruff, Richard]

> Virtual clocks should soon be eliminated from the OMAP clock tree.  (
> "Virtual clocks" is here used to refer a clock that does not have a
> referent in the hardware; the usage of the term in the code is loose.)
> So far as I know, all of the OMAP virtual clocks have either turned out to
> be superfluous (and prone to spinlock recursion bugs), or to be better
> implemented outside of the clock framework (such as the virtual OPP
> clocks).  We've eliminated the former.  We should be able to eliminate the
> latter in a few months.

What is your idea for replacement of virtual OPPs?

Especially on OMAP2 they were useful with tightly coupled clock sets.  Some later versions of 2420 were characterized such that some of the dependencies could be ignored but that is not the case for all of them.  The centerline design as I understand it wasn't for this. It just happened that enough margin was there in some binn'ed lots to do this.

Regards,
Richard W.

RE: [PATCH F 06/12] OMAP2/3 clock: every clock must have a clkdm

[Paul Walmsley]

Hi Richard,

On Tue, 3 Feb 2009, Woodruff, Richard wrote:

> > Virtual clocks should soon be eliminated from the OMAP clock tree.  (
> > "Virtual clocks" is here used to refer a clock that does not have a
> > referent in the hardware; the usage of the term in the code is loose.)
> > So far as I know, all of the OMAP virtual clocks have either turned out to
> > be superfluous (and prone to spinlock recursion bugs), or to be better
> > implemented outside of the clock framework (such as the virtual OPP
> > clocks).  We've eliminated the former.  We should be able to eliminate the
> > latter in a few months.
> 
> What is your idea for replacement of virtual OPPs?
> 
> Especially on OMAP2 they were useful with tightly coupled clock sets.  
> Some later versions of 2420 were characterized such that some of the 
> dependencies could be ignored but that is not the case for all of them.  
> The centerline design as I understand it wasn't for this. It just 
> happened that enough margin was there in some binn'ed lots to do this.

The rates and voltages would still be tightly coupled; the OPP change just 
wouldn't be handled by the clock framework.  Will send patches to you once 
the code is fully baked, the patches would probably the best thing to 
comment on...


- Paul