linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ep93xx: introduce clk parent
@ 2009-09-16 22:57 H Hartley Sweeten
  2009-09-23 22:48 ` H Hartley Sweeten
  0 siblings, 1 reply; 18+ messages in thread
From: H Hartley Sweeten @ 2009-09-16 22:57 UTC (permalink / raw)
  To: linux-arm-kernel

The clock generation system in the ep93xx uses two external oscillator's
and two internal PLL's to derive all the internal clocks.  Many of these
internal clocks can be stopped to save power.

This introduces a "parent" hierarchy for the clocks so that the users
count can be correctly tracked for power management.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Ryan Mallon <ryan@bluewatersys.com>

---

diff --git a/arch/arm/mach-ep93xx/clock.c b/arch/arm/mach-ep93xx/clock.c
index 3dd0e2a..11a2fd5 100644
--- a/arch/arm/mach-ep93xx/clock.c
+++ b/arch/arm/mach-ep93xx/clock.c
@@ -23,6 +23,7 @@
 
 
 struct clk {
+	struct clk	*parent;
 	unsigned long	rate;
 	int		users;
 	int		sw_locked;
@@ -39,89 +40,120 @@ static unsigned long get_uart_rate(struct clk *clk);
 static int set_keytchclk_rate(struct clk *clk, unsigned long rate);
 
 
+static struct clk clk_xtali = {
+	.rate		= EP93XX_EXT_CLK_RATE,
+};
 static struct clk clk_uart1 = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_DEVCFG,
 	.enable_mask	= EP93XX_SYSCON_DEVCFG_U1EN,
 	.get_rate	= get_uart_rate,
 };
 static struct clk clk_uart2 = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_DEVCFG,
 	.enable_mask	= EP93XX_SYSCON_DEVCFG_U2EN,
 	.get_rate	= get_uart_rate,
 };
 static struct clk clk_uart3 = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_DEVCFG,
 	.enable_mask	= EP93XX_SYSCON_DEVCFG_U3EN,
 	.get_rate	= get_uart_rate,
 };
-static struct clk clk_pll1;
-static struct clk clk_f;
-static struct clk clk_h;
-static struct clk clk_p;
-static struct clk clk_pll2;
+static struct clk clk_pll1 = {
+	.parent		= &clk_xtali,
+};
+static struct clk clk_f = {
+	.parent		= &clk_pll1,
+};
+static struct clk clk_h = {
+	.parent		= &clk_pll1,
+};
+static struct clk clk_p = {
+	.parent		= &clk_pll1,
+};
+static struct clk clk_pll2 = {
+	.parent		= &clk_xtali,
+};
 static struct clk clk_usb_host = {
+	.parent		= &clk_pll2,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_USH_EN,
 };
 static struct clk clk_keypad = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_KEYTCHCLKDIV,
 	.enable_mask	= EP93XX_SYSCON_KEYTCHCLKDIV_KEN,
 	.set_rate	= set_keytchclk_rate,
 };
 static struct clk clk_pwm = {
+	.parent		= &clk_xtali,
 	.rate		= EP93XX_EXT_CLK_RATE,
 };
 
 /* DMA Clocks */
 static struct clk clk_m2p0 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P0,
 };
 static struct clk clk_m2p1 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P1,
 };
 static struct clk clk_m2p2 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P2,
 };
 static struct clk clk_m2p3 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P3,
 };
 static struct clk clk_m2p4 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P4,
 };
 static struct clk clk_m2p5 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P5,
 };
 static struct clk clk_m2p6 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P6,
 };
 static struct clk clk_m2p7 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P7,
 };
 static struct clk clk_m2p8 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P8,
 };
 static struct clk clk_m2p9 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P9,
 };
 static struct clk clk_m2m0 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M0,
 };
 static struct clk clk_m2m1 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M1,
 };
@@ -130,6 +162,7 @@ static struct clk clk_m2m1 = {
 	{ .dev_id = dev, .con_id = con, .clk = ck }
 
 static struct clk_lookup clocks[] = {
+	INIT_CK(NULL,			"xtali",	&clk_xtali),
 	INIT_CK("apb:uart1",		NULL,		&clk_uart1),
 	INIT_CK("apb:uart2",		NULL,		&clk_uart2),
 	INIT_CK("apb:uart3",		NULL,		&clk_uart3),
@@ -158,15 +191,20 @@ static struct clk_lookup clocks[] = {
 
 int clk_enable(struct clk *clk)
 {
-	if (!clk->users++ && clk->enable_reg) {
-		u32 value;
-
-		value = __raw_readl(clk->enable_reg);
-		value |= clk->enable_mask;
-		if (clk->sw_locked)
-			ep93xx_syscon_swlocked_write(value, clk->enable_reg);
-		else
-			__raw_writel(value, clk->enable_reg);
+	if (!clk->users++) {
+		if (clk->parent)
+			clk_enable(clk->parent);
+
+		if (clk->enable_reg) {
+			u32 v;
+
+			v = __raw_readl(clk->enable_reg);
+			v |= clk->enable_mask;
+			if (clk->sw_locked)
+				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+			else
+				__raw_writel(v, clk->enable_reg);
+		}
 	}
 
 	return 0;
@@ -175,28 +213,34 @@ EXPORT_SYMBOL(clk_enable);
 
 void clk_disable(struct clk *clk)
 {
-	if (!--clk->users && clk->enable_reg) {
-		u32 value;
-
-		value = __raw_readl(clk->enable_reg);
-		value &= ~clk->enable_mask;
-		if (clk->sw_locked)
-			ep93xx_syscon_swlocked_write(value, clk->enable_reg);
-		else
-			__raw_writel(value, clk->enable_reg);
+	if (!--clk->users) {
+		if (clk->enable_reg) {
+			u32 v;
+
+			v = __raw_readl(clk->enable_reg);
+			v &= ~clk->enable_mask;
+			if (clk->sw_locked)
+				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+			else
+				__raw_writel(v, clk->enable_reg);
+		}
+
+		if (clk->parent)
+			clk_disable(clk->parent);
 	}
 }
 EXPORT_SYMBOL(clk_disable);
 
 static unsigned long get_uart_rate(struct clk *clk)
 {
+	unsigned long rate = clk_get_rate(clk->parent);
 	u32 value;
 
 	value = __raw_readl(EP93XX_SYSCON_PWRCNT);
 	if (value & EP93XX_SYSCON_PWRCNT_UARTBAUD)
-		return EP93XX_EXT_CLK_RATE;
+		return rate;
 	else
-		return EP93XX_EXT_CLK_RATE / 2;
+		return rate / 2;
 }
 
 unsigned long clk_get_rate(struct clk *clk)
@@ -258,7 +302,7 @@ static unsigned long calc_pll_rate(u32 config_word)
 	unsigned long long rate;
 	int i;
 
-	rate = EP93XX_EXT_CLK_RATE;
+	rate = clk_xtali.rate;
 	rate *= ((config_word >> 11) & 0x1f) + 1;		/* X1FBD */
 	rate *= ((config_word >> 5) & 0x3f) + 1;		/* X2FBD */
 	do_div(rate, (config_word & 0x1f) + 1);			/* X2IPD */
@@ -291,7 +335,7 @@ static int __init ep93xx_clock_init(void)
 
 	value = __raw_readl(EP93XX_SYSCON_CLOCK_SET1);
 	if (!(value & 0x00800000)) {			/* PLL1 bypassed?  */
-		clk_pll1.rate = EP93XX_EXT_CLK_RATE;
+		clk_pll1.rate = clk_xtali.rate;
 	} else {
 		clk_pll1.rate = calc_pll_rate(value);
 	}
@@ -302,7 +346,7 @@ static int __init ep93xx_clock_init(void)
 
 	value = __raw_readl(EP93XX_SYSCON_CLOCK_SET2);
 	if (!(value & 0x00080000)) {			/* PLL2 bypassed?  */
-		clk_pll2.rate = EP93XX_EXT_CLK_RATE;
+		clk_pll2.rate = clk_xtali.rate;
 	} else if (value & 0x00040000) {		/* PLL2 enabled?  */
 		clk_pll2.rate = calc_pll_rate(value);
 	} else { 

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-09-16 22:57 [PATCH] ep93xx: introduce clk parent H Hartley Sweeten
@ 2009-09-23 22:48 ` H Hartley Sweeten
  2009-09-30 19:53   ` Ryan Mallon
  0 siblings, 1 reply; 18+ messages in thread
From: H Hartley Sweeten @ 2009-09-23 22:48 UTC (permalink / raw)
  To: linux-arm-kernel

The clock generation system in the ep93xx uses two external oscillator's
and two internal PLLs to derive all the internal clocks.  Many of these
internal clocks can be stopped to save power.

This introduces a "parent" hierarchy for the clocks so that the users
count can be correctly tracked for power management.

The "parent" for the video clock can either be one of the PLL outputs
or the external oscillator.  In order to correctly track the "parent"
for the video clock calc_clk_div() needed to be modified.  It now
returns an error code if the desired rate cannot be generated.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Cc: Ryan Mallon <ryan@bluewatersys.com>

---

Rebased to 2.6.31-gitcurr now that the ep93xx branch is stable.

diff --git a/arch/arm/mach-ep93xx/clock.c b/arch/arm/mach-ep93xx/clock.c
index dda19cd..2723cb7 100644
--- a/arch/arm/mach-ep93xx/clock.c
+++ b/arch/arm/mach-ep93xx/clock.c
@@ -23,6 +23,7 @@
 
 
 struct clk {
+	struct clk	*parent;
 	unsigned long	rate;
 	int		users;
 	int		sw_locked;
@@ -39,40 +40,60 @@ static unsigned long get_uart_rate(struct clk *clk);
 static int set_keytchclk_rate(struct clk *clk, unsigned long rate);
 static int set_div_rate(struct clk *clk, unsigned long rate);
 
+
+static struct clk clk_xtali = {
+	.rate		= EP93XX_EXT_CLK_RATE,
+};
 static struct clk clk_uart1 = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_DEVCFG,
 	.enable_mask	= EP93XX_SYSCON_DEVCFG_U1EN,
 	.get_rate	= get_uart_rate,
 };
 static struct clk clk_uart2 = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_DEVCFG,
 	.enable_mask	= EP93XX_SYSCON_DEVCFG_U2EN,
 	.get_rate	= get_uart_rate,
 };
 static struct clk clk_uart3 = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_DEVCFG,
 	.enable_mask	= EP93XX_SYSCON_DEVCFG_U3EN,
 	.get_rate	= get_uart_rate,
 };
-static struct clk clk_pll1;
-static struct clk clk_f;
-static struct clk clk_h;
-static struct clk clk_p;
-static struct clk clk_pll2;
+static struct clk clk_pll1 = {
+	.parent		= &clk_xtali,
+};
+static struct clk clk_f = {
+	.parent		= &clk_pll1,
+};
+static struct clk clk_h = {
+	.parent		= &clk_pll1,
+};
+static struct clk clk_p = {
+	.parent		= &clk_pll1,
+};
+static struct clk clk_pll2 = {
+	.parent		= &clk_xtali,
+};
 static struct clk clk_usb_host = {
+	.parent		= &clk_pll2,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_USH_EN,
 };
 static struct clk clk_keypad = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_KEYTCHCLKDIV,
 	.enable_mask	= EP93XX_SYSCON_KEYTCHCLKDIV_KEN,
 	.set_rate	= set_keytchclk_rate,
 };
 static struct clk clk_pwm = {
+	.parent		= &clk_xtali,
 	.rate		= EP93XX_EXT_CLK_RATE,
 };
 
@@ -85,50 +106,62 @@ static struct clk clk_video = {
 
 /* DMA Clocks */
 static struct clk clk_m2p0 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P0,
 };
 static struct clk clk_m2p1 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P1,
 };
 static struct clk clk_m2p2 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P2,
 };
 static struct clk clk_m2p3 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P3,
 };
 static struct clk clk_m2p4 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P4,
 };
 static struct clk clk_m2p5 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P5,
 };
 static struct clk clk_m2p6 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P6,
 };
 static struct clk clk_m2p7 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P7,
 };
 static struct clk clk_m2p8 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P8,
 };
 static struct clk clk_m2p9 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P9,
 };
 static struct clk clk_m2m0 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M0,
 };
 static struct clk clk_m2m1 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M1,
 };
@@ -137,6 +170,7 @@ static struct clk clk_m2m1 = {
 	{ .dev_id = dev, .con_id = con, .clk = ck }
 
 static struct clk_lookup clocks[] = {
+	INIT_CK(NULL,			"xtali",	&clk_xtali),
 	INIT_CK("apb:uart1",		NULL,		&clk_uart1),
 	INIT_CK("apb:uart2",		NULL,		&clk_uart2),
 	INIT_CK("apb:uart3",		NULL,		&clk_uart3),
@@ -166,15 +200,20 @@ static struct clk_lookup clocks[] = {
 
 int clk_enable(struct clk *clk)
 {
-	if (!clk->users++ && clk->enable_reg) {
-		u32 value;
-
-		value = __raw_readl(clk->enable_reg);
-		value |= clk->enable_mask;
-		if (clk->sw_locked)
-			ep93xx_syscon_swlocked_write(value, clk->enable_reg);
-		else
-			__raw_writel(value, clk->enable_reg);
+	if (!clk->users++) {
+		if (clk->parent)
+			clk_enable(clk->parent);
+
+		if (clk->enable_reg) {
+			u32 v;
+
+			v = __raw_readl(clk->enable_reg);
+			v |= clk->enable_mask;
+			if (clk->sw_locked)
+				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+			else
+				__raw_writel(v, clk->enable_reg);
+		}
 	}
 
 	return 0;
@@ -183,28 +222,34 @@ EXPORT_SYMBOL(clk_enable);
 
 void clk_disable(struct clk *clk)
 {
-	if (!--clk->users && clk->enable_reg) {
-		u32 value;
+	if (!--clk->users) {
+		if (clk->enable_reg) {
+			u32 v;
+
+			v = __raw_readl(clk->enable_reg);
+			v &= ~clk->enable_mask;
+			if (clk->sw_locked)
+				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+			else
+				__raw_writel(v, clk->enable_reg);
+		}
 
-		value = __raw_readl(clk->enable_reg);
-		value &= ~clk->enable_mask;
-		if (clk->sw_locked)
-			ep93xx_syscon_swlocked_write(value, clk->enable_reg);
-		else
-			__raw_writel(value, clk->enable_reg);
+		if (clk->parent)
+			clk_disable(clk->parent);
 	}
 }
 EXPORT_SYMBOL(clk_disable);
 
 static unsigned long get_uart_rate(struct clk *clk)
 {
+	unsigned long rate = clk_get_rate(clk->parent);
 	u32 value;
 
 	value = __raw_readl(EP93XX_SYSCON_PWRCNT);
 	if (value & EP93XX_SYSCON_PWRCNT_UARTBAUD)
-		return EP93XX_EXT_CLK_RATE;
+		return rate;
 	else
-		return EP93XX_EXT_CLK_RATE / 2;
+		return rate / 2;
 }
 
 unsigned long clk_get_rate(struct clk *clk)
@@ -244,16 +289,16 @@ static int set_keytchclk_rate(struct clk *clk, unsigned long rate)
 	return 0;
 }
 
-static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
-				  int *pdiv, int *div)
+static int calc_clk_div(struct clk *clk, unsigned long rate,
+			int *psel, int *esel, int *pdiv, int *div)
 {
-	unsigned long max_rate, best_rate = 0,
-		actual_rate = 0, mclk_rate = 0, rate_err = -1;
+	struct clk *mclk;
+	unsigned long max_rate, actual_rate, mclk_rate, rate_err = -1;
 	int i, found = 0, __div = 0, __pdiv = 0;
 
 	/* Don't exceed the maximum rate */
 	max_rate = max(max(clk_pll1.rate / 4, clk_pll2.rate / 4),
-		       (unsigned long)EP93XX_EXT_CLK_RATE / 4);
+		       clk_xtali.rate / 4);
 	rate = min(rate, max_rate);
 
 	/*
@@ -267,11 +312,12 @@ static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
 	 */
 	for (i = 0; i < 3; i++) {
 		if (i == 0)
-			mclk_rate = EP93XX_EXT_CLK_RATE * 2;
+			mclk = &clk_xtali;
 		else if (i == 1)
-			mclk_rate = clk_pll1.rate * 2;
-		else if (i == 2)
-			mclk_rate = clk_pll2.rate * 2;
+			mclk = &clk_pll1;
+		else
+			mclk = &clk_pll2;
+		mclk_rate = mclk->rate * 2;
 
 		/* Try each predivider value */
 		for (__pdiv = 4; __pdiv <= 6; __pdiv++) {
@@ -286,7 +332,8 @@ static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
 				*div = __div;
 				*psel = (i == 2);
 				*esel = (i != 0);
-				best_rate = actual_rate;
+				clk->parent = mclk;
+				clk->rate = actual_rate;
 				rate_err = abs(actual_rate - rate);
 				found = 1;
 			}
@@ -294,21 +341,19 @@ static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
 	}
 
 	if (!found)
-		return 0;
+		return -EINVAL;
 
-	return best_rate;
+	return 0;
 }
 
 static int set_div_rate(struct clk *clk, unsigned long rate)
 {
-	unsigned long actual_rate;
-	int psel = 0, esel = 0, pdiv = 0, div = 0;
+	int err, psel = 0, esel = 0, pdiv = 0, div = 0;
 	u32 val;
 
-	actual_rate = calc_clk_div(rate, &psel, &esel, &pdiv, &div);
-	if (actual_rate == 0)
-		return -EINVAL;
-	clk->rate = actual_rate;
+	err = calc_clk_div(clk, rate, &psel, &esel, &pdiv, &div);
+	if (err)
+		return err;
 
 	/* Clear the esel, psel, pdiv and div bits */
 	val = __raw_readl(clk->enable_reg);
@@ -344,7 +389,7 @@ static unsigned long calc_pll_rate(u32 config_word)
 	unsigned long long rate;
 	int i;
 
-	rate = EP93XX_EXT_CLK_RATE;
+	rate = clk_xtali.rate;
 	rate *= ((config_word >> 11) & 0x1f) + 1;		/* X1FBD */
 	rate *= ((config_word >> 5) & 0x3f) + 1;		/* X2FBD */
 	do_div(rate, (config_word & 0x1f) + 1);			/* X2IPD */
@@ -377,7 +422,7 @@ static int __init ep93xx_clock_init(void)
 
 	value = __raw_readl(EP93XX_SYSCON_CLOCK_SET1);
 	if (!(value & 0x00800000)) {			/* PLL1 bypassed?  */
-		clk_pll1.rate = EP93XX_EXT_CLK_RATE;
+		clk_pll1.rate = clk_xtali.rate;
 	} else {
 		clk_pll1.rate = calc_pll_rate(value);
 	}
@@ -388,7 +433,7 @@ static int __init ep93xx_clock_init(void)
 
 	value = __raw_readl(EP93XX_SYSCON_CLOCK_SET2);
 	if (!(value & 0x00080000)) {			/* PLL2 bypassed?  */
-		clk_pll2.rate = EP93XX_EXT_CLK_RATE;
+		clk_pll2.rate = clk_xtali.rate;
 	} else if (value & 0x00040000) {		/* PLL2 enabled?  */
 		clk_pll2.rate = calc_pll_rate(value);
 	} else { 

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-09-23 22:48 ` H Hartley Sweeten
@ 2009-09-30 19:53   ` Ryan Mallon
  2009-09-30 21:30     ` H Hartley Sweeten
  0 siblings, 1 reply; 18+ messages in thread
