Second batch of Tegra clock updates for 2.6.39

PATCH In review Colin Cross 3 participants 22 messages Feb 22, 2011 – Feb 23, 2011

Subsystems (3)

Arm (32-bit) Arm sub-architectures Tegra architecture support

lore

Share & tools

lore

Colin Cross

author

2011-02-22 02:39 UTC #0 lore raw

cover Second batch of Tegra clock updates for 2.6.39

This patch series contains two small patches out of the previous series that needed fixes, as well as some new ones to disable unused clocks during boot and related bug fixes.

Patch 4 converts the Harmony and Trimslice boards to use the new init_early machine handler. Any boards that go in after this patch will need to be updated to match, and this patch will need to be updated if any boards go in before this patch.

Tested-bys on Harmony and Trimslice would be appreciated.

Colin Cross

author

2011-02-22 02:39 UTC #1 lore raw

PATCH 1/7 ARM: tegra: clock: Refcount periph clock enables

Some peripheral clocks share enable bits. Refcount the enables so that calling clk_disable on one clock will not turn off another clock.

Signed-off-by Colin Cross <[email protected]>

---
 arch/arm/mach-tegra/tegra2_clocks.c |   37 +++++++++++++++++++++++++++++-----
 1 files changed, 31 insertions(+), 6 deletions(-)

arch/arm/mach-tegra/tegra2_clocks.c +31 -6

index 59e77ba..2ca8b74 100644
@@ -159,6 +159,12 @@ static void __iomem *reg_pmc_base = IO_ADDRESS(TEGRA_PMC_BASE);
  */
 static DEFINE_SPINLOCK(clock_register_lock);
 
+/*
+ * Some peripheral clocks share an enable bit, so refcount the enable bits
+ * in registers CLK_ENABLE_L, CLK_ENABLE_H, and CLK_ENABLE_U
+ */
+static int tegra_periph_clk_enable_refcount[3 * 32];
+
 #define clk_writel(value, reg) \
 	__raw_writel(value, (u32)reg_clk_base + (reg))
 #define clk_readl(reg) \

@@ -952,8 +958,17 @@ static void tegra2_periph_clk_init(struct clk *c)
 static int tegra2_periph_clk_enable(struct clk *c)
 {
 	u32 val;
+	unsigned long flags;
+	int refcount;
 	pr_debug("%s on clock %s\n", __func__, c->name);
 
+	spin_lock_irqsave(&clock_register_lock, flags);
+
+	refcount = tegra_periph_clk_enable_refcount[c->u.periph.clk_num]++;
+
+	if (refcount > 1)
+		goto out;
+
 	clk_writel(PERIPH_CLK_TO_ENB_BIT(c),
 		CLK_OUT_ENB_SET + PERIPH_CLK_TO_ENB_SET_REG(c));
 	if (!(c->flags & PERIPH_NO_RESET) && !(c->flags & PERIPH_MANUAL_RESET))

@@ -966,15 +981,29 @@ static int tegra2_periph_clk_enable(struct clk *c)
 		val |= 0x3 << 24;
 		clk_writel(val, c->reg);
 	}
+
+out:
+	spin_unlock_irqrestore(&clock_register_lock, flags);
+
 	return 0;
 }
 
 static void tegra2_periph_clk_disable(struct clk *c)
 {
+	unsigned long flags;
+
 	pr_debug("%s on clock %s\n", __func__, c->name);
 
-	clk_writel(PERIPH_CLK_TO_ENB_BIT(c),
-		CLK_OUT_ENB_CLR + PERIPH_CLK_TO_ENB_SET_REG(c));
+	spin_lock_irqsave(&clock_register_lock, flags);
+
+	if (c->refcnt)
+		tegra_periph_clk_enable_refcount[c->u.periph.clk_num]--;
+
+	if (tegra_periph_clk_enable_refcount[c->u.periph.clk_num] == 0)
+		clk_writel(PERIPH_CLK_TO_ENB_BIT(c),
+			CLK_OUT_ENB_CLR + PERIPH_CLK_TO_ENB_SET_REG(c));
+
+	spin_unlock_irqrestore(&clock_register_lock, flags);
 }
 
 static void tegra2_periph_clk_reset(struct clk *c, bool assert)

