[PATCH 0/3] 24xx clock fixes: DSP, 54MHz APLL

[PATCH 0/3] 24xx clock fixes: DSP, 54MHz APLL

[Paul Walmsley]

Hello,

these patches clean up the structure of the DSP clocks on the OMAP24xx
platforms.  They remove redundant clocks and rate structures, and fix
the DSP driver code to use the correct clock name on 2430.  DSP
i-clock source selection is moved into a separate clock,
dsp_irate_ick, to facilitate some of this consolidation.  

Also, in omap2_clk_fixed_enable(), it turns out that we were looking for
the wrong IDLEST bit to test if the 54MHz APLL was locked - fix this.

This series resolves the boot-time clock warnings that appear on
2430SDP:

    clock: Could not find parent clock iva2_1_fck in clksel array of
    clock iva2_1_ick

    omapdsp: could not acquire dsp_ick handle

    Clock apll54_ck didn't enable in 100000 tries


Boot-tested on N800 and 2430SDP.  Comments welcome.


- Paul


diffstat:

 arch/arm/mach-omap2/clock24xx.c     |    2
 arch/arm/mach-omap2/clock24xx.h     |  180 ++++++++++++------------------------
 drivers/dsp/dspgateway/dsp_common.c |    8 +
 3 files changed, 72 insertions(+), 118 deletions(-)

size:

   text    data     bss     dec     hex filename
2741640  141712   84116 2967468  2d47ac vmlinux.n800.orig
2741244  141712   84116 2967072  2d4620 vmlinux.n800.patched

[PATCH 1/3] 2430 clock: remove redundant iva2_1_fck

[Paul Walmsley]