From: Ryan Mallon @ 2009-09-30 19:53 UTC (permalink / raw)
  To: linux-arm-kernel

H Hartley Sweeten wrote:
> The clock generation system in the ep93xx uses two external oscillator's
> and two internal PLLs to derive all the internal clocks.  Many of these
> internal clocks can be stopped to save power.
> 
> This introduces a "parent" hierarchy for the clocks so that the users
> count can be correctly tracked for power management.
> 
> The "parent" for the video clock can either be one of the PLL outputs
> or the external oscillator.  In order to correctly track the "parent"
> for the video clock calc_clk_div() needed to be modified.  It now
> returns an error code if the desired rate cannot be generated.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Cc: Ryan Mallon <ryan@bluewatersys.com>
> 
> ---
> 

Acked-by: Ryan Mallon <ryan@bluewatersys.com>

-- 
Bluewater Systems Ltd - ARM Technology Solution Centre

Ryan Mallon         		5 Amuri Park, 404 Barbadoes St
ryan at bluewatersys.com         	PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com	New Zealand
Phone: +64 3 3779127		Freecall: Australia 1800 148 751
Fax:   +64 3 3779135			  USA 1800 261 2934

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-09-30 19:53   ` Ryan Mallon
@ 2009-09-30 21:30     ` H Hartley Sweeten
  2009-10-01 14:59       ` Russell King - ARM Linux
  0 siblings, 1 reply; 18+ messages in thread
From: H Hartley Sweeten @ 2009-09-30 21:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Wednesday, September 30, 2009 12:54 PM, Ryan Mallon wrote:
> H Hartley Sweeten wrote:
>> The clock generation system in the ep93xx uses two external oscillator's
>> and two internal PLLs to derive all the internal clocks.  Many of these
>> internal clocks can be stopped to save power.
>> 
>> This introduces a "parent" hierarchy for the clocks so that the users
>> count can be correctly tracked for power management.
>> 
>> The "parent" for the video clock can either be one of the PLL outputs
>> or the external oscillator.  In order to correctly track the "parent"
>> for the video clock calc_clk_div() needed to be modified.  It now
>> returns an error code if the desired rate cannot be generated.
>> 
>> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
>> Cc: Ryan Mallon <ryan@bluewatersys.com>
>> 
>> ---
>> 
>
> Acked-by: Ryan Mallon <ryan@bluewatersys.com>

Russell,

As the clkdev maintainer, do you see any problems with this?

Thanks,
Hartley

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-09-30 21:30     ` H Hartley Sweeten
@ 2009-10-01 14:59       ` Russell King - ARM Linux
  2009-10-01 16:32         ` H Hartley Sweeten
  0 siblings, 1 reply; 18+ messages in thread