@@ -2076,7 +2105,6 @@ struct clk tegra_list_clks[] = {
 	PERIPH_CLK("timer",	"timer",		NULL,	5,	0,	26000000,  mux_clk_m,			0),
 	PERIPH_CLK("i2s1",	"i2s.0",		NULL,	11,	0x100,	26000000,  mux_pllaout0_audio2x_pllp_clkm,	MUX | DIV_U71),
 	PERIPH_CLK("i2s2",	"i2s.1",		NULL,	18,	0x104,	26000000,  mux_pllaout0_audio2x_pllp_clkm,	MUX | DIV_U71),
-	/* FIXME: spdif has 2 clocks but 1 enable */
 	PERIPH_CLK("spdif_out",	"spdif_out",		NULL,	10,	0x108,	100000000, mux_pllaout0_audio2x_pllp_clkm,	MUX | DIV_U71),
 	PERIPH_CLK("spdif_in",	"spdif_in",		NULL,	10,	0x10c,	100000000, mux_pllp_pllc_pllm,		MUX | DIV_U71),
 	PERIPH_CLK("pwm",	"pwm",			NULL,	17,	0x110,	432000000, mux_pllp_pllc_audio_clkm_clk32,	MUX | DIV_U71),

@@ -2089,7 +2117,6 @@ struct clk tegra_list_clks[] = {
 	PERIPH_CLK("sbc4",	"spi_tegra.3",		NULL,	68,	0x1b4,	160000000, mux_pllp_pllc_pllm_clkm,	MUX | DIV_U71),
 	PERIPH_CLK("ide",	"ide",			NULL,	25,	0x144,	100000000, mux_pllp_pllc_pllm_clkm,	MUX | DIV_U71), /* requires min voltage */
 	PERIPH_CLK("ndflash",	"tegra_nand",		NULL,	13,	0x160,	164000000, mux_pllp_pllc_pllm_clkm,	MUX | DIV_U71), /* scales with voltage */
-	/* FIXME: vfir shares an enable with uartb */
 	PERIPH_CLK("vfir",	"vfir",			NULL,	7,	0x168,	72000000,  mux_pllp_pllc_pllm_clkm,	MUX | DIV_U71),
 	PERIPH_CLK("sdmmc1",	"sdhci-tegra.0",	NULL,	14,	0x150,	52000000,  mux_pllp_pllc_pllm_clkm,	MUX | DIV_U71), /* scales with voltage */
 	PERIPH_CLK("sdmmc2",	"sdhci-tegra.1",	NULL,	9,	0x154,	52000000,  mux_pllp_pllc_pllm_clkm,	MUX | DIV_U71), /* scales with voltage */

@@ -2120,13 +2147,11 @@ struct clk tegra_list_clks[] = {
 	PERIPH_CLK("uarte",	"uart.4",		NULL,	66,	0x1c4,	600000000, mux_pllp_pllc_pllm_clkm,	MUX),
 	PERIPH_CLK("3d",	"3d",			NULL,	24,	0x158,	300000000, mux_pllm_pllc_pllp_plla,	MUX | DIV_U71 | PERIPH_MANUAL_RESET), /* scales with voltage and process_id */
 	PERIPH_CLK("2d",	"2d",			NULL,	21,	0x15c,	300000000, mux_pllm_pllc_pllp_plla,	MUX | DIV_U71), /* scales with voltage and process_id */
-	/* FIXME: vi and vi_sensor share an enable */
 	PERIPH_CLK("vi",	"tegra_camera",		"vi",	20,	0x148,	150000000, mux_pllm_pllc_pllp_plla,	MUX | DIV_U71), /* scales with voltage and process_id */
 	PERIPH_CLK("vi_sensor",	"tegra_camera",		"vi_sensor",	20,	0x1a8,	150000000, mux_pllm_pllc_pllp_plla,	MUX | DIV_U71 | PERIPH_NO_RESET), /* scales with voltage and process_id */
 	PERIPH_CLK("epp",	"epp",			NULL,	19,	0x16c,	300000000, mux_pllm_pllc_pllp_plla,	MUX | DIV_U71), /* scales with voltage and process_id */
 	PERIPH_CLK("mpe",	"mpe",			NULL,	60,	0x170,	250000000, mux_pllm_pllc_pllp_plla,	MUX | DIV_U71), /* scales with voltage and process_id */
 	PERIPH_CLK("host1x",	"host1x",		NULL,	28,	0x180,	166000000, mux_pllm_pllc_pllp_plla,	MUX | DIV_U71), /* scales with voltage and process_id */
-	/* FIXME: cve and tvo share an enable	*/
 	PERIPH_CLK("cve",	"cve",			NULL,	49,	0x140,	250000000, mux_pllp_plld_pllc_clkm,	MUX | DIV_U71), /* requires min voltage */
 	PERIPH_CLK("tvo",	"tvo",			NULL,	49,	0x188,	250000000, mux_pllp_plld_pllc_clkm,	MUX | DIV_U71), /* requires min voltage */
 	PERIPH_CLK("hdmi",	"hdmi",			NULL,	51,	0x18c,	600000000, mux_pllp_plld_pllc_clkm,	MUX | DIV_U71), /* requires min voltage */

Signature · 1 line

1.7.3.1

Olof Johansson

Acked-by on v1 1/7

2011-02-22 05:25 UTC #2 lore raw

Re: [PATCH 1/7] ARM: tegra: clock: Refcount periph clock enables

Quoted 5 lines · Colin Cross

Some peripheral clocks share enable bits. ?Refcount the enables so that calling clk_disable on one clock will not turn off another clock.
Signed-off-by Colin Cross <[email protected]>

Acked-by Olof Johansson <[email protected]>

Colin Cross

author

2011-02-22 02:39 UTC #3 lore raw

PATCH 2/7 ARM: tegra: clock: Round rate before setting rate

In reply to Colin Cross

Call the clock's round_rate op, if it exists, before calling the set_rate op. This will help later when dvfs is added, dvfs needs to know what the final rate will be before the frequency changes.

Also requires fixes to the round rate functions to ensure calling round rate and then set rate will not cause the frequency to be rounded down twice. When picking clock divider values, the clock framework picks the closest frequency that is lower than the requested frequency. If the new frequency calculated from the divider value is rounded down, and then passed to set_rate, it will get rounded down again, possibly resulting in a frequency two steps lower than the original requested frequency.

Fix the problem by rounding up when calculating the frequency coming out of a clock divider, so if that frequency is requested again, the same divider value will be picked.

Signed-off-by Colin Cross <[email protected]>

---
 arch/arm/mach-tegra/clock.c         |   12 ++++++++++++
 arch/arm/mach-tegra/tegra2_clocks.c |    8 ++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

arch/arm/mach-tegra/clock.c +12 -0

index 165aa9c..e028320 100644
@@ -86,6 +86,7 @@ static unsigned long clk_predict_rate_from_parent(struct clk *c, struct clk *p)
 
 	if (c->mul != 0 && c->div != 0) {
 		rate *= c->mul;
+		rate += c->div - 1; /* round up */
 		do_div(rate, c->div);
 	}

@@ -240,12 +241,23 @@ EXPORT_SYMBOL(clk_get_parent);
 
 int clk_set_rate_locked(struct clk *c, unsigned long rate)
 {
+	long new_rate;
+
 	if (!c->ops || !c->ops->set_rate)
 		return -ENOSYS;
 
 	if (rate > c->max_rate)
 		rate = c->max_rate;
 
+	if (c->ops && c->ops->round_rate) {
+		new_rate = c->ops->round_rate(c, rate);
+
+		if (new_rate < 0)
+			return new_rate;
+
+		rate = new_rate;
+	}
+
 	return c->ops->set_rate(c, rate);
 }

arch/arm/mach-tegra/tegra2_clocks.c +4 -4

index 2ca8b74..73e112f 100644
@@ -898,9 +898,9 @@ static long tegra2_pll_div_clk_round_rate(struct clk *c, unsigned long rate)
 		divider = clk_div71_get_divider(parent_rate, rate);
 		if (divider < 0)
 			return divider;
-		return parent_rate * 2 / (divider + 2);
+		return DIV_ROUND_UP(parent_rate * 2, divider + 2);
 	} else if (c->flags & DIV_2) {
-		return parent_rate / 2;
+		return DIV_ROUND_UP(parent_rate, 2);
 	}
 	return -EINVAL;
 }

@@ -1092,12 +1092,12 @@ static long tegra2_periph_clk_round_rate(struct clk *c,
 		if (divider < 0)
 			return divider;
 
-		return parent_rate * 2 / (divider + 2);
+		return DIV_ROUND_UP(parent_rate * 2, divider + 2);
 	} else if (c->flags & DIV_U16) {
 		divider = clk_div16_get_divider(parent_rate, rate);
 		if (divider < 0)
 			return divider;
-		return parent_rate / (divider + 1);
+		return DIV_ROUND_UP(parent_rate, divider + 1);
 	}
 	return -EINVAL;
 }