[-- Attachment #1: fix-2430-clock-problems.patch --]
[-- Type: text/plain, Size: 5286 bytes --]

iva2_1_fck, the 2430 DSP functional clock, is the same clock (from the
software's perspective) as dsp_fck - same registers, same bits, same
rates.  So, get rid of iva2_1_fck, and switch iva2_1_ick to take its
clock from dsp_fck instead, per the TRM.  Mark dsp_fck as existing on
all 24xx chips, not just 2420.  These changes resolve the "clock:
Could not find parent clock iva2_1_fck in clksel array of clock
iva2_1_ick" message that appears on boot.

Signed-off-by: Paul Walmsley <paul@pwsan.com>

---
 arch/arm/mach-omap2/clock24xx.h |  103 ++++++++++++----------------------------
 1 file changed, 33 insertions(+), 70 deletions(-)

Index: linux-omap/arch/arm/mach-omap2/clock24xx.h
===================================================================
--- linux-omap.orig/arch/arm/mach-omap2/clock24xx.h	2007-12-03 15:02:58.000000000 -0700
+++ linux-omap/arch/arm/mach-omap2/clock24xx.h	2007-12-03 15:03:26.000000000 -0700
@@ -1000,71 +1000,9 @@
 /*
  * DSP (2430-IVA2.1) (2420-UMA+IVA1) clock domain
  * Clocks:
- *	2430: IVA2.1_FCLK, IVA2.1_ICLK
+ *	2430: IVA2.1_FCLK (really just DSP_FCLK), IVA2.1_ICLK
  *	2420: UMA_FCLK, UMA_ICLK, IVA_MPU, IVA_COP
- */
-/* XXX Okay, this is dumb.  iva2_1fck and dsp_fck are the same clock.
- * they should just be treated as such.
- */
-
-/* iva2_1_fck */
-static const struct clksel_rate iva2_1_fck_core_rates[] = {
-	{ .div = 1, .val = 1, .flags = RATE_IN_24XX | DEFAULT_RATE },
-	{ .div = 2, .val = 2, .flags = RATE_IN_24XX },
-	{ .div = 3, .val = 3, .flags = RATE_IN_24XX },
-	{ .div = 4, .val = 4, .flags = RATE_IN_24XX },
-	{ .div = 6, .val = 6, .flags = RATE_IN_242X },
-	{ .div = 8, .val = 8, .flags = RATE_IN_242X },
-	{ .div = 12, .val = 12, .flags = RATE_IN_242X },
-	{ .div = 0 },
-};
-
-static const struct clksel iva2_1_fck_clksel[] = {
-	{ .parent = &core_ck, .rates = iva2_1_fck_core_rates },
-	{ .parent = NULL }
-};
-
-static struct clk iva2_1_fck = {
-	.name		= "iva2_1_fck",
-	.parent		= &core_ck,
-	.flags		= CLOCK_IN_OMAP243X | DELAYED_APP | RATE_PROPAGATES |
-				CONFIG_PARTICIPANT,
-	.enable_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_FCLKEN),
-	.enable_bit	= OMAP24XX_CM_FCLKEN_DSP_EN_DSP_SHIFT,
-	.clksel_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_CLKSEL),
-	.clksel_mask	= OMAP24XX_CLKSEL_DSP_MASK,
-	.clksel		= iva2_1_fck_clksel,
-	.recalc		= &omap2_clksel_recalc,
-	.round_rate	= &omap2_clksel_round_rate,
-	.set_rate	= &omap2_clksel_set_rate
-};
-
-/* iva2_1_ick */
-static const struct clksel_rate iva2_1_ick_core_rates[] = {
-	{ .div = 1, .val = 1, .flags = RATE_IN_24XX | DEFAULT_RATE },
-	{ .div = 2, .val = 2, .flags = RATE_IN_24XX },
-	{ .div = 3, .val = 3, .flags = RATE_IN_243X },
-	{ .div = 0 },
-};
-
-static const struct clksel iva2_1_ick_clksel[] = {
-	{ .parent = &core_ck, .rates = iva2_1_ick_core_rates },
-	{ .parent = NULL }
-};
-
-static struct clk iva2_1_ick = {
-	.name		= "iva2_1_ick",
-	.parent		= &iva2_1_fck,
-	.flags		= CLOCK_IN_OMAP243X | DELAYED_APP | CONFIG_PARTICIPANT,
-	.clksel_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_CLKSEL),
-	.clksel_mask	= OMAP24XX_CLKSEL_DSP_IF_MASK,
-	.clksel		= iva2_1_ick_clksel,
-	.recalc		= &omap2_clksel_recalc,
-	.round_rate	= &omap2_clksel_round_rate,
-	.set_rate	= &omap2_clksel_set_rate
-};
-
-/*
+ *
  * Won't be too specific here. The core clock comes into this block
  * it is divided then tee'ed. One branch goes directly to xyz enable
  * controls. The other branch gets further divided by 2 then possibly
@@ -1089,7 +1027,7 @@
 static struct clk dsp_fck = {
 	.name		= "dsp_fck",
 	.parent		= &core_ck,
-	.flags		= CLOCK_IN_OMAP242X | DELAYED_APP |
+	.flags		= CLOCK_IN_OMAP242X | CLOCK_IN_OMAP243X | DELAYED_APP |
 				CONFIG_PARTICIPANT | RATE_PROPAGATES,
 	.enable_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_FCLKEN),
 	.enable_bit	= OMAP24XX_CM_FCLKEN_DSP_EN_DSP_SHIFT,
@@ -1125,6 +1063,32 @@
 	.recalc		= &omap2_clksel_recalc,
 };
 
+/* iva2_1_ick */
+static const struct clksel_rate iva2_1_ick_core_rates[] = {
+	{ .div = 1, .val = 1, .flags = RATE_IN_24XX | DEFAULT_RATE },
+	{ .div = 2, .val = 2, .flags = RATE_IN_24XX },
+	{ .div = 3, .val = 3, .flags = RATE_IN_243X },
+	{ .div = 0 },
+};
+
+static const struct clksel iva2_1_ick_clksel[] = {
+	{ .parent = &dsp_fck, .rates = iva2_1_ick_core_rates },
+	{ .parent = NULL }
+};
+
+/* 2430 only - dsp_ick is also controlled by EN_DSP on 2430 */
+static struct clk iva2_1_ick = {
+	.name		= "iva2_1_ick",
+	.parent		= &dsp_fck,
+	.flags		= CLOCK_IN_OMAP243X | DELAYED_APP | CONFIG_PARTICIPANT,
+	.clksel_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_CLKSEL),
+	.clksel_mask	= OMAP24XX_CLKSEL_DSP_IF_MASK,
+	.clksel		= iva2_1_ick_clksel,
+	.recalc		= &omap2_clksel_recalc,
+	.round_rate	= &omap2_clksel_round_rate,
+	.set_rate	= &omap2_clksel_set_rate
+};
+
 static const struct clksel_rate iva1_ifck_core_rates[] = {
 	{ .div = 1, .val = 1, .flags = RATE_IN_242X | DEFAULT_RATE },
 	{ .div = 2, .val = 2, .flags = RATE_IN_242X },
@@ -2563,12 +2527,11 @@
 	/* mpu domain clocks */
 	&mpu_ck,
 	/* dsp domain clocks */
-	&iva2_1_fck,		/* 2430 */
-	&iva2_1_ick,
-	&dsp_ick,		/* 2420 */
+	&dsp_ick,
 	&dsp_fck,
-	&iva1_ifck,
-	&iva1_mpu_int_ifck,
+	&iva2_1_ick,		/* 243x */
+	&iva1_ifck,		/* 242x */
+	&iva1_mpu_int_ifck,	/* 242x */
 	/* GFX domain clocks */
 	&gfx_3d_fck,
 	&gfx_2d_fck,

-- 

[PATCH 2/3] 24xx clock: clarify clock structure; correct parent clock

[Paul Walmsley]

[-- Attachment #1: dsp_clock_cleanup.patch --]
[-- Type: text/plain, Size: 6143 bytes --]

Separate the DSP I-clock rate clksel settings from dsp_ick, and use it
to create a new clock, dsp_irate_ick.  Use this new clock as the
parent of dsp_ick and iva2_1_ick.  dsp_ick now simply controls whether
the IPI and ISP clocks are enabled on 2420.

Also, correct the parent clock of the DSP interface clock to be dsp_fck, 
not core_ck, per the 2420 TRM figure 5-15 and 2430 TRM figure 4-16.

Fix the OMAP DSP code to use the correct clock name on 2430.  This 
avoids the "omapdsp: could not acquire dsp_ick handle" warning that appears 
at boot.

While we are here, we also remove some redundant clksel_rate arrays -
iva1_ifck_core_rates[] and iva2_1_ick_core_rates[].


Signed-off-by: Paul Walmsley <paul@pwsan.com>

---
 arch/arm/mach-omap2/clock24xx.h     |   77 ++++++++++++++----------------------
 drivers/dsp/dspgateway/dsp_common.c |    8 +++
 2 files changed, 38 insertions(+), 47 deletions(-)

Index: linux-omap/arch/arm/mach-omap2/clock24xx.h
===================================================================
--- linux-omap.orig/arch/arm/mach-omap2/clock24xx.h	2007-12-03 15:03:26.000000000 -0700
+++ linux-omap/arch/arm/mach-omap2/clock24xx.h	2007-12-03 15:04:28.000000000 -0700
@@ -1039,70 +1039,52 @@
 	.set_rate	= &omap2_clksel_set_rate
 };
 
-static const struct clksel_rate dsp_ick_core_rates[] = {
+/* DSP interface clock */
+static const struct clksel_rate dsp_irate_ick_rates[] = {
 	{ .div = 1, .val = 1, .flags = RATE_IN_24XX | DEFAULT_RATE },
 	{ .div = 2, .val = 2, .flags = RATE_IN_24XX },
 	{ .div = 3, .val = 3, .flags = RATE_IN_243X },
 	{ .div = 0 },
 };
 
-static const struct clksel dsp_ick_clksel[] = {
-	{ .parent = &core_ck, .rates = dsp_ick_core_rates },
+static const struct clksel dsp_irate_ick_clksel[] = {
+	{ .parent = &dsp_fck, .rates = dsp_irate_ick_rates },
 	{ .parent = NULL }
 };
 
-static struct clk dsp_ick = {
-	.name		= "dsp_ick",	 /* apparently ipi and isp */
-	.parent		= &core_ck,
-	.flags		= CLOCK_IN_OMAP242X | DELAYED_APP | CONFIG_PARTICIPANT,
-	.enable_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_ICLKEN),
-	.enable_bit	= OMAP2420_EN_DSP_IPI_SHIFT,		/* for ipi */
+/*
+ * This clock does not exist as such in the TRM, but is added to
+ * separate source selection from  XXX
+ */
+static struct clk dsp_irate_ick = {
+	.name		= "dsp_irate_ick",
+	.parent		= &dsp_fck,
+	.flags		= CLOCK_IN_OMAP242X | CLOCK_IN_OMAP243X | DELAYED_APP |
+				CONFIG_PARTICIPANT | PARENT_CONTROLS_CLOCK,
 	.clksel_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_CLKSEL),
 	.clksel_mask	= OMAP24XX_CLKSEL_DSP_IF_MASK,
-	.clksel		= dsp_ick_clksel,
+	.clksel		= dsp_irate_ick_clksel,
 	.recalc		= &omap2_clksel_recalc,
+	.round_rate	= &omap2_clksel_round_rate,
+	.set_rate	      = &omap2_clksel_set_rate
 };
 
-/* iva2_1_ick */
-static const struct clksel_rate iva2_1_ick_core_rates[] = {
-	{ .div = 1, .val = 1, .flags = RATE_IN_24XX | DEFAULT_RATE },
-	{ .div = 2, .val = 2, .flags = RATE_IN_24XX },
-	{ .div = 3, .val = 3, .flags = RATE_IN_243X },
-	{ .div = 0 },
-};
-
-static const struct clksel iva2_1_ick_clksel[] = {
-	{ .parent = &dsp_fck, .rates = iva2_1_ick_core_rates },
-	{ .parent = NULL }
+/* 2420 only */
+static struct clk dsp_ick = {
+	.name		= "dsp_ick",	 /* apparently ipi and isp */
+	.parent		= &dsp_irate_ick,
+	.flags		= CLOCK_IN_OMAP242X | DELAYED_APP | CONFIG_PARTICIPANT,
+	.enable_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_ICLKEN),
+	.enable_bit	= OMAP2420_EN_DSP_IPI_SHIFT,	      /* for ipi */
 };
 
-/* 2430 only - dsp_ick is also controlled by EN_DSP on 2430 */
+/* 2430 only - EN_DSP controls both dsp fclk and iclk on 2430 */
 static struct clk iva2_1_ick = {
 	.name		= "iva2_1_ick",
-	.parent		= &dsp_fck,
+	.parent		= &dsp_irate_ick,
 	.flags		= CLOCK_IN_OMAP243X | DELAYED_APP | CONFIG_PARTICIPANT,
-	.clksel_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_CLKSEL),
-	.clksel_mask	= OMAP24XX_CLKSEL_DSP_IF_MASK,
-	.clksel		= iva2_1_ick_clksel,
-	.recalc		= &omap2_clksel_recalc,
-	.round_rate	= &omap2_clksel_round_rate,
-	.set_rate	= &omap2_clksel_set_rate
-};
-
-static const struct clksel_rate iva1_ifck_core_rates[] = {
-	{ .div = 1, .val = 1, .flags = RATE_IN_242X | DEFAULT_RATE },
-	{ .div = 2, .val = 2, .flags = RATE_IN_242X },
-	{ .div = 3, .val = 3, .flags = RATE_IN_242X },
-	{ .div = 4, .val = 4, .flags = RATE_IN_242X },
-	{ .div = 6, .val = 6, .flags = RATE_IN_242X },
-	{ .div = 8, .val = 8, .flags = RATE_IN_242X },
-	{ .div = 12, .val = 12, .flags = RATE_IN_242X },
-	{ .div = 0 },
-};
-
-static const struct clksel iva1_ifck_clksel[] = {
-	{ .parent = &core_ck, .rates = iva1_ifck_core_rates },
-	{ .parent = NULL }
+	.enable_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_FCLKEN),
+	.enable_bit	= OMAP24XX_CM_FCLKEN_DSP_EN_DSP_SHIFT,
 };
 
 static struct clk iva1_ifck = {
@@ -1114,7 +1096,7 @@
 	.enable_bit	= OMAP2420_EN_IVA_COP_SHIFT,
 	.clksel_reg	= OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_CLKSEL),
 	.clksel_mask	= OMAP2420_CLKSEL_IVA_MASK,