From: Russell King - ARM Linux @ 2009-10-01 14:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 30, 2009 at 05:30:28PM -0400, H Hartley Sweeten wrote:
> On Wednesday, September 30, 2009 12:54 PM, Ryan Mallon wrote:
> > H Hartley Sweeten wrote:
> >> The clock generation system in the ep93xx uses two external oscillator's
> >> and two internal PLLs to derive all the internal clocks.  Many of these
> >> internal clocks can be stopped to save power.
> >> 
> >> This introduces a "parent" hierarchy for the clocks so that the users
> >> count can be correctly tracked for power management.
> >> 
> >> The "parent" for the video clock can either be one of the PLL outputs
> >> or the external oscillator.  In order to correctly track the "parent"
> >> for the video clock calc_clk_div() needed to be modified.  It now
> >> returns an error code if the desired rate cannot be generated.
> >> 
> >> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> >> Cc: Ryan Mallon <ryan@bluewatersys.com>
> >> 
> >> ---
> >> 
> >
> > Acked-by: Ryan Mallon <ryan@bluewatersys.com>
> 
> Russell,
> 
> As the clkdev maintainer, do you see any problems with this?

Only comment is that you should really consider some locking in
clk_enable and clk_disable to prevent the use count being corrupted
by simultaneous calls.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-01 14:59       ` Russell King - ARM Linux
@ 2009-10-01 16:32         ` H Hartley Sweeten
  2009-10-08 21:28           ` Christian Gagneraud
  0 siblings, 1 reply; 18+ messages in thread
From: H Hartley Sweeten @ 2009-10-01 16:32 UTC (permalink / raw)
  To: linux-arm-kernel

The clock generation system in the ep93xx uses two external oscillator's
and two internal PLLs to derive all the internal clocks.  Many of these
internal clocks can be stopped to save power.

This introduces a "parent" hierarchy for the clocks so that the users
count can be correctly tracked for power management.

The "parent" for the video clock can either be one of the PLL outputs
or the external oscillator.  In order to correctly track the "parent"
for the video clock calc_clk_div() needed to be modified.  It now
returns an error code if the desired rate cannot be generated.

Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
Acked-by: Ryan Mallon <ryan@bluewatersys.com>

---

Added locking for clk_enable and clk_disable to prevent the use count
being corrupted by simultaneous calls.


diff --git a/arch/arm/mach-ep93xx/clock.c b/arch/arm/mach-ep93xx/clock.c
index dda19cd..1d0f9d8 100644
--- a/arch/arm/mach-ep93xx/clock.c
+++ b/arch/arm/mach-ep93xx/clock.c
@@ -16,13 +16,16 @@
 #include <linux/module.h>
 #include <linux/string.h>
 #include <linux/io.h>
+#include <linux/spinlock.h>
+
+#include <mach/hardware.h>
 
 #include <asm/clkdev.h>
 #include <asm/div64.h>
-#include <mach/hardware.h>
 
 
 struct clk {
+	struct clk	*parent;
 	unsigned long	rate;
 	int		users;
 	int		sw_locked;
@@ -39,40 +42,60 @@ static unsigned long get_uart_rate(struct clk *clk);
 static int set_keytchclk_rate(struct clk *clk, unsigned long rate);
 static int set_div_rate(struct clk *clk, unsigned long rate);
 
+
+static struct clk clk_xtali = {
+	.rate		= EP93XX_EXT_CLK_RATE,
+};
 static struct clk clk_uart1 = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_DEVCFG,
 	.enable_mask	= EP93XX_SYSCON_DEVCFG_U1EN,
 	.get_rate	= get_uart_rate,
 };
 static struct clk clk_uart2 = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_DEVCFG,
 	.enable_mask	= EP93XX_SYSCON_DEVCFG_U2EN,
 	.get_rate	= get_uart_rate,
 };
 static struct clk clk_uart3 = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_DEVCFG,
 	.enable_mask	= EP93XX_SYSCON_DEVCFG_U3EN,
 	.get_rate	= get_uart_rate,
 };
-static struct clk clk_pll1;
-static struct clk clk_f;
-static struct clk clk_h;
-static struct clk clk_p;
-static struct clk clk_pll2;
+static struct clk clk_pll1 = {
+	.parent		= &clk_xtali,
+};
+static struct clk clk_f = {
+	.parent		= &clk_pll1,
+};
+static struct clk clk_h = {
+	.parent		= &clk_pll1,
+};
+static struct clk clk_p = {
+	.parent		= &clk_pll1,
+};
+static struct clk clk_pll2 = {
+	.parent		= &clk_xtali,
+};
 static struct clk clk_usb_host = {
+	.parent		= &clk_pll2,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_USH_EN,
 };
 static struct clk clk_keypad = {
+	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_KEYTCHCLKDIV,
 	.enable_mask	= EP93XX_SYSCON_KEYTCHCLKDIV_KEN,
 	.set_rate	= set_keytchclk_rate,
 };
 static struct clk clk_pwm = {
+	.parent		= &clk_xtali,
 	.rate		= EP93XX_EXT_CLK_RATE,
 };
 
@@ -85,50 +108,62 @@ static struct clk clk_video = {
 
 /* DMA Clocks */
 static struct clk clk_m2p0 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P0,
 };
 static struct clk clk_m2p1 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P1,
 };
 static struct clk clk_m2p2 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P2,
 };
 static struct clk clk_m2p3 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P3,
 };
 static struct clk clk_m2p4 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P4,
 };
 static struct clk clk_m2p5 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P5,
 };
 static struct clk clk_m2p6 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P6,
 };
 static struct clk clk_m2p7 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P7,
 };
 static struct clk clk_m2p8 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P8,
 };
 static struct clk clk_m2p9 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P9,
 };
 static struct clk clk_m2m0 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M0,
 };
 static struct clk clk_m2m1 = {
+	.parent		= &clk_h,
 	.enable_reg	= EP93XX_SYSCON_PWRCNT,
 	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M1,
 };
@@ -137,6 +172,7 @@ static struct clk clk_m2m1 = {
 	{ .dev_id = dev, .con_id = con, .clk = ck }
 
 static struct clk_lookup clocks[] = {
+	INIT_CK(NULL,			"xtali",	&clk_xtali),
 	INIT_CK("apb:uart1",		NULL,		&clk_uart1),
 	INIT_CK("apb:uart2",		NULL,		&clk_uart2),
 	INIT_CK("apb:uart3",		NULL,		&clk_uart3),
@@ -163,48 +199,84 @@ static struct clk_lookup clocks[] = {
 	INIT_CK(NULL,			"m2m1",		&clk_m2m1),
 };
 
+static DEFINE_SPINLOCK(clk_lock);
+
+static void __clk_enable(struct clk *clk)
+{
+	if (!clk->users++) {
+		if (clk->parent)
+			__clk_enable(clk->parent);
+
+		if (clk->enable_reg) {
+			u32 v;
+
+			v = __raw_readl(clk->enable_reg);
+			v |= clk->enable_mask;
+			if (clk->sw_locked)
+				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+			else
+				__raw_writel(v, clk->enable_reg);
+		}
+	}
+}
 
 int clk_enable(struct clk *clk)
 {
-	if (!clk->users++ && clk->enable_reg) {
-		u32 value;
+	unsigned long flags;
 
-		value = __raw_readl(clk->enable_reg);
-		value |= clk->enable_mask;
-		if (clk->sw_locked)
-			ep93xx_syscon_swlocked_write(value, clk->enable_reg);
-		else
-			__raw_writel(value, clk->enable_reg);
-	}
+	if (!clk)
+		return -EINVAL;
+
+	spin_lock_irqsave(&clk_lock, flags);
+	__clk_enable(clk);
+	spin_unlock_irqrestore(&clk_lock, flags);
 
 	return 0;
 }
 EXPORT_SYMBOL(clk_enable);
 
-void clk_disable(struct clk *clk)
+static void __clk_disable(struct clk *clk)
 {
-	if (!--clk->users && clk->enable_reg) {
-		u32 value;
+	if (!--clk->users) {
+		if (clk->enable_reg) {
+			u32 v;
+
+			v = __raw_readl(clk->enable_reg);
+			v &= ~clk->enable_mask;
+			if (clk->sw_locked)
+				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
+			else
+				__raw_writel(v, clk->enable_reg);
+		}
 
-		value = __raw_readl(clk->enable_reg);
-		value &= ~clk->enable_mask;
-		if (clk->sw_locked)
-			ep93xx_syscon_swlocked_write(value, clk->enable_reg);
-		else
-			__raw_writel(value, clk->enable_reg);
+		if (clk->parent)
+			__clk_disable(clk->parent);
 	}
 }
+
+void clk_disable(struct clk *clk)
+{
+	unsigned long flags;
+
+	if (!clk)
+		return;
+
+	spin_lock_irqsave(&clk_lock, flags);
+	__clk_disable(clk);
+	spin_unlock_irqrestore(&clk_lock, flags);
+}
 EXPORT_SYMBOL(clk_disable);
 
 static unsigned long get_uart_rate(struct clk *clk)
 {
+	unsigned long rate = clk_get_rate(clk->parent);
 	u32 value;
 
 	value = __raw_readl(EP93XX_SYSCON_PWRCNT);
 	if (value & EP93XX_SYSCON_PWRCNT_UARTBAUD)
-		return EP93XX_EXT_CLK_RATE;
+		return rate;
 	else
-		return EP93XX_EXT_CLK_RATE / 2;
+		return rate / 2;
 }
 
 unsigned long clk_get_rate(struct clk *clk)
@@ -244,16 +316,16 @@ static int set_keytchclk_rate(struct clk *clk, unsigned long rate)
 	return 0;
 }
 
-static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
-				  int *pdiv, int *div)
+static int calc_clk_div(struct clk *clk, unsigned long rate,
+			int *psel, int *esel, int *pdiv, int *div)
 {
-	unsigned long max_rate, best_rate = 0,
-		actual_rate = 0, mclk_rate = 0, rate_err = -1;
+	struct clk *mclk;
+	unsigned long max_rate, actual_rate, mclk_rate, rate_err = -1;
 	int i, found = 0, __div = 0, __pdiv = 0;
 
 	/* Don't exceed the maximum rate */
 	max_rate = max(max(clk_pll1.rate / 4, clk_pll2.rate / 4),
-		       (unsigned long)EP93XX_EXT_CLK_RATE / 4);
+		       clk_xtali.rate / 4);
 	rate = min(rate, max_rate);
 
 	/*
@@ -267,11 +339,12 @@ static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
 	 */
 	for (i = 0; i < 3; i++) {
 		if (i == 0)
-			mclk_rate = EP93XX_EXT_CLK_RATE * 2;
+			mclk = &clk_xtali;
 		else if (i == 1)
-			mclk_rate = clk_pll1.rate * 2;
-		else if (i == 2)
-			mclk_rate = clk_pll2.rate * 2;
+			mclk = &clk_pll1;
+		else
+			mclk = &clk_pll2;
+		mclk_rate = mclk->rate * 2;
 
 		/* Try each predivider value */
 		for (__pdiv = 4; __pdiv <= 6; __pdiv++) {
@@ -286,7 +359,8 @@ static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
 				*div = __div;
 				*psel = (i == 2);
 				*esel = (i != 0);
-				best_rate = actual_rate;
+				clk->parent = mclk;
+				clk->rate = actual_rate;
 				rate_err = abs(actual_rate - rate);
 				found = 1;
 			}
@@ -294,21 +368,19 @@ static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
 	}
 
 	if (!found)
-		return 0;
+		return -EINVAL;
 
-	return best_rate;
+	return 0;
 }
 
 static int set_div_rate(struct clk *clk, unsigned long rate)
 {
-	unsigned long actual_rate;
-	int psel = 0, esel = 0, pdiv = 0, div = 0;
+	int err, psel = 0, esel = 0, pdiv = 0, div = 0;
 	u32 val;
 
-	actual_rate = calc_clk_div(rate, &psel, &esel, &pdiv, &div);
-	if (actual_rate == 0)
-		return -EINVAL;
-	clk->rate = actual_rate;
+	err = calc_clk_div(clk, rate, &psel, &esel, &pdiv, &div);
+	if (err)
+		return err;
 
 	/* Clear the esel, psel, pdiv and div bits */
 	val = __raw_readl(clk->enable_reg);
@@ -344,7 +416,7 @@ static unsigned long calc_pll_rate(u32 config_word)
 	unsigned long long rate;
 	int i;
 
-	rate = EP93XX_EXT_CLK_RATE;
+	rate = clk_xtali.rate;
 	rate *= ((config_word >> 11) & 0x1f) + 1;		/* X1FBD */
 	rate *= ((config_word >> 5) & 0x3f) + 1;		/* X2FBD */
 	do_div(rate, (config_word & 0x1f) + 1);			/* X2IPD */
@@ -377,7 +449,7 @@ static int __init ep93xx_clock_init(void)
 
 	value = __raw_readl(EP93XX_SYSCON_CLOCK_SET1);
 	if (!(value & 0x00800000)) {			/* PLL1 bypassed?  */
-		clk_pll1.rate = EP93XX_EXT_CLK_RATE;
+		clk_pll1.rate = clk_xtali.rate;
 	} else {
 		clk_pll1.rate = calc_pll_rate(value);
 	}
@@ -388,7 +460,7 @@ static int __init ep93xx_clock_init(void)
 
 	value = __raw_readl(EP93XX_SYSCON_CLOCK_SET2);
 	if (!(value & 0x00080000)) {			/* PLL2 bypassed?  */
-		clk_pll2.rate = EP93XX_EXT_CLK_RATE;
+		clk_pll2.rate = clk_xtali.rate;
 	} else if (value & 0x00040000) {		/* PLL2 enabled?  */
 		clk_pll2.rate = calc_pll_rate(value);
 	} else { 

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-01 16:32         ` H Hartley Sweeten
@ 2009-10-08 21:28           ` Christian Gagneraud
  2009-10-08 21:34             ` Russell King - ARM Linux
  2009-10-08 22:40             ` H Hartley Sweeten
  0 siblings, 2 replies; 18+ messages in thread
From: Christian Gagneraud @ 2009-10-08 21:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

What's the status of this patch? It does not appear on the patch 
tracker, and it's not in linus tree (or I'm missing something).

I'm currently working on adding ADC support (no touchscreen for now) 
and the problem is that touchscreen (ADC) and keypad use the same 
clock. With this patch applied it would be possible to define the 
shared ep93xx-keytch clock and add ep93xx-keypad and 
ep93xx-touchscreen as children.


Regards,
Chris


H Hartley Sweeten wrote:
> The clock generation system in the ep93xx uses two external oscillator's
> and two internal PLLs to derive all the internal clocks.  Many of these
> internal clocks can be stopped to save power.
> 
> This introduces a "parent" hierarchy for the clocks so that the users
> count can be correctly tracked for power management.
> 
> The "parent" for the video clock can either be one of the PLL outputs
> or the external oscillator.  In order to correctly track the "parent"
> for the video clock calc_clk_div() needed to be modified.  It now
> returns an error code if the desired rate cannot be generated.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com>
> Acked-by: Ryan Mallon <ryan@bluewatersys.com>
> 
> ---
> 
> Added locking for clk_enable and clk_disable to prevent the use count
> being corrupted by simultaneous calls.
> 
> 
> diff --git a/arch/arm/mach-ep93xx/clock.c b/arch/arm/mach-ep93xx/clock.c
> index dda19cd..1d0f9d8 100644
> --- a/arch/arm/mach-ep93xx/clock.c
> +++ b/arch/arm/mach-ep93xx/clock.c
> @@ -16,13 +16,16 @@
>  #include <linux/module.h>
>  #include <linux/string.h>
>  #include <linux/io.h>
> +#include <linux/spinlock.h>
> +
> +#include <mach/hardware.h>
>  
>  #include <asm/clkdev.h>
>  #include <asm/div64.h>
> -#include <mach/hardware.h>
>  
>  
>  struct clk {
> +	struct clk	*parent;
>  	unsigned long	rate;
>  	int		users;
>  	int		sw_locked;
> @@ -39,40 +42,60 @@ static unsigned long get_uart_rate(struct clk *clk);
>  static int set_keytchclk_rate(struct clk *clk, unsigned long rate);
>  static int set_div_rate(struct clk *clk, unsigned long rate);
>  
> +
> +static struct clk clk_xtali = {
> +	.rate		= EP93XX_EXT_CLK_RATE,
> +};
>  static struct clk clk_uart1 = {
> +	.parent		= &clk_xtali,
>  	.sw_locked	= 1,
>  	.enable_reg	= EP93XX_SYSCON_DEVCFG,
>  	.enable_mask	= EP93XX_SYSCON_DEVCFG_U1EN,
>  	.get_rate	= get_uart_rate,
>  };
>  static struct clk clk_uart2 = {
> +	.parent		= &clk_xtali,
>  	.sw_locked	= 1,
>  	.enable_reg	= EP93XX_SYSCON_DEVCFG,
>  	.enable_mask	= EP93XX_SYSCON_DEVCFG_U2EN,
>  	.get_rate	= get_uart_rate,
>  };
>  static struct clk clk_uart3 = {
> +	.parent		= &clk_xtali,
>  	.sw_locked	= 1,
>  	.enable_reg	= EP93XX_SYSCON_DEVCFG,
>  	.enable_mask	= EP93XX_SYSCON_DEVCFG_U3EN,
>  	.get_rate	= get_uart_rate,
>  };
> -static struct clk clk_pll1;
> -static struct clk clk_f;
> -static struct clk clk_h;
> -static struct clk clk_p;
> -static struct clk clk_pll2;
> +static struct clk clk_pll1 = {
> +	.parent		= &clk_xtali,
> +};
> +static struct clk clk_f = {
> +	.parent		= &clk_pll1,
> +};
> +static struct clk clk_h = {
> +	.parent		= &clk_pll1,
> +};
> +static struct clk clk_p = {
> +	.parent		= &clk_pll1,
> +};
> +static struct clk clk_pll2 = {
> +	.parent		= &clk_xtali,
> +};
>  static struct clk clk_usb_host = {
> +	.parent		= &clk_pll2,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_USH_EN,
>  };
>  static struct clk clk_keypad = {
> +	.parent		= &clk_xtali,
>  	.sw_locked	= 1,
>  	.enable_reg	= EP93XX_SYSCON_KEYTCHCLKDIV,
>  	.enable_mask	= EP93XX_SYSCON_KEYTCHCLKDIV_KEN,
>  	.set_rate	= set_keytchclk_rate,
>  };
>  static struct clk clk_pwm = {
> +	.parent		= &clk_xtali,
>  	.rate		= EP93XX_EXT_CLK_RATE,
>  };
>  
> @@ -85,50 +108,62 @@ static struct clk clk_video = {
>  
>  /* DMA Clocks */
>  static struct clk clk_m2p0 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P0,
>  };
>  static struct clk clk_m2p1 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P1,
>  };
>  static struct clk clk_m2p2 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P2,
>  };
>  static struct clk clk_m2p3 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P3,
>  };
>  static struct clk clk_m2p4 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P4,
>  };
>  static struct clk clk_m2p5 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P5,
>  };
>  static struct clk clk_m2p6 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P6,
>  };
>  static struct clk clk_m2p7 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P7,
>  };
>  static struct clk clk_m2p8 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P8,
>  };
>  static struct clk clk_m2p9 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2P9,
>  };
>  static struct clk clk_m2m0 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M0,
>  };
>  static struct clk clk_m2m1 = {
> +	.parent		= &clk_h,
>  	.enable_reg	= EP93XX_SYSCON_PWRCNT,
>  	.enable_mask	= EP93XX_SYSCON_PWRCNT_DMA_M2M1,
>  };
> @@ -137,6 +172,7 @@ static struct clk clk_m2m1 = {
>  	{ .dev_id = dev, .con_id = con, .clk = ck }
>  
>  static struct clk_lookup clocks[] = {
> +	INIT_CK(NULL,			"xtali",	&clk_xtali),
>  	INIT_CK("apb:uart1",		NULL,		&clk_uart1),
>  	INIT_CK("apb:uart2",		NULL,		&clk_uart2),
>  	INIT_CK("apb:uart3",		NULL,		&clk_uart3),
> @@ -163,48 +199,84 @@ static struct clk_lookup clocks[] = {
>  	INIT_CK(NULL,			"m2m1",		&clk_m2m1),
>  };
>  
> +static DEFINE_SPINLOCK(clk_lock);
> +
> +static void __clk_enable(struct clk *clk)
> +{
> +	if (!clk->users++) {
> +		if (clk->parent)
> +			__clk_enable(clk->parent);
> +
> +		if (clk->enable_reg) {
> +			u32 v;
> +
> +			v = __raw_readl(clk->enable_reg);
> +			v |= clk->enable_mask;
> +			if (clk->sw_locked)
> +				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
> +			else
> +				__raw_writel(v, clk->enable_reg);
> +		}
> +	}
> +}
>  
>  int clk_enable(struct clk *clk)
>  {
> -	if (!clk->users++ && clk->enable_reg) {
> -		u32 value;
> +	unsigned long flags;
>  
> -		value = __raw_readl(clk->enable_reg);
> -		value |= clk->enable_mask;
> -		if (clk->sw_locked)
> -			ep93xx_syscon_swlocked_write(value, clk->enable_reg);
> -		else
> -			__raw_writel(value, clk->enable_reg);
> -	}
> +	if (!clk)
> +		return -EINVAL;
> +
> +	spin_lock_irqsave(&clk_lock, flags);
> +	__clk_enable(clk);
> +	spin_unlock_irqrestore(&clk_lock, flags);
>  
>  	return 0;
>  }
>  EXPORT_SYMBOL(clk_enable);
>  
> -void clk_disable(struct clk *clk)
> +static void __clk_disable(struct clk *clk)
>  {
> -	if (!--clk->users && clk->enable_reg) {
> -		u32 value;
> +	if (!--clk->users) {
> +		if (clk->enable_reg) {
> +			u32 v;
> +
> +			v = __raw_readl(clk->enable_reg);
> +			v &= ~clk->enable_mask;
> +			if (clk->sw_locked)
> +				ep93xx_syscon_swlocked_write(v, clk->enable_reg);
> +			else
> +				__raw_writel(v, clk->enable_reg);
> +		}
>  
> -		value = __raw_readl(clk->enable_reg);
> -		value &= ~clk->enable_mask;
> -		if (clk->sw_locked)
> -			ep93xx_syscon_swlocked_write(value, clk->enable_reg);
> -		else
> -			__raw_writel(value, clk->enable_reg);
> +		if (clk->parent)
> +			__clk_disable(clk->parent);
>  	}
>  }
> +
> +void clk_disable(struct clk *clk)
> +{
> +	unsigned long flags;
> +
> +	if (!clk)
> +		return;
> +
> +	spin_lock_irqsave(&clk_lock, flags);
> +	__clk_disable(clk);
> +	spin_unlock_irqrestore(&clk_lock, flags);
> +}
>  EXPORT_SYMBOL(clk_disable);
>  
>  static unsigned long get_uart_rate(struct clk *clk)
>  {
> +	unsigned long rate = clk_get_rate(clk->parent);
>  	u32 value;
>  
>  	value = __raw_readl(EP93XX_SYSCON_PWRCNT);
>  	if (value & EP93XX_SYSCON_PWRCNT_UARTBAUD)
> -		return EP93XX_EXT_CLK_RATE;
> +		return rate;
>  	else
> -		return EP93XX_EXT_CLK_RATE / 2;
> +		return rate / 2;
>  }
>  
>  unsigned long clk_get_rate(struct clk *clk)
> @@ -244,16 +316,16 @@ static int set_keytchclk_rate(struct clk *clk, unsigned long rate)
>  	return 0;
>  }
>  
> -static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
> -				  int *pdiv, int *div)
> +static int calc_clk_div(struct clk *clk, unsigned long rate,
> +			int *psel, int *esel, int *pdiv, int *div)
>  {
> -	unsigned long max_rate, best_rate = 0,
> -		actual_rate = 0, mclk_rate = 0, rate_err = -1;
> +	struct clk *mclk;
> +	unsigned long max_rate, actual_rate, mclk_rate, rate_err = -1;
>  	int i, found = 0, __div = 0, __pdiv = 0;
>  
>  	/* Don't exceed the maximum rate */
>  	max_rate = max(max(clk_pll1.rate / 4, clk_pll2.rate / 4),
> -		       (unsigned long)EP93XX_EXT_CLK_RATE / 4);
> +		       clk_xtali.rate / 4);
>  	rate = min(rate, max_rate);
>  
>  	/*
> @@ -267,11 +339,12 @@ static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
>  	 */
>  	for (i = 0; i < 3; i++) {
>  		if (i == 0)
> -			mclk_rate = EP93XX_EXT_CLK_RATE * 2;
> +			mclk = &clk_xtali;
>  		else if (i == 1)
> -			mclk_rate = clk_pll1.rate * 2;
> -		else if (i == 2)
> -			mclk_rate = clk_pll2.rate * 2;
> +			mclk = &clk_pll1;
> +		else
> +			mclk = &clk_pll2;
> +		mclk_rate = mclk->rate * 2;
>  
>  		/* Try each predivider value */
>  		for (__pdiv = 4; __pdiv <= 6; __pdiv++) {
> @@ -286,7 +359,8 @@ static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
>  				*div = __div;
>  				*psel = (i == 2);
>  				*esel = (i != 0);
> -				best_rate = actual_rate;
> +				clk->parent = mclk;
> +				clk->rate = actual_rate;
>  				rate_err = abs(actual_rate - rate);
>  				found = 1;
>  			}
> @@ -294,21 +368,19 @@ static unsigned long calc_clk_div(unsigned long rate, int *psel, int *esel,
>  	}
>  
>  	if (!found)
> -		return 0;
> +		return -EINVAL;
>  
> -	return best_rate;
> +	return 0;
>  }
>  
>  static int set_div_rate(struct clk *clk, unsigned long rate)
>  {
> -	unsigned long actual_rate;
> -	int psel = 0, esel = 0, pdiv = 0, div = 0;
> +	int err, psel = 0, esel = 0, pdiv = 0, div = 0;
>  	u32 val;
>  
> -	actual_rate = calc_clk_div(rate, &psel, &esel, &pdiv, &div);
> -	if (actual_rate == 0)
> -		return -EINVAL;
> -	clk->rate = actual_rate;
> +	err = calc_clk_div(clk, rate, &psel, &esel, &pdiv, &div);
> +	if (err)
> +		return err;
>  
>  	/* Clear the esel, psel, pdiv and div bits */
>  	val = __raw_readl(clk->enable_reg);
> @@ -344,7 +416,7 @@ static unsigned long calc_pll_rate(u32 config_word)
>  	unsigned long long rate;
>  	int i;
>  
> -	rate = EP93XX_EXT_CLK_RATE;
> +	rate = clk_xtali.rate;
>  	rate *= ((config_word >> 11) & 0x1f) + 1;		/* X1FBD */
>  	rate *= ((config_word >> 5) & 0x3f) + 1;		/* X2FBD */
>  	do_div(rate, (config_word & 0x1f) + 1);			/* X2IPD */
> @@ -377,7 +449,7 @@ static int __init ep93xx_clock_init(void)
>  
>  	value = __raw_readl(EP93XX_SYSCON_CLOCK_SET1);
>  	if (!(value & 0x00800000)) {			/* PLL1 bypassed?  */
> -		clk_pll1.rate = EP93XX_EXT_CLK_RATE;
> +		clk_pll1.rate = clk_xtali.rate;
>  	} else {
>  		clk_pll1.rate = calc_pll_rate(value);
>  	}
> @@ -388,7 +460,7 @@ static int __init ep93xx_clock_init(void)
>  
>  	value = __raw_readl(EP93XX_SYSCON_CLOCK_SET2);
>  	if (!(value & 0x00080000)) {			/* PLL2 bypassed?  */
> -		clk_pll2.rate = EP93XX_EXT_CLK_RATE;
> +		clk_pll2.rate = clk_xtali.rate;
>  	} else if (value & 0x00040000) {		/* PLL2 enabled?  */
>  		clk_pll2.rate = calc_pll_rate(value);
>  	} else { 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-08 21:28           ` Christian Gagneraud
@ 2009-10-08 21:34             ` Russell King - ARM Linux
  2009-10-08 22:09               ` Christian Gagneraud
  2009-10-08 22:32               ` H Hartley Sweeten
  2009-10-08 22:40             ` H Hartley Sweeten
  1 sibling, 2 replies; 18+ messages in thread