Signature · 1 line

1.7.3.1

Olof Johansson

Acked-by on v1 2/7

2011-02-22 05:26 UTC #4 lore raw

Re: [PATCH 2/7] ARM: tegra: clock: Round rate before setting rate

Quoted 15 lines · Colin Cross

Call the clock's round_rate op, if it exists, before calling the set_rate op. ?This will help later when dvfs is added, dvfs needs to know what the final rate will be before the frequency changes.
Also requires fixes to the round rate functions to ensure calling round rate and then set rate will not cause the frequency to be rounded down twice. ?When picking clock divider values, the clock framework picks the closest frequency that is lower than the requested frequency. ?If the new frequency calculated from the divider value is rounded down, and then passed to set_rate, it will get rounded down again, possibly resulting in a frequency two steps lower than the original requested frequency.

Quoted 5 lines

Fix the problem by rounding up when calculating the frequency coming out of a clock divider, so if that frequency is requested again, the same divider value will be picked.
Signed-off-by Colin Cross <[email protected]>

Acked-by Olof Johansson <[email protected]>

Colin Cross

author

2011-02-22 02:39 UTC #5 lore raw

PATCH 3/7 ARM: tegra: clock: Check for clk_num == 0

In reply to Colin Cross

Peripheral clocks that have no clock enable bit in the enable registers have their clk_num set to 0. Bit 0 in the clock enable registers is the CPU clock. Prevent disables on these peripheral clocks from accidentally disabling the CPU clock.

Signed-off-by Colin Cross <[email protected]>

---
 arch/arm/mach-tegra/tegra2_clocks.c |   25 +++++++++++++++++++++++++
 1 files changed, 25 insertions(+), 0 deletions(-)

arch/arm/mach-tegra/tegra2_clocks.c +25 -0

index 73e112f..3015a2c 100644
@@ -946,9 +946,14 @@ static void tegra2_periph_clk_init(struct clk *c)
 	}
 
 	c->state = ON;
+
+	if (!c->u.periph.clk_num)
+		return;
+
 	if (!(clk_readl(CLK_OUT_ENB + PERIPH_CLK_TO_ENB_REG(c)) &
 			PERIPH_CLK_TO_ENB_BIT(c)))
 		c->state = OFF;
+
 	if (!(c->flags & PERIPH_NO_RESET))
 		if (clk_readl(RST_DEVICES + PERIPH_CLK_TO_ENB_REG(c)) &
 				PERIPH_CLK_TO_ENB_BIT(c))

@@ -962,6 +967,9 @@ static int tegra2_periph_clk_enable(struct clk *c)
 	int refcount;
 	pr_debug("%s on clock %s\n", __func__, c->name);
 
+	if (!c->u.periph.clk_num)
+		return 0;
+
 	spin_lock_irqsave(&clock_register_lock, flags);
 
 	refcount = tegra_periph_clk_enable_refcount[c->u.periph.clk_num]++;

@@ -994,6 +1002,9 @@ static void tegra2_periph_clk_disable(struct clk *c)
 
 	pr_debug("%s on clock %s\n", __func__, c->name);
 
+	if (!c->u.periph.clk_num)
+		return;
+
 	spin_lock_irqsave(&clock_register_lock, flags);
 
 	if (c->refcnt)

