diff mbox

[RFC,4/7] arm: tegra: Fix PWM clock programming

Message ID 1324377138-32129-5-git-send-email-thierry.reding@avionic-design.de
State New, archived
Headers show

Commit Message

Thierry Reding Dec. 20, 2011, 10:32 a.m. UTC 
From: Simon Que <sque@chromium.org>

PWM clock source registers in Tegra 2 have different clock source selection bit
fields than other registers.  PWM clock source bits in CLK_SOURCE_PWM_0 register
are located at bit field bit[30:28] while others are at bit field bit[31:30] in
their respective clock source register.

This patch updates the clock programming to correctly reflect that, by adding a
flag to indicate the alternate bit field format and checking for it when
selecting a clock source (parent clock).

Also, adjusts for the frequency divider being offset by 1.

Signed-off-by: Bill Huang <bilhuang@nvidia.com>
Signed-off-by: Simon Que <sque@chromium.org>
---
 arch/arm/mach-tegra/clock.h         |    1 +
 arch/arm/mach-tegra/tegra2_clocks.c |   28 ++++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

Comments

Stephen Warren Dec. 20, 2011, 10:57 p.m. UTC | #1 
Thierry Reding wrote at Tuesday, December 20, 2011 3:32 AM:
> From: Simon Que <sque@chromium.org>
> 
> PWM clock source registers in Tegra 2 have different clock source selection bit
> fields than other registers.  PWM clock source bits in CLK_SOURCE_PWM_0 register
> are located at bit field bit[30:28] while others are at bit field bit[31:30] in
> their respective clock source register.
> 
> This patch updates the clock programming to correctly reflect that, by adding a
> flag to indicate the alternate bit field format and checking for it when
> selecting a clock source (parent clock).
> 
> Also, adjusts for the frequency divider being offset by 1.

