Skip to contentSkip to conversation

24xx clock fixes: DSP, 54MHz APLL

PATCH In review Paul Walmsley 2 participants 5 messages Dec 3, 2007 – Dec 5, 2007

Subsystems (4)
Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP clock framework support
lore
Share & tools
lore

Paul Walmsley

author maintainer
#0 lore raw

cover 24xx clock fixes: DSP, 54MHz APLL

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
author maintainer
#1 lore raw

PATCH 1/3 2430 clock: remove redundant iva2_1_fck

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 <[email protected]>
---
 arch/arm/mach-omap2/clock24xx.h |  103 ++++++++++++----------------------------
 1 file changed, 33 insertions(+), 70 deletions(-)
arch/arm/mach-omap2/clock24xx.h +33 -70
Index: linux-omap/arch/arm/mach-omap2/clock24xx.h
===================================================================
@@ -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,
author maintainer
#2 lore raw

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

In reply to Paul Walmsley

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 <[email protected]>
---
 arch/arm/mach-omap2/clock24xx.h     |   77 ++++++++++++++----------------------
 drivers/dsp/dspgateway/dsp_common.c |    8 +++
 2 files changed, 38 insertions(+), 47 deletions(-)
arch/arm/mach-omap2/clock24xx.h +30 -47
Index: linux-omap/arch/arm/mach-omap2/clock24xx.h
===================================================================
@@ -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 */
drivers/dsp/dspgateway/dsp_common.c +8 -0
Index: linux-omap/drivers/dsp/dspgateway/dsp_common.c
===================================================================
@@ -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)
author maintainer
#3 lore raw

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

In reply to Paul Walmsley

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 <[email protected]>
---
 arch/arm/mach-omap2/clock24xx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm/mach-omap2/clock24xx.c +1 -1
Index: linux-omap/arch/arm/mach-omap2/clock24xx.c
===================================================================
@@ -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);
maintainer
#4 lore raw
In reply to Paul Walmsley
Quoted 17 lines · 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

Quoted 6 lines

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