@@ -1012,6 +1023,9 @@ static void tegra2_periph_clk_reset(struct clk *c, bool assert)
 
 	pr_debug("%s %s on clock %s\n", __func__,
 		 assert ? "assert" : "deassert", c->name);
+
+	BUG_ON(!c->u.periph.clk_num);
+
 	if (!(c->flags & PERIPH_NO_RESET))
 		clk_writel(PERIPH_CLK_TO_ENB_BIT(c),
 			   base + PERIPH_CLK_TO_ENB_SET_REG(c));

@@ -1182,6 +1196,10 @@ static void tegra2_clk_double_init(struct clk *c)
 	c->mul = 2;
 	c->div = 1;
 	c->state = ON;
+
+	if (!c->u.periph.clk_num)
+		return;
+
 	if (!(clk_readl(CLK_OUT_ENB + PERIPH_CLK_TO_ENB_REG(c)) &
 			PERIPH_CLK_TO_ENB_BIT(c)))
 		c->state = OFF;

@@ -1269,6 +1287,9 @@ static void tegra2_cdev_clk_init(struct clk *c)
 	/* We could un-tristate the cdev1 or cdev2 pingroup here; this is
 	 * currently done in the pinmux code. */
 	c->state = ON;
+
+	BUG_ON(!c->u.periph.clk_num);
+
 	if (!(clk_readl(CLK_OUT_ENB + PERIPH_CLK_TO_ENB_REG(c)) &
 			PERIPH_CLK_TO_ENB_BIT(c)))
 		c->state = OFF;