-	.clksel		= iva1_ifck_clksel,
+	.clksel		= dsp_fck_clksel,
 	.recalc		= &omap2_clksel_recalc,
 	.round_rate	= &omap2_clksel_round_rate,
 	.set_rate	= &omap2_clksel_set_rate
@@ -2527,8 +2509,9 @@
 	/* mpu domain clocks */
 	&mpu_ck,
 	/* dsp domain clocks */
-	&dsp_ick,
 	&dsp_fck,
+	&dsp_irate_ick,
+	&dsp_ick,		/* 242x */
 	&iva2_1_ick,		/* 243x */
 	&iva1_ifck,		/* 242x */
 	&iva1_mpu_int_ifck,	/* 242x */
Index: linux-omap/drivers/dsp/dspgateway/dsp_common.c
===================================================================
--- linux-omap.orig/drivers/dsp/dspgateway/dsp_common.c	2007-12-03 15:02:58.000000000 -0700
+++ linux-omap/drivers/dsp/dspgateway/dsp_common.c	2007-12-03 15:03:29.000000000 -0700
@@ -347,7 +347,15 @@
 		return PTR_ERR(dsp_fck_handle);
 	}
 
+# if defined(CONFIG_ARCH_OMAP2420)
 	dsp_ick_handle = clk_get(NULL, "dsp_ick");