From: Russell King - ARM Linux @ 2009-10-08 21:34 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 08, 2009 at 10:28:46PM +0100, Christian Gagneraud wrote:
> I'm currently working on adding ADC support (no touchscreen for now) and 
> the problem is that touchscreen (ADC) and keypad use the same clock. With 
> this patch applied it would be possible to define the shared 
> ep93xx-keytch clock and add ep93xx-keypad and ep93xx-touchscreen as 
> children.

If it's a shared clock with common control, there's nothing to stop you
returning the same clock for both of them - that's how the clk API is
supposed to work.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-08 21:34             ` Russell King - ARM Linux
@ 2009-10-08 22:09               ` Christian Gagneraud
  2009-10-08 23:05                 ` H Hartley Sweeten
  2009-10-08 22:32               ` H Hartley Sweeten
  1 sibling, 1 reply; 18+ messages in thread
From: Christian Gagneraud @ 2009-10-08 22:09 UTC (permalink / raw)
  To: linux-arm-kernel

Russell King - ARM Linux wrote:
> On Thu, Oct 08, 2009 at 10:28:46PM +0100, Christian Gagneraud wrote:
>> I'm currently working on adding ADC support (no touchscreen for now) and 
>> the problem is that touchscreen (ADC) and keypad use the same clock. With 
>> this patch applied it would be possible to define the shared 
>> ep93xx-keytch clock and add ep93xx-keypad and ep93xx-touchscreen as 
>> children.
> 
> If it's a shared clock with common control, there's nothing to stop you
> returning the same clock for both of them - that's how the clk API is
> supposed to work.