That last line applies to the original patch in the ChromeOS tree, but
not to the patch you posted (the edit to arch/arm/mach-tegra/pwm.c that
was in the original patch isn't part of this patch).
Thierry Reding Dec. 21, 2011, 9:12 a.m. UTC | #2 
* Stephen Warren wrote:
> Thierry Reding wrote at Tuesday, December 20, 2011 3:32 AM:
> > From: Simon Que <sque@chromium.org>
> > 
> > PWM clock source registers in Tegra 2 have different clock source selection bit
> > fields than other registers.  PWM clock source bits in CLK_SOURCE_PWM_0 register
> > are located at bit field bit[30:28] while others are at bit field bit[31:30] in
> > their respective clock source register.
> > 
> > This patch updates the clock programming to correctly reflect that, by adding a
> > flag to indicate the alternate bit field format and checking for it when
> > selecting a clock source (parent clock).
> > 
> > Also, adjusts for the frequency divider being offset by 1.
> 
> That last line applies to the original patch in the ChromeOS tree, but
> not to the patch you posted (the edit to arch/arm/mach-tegra/pwm.c that
> was in the original patch isn't part of this patch).

Right, I've adjusted the commit message to take that into account. I assume
the commit now also requires my Signed-off-by because I actually modified the
patch? This would be true even in the previous version because I had to make
some small adjustments.

Thierry
Stephen Warren Dec. 21, 2011, 4:44 p.m. UTC | #3 
Thierry Reding wrote at Wednesday, December 21, 2011 2:12 AM:
> * Stephen Warren wrote:
> > Thierry Reding wrote at Tuesday, December 20, 2011 3:32 AM:
> > > From: Simon Que <sque@chromium.org>
> > >
> > > PWM clock source registers in Tegra 2 have different clock source selection bit
...
> > > Also, adjusts for the frequency divider being offset by 1.
> >
> > That last line applies to the original patch in the ChromeOS tree, but
> > not to the patch you posted (the edit to arch/arm/mach-tegra/pwm.c that
> > was in the original patch isn't part of this patch).
> 
> Right, I've adjusted the commit message to take that into account. I assume
> the commit now also requires my Signed-off-by because I actually modified the
> patch? This would be true even in the previous version because I had to make
> some small adjustments.

All patches you send need you S-o-b line. This is true whether you
modified the patch or not.

When modifying a patch someone else wrote, it's typical to include some
notes on what you changed, e.g.:

Signed-off-by: Original author <...>
[swarren: Fixed checkpatch warnings]
Signed-off-by: You <...>

Or perhaps for larger changes that don't inline between the S-o-b very
well, put it above all the S-o-b.
diff mbox

Patch

diff --git a/arch/arm/mach-tegra/clock.h b/arch/arm/mach-tegra/clock.h
index 688316a..ebf4bcd 100644
--- a/arch/arm/mach-tegra/clock.h
+++ b/arch/arm/mach-tegra/clock.h
@@ -39,6 +39,7 @@ 
 #define PERIPH_MANUAL_RESET	(1 << 12)
 #define PLL_ALT_MISC_REG	(1 << 13)
 #define PLLU			(1 << 14)
+#define PERIPH_SOURCE_CLK_4BIT	(1 << 15)
 #define ENABLE_ON_INIT		(1 << 28)
 
 struct clk;
diff --git a/arch/arm/mach-tegra/tegra2_clocks.c b/arch/arm/mach-tegra/tegra2_clocks.c
index 371869d..21729fa 100644
--- a/arch/arm/mach-tegra/tegra2_clocks.c
+++ b/arch/arm/mach-tegra/tegra2_clocks.c
@@ -69,6 +69,8 @@ 
 
 #define PERIPH_CLK_SOURCE_MASK		(3<<30)
 #define PERIPH_CLK_SOURCE_SHIFT		30
+#define PERIPH_CLK_SOURCE_4BIT_MASK	(7<<28)
+#define PERIPH_CLK_SOURCE_4BIT_SHIFT	28
 #define PERIPH_CLK_SOURCE_ENABLE	(1<<28)
 #define PERIPH_CLK_SOURCE_DIVU71_MASK	0xFF
 #define PERIPH_CLK_SOURCE_DIVU16_MASK	0xFFFF
@@ -920,9 +922,16 @@  static void tegra2_periph_clk_init(struct clk *c)
 	u32 val = clk_readl(c->reg);
 	const struct clk_mux_sel *mux = NULL;
 	const struct clk_mux_sel *sel;
+	u32 shift;
+
+	if (c->flags & PERIPH_SOURCE_CLK_4BIT)
+		shift = PERIPH_CLK_SOURCE_4BIT_SHIFT;
+	else
+		shift = PERIPH_CLK_SOURCE_SHIFT;
+
 	if (c->flags & MUX) {
 		for (sel = c->inputs; sel->input != NULL; sel++) {
-			if (val >> PERIPH_CLK_SOURCE_SHIFT == sel->value)
+			if (val >> shift == sel->value)
 				mux = sel;
 		}
 		BUG_ON(!mux);
@@ -1035,12 +1044,23 @@  static int tegra2_periph_clk_set_parent(struct clk *c, struct clk *p)
 {
 	u32 val;
 	const struct clk_mux_sel *sel;
+	u32 mask, shift;
+
 	pr_debug("%s: %s %s\n", __func__, c->name, p->name);
+
+	if (c->flags & PERIPH_SOURCE_CLK_4BIT) {
+		mask = PERIPH_CLK_SOURCE_4BIT_MASK;
+		shift = PERIPH_CLK_SOURCE_4BIT_SHIFT;
+	} else {
+		mask = PERIPH_CLK_SOURCE_MASK;
+		shift = PERIPH_CLK_SOURCE_SHIFT;
+	}
+
 	for (sel = c->inputs; sel->input != NULL; sel++) {
 		if (sel->input == p) {
 			val = clk_readl(c->reg);
-			val &= ~PERIPH_CLK_SOURCE_MASK;
-			val |= (sel->value) << PERIPH_CLK_SOURCE_SHIFT;
+			val &= ~mask;
+			val |= (sel->value) << shift;
 
 			if (c->refcnt)
 				clk_enable(p);
@@ -2133,7 +2153,7 @@  static struct clk tegra_list_clks[] = {
 	PERIPH_CLK("i2s2",	"tegra-i2s.1",		NULL,	18,	0x104,	26000000,  mux_pllaout0_audio2x_pllp_clkm,	MUX | DIV_U71),
 	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),
+	PERIPH_CLK("pwm",	"pwm",			NULL,	17,	0x110,	432000000, mux_pllp_pllc_audio_clkm_clk32,	MUX | DIV_U71 | PERIPH_SOURCE_CLK_4BIT),
 	PERIPH_CLK("spi",	"spi",			NULL,	43,	0x114,	40000000,  mux_pllp_pllc_pllm_clkm,	MUX | DIV_U71),
 	PERIPH_CLK("xio",	"xio",			NULL,	45,	0x120,	150000000, mux_pllp_pllc_pllm_clkm,	MUX | DIV_U71),
 	PERIPH_CLK("twc",	"twc",			NULL,	16,	0x12c,	150000000, mux_pllp_pllc_pllm_clkm,	MUX | DIV_U71),