+# elif defined(CONFIG_ARCH_OMAP2430)
+	/*
+	 * 2430 has no separate switch for DSP ICLK, but this at least
+	 * involves the minimal change to the rest of the code.
+	 */
+	dsp_ick_handle = clk_get(NULL, "iva2_1_ick");
+# endif
 	if (IS_ERR(dsp_ick_handle)) {
 		printk(KERN_ERR "omapdsp: could not acquire dsp_ick handle.\n");
 		if (dsp_fck_handle != NULL)

-- 

[PATCH 3/3] 24xx clock: Fix 54MHz APLL readiness test

[Paul Walmsley]

[-- Attachment #1: fix_apll_54m_readiness_test.patch --]
[-- Type: text/plain, Size: 877 bytes --]

omap2_clk_fixed_enable() was checking the wrong bit to determine if
the 54MHz APLL was ready, causing a "Clock apll54_ck didn't enable in
100000 tries" warning message on 2430SDP:

Signed-off-by: Paul Walmsley <paul@pwsan.com>

---
 arch/arm/mach-omap2/clock24xx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-omap/arch/arm/mach-omap2/clock24xx.c
===================================================================
--- linux-omap.orig/arch/arm/mach-omap2/clock24xx.c	2007-11-21 00:26:06.000000000 -0700
+++ linux-omap/arch/arm/mach-omap2/clock24xx.c	2007-12-03 16:07:46.000000000 -0700
@@ -115,7 +115,7 @@
 	if (clk == &apll96_ck)
 		cval = OMAP24XX_ST_96M_APLL;
 	else if (clk == &apll54_ck)
-		cval = OMAP24XX_ST_54M_CLK;
+		cval = OMAP24XX_ST_54M_APLL;
 
 	omap2_wait_clock_ready(OMAP_CM_REGADDR(PLL_MOD, CM_IDLEST), cval,
 			    clk->name);

-- 

Re: [PATCH 0/3] 24xx clock fixes: DSP, 54MHz APLL

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [071203 15:30]:
> Hello,
> 
> these patches clean up the structure of the DSP clocks on the OMAP24xx
> platforms.  They remove redundant clocks and rate structures, and fix
> the DSP driver code to use the correct clock name on 2430.  DSP
> i-clock source selection is moved into a separate clock,
> dsp_irate_ick, to facilitate some of this consolidation.  
> 
> Also, in omap2_clk_fixed_enable(), it turns out that we were looking for
> the wrong IDLEST bit to test if the 54MHz APLL was locked - fix this.
> 
> This series resolves the boot-time clock warnings that appear on
> 2430SDP:
> 
>     clock: Could not find parent clock iva2_1_fck in clksel array of
>     clock iva2_1_ick
> 
>     omapdsp: could not acquire dsp_ick handle
> 
>     Clock apll54_ck didn't enable in 100000 tries
> 
> 
> Boot-tested on N800 and 2430SDP.  Comments welcome.

Pushing today.

Tony