Thanks, actually i was confused, the 2 clocks share the same 
configuration register, but have separate clock divisor and clock 
enable. So there's no need for clock parents. I just need to create a 
new clkdev for the TS/ADC.

Regards,
Chris


> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-08 21:34             ` Russell King - ARM Linux
  2009-10-08 22:09               ` Christian Gagneraud
@ 2009-10-08 22:32               ` H Hartley Sweeten
  2009-10-08 22:38                 ` Russell King - ARM Linux
  1 sibling, 1 reply; 18+ messages in thread
From: H Hartley Sweeten @ 2009-10-08 22:32 UTC (permalink / raw)
  To: linux-arm-kernel

Russell,

Did you see the locking I added for clk_enable / clk_disable?  Will that
cover the concern you had earlier? 

Regards,
Hartley

-----Original Message-----
From: linux-arm-kernel-bounces@lists.infradead.org [mailto:linux-arm-kernel-bounces at lists.infradead.org] On Behalf Of Russell King - ARM Linux
Sent: Thursday, October 08, 2009 2:34 PM
To: Christian Gagneraud
Cc: linux-arm-kernel
Subject: Re: [PATCH] ep93xx: introduce clk parent

On Thu, Oct 08, 2009 at 10:28:46PM +0100, Christian Gagneraud wrote:
> I'm currently working on adding ADC support (no touchscreen for now) and 
> the problem is that touchscreen (ADC) and keypad use the same clock. With 
> this patch applied it would be possible to define the shared 
> ep93xx-keytch clock and add ep93xx-keypad and ep93xx-touchscreen as 
> children.