@@ -1276,6 +1297,8 @@ static void tegra2_cdev_clk_init(struct clk *c)
 
 static int tegra2_cdev_clk_enable(struct clk *c)
 {
+	BUG_ON(!c->u.periph.clk_num);
+
 	clk_writel(PERIPH_CLK_TO_ENB_BIT(c),
 		CLK_OUT_ENB_SET + PERIPH_CLK_TO_ENB_SET_REG(c));
 	return 0;

@@ -1283,6 +1306,8 @@ static int tegra2_cdev_clk_enable(struct clk *c)
 
 static void tegra2_cdev_clk_disable(struct clk *c)
 {
+	BUG_ON(!c->u.periph.clk_num);
+
 	clk_writel(PERIPH_CLK_TO_ENB_BIT(c),
 		CLK_OUT_ENB_CLR + PERIPH_CLK_TO_ENB_SET_REG(c));
 }

Signature · 1 line

1.7.3.1

Olof Johansson

Acked-by on v1 3/7

2011-02-22 05:29 UTC #6 lore raw

Re: [PATCH 3/7] ARM: tegra: clock: Check for clk_num == 0

Quoted 7 lines · Colin Cross

Peripheral clocks that have no clock enable bit in the enable registers have their clk_num set to 0. ?Bit 0 in the clock enable registers is the CPU clock. Prevent disables on these peripheral clocks from accidentally disabling the CPU clock.
Signed-off-by Colin Cross <[email protected]>

Acked-by Olof Johansson <[email protected]>

May I suggest a slightly more descriptive patch subject though? "ARM:
tegra: clock: prevent accidental disables of cpu clock" or similar, maybe.

-Olof

Colin Cross

author

2011-02-22 02:39 UTC #7 lore raw

PATCH 4/7 ARM: tegra: Move tegra_common_init to tegra_init_early

In reply to Colin Cross

Move tegra_common_init to tegra_init_early, and set it as the init_early entry in the machine struct. Initializes the clocks earlier so that timers can enable their clocks.

Also reorders the members in the Harmony and Trimslice boards' machine structs to match the order they are called in.

Signed-off-by Colin Cross <[email protected]>

---
 arch/arm/mach-tegra/board-harmony.c   |    7 +++----
 arch/arm/mach-tegra/board-trimslice.c |    7 +++----
 arch/arm/mach-tegra/board.h           |    2 +-
 arch/arm/mach-tegra/common.c          |    2 +-
 4 files changed, 8 insertions(+), 10 deletions(-)

arch/arm/mach-tegra/board-harmony.c +3 -4

index b9dbdb1..368ddff 100644
@@ -104,8 +104,6 @@ static __initdata struct tegra_clk_init_table harmony_clk_init_table[] = {
 
 static void __init tegra_harmony_init(void)
 {
-	tegra_common_init();
-
 	tegra_clk_init_from_table(harmony_clk_init_table);
 
 	harmony_pinmux_init();

@@ -116,8 +114,9 @@ static void __init tegra_harmony_init(void)
 MACHINE_START(HARMONY, "harmony")
 	.boot_params  = 0x00000100,
 	.fixup		= tegra_harmony_fixup,
-	.init_irq       = tegra_init_irq,
-	.init_machine   = tegra_harmony_init,
 	.map_io         = tegra_map_common_io,
+	.init_early	= tegra_init_early,
+	.init_irq       = tegra_init_irq,
 	.timer          = &tegra_timer,
+	.init_machine   = tegra_harmony_init,
 MACHINE_END

arch/arm/mach-tegra/board-trimslice.c +3 -4

index ef233b2..0f3081a 100644
@@ -85,8 +85,6 @@ subsys_initcall(tegra_trimslice_pci_init);
 
 static void __init tegra_trimslice_init(void)
 {
-	tegra_common_init();
-
 	tegra_clk_init_from_table(trimslice_clk_init_table);
 
 	trimslice_pinmux_init();

@@ -97,8 +95,9 @@ static void __init tegra_trimslice_init(void)
 MACHINE_START(TRIMSLICE, "trimslice")
 	.boot_params	= 0x00000100,
 	.fixup		= tegra_trimslice_fixup,
-	.init_irq       = tegra_init_irq,
-	.init_machine   = tegra_trimslice_init,
 	.map_io         = tegra_map_common_io,
+	.init_early	= tegra_init_early,
+	.init_irq       = tegra_init_irq,
 	.timer          = &tegra_timer,
+	.init_machine   = tegra_trimslice_init,
 MACHINE_END

arch/arm/mach-tegra/board.h +1 -1

index b3f9c94..1d14df7 100644
@@ -25,7 +25,7 @@
 
 void tegra_assert_system_reset(char mode, const char *cmd);
 
-void __init tegra_common_init(void);
+void __init tegra_init_early(void);
 void __init tegra_map_common_io(void);
 void __init tegra_init_irq(void);
 void __init tegra_init_clock(void);

arch/arm/mach-tegra/common.c +1 -1

index 54826b8..14fa533 100644
@@ -72,7 +72,7 @@ void __init tegra_init_cache(void)
 
 }
 
-void __init tegra_common_init(void)
+void __init tegra_init_early(void)
 {
 	tegra_init_fuse();
 	tegra_init_clock();

Signature · 1 line

1.7.3.1

Olof Johansson

Acked-by on v1 4/7

2011-02-22 05:30 UTC #8 lore raw

Re: [PATCH 4/7] ARM: tegra: Move tegra_common_init to tegra_init_early

Quoted 10 lines · Colin Cross

Move tegra_common_init to tegra_init_early, and set it as the init_early entry in the machine struct. Initializes the clocks earlier so that timers can enable their clocks.
Also reorders the members in the Harmony and Trimslice boards' machine structs to match the order they are called in.
Signed-off-by Colin Cross <[email protected]>

Acked-by Olof Johansson <[email protected]>

Please push this ASAP so I can rebase my series, even if some of the others might need respin. :)

-Olof

Colin Cross

author

2011-02-22 02:39 UTC #9 lore raw

PATCH 5/7 ARM: tegra: timer: Enable timer and rtc clocks

In reply to Colin Cross

Enable the timer and rtc clocks to prevent them being turned off by the bootloader clock disabling code.

Signed-off-by Colin Cross <[email protected]>

---
 arch/arm/mach-tegra/timer.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

arch/arm/mach-tegra/timer.c +12 -2

index ffa6a68..31b4f56 100644
@@ -18,6 +18,7 @@
  */
 
 #include <linux/init.h>
+#include <linux/err.h>
 #include <linux/sched.h>
 #include <linux/time.h>
 #include <linux/interrupt.h>

@@ -193,9 +194,20 @@ static struct irqaction tegra_timer_irq = {
 
 static void __init tegra_init_timer(void)
 {
+	struct clk *clk;
 	unsigned long rate = clk_measure_input_freq();
 	int ret;
 
+	clk = clk_get_sys("timer", NULL);
+	BUG_ON(IS_ERR(clk));
+	clk_enable(clk);
+	clk_put(clk);
+
+	clk = clk_get_sys("rtc-tegra", NULL);
+	BUG_ON(IS_ERR(clk));
+	clk_enable(clk);
+	clk_put(clk);
+
 #ifdef CONFIG_HAVE_ARM_TWD
 	twd_base = IO_ADDRESS(TEGRA_ARM_PERIF_BASE + 0x600);
 #endif

@@ -239,8 +251,6 @@ static void __init tegra_init_timer(void)
 	tegra_clockevent.cpumask = cpu_all_mask;
 	tegra_clockevent.irq = tegra_timer_irq.irq;
 	clockevents_register_device(&tegra_clockevent);
-
-	return;
 }
 
 struct sys_timer tegra_timer = {

Signature · 1 line

1.7.3.1

Olof Johansson

2011-02-22 05:32 UTC #10 lore raw

Re: [PATCH 5/7] ARM: tegra: timer: Enable timer and rtc clocks

Hi,

Quoted 27 lines · Colin Cross

Enable the timer and rtc clocks to prevent them being turned off by the bootloader clock disabling code.
Signed-off-by Colin Cross <[email protected]>
---
?arch/arm/mach-tegra/timer.c | ? 14 ++++++++++++-- ?1 files changed, 12 insertions(+), 2 deletions(-)
arch/arm/mach-tegra/timer.c +0 -0
index ffa6a68..31b4f56 100644
@@ -18,6 +18,7 @@
?*/
?#include <linux/init.h>
+#include <linux/err.h>
?#include <linux/sched.h>
?#include <linux/time.h>
?#include <linux/interrupt.h>
arch/arm/mach-tegra/timer.c +1 -0
@@ -193,9 +194,20 @@ static struct irqaction tegra_timer_irq = {
 
 ?static void __init tegra_init_timer(void)
 ?{
+ ? ? ? struct clk *clk;
 ? ? ? ?unsigned long rate = clk_measure_input_freq();
 ? ? ? ?int ret;

arch/arm/mach-tegra/timer.c static struct irqaction tegra_timer_irq = { · line 209

Quoted 10 lines

arch/arm/mach-tegra/timer.c +9 -0

 
+ ? ? ? clk = clk_get_sys("timer", NULL);
+ ? ? ? BUG_ON(IS_ERR(clk));
+ ? ? ? clk_enable(clk);
+ ? ? ? clk_put(clk);
+
+ ? ? ? clk = clk_get_sys("rtc-tegra", NULL);
+ ? ? ? BUG_ON(IS_ERR(clk));
+ ? ? ? clk_enable(clk);
+ ? ? ? clk_put(clk);

Why initialize the rtc clock here? Not all boards use it and instead use the rtc on the pmic. Seems wasteful to clock it unless the driver for it is probed and configured.

-Olof

Colin Cross

author

2011-02-22 08:00 UTC #11 lore raw

Re: [PATCH 5/7] ARM: tegra: timer: Enable timer and rtc clocks

Quoted 32 lines · Olof Johansson

Hi,
Quoted 28 lines · Colin Cross
Enable the timer and rtc clocks to prevent them being turned off by the bootloader clock disabling code.
Signed-off-by Colin Cross <[email protected]>
---
?arch/arm/mach-tegra/timer.c | ? 14 ++++++++++++-- ?1 files changed, 12 insertions(+), 2 deletions(-)
arch/arm/mach-tegra/timer.c +0 -0
index ffa6a68..31b4f56 100644
@@ -18,6 +18,7 @@
?*/
?#include <linux/init.h>
+#include <linux/err.h>
?#include <linux/sched.h>
?#include <linux/time.h>
?#include <linux/interrupt.h>
arch/arm/mach-tegra/timer.c +1 -0
@@ -193,9 +194,20 @@ static struct irqaction tegra_timer_irq = {
 
 ?static void __init tegra_init_timer(void)
 ?{
+ ? ? ? struct clk *clk;
 ? ? ? ?unsigned long rate = clk_measure_input_freq();
 ? ? ? ?int ret;
 

Quoted 13 lines

arch/arm/mach-tegra/timer.c static struct irqaction tegra_timer_irq = { · line 209
Quoted 9 lines
arch/arm/mach-tegra/timer.c +9 -0
+ ? ? ? clk = clk_get_sys("timer", NULL);
+ ? ? ? BUG_ON(IS_ERR(clk));
+ ? ? ? clk_enable(clk);
+ ? ? ? clk_put(clk);
+
+ ? ? ? clk = clk_get_sys("rtc-tegra", NULL);
+ ? ? ? BUG_ON(IS_ERR(clk));
+ ? ? ? clk_enable(clk);
+ ? ? ? clk_put(clk);
Why initialize the rtc clock here? Not all boards use it and instead use the rtc on the pmic. Seems wasteful to clock it unless the driver for it is probed and configured.

timekeeping_suspend uses read_persistent_clock and not the RTC driver to track time in suspend. The Tegra implementation of read_persistent_clock reads from the internal RTC registers. Its a 32 kHz clock, so it's power usage is negligible, and it is required on all Tegra boards.

Olof Johansson

Acked-by

2011-02-22 15:28 UTC #12 lore raw

Re: [PATCH 5/7] ARM: tegra: timer: Enable timer and rtc clocks

Quoted 47 lines · Colin Cross

Quoted 32 lines · Olof Johansson
Hi,
Quoted 28 lines · Colin Cross
Enable the timer and rtc clocks to prevent them being turned off by the bootloader clock disabling code.
Signed-off-by Colin Cross <[email protected]>
---
?arch/arm/mach-tegra/timer.c | ? 14 ++++++++++++-- ?1 files changed, 12 insertions(+), 2 deletions(-)
arch/arm/mach-tegra/timer.c +0 -0
index ffa6a68..31b4f56 100644
@@ -18,6 +18,7 @@
?*/
?#include <linux/init.h>
+#include <linux/err.h>
?#include <linux/sched.h>
?#include <linux/time.h>
?#include <linux/interrupt.h>
arch/arm/mach-tegra/timer.c +1 -0
@@ -193,9 +194,20 @@ static struct irqaction tegra_timer_irq = {
 
 ?static void __init tegra_init_timer(void)
 ?{
+ ? ? ? struct clk *clk;
 ? ? ? ?unsigned long rate = clk_measure_input_freq();
 ? ? ? ?int ret;
 
Quoted 13 lines
arch/arm/mach-tegra/timer.c static struct irqaction tegra_timer_irq = { · line 209
Quoted 9 lines
arch/arm/mach-tegra/timer.c +9 -0
+ ? ? ? clk = clk_get_sys("timer", NULL);
+ ? ? ? BUG_ON(IS_ERR(clk));
+ ? ? ? clk_enable(clk);
+ ? ? ? clk_put(clk);
+
+ ? ? ? clk = clk_get_sys("rtc-tegra", NULL);
+ ? ? ? BUG_ON(IS_ERR(clk));
+ ? ? ? clk_enable(clk);
+ ? ? ? clk_put(clk);
Why initialize the rtc clock here? Not all boards use it and instead use the rtc on the pmic. Seems wasteful to clock it unless the driver for it is probed and configured.

Quoted 5 lines

timekeeping_suspend uses read_persistent_clock and not the RTC driver to track time in suspend. ?The Tegra implementation of read_persistent_clock reads from the internal RTC registers. ?Its a 32 kHz clock, so it's power usage is negligible, and it is required on all Tegra boards.

Ah, right. A short comment to that effect near the clock enablement would be useful, but other than that:

Acked-by Olof Johansson <[email protected]>

-Olof

Russell King - ARM Linux

2011-02-22 20:56 UTC #13 lore raw

Re: [PATCH 5/7] ARM: tegra: timer: Enable timer and rtc clocks

In reply to Colin Cross

arch/arm/mach-tegra/timer.c static struct irqaction tegra_timer_irq = { · line 209

Quoted 13 lines · Colin Cross

arch/arm/mach-tegra/timer.c +10 -0

+	struct clk *clk;
 	unsigned long rate = clk_measure_input_freq();
 	int ret;
 
+	clk = clk_get_sys("timer", NULL);
+	BUG_ON(IS_ERR(clk));
+	clk_enable(clk);
+	clk_put(clk);
+
+	clk = clk_get_sys("rtc-tegra", NULL);
+	BUG_ON(IS_ERR(clk));
+	clk_enable(clk);
+	clk_put(clk);

It's probably better to leave them get'd if you're continuing to use them and just forget the reference.

Colin Cross

author

2011-02-23 17:39 UTC #14 lore raw

Re: [PATCH 5/7] ARM: tegra: timer: Enable timer and rtc clocks

Quoted 15 lines · Russell King - ARM Linux

Quoted 4 lines · Colin Cross
+ ? ? struct clk *clk;
? ? ? unsigned long rate = clk_measure_input_freq(); ? ? ? int ret;
Quoted 9 lines
+ ? ? clk = clk_get_sys("timer", NULL);
+ ? ? BUG_ON(IS_ERR(clk));
+ ? ? clk_enable(clk);
+ ? ? clk_put(clk);
+
+ ? ? clk = clk_get_sys("rtc-tegra", NULL);
+ ? ? BUG_ON(IS_ERR(clk));
+ ? ? clk_enable(clk);
+ ? ? clk_put(clk);

Quoted 3 lines

It's probably better to leave them get'd if you're continuing to use them and just forget the reference.

Good point, fixed.

Colin Cross

author

2011-02-22 02:39 UTC #15 lore raw

PATCH 6/7 ARM: tegra: common: Enable core clocks

In reply to Colin Cross

Enable the cpu, emc (memory controller) and csite (debug and trace controller) clocks during init to prevent them from being disabled by the bootloader clock disabling code.

Signed-off-by Colin Cross <[email protected]>

---
 arch/arm/mach-tegra/common.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

arch/arm/mach-tegra/common.c +3 -0

index 14fa533..516e100 100644
@@ -56,6 +56,9 @@ static __initdata struct tegra_clk_init_table common_clk_init_table[] = {
 	{ "sclk",	"pll_p_out4",	108000000,	true },
 	{ "hclk",	"sclk",		108000000,	true },
 	{ "pclk",	"hclk",		54000000,	true },
+	{ "csite",	NULL,		0,		true },
+	{ "emc",	NULL,		0,		true },
+	{ "cpu",	NULL,		0,		true },
 	{ NULL,		NULL,		0,		0},
 };

Signature · 1 line

1.7.3.1

Olof Johansson

Acked-by on v1 6/7

2011-02-22 05:32 UTC #16 lore raw

Re: [PATCH 6/7] ARM: tegra: common: Enable core clocks

Quoted 5 lines · Colin Cross

Enable the cpu, emc (memory controller) and csite (debug and trace controller) clocks during init to prevent them from being disabled by the bootloader clock disabling code.
Signed-off-by Colin Cross <[email protected]>

Acked-by Olof Johansson <[email protected]>

Colin Cross

author

2011-02-22 02:39 UTC #17 lore raw

PATCH 7/7 ARM: tegra: clock: Disable clocks left on by bootloader

In reply to Colin Cross

Iterates through all clocks, disabling any for which the refcount is 0 but the clock init detected the bootloader left the clock on. Can be disabled with command line tegra_clock.disable_boot_clocks=N

Signed-off-by Colin Cross <[email protected]>

---
 arch/arm/mach-tegra/clock.c |   44 +++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

arch/arm/mach-tegra/clock.c +44 -0

index e028320..6d686ff 100644
@@ -33,6 +33,9 @@
 #include "board.h"
 #include "clock.h"
 
+#undef MODULE_PARAM_PREFIX
+#define MODULE_PARAM_PREFIX "tegra_clock."
+
 /*
  * Locking:
  *

@@ -416,6 +419,47 @@ void tegra_sdmmc_tap_delay(struct clk *c, int delay)
 	spin_unlock_irqrestore(&c->spinlock, flags);
 }
 
+static bool tegra_disable_boot_clocks = true;
+module_param_named(disable_boot_clocks, tegra_disable_boot_clocks, bool,
+	S_IRUGO | S_IWUSR | S_IWGRP);
+
+/*
+ * Iterate through all clocks, disabling any for which the refcount is 0
+ * but the clock init detected the bootloader left the clock on.
+ */
+static int __init tegra_init_disable_boot_clocks(void)
+{
+	struct clk *c;
+
+	mutex_lock(&clock_list_lock);
+
+	list_for_each_entry(c, &clocks, node) {
+		spin_lock_irq(&c->spinlock);
+
+		if (c->refcnt == 0 && c->state == ON &&
+				c->ops && c->ops->disable) {
+			pr_warn_once("%s clocks left on by bootloader:\n",
+				tegra_disable_boot_clocks ?
+					"Disabling" :
+					"Prevented disabling");
+
+			pr_warn("   %s\n", c->name);
+
+			if (tegra_disable_boot_clocks) {
+				c->ops->disable(c);
+				c->state = OFF;
+			}
+		}
+
+		spin_unlock_irq(&c->spinlock);
+	}
+
+	mutex_unlock(&clock_list_lock);
+
+	return 0;
+}
+late_initcall(tegra_init_disable_boot_clocks);
+
 #ifdef CONFIG_DEBUG_FS
 
 static int __clk_lock_all_spinlocks(void)

Signature · 1 line

1.7.3.1

Olof Johansson

2011-02-22 05:43 UTC #18 lore raw

Re: [PATCH 7/7] ARM: tegra: clock: Disable clocks left on by bootloader

Hi,

Quoted 25 lines · Colin Cross

Iterates through all clocks, disabling any for which the refcount is 0 but the clock init detected the bootloader left the clock on. ?Can be disabled with command line tegra_clock.disable_boot_clocks=N
Signed-off-by Colin Cross <[email protected]>
---
?arch/arm/mach-tegra/clock.c | ? 44 +++++++++++++++++++++++++++++++++++++++++++ ?1 files changed, 44 insertions(+), 0 deletions(-)
arch/arm/mach-tegra/clock.c +0 -0
index e028320..6d686ff 100644
@@ -33,6 +33,9 @@
?#include "board.h"
?#include "clock.h"
arch/arm/mach-tegra/clock.c +3 -0
+#undef MODULE_PARAM_PREFIX
+#define MODULE_PARAM_PREFIX "tegra_clock."
+
?/*
?* Locking:
?*

arch/arm/mach-tegra/clock.c void tegra_sdmmc_tap_delay(struct clk *c, int delay) · line 424

Quoted 6 lines

arch/arm/mach-tegra/clock.c +3 -0

@@ -416,6 +419,47 @@ void tegra_sdmmc_tap_delay(struct clk *c, int delay)
 ? ? ? ?spin_unlock_irqrestore(&c->spinlock, flags);
 ?}
 
+static bool tegra_disable_boot_clocks = true;
+module_param_named(disable_boot_clocks, tegra_disable_boot_clocks, bool,
+ ? ? ? S_IRUGO | S_IWUSR | S_IWGRP);

I suggest doing this as an early_param instead. I know it's not truly an early param, but it's the easier way to do non-module bootargs, i.e. by not requiring a (fake) module prefix. It'd be a little cleaner, in my opinion. The variable name itself is unique enough to not need a module prefix for namespace reasons.

Also, Documentation/kernel-parameters.txt should be updated with it.

-Olof

Colin Cross

author

2011-02-22 08:05 UTC #19 lore raw

Re: [PATCH 7/7] ARM: tegra: clock: Disable clocks left on by bootloader

Quoted 31 lines · Olof Johansson

Hi,
Quoted 28 lines · Colin Cross
Iterates through all clocks, disabling any for which the refcount is 0 but the clock init detected the bootloader left the clock on. ?Can be disabled with command line tegra_clock.disable_boot_clocks=N
Signed-off-by Colin Cross <[email protected]>
---
?arch/arm/mach-tegra/clock.c | ? 44 +++++++++++++++++++++++++++++++++++++++++++ ?1 files changed, 44 insertions(+), 0 deletions(-)
arch/arm/mach-tegra/clock.c +0 -0
index e028320..6d686ff 100644
@@ -33,6 +33,9 @@
?#include "board.h"
?#include "clock.h"
arch/arm/mach-tegra/clock.c +3 -0
+#undef MODULE_PARAM_PREFIX
+#define MODULE_PARAM_PREFIX "tegra_clock."
+
?/*
?* Locking:
?*
arch/arm/mach-tegra/clock.c +0 -0
@@ -416,6 +419,47 @@ void tegra_sdmmc_tap_delay(struct clk *c, int delay)
 ? ? ? ?spin_unlock_irqrestore(&c->spinlock, flags);
 ?}
 

Quoted 9 lines

arch/arm/mach-tegra/clock.c void tegra_sdmmc_tap_delay(struct clk *c, int delay)
Quoted 3 lines
arch/arm/mach-tegra/clock.c +3 -0
+static bool tegra_disable_boot_clocks = true;
+module_param_named(disable_boot_clocks, tegra_disable_boot_clocks, bool,
+ ? ? ? S_IRUGO | S_IWUSR | S_IWGRP);
I suggest doing this as an early_param instead. I know it's not truly an early param, but it's the easier way to do non-module bootargs, i.e. by not requiring a (fake) module prefix. It'd be a little cleaner, in my opinion. The variable name itself is unique enough to not need a module prefix for namespace reasons.

It doesn't need to be an early_param, I can make it a __setup.

Quoted 1 line

Also, Documentation/kernel-parameters.txt should be updated with it.

Is it really worth adding every debug flag on every platform to kernel-parametes.txt? I could add the flag to the warning message instead.

Olof Johansson

2011-02-22 15:30 UTC #20 lore raw

Re: [PATCH 7/7] ARM: tegra: clock: Disable clocks left on by bootloader

Hi,

Quoted 1 line · Colin Cross

It doesn't need to be an early_param, I can make it a __setup.

Sounds good.

Quoted 4 lines

Quoted 1 line
Also, Documentation/kernel-parameters.txt should be updated with it.
Is it really worth adding every debug flag on every platform to kernel-parametes.txt? ?I could add the flag to the warning message instead.

It looks like most arm sub-archs haven't added theirs, so it's OK to leave out. I'm not too bothered either way myself.

-Olof

Colin Cross

author

2011-02-22 19:38 UTC #21 lore raw

In reply to Colin Cross

I pushed everything except the last patch that disables bootloader clocks to for-next. I'll repost the last change with Olof's comments addressed.

Quoted 12 lines · Colin Cross

This patch series contains two small patches out of the previous series that needed fixes, as well as some new ones to disable unused clocks during boot and related bug fixes.
Patch 4 converts the Harmony and Trimslice boards to use the new init_early machine handler. ?Any boards that go in after this patch will need to be updated to match, and this patch will need to be updated if any boards go in before this patch.
Tested-bys on Harmony and Trimslice would be appreciated.