If it's a shared clock with common control, there's nothing to stop you
returning the same clock for both of them - that's how the clk API is
supposed to work.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-08 22:32               ` H Hartley Sweeten
@ 2009-10-08 22:38                 ` Russell King - ARM Linux
  2009-10-08 22:45                   ` H Hartley Sweeten
  0 siblings, 1 reply; 18+ messages in thread
From: Russell King - ARM Linux @ 2009-10-08 22:38 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Oct 08, 2009 at 06:32:49PM -0400, H Hartley Sweeten wrote:
> Did you see the locking I added for clk_enable / clk_disable?  Will that
> cover the concern you had earlier? 

Yes, and yes.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-08 21:28           ` Christian Gagneraud
  2009-10-08 21:34             ` Russell King - ARM Linux
@ 2009-10-08 22:40             ` H Hartley Sweeten
  2009-10-08 23:14               ` Christian Gagneraud
  1 sibling, 1 reply; 18+ messages in thread
From: H Hartley Sweeten @ 2009-10-08 22:40 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday, October 08, 2009 2:29 PM, Christian Gagneraud wrote:
> Hi,
> 
> What's the status of this patch? It does not appear on the patch 
> tracker, and it's not in linus tree (or I'm missing something).
> 
> I'm currently working on adding ADC support (no touchscreen for now) 
> and the problem is that touchscreen (ADC) and keypad use the same 
> clock. With this patch applied it would be possible to define the 
> shared ep93xx-keytch clock and add ep93xx-keypad and 
> ep93xx-touchscreen as children.

Christian,

I have a patch for clock.c that adds the keypad clock.  This will
also give you what you need to configure the adc clock.  Hopefully
I will get a chance to post that patch in the next couple days.

As Russell already mentioned, the clk parent patch does not effect
the adc clock.  The common clock between them is actually the external
crystal.

The only thing they share is the configuration register used to set
the clock divisor and to enable the clock.  For the adc clock you
will just need to add:

+static struct clk clk_adc = {
+	.parent		= &clk_xtali,
+	.sw_locked	= 1,
+	.enable_reg	= EP93XX_SYSCON_KEYTCHCLKDIV,
+	.enable_mask	= EP93XX_SYSCON_KEYTCHCLKDIV_TSEN,
+	.set_rate	= set_keytchclk_rate,
+};

And of course the proper INIT_CK() for clk_adc.  The set_rate
callback will be the same one used for the keypad, which will
be in my coming patch.

Regards,
Hartley

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-08 22:38                 ` Russell King - ARM Linux
@ 2009-10-08 22:45                   ` H Hartley Sweeten
  0 siblings, 0 replies; 18+ messages in thread
From: H Hartley Sweeten @ 2009-10-08 22:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday, October 08, 2009 3:38 PM, Russell King wrote:
> On Thu, Oct 08, 2009 at 06:32:49PM -0400, H Hartley Sweeten wrote:
>> Did you see the locking I added for clk_enable / clk_disable?  Will that
>> cover the concern you had earlier? 
>
> Yes, and yes.

Thank you.  Added to patch tracker as 5756/1.

Regards,
Hartley

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-08 22:09               ` Christian Gagneraud
@ 2009-10-08 23:05                 ` H Hartley Sweeten
  0 siblings, 0 replies; 18+ messages in thread
From: H Hartley Sweeten @ 2009-10-08 23:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday, October 08, 2009 3:10 PM, Christian Gagneraud wrote:
> Russell King - ARM Linux wrote:
>> On Thu, Oct 08, 2009 at 10:28:46PM +0100, Christian Gagneraud wrote:
>>> I'm currently working on adding ADC support (no touchscreen for now) and 
>>> the problem is that touchscreen (ADC) and keypad use the same clock. With 
>>> this patch applied it would be possible to define the shared 
>>> ep93xx-keytch clock and add ep93xx-keypad and ep93xx-touchscreen as 
>>> children.
>> 
>> If it's a shared clock with common control, there's nothing to stop you
>> returning the same clock for both of them - that's how the clk API is
>> supposed to work.
>
> Thanks, actually i was confused, the 2 clocks share the same 
> configuration register, but have separate clock divisor and clock 
> enable. So there's no need for clock parents. I just need to create a 
> new clkdev for the TS/ADC.

Doh!

The keypad clock is already in clock.c, as is the set_rate callback.  The
necessary Syscon defines are also already in ep93xx-regs.h.  I just put
the clk parent patch into Russell's patch tracker so all you need add for
the adc clock is:

 static struct clk clk_keypad = {
	.parent		= &clk_xtali,
 	.sw_locked	= 1,
 	.enable_reg	= EP93XX_SYSCON_KEYTCHCLKDIV,
 	.enable_mask	= EP93XX_SYSCON_KEYTCHCLKDIV_KEN,
 	.set_rate	= set_keytchclk_rate,
 };
+static struct clk clk_adc = {
+	.parent		= &clk_xtali,
+	.sw_locked	= 1,
+	.enable_reg	= EP93XX_SYSCON_KEYTCHCLKDIV,
+	.enable_mask	= EP93XX_SYSCON_KEYTCHCLKDIV_TSEN,
+	.set_rate	= set_keytchclk_rate,
+};
 static struct clk clk_pwm = {
 	.rate		= EP93XX_EXT_CLK_RATE,
 };

And:

	INIT_CK("ep93xx-keypad",	NULL,		&clk_keypad),
+	INIT_CK("ep93xx-adc",		NULL,		&clk_adc),
	INIT_CK("ep93xx-fb",		NULL,		&clk_video),


Make sure you look at set_keytchclk_rate() to see how the adc
clock is configured.  Basically the only two rates that are valid
are EP93XX_KEYTCHCLK_DIV4 and EP93XX_KEYTCHCLK_DIV16.

Regards,
Hartley

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-08 22:40             ` H Hartley Sweeten
@ 2009-10-08 23:14               ` Christian Gagneraud
  2009-10-08 23:25                 ` H Hartley Sweeten
  0 siblings, 1 reply; 18+ messages in thread
From: Christian Gagneraud @ 2009-10-08 23:14 UTC (permalink / raw)
  To: linux-arm-kernel

H Hartley Sweeten wrote:
> On Thursday, October 08, 2009 2:29 PM, Christian Gagneraud wrote:
>> Hi,
>>
>> What's the status of this patch? It does not appear on the patch 
>> tracker, and it's not in linus tree (or I'm missing something).
>>
>> I'm currently working on adding ADC support (no touchscreen for now) 
>> and the problem is that touchscreen (ADC) and keypad use the same 
>> clock. With this patch applied it would be possible to define the 
>> shared ep93xx-keytch clock and add ep93xx-keypad and 
>> ep93xx-touchscreen as children.
> 
> Christian,
> 
> I have a patch for clock.c that adds the keypad clock.  This will
> also give you what you need to configure the adc clock.  Hopefully
> I will get a chance to post that patch in the next couple days.
> 
> As Russell already mentioned, the clk parent patch does not effect
> the adc clock.  The common clock between them is actually the external
> crystal.
> 
> The only thing they share is the configuration register used to set
> the clock divisor and to enable the clock.  For the adc clock you
> will just need to add:
> 
> +static struct clk clk_adc = {
> +	.parent		= &clk_xtali,
> +	.sw_locked	= 1,
> +	.enable_reg	= EP93XX_SYSCON_KEYTCHCLKDIV,
> +	.enable_mask	= EP93XX_SYSCON_KEYTCHCLKDIV_TSEN,
> +	.set_rate	= set_keytchclk_rate,
> +};
> 
> And of course the proper INIT_CK() for clk_adc.  The set_rate
> callback will be the same one used for the keypad, which will
> be in my coming patch.

Hartley,

The keypad clock is already in as is the et_keytchclk_rate too.

Can you explain me by the way why the keypad declares the clock with 
dev_id="ep93xx-keypad" and no con_id, but the pwm declares the clock 
with no dev_id and with con_id="clk_pwm"
Which of dev and con is the consumer and which one is the producer?
I'm a bit confuse between the platform_driver.name, the clock.dev_id 
and the clock.con_id

As well I've noticed that the EP93XX keypad input driver does a 
clock_get("clk_key"), but this clock doesn't exist. or did I miss 
something.


Regards,
Chris

> 
> Regards,
> Hartley

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-08 23:14               ` Christian Gagneraud
@ 2009-10-08 23:25                 ` H Hartley Sweeten
  2009-10-09 17:11                   ` Christian Gagneraud
  0 siblings, 1 reply; 18+ messages in thread
From: H Hartley Sweeten @ 2009-10-08 23:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Thursday, October 08, 2009 4:15 PM, Christian Gagneraud wrote:
> Hartley,
>
> The keypad clock is already in as is the et_keytchclk_rate too.

Saw that.  Already responded to you about it.

> Can you explain me by the way why the keypad declares the clock with 
> dev_id="ep93xx-keypad" and no con_id, but the pwm declares the clock 
> with no dev_id and with con_id="clk_pwm"
> Which of dev and con is the consumer and which one is the producer?
> I'm a bit confuse between the platform_driver.name, the clock.dev_id 
> and the clock.con_id
>
> As well I've noticed that the EP93XX keypad input driver does a 
> clock_get("clk_key"), but this clock doesn't exist. or did I miss 
> something.

No, I did...

When the input group picked up the keypad driver I didn't get the core
support files reviewed/merged.  So basically the keypad driver is
currently broken.

Patch 5578/1 was the first patch needed to fix this.  During the review
for it, the original use intended for by the driver changed.  So I'm in
the process of fixing the keypad driver.  Hopefully I'll get this all
resolved soon.

As far as the dev/con id's.

I could have created two clocks for the pwms.  One for dev_id="pwm.0" and
one for dev_id="pwm.1" which is how the two pwm ports get identified when
used.  But, since the pwm clock cannot be disabled it seemed cleaner to
just add one clock, con_id="clk_pwm", since the clock is common for both
pwm controllers.  Look at drivers/misc/ep93xx_pwm.c and you will see that
the clk_get() call passes the dev_id and the con_id.  Since there is not
a matching dev_id in the ep93xx clocks[], the match occurs based on the
con_id.

Make sense?

Regards,
Hartley

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-08 23:25                 ` H Hartley Sweeten
@ 2009-10-09 17:11                   ` Christian Gagneraud
  2009-10-09 17:30                     ` H Hartley Sweeten
  0 siblings, 1 reply; 18+ messages in thread
From: Christian Gagneraud @ 2009-10-09 17:11 UTC (permalink / raw)
  To: linux-arm-kernel

H Hartley Sweeten wrote:
> On Thursday, October 08, 2009 4:15 PM, Christian Gagneraud wrote:
>> Hartley,
>>
>> The keypad clock is already in as is the et_keytchclk_rate too.
> 
> Saw that.  Already responded to you about it.
> 
>> Can you explain me by the way why the keypad declares the clock with 
>> dev_id="ep93xx-keypad" and no con_id, but the pwm declares the clock 
>> with no dev_id and with con_id="clk_pwm"
>> Which of dev and con is the consumer and which one is the producer?
>> I'm a bit confuse between the platform_driver.name, the clock.dev_id 
>> and the clock.con_id
>>
>> As well I've noticed that the EP93XX keypad input driver does a 
>> clock_get("clk_key"), but this clock doesn't exist. or did I miss 
>> something.
> 
> No, I did...
> 
> When the input group picked up the keypad driver I didn't get the core
> support files reviewed/merged.  So basically the keypad driver is
> currently broken.
> 
> Patch 5578/1 was the first patch needed to fix this.  During the review
> for it, the original use intended for by the driver changed.  So I'm in
> the process of fixing the keypad driver.  Hopefully I'll get this all
> resolved soon.
> 
> As far as the dev/con id's.
> 
> I could have created two clocks for the pwms.  One for dev_id="pwm.0" and
> one for dev_id="pwm.1" which is how the two pwm ports get identified when
> used.  But, since the pwm clock cannot be disabled it seemed cleaner to
> just add one clock, con_id="clk_pwm", since the clock is common for both
> pwm controllers.  Look at drivers/misc/ep93xx_pwm.c and you will see that
> the clk_get() call passes the dev_id and the con_id.  Since there is not
> a matching dev_id in the ep93xx clocks[], the match occurs based on the
> con_id.

OK, I'll do that.

I've noticed as well that there's missing bits from Matthieu's 
0024-ep93xx_pwm.patch, that's almost nothing, but I guess that was the 
main reason Matthieu added PWM support to EP93XX:

diff --git a/arch/arm/mach-ep93xx/ts72xx.c b/arch/arm/mach-ep93xx/ts72xx.c
index 086d069..ee7baeb 100644
--- a/arch/arm/mach-ep93xx/ts72xx.c
+++ b/arch/arm/mach-ep93xx/ts72xx.c
@@ -303,6 +303,9 @@ static void __init ts72xx_init_machine(void)
         spi_register_board_info(ts72xx_spi_bus, 
ARRAY_SIZE(ts72xx_spi_bus));
         #endif

+       /* PWM1 is DIO_6 on TS-72xx header */
+       ep93xx_register_pwm(1, 1);
+
         ep93xx_register_eth(&ts72xx_eth_data, 1);
  }

> 
> Make sense?

Yes it does! :)

Regards,
Chris

> 
> Regards,
> Hartley

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] ep93xx: introduce clk parent
  2009-10-09 17:11                   ` Christian Gagneraud
@ 2009-10-09 17:30                     ` H Hartley Sweeten
  0 siblings, 0 replies; 18+ messages in thread
From: H Hartley Sweeten @ 2009-10-09 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, October 09, 2009 10:11 AM, Christian Gagneraud wrote:
> I've noticed as well that there's missing bits from Matthieu's 
> 0024-ep93xx_pwm.patch, that's almost nothing, but I guess that was the 
> main reason Matthieu added PWM support to EP93XX:
> 
> diff --git a/arch/arm/mach-ep93xx/ts72xx.c b/arch/arm/mach-ep93xx/ts72xx.c
> index 086d069..ee7baeb 100644
> --- a/arch/arm/mach-ep93xx/ts72xx.c
> +++ b/arch/arm/mach-ep93xx/ts72xx.c
> @@ -303,6 +303,9 @@ static void __init ts72xx_init_machine(void)
>          spi_register_board_info(ts72xx_spi_bus, 
> ARRAY_SIZE(ts72xx_spi_bus));
>          #endif
> 
> +       /* PWM1 is DIO_6 on TS-72xx header */
> +       ep93xx_register_pwm(1, 1);
> +
>          ep93xx_register_eth(&ts72xx_eth_data, 1);
>   }

Matthieu must have rebased his yahoo patches recently.  The copy I have
registered his old driver as:

+	// DIO_6 on TS-72xx header
+	#if defined(CONFIG_EP93XX_PWM) || defined(CONFIG_EP93XX_PWM_MODULE)
+	(void) platform_device_register_simple("ep93xx-pwm", 1, NULL, 0);
+	#endif 

When I submitted the updated pwm driver I didn't add the code to register
the pwm peripheral in any of the platform init's.

If you want to use the pwm signals just add the ep93xx_register_pwm() call
to your init_machine().

Regards,
Hartley

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2009-10-09 17:30 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-09-16 22:57 [PATCH] ep93xx: introduce clk parent H Hartley Sweeten
2009-09-23 22:48 ` H Hartley Sweeten
2009-09-30 19:53   ` Ryan Mallon
2009-09-30 21:30     ` H Hartley Sweeten
2009-10-01 14:59       ` Russell King - ARM Linux
2009-10-01 16:32         ` H Hartley Sweeten
2009-10-08 21:28           ` Christian Gagneraud
2009-10-08 21:34             ` Russell King - ARM Linux
2009-10-08 22:09               ` Christian Gagneraud
2009-10-08 23:05                 ` H Hartley Sweeten
2009-10-08 22:32               ` H Hartley Sweeten
2009-10-08 22:38                 ` Russell King - ARM Linux
2009-10-08 22:45                   ` H Hartley Sweeten
2009-10-08 22:40             ` H Hartley Sweeten
2009-10-08 23:14               ` Christian Gagneraud
2009-10-08 23:25                 ` H Hartley Sweeten
2009-10-09 17:11                   ` Christian Gagneraud
2009-10-09 17:30                     ` H Hartley Sweeten

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).