[PATCH] sh: clkfwk: Changed the init function

[PATCH] sh: clkfwk: Changed the init function

[Francesco VIRLINZI]

[-- Attachment #1: Type: text/plain, Size: 231 bytes --]

Hi all

This patch changes the init field in the clk_ops structure.
Moreover it changes how the init function is used.
Now it's called during registration and if something was wrong the clock 
isn't registered.

Regards
 Francesco

[-- Attachment #2: 0002-sh-clkfwk-Changed-the-init-callback-from-void-to-i.patch --]
[-- Type: text/x-patch, Size: 14672 bytes --]

From 24022f871a04d09a0884fe7e8185871081cad44a Mon Sep 17 00:00:00 2001
From: Francesco Virlinzi <francesco.virlinzi@st.com>
Date: Fri, 13 Mar 2009 09:03:04 +0100
Subject: [PATCH] sh: clkfwk: Changed the init callback from void to int

This patch changes the clk_ops.init function pointer
to return a value.
Moreover it change the clk_register function to call
always the clk_ops.init function to initilize the clock
if there is an error the clock _isn't_ registered.

Signed-off-by: Francesco Virlinzi <francesco.virlinzi@st.com>
---
 arch/sh/include/asm/clock.h            |    2 +-
 arch/sh/kernel/cpu/clock.c             |   18 +++++-------------
 arch/sh/kernel/cpu/sh2/clock-sh7619.c  |    3 ++-
 arch/sh/kernel/cpu/sh2a/clock-sh7201.c |    3 ++-
 arch/sh/kernel/cpu/sh2a/clock-sh7203.c |    3 ++-
 arch/sh/kernel/cpu/sh2a/clock-sh7206.c |    3 ++-
 arch/sh/kernel/cpu/sh3/clock-sh3.c     |    3 ++-
 arch/sh/kernel/cpu/sh3/clock-sh7705.c  |    3 ++-
 arch/sh/kernel/cpu/sh3/clock-sh7706.c  |    3 ++-
 arch/sh/kernel/cpu/sh3/clock-sh7709.c  |    3 ++-
 arch/sh/kernel/cpu/sh3/clock-sh7712.c  |    3 ++-
 arch/sh/kernel/cpu/sh4/clock-sh4-202.c |    3 ++-
 arch/sh/kernel/cpu/sh4/clock-sh4.c     |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7722.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7763.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7770.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7780.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7785.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7786.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-shx3.c   |    3 ++-
 arch/sh/kernel/cpu/sh5/clock-sh5.c     |    3 ++-
 21 files changed, 44 insertions(+), 33 deletions(-)

diff --git a/arch/sh/include/asm/clock.h b/arch/sh/include/asm/clock.h
index 2f6c962..f7a5899 100644
--- a/arch/sh/include/asm/clock.h
+++ b/arch/sh/include/asm/clock.h
@@ -10,7 +10,7 @@
 struct clk;
 
 struct clk_ops {
-	void (*init)(struct clk *clk);
+	int (*init)(struct clk *clk);
 	void (*enable)(struct clk *clk);
 	void (*disable)(struct clk *clk);
 	void (*recalc)(struct clk *clk);
diff --git a/arch/sh/kernel/cpu/clock.c b/arch/sh/kernel/cpu/clock.c
index e2d2451..87a54b5 100644
--- a/arch/sh/kernel/cpu/clock.c
+++ b/arch/sh/kernel/cpu/clock.c
@@ -92,17 +92,6 @@ static void propagate_rate(struct clk *clk)
 
 static int __clk_enable(struct clk *clk)
 {
-	/*
-	 * See if this is the first time we're enabling the clock, some
-	 * clocks that are always enabled still require "special"
-	 * initialization. This is especially true if the clock mode
-	 * changes and the clock needs to hunt for the proper set of
-	 * divisors to use before it can effectively recalc.
-	 */
-	if (unlikely(atomic_read(&clk->kref.refcount) == 1))
-		if (clk->ops && clk->ops->init)
-			clk->ops->init(clk);
-
 	kref_get(&clk->kref);
 
 	if (clk->flags & CLK_ALWAYS_ENABLED)
@@ -167,6 +156,11 @@ EXPORT_SYMBOL_GPL(clk_disable);
 
 int clk_register(struct clk *clk)
 {
+
+	if (clk->ops && clk->ops->init)
+		if (clk->ops->init(clk))
+			return -EINVAL;
+
 	mutex_lock(&clock_list_sem);
 
 	list_add(&clk->node, &clock_list);
@@ -176,8 +170,6 @@ int clk_register(struct clk *clk)
 
 	if (clk->flags & CLK_ALWAYS_ENABLED) {
 		pr_debug( "Clock '%s' is ALWAYS_ENABLED\n", clk->name);
-		if (clk->ops && clk->ops->init)
-			clk->ops->init(clk);
 		if (clk->ops && clk->ops->enable)
 			clk->ops->enable(clk);
 		pr_debug( "Enabled.");
diff --git a/arch/sh/kernel/cpu/sh2/clock-sh7619.c b/arch/sh/kernel/cpu/sh2/clock-sh7619.c
index d2c1579..5549534 100644
--- a/arch/sh/kernel/cpu/sh2/clock-sh7619.c
+++ b/arch/sh/kernel/cpu/sh2/clock-sh7619.c
@@ -29,9 +29,10 @@ static const int pfc_divisors[] = {1,2,0,4};
 #error "Illigal Clock Mode!"
 #endif
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= PLL2 * pll1rate[(ctrl_inw(FREQCR) >> 8) & 7];
+	return 0;
 }
 
 static struct clk_ops sh7619_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh2a/clock-sh7201.c b/arch/sh/kernel/cpu/sh2a/clock-sh7201.c
index 4a5e597..90a183a 100644
--- a/arch/sh/kernel/cpu/sh2a/clock-sh7201.c
+++ b/arch/sh/kernel/cpu/sh2a/clock-sh7201.c
@@ -32,9 +32,10 @@ static const int pfc_divisors[]={1,2,3,4,6,8,12};
 #error "Illegal Clock Mode!"
 #endif
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate = 10000000 * PLL2 * pll1rate[(ctrl_inw(FREQCR) >> 8) & 0x0007];
+	return 0;
 }
 
 static struct clk_ops sh7201_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh2a/clock-sh7203.c b/arch/sh/kernel/cpu/sh2a/clock-sh7203.c
index fb78132..2efd225 100644
--- a/arch/sh/kernel/cpu/sh2a/clock-sh7203.c
+++ b/arch/sh/kernel/cpu/sh2a/clock-sh7203.c
@@ -37,9 +37,10 @@ static const int pfc_divisors[]={1,2,3,4,6,8,12};
 #error "Illegal Clock Mode!"
 #endif
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pll1rate[(ctrl_inw(FREQCR) >> 8) & 0x0003] * PLL2 ;
+	return 0;
 }
 
 static struct clk_ops sh7203_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh2a/clock-sh7206.c b/arch/sh/kernel/cpu/sh2a/clock-sh7206.c
index 82d7f99..f374c7e 100644
--- a/arch/sh/kernel/cpu/sh2a/clock-sh7206.c
+++ b/arch/sh/kernel/cpu/sh2a/clock-sh7206.c
@@ -32,9 +32,10 @@ static const int pfc_divisors[]={1,2,3,4,6,8,12};
 #error "Illigal Clock Mode!"
 #endif
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= PLL2 * pll1rate[(ctrl_inw(FREQCR) >> 8) & 0x0007];
+	return 0;
 }
 
 static struct clk_ops sh7206_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh3/clock-sh3.c b/arch/sh/kernel/cpu/sh3/clock-sh3.c
index c3c9459..47f3591 100644
--- a/arch/sh/kernel/cpu/sh3/clock-sh3.c
+++ b/arch/sh/kernel/cpu/sh3/clock-sh3.c
@@ -26,12 +26,13 @@ static int stc_multipliers[] = { 1, 2, 3, 4, 6, 1, 1, 1 };
 static int ifc_divisors[]    = { 1, 2, 3, 4, 1, 1, 1, 1 };
 static int pfc_divisors[]    = { 1, 2, 3, 4, 6, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	int frqcr = ctrl_inw(FRQCR);
 	int idx = ((frqcr & 0x2000) >> 11) | (frqcr & 0x0003);
 
 	clk->rate *= pfc_divisors[idx];
+	return 0;
 }
 
 static struct clk_ops sh3_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh3/clock-sh7705.c b/arch/sh/kernel/cpu/sh3/clock-sh7705.c
index dfdbf32..2943c46 100644
--- a/arch/sh/kernel/cpu/sh3/clock-sh7705.c
+++ b/arch/sh/kernel/cpu/sh3/clock-sh7705.c
@@ -30,9 +30,10 @@ static int stc_multipliers[] = { 1, 2, 3, 4, 6, 1, 1, 1 };
 static int ifc_divisors[]    = { 1, 2, 3, 4, 1, 1, 1, 1 };
 static int pfc_divisors[]    = { 1, 2, 3, 4, 6, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[ctrl_inw(FRQCR) & 0x0003];
+	return 0;
 }
 
 static struct clk_ops sh7705_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh3/clock-sh7706.c b/arch/sh/kernel/cpu/sh3/clock-sh7706.c
index 0cf96f9..6397122 100644
--- a/arch/sh/kernel/cpu/sh3/clock-sh7706.c
+++ b/arch/sh/kernel/cpu/sh3/clock-sh7706.c
@@ -22,12 +22,13 @@ static int stc_multipliers[] = { 1, 2, 4, 1, 3, 6, 1, 1 };
 static int ifc_divisors[]    = { 1, 2, 4, 1, 3, 1, 1, 1 };
 static int pfc_divisors[]    = { 1, 2, 4, 1, 3, 6, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	int frqcr = ctrl_inw(FRQCR);
 	int idx = ((frqcr & 0x2000) >> 11) | (frqcr & 0x0003);
 
 	clk->rate *= pfc_divisors[idx];
+	return 0;
 }
 
 static struct clk_ops sh7706_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh3/clock-sh7709.c b/arch/sh/kernel/cpu/sh3/clock-sh7709.c
index b791a29..4cc4006 100644
--- a/arch/sh/kernel/cpu/sh3/clock-sh7709.c
+++ b/arch/sh/kernel/cpu/sh3/clock-sh7709.c
@@ -29,12 +29,13 @@ static void set_bus_parent(struct clk *clk)
 	clk_put(bus_clk);
 }
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	int frqcr = ctrl_inw(FRQCR);
 	int idx = ((frqcr & 0x2000) >> 11) | (frqcr & 0x0003);
 
 	clk->rate *= pfc_divisors[idx];
+	return 0;
 }
 
 static struct clk_ops sh7709_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh3/clock-sh7712.c b/arch/sh/kernel/cpu/sh3/clock-sh7712.c
index 54f54df..50c8033 100644
--- a/arch/sh/kernel/cpu/sh3/clock-sh7712.c
+++ b/arch/sh/kernel/cpu/sh3/clock-sh7712.c
@@ -21,12 +21,13 @@
 static int multipliers[] = { 1, 2, 3 };
 static int divisors[]    = { 1, 2, 3, 4, 6 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	int frqcr = ctrl_inw(FRQCR);
 	int idx = (frqcr & 0x0300) >> 8;
 
 	clk->rate *= multipliers[idx];
+	return 0;
 }
 
 static struct clk_ops sh7712_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4/clock-sh4-202.c b/arch/sh/kernel/cpu/sh4/clock-sh4-202.c
index a334294..2c46a51 100644
--- a/arch/sh/kernel/cpu/sh4/clock-sh4-202.c
+++ b/arch/sh/kernel/cpu/sh4/clock-sh4-202.c
@@ -66,7 +66,7 @@ static struct clk sh4202_femi_clk = {
 	.ops		= &sh4202_femi_clk_ops,
 };
 
-static void shoc_clk_init(struct clk *clk)
+static int shoc_clk_init(struct clk *clk)
 {
 	int i;
 
@@ -88,6 +88,7 @@ static void shoc_clk_init(struct clk *clk)
 	}
 
 	WARN_ON(i == ARRAY_SIZE(frqcr3_divisors));	/* Undefined clock */
+	return 0;
 }
 
 static void shoc_clk_recalc(struct clk *clk)
diff --git a/arch/sh/kernel/cpu/sh4/clock-sh4.c b/arch/sh/kernel/cpu/sh4/clock-sh4.c
index dca9f87..70794ac 100644
--- a/arch/sh/kernel/cpu/sh4/clock-sh4.c
+++ b/arch/sh/kernel/cpu/sh4/clock-sh4.c
@@ -26,9 +26,10 @@ static int ifc_divisors[] = { 1, 2, 3, 4, 6, 8, 1, 1 };
 #define bfc_divisors ifc_divisors	/* Same */
 static int pfc_divisors[] = { 2, 3, 4, 6, 8, 2, 2, 2 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[ctrl_inw(FRQCR) & 0x0007];
+	return 0;
 }
 
 static struct clk_ops sh4_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7722.c b/arch/sh/kernel/cpu/sh4a/clock-sh7722.c
index 0e174af..bbfbd6f 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7722.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7722.c
@@ -148,12 +148,13 @@ static void master_clk_recalc(struct clk *clk)
 	clk->rate = CONFIG_SH_PCLK_FREQ * (((frqcr >> 24) & 0x1f) + 1);
 }
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->parent = NULL;
 	clk->flags |= CLK_RATE_PROPAGATES;
 	clk->rate = CONFIG_SH_PCLK_FREQ;
 	master_clk_recalc(clk);
+	return 0;
 }
 
 
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7763.c b/arch/sh/kernel/cpu/sh4a/clock-sh7763.c
index 3177d0d..934fbdd 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7763.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7763.c
@@ -20,9 +20,10 @@ static int bfc_divisors[] = { 1, 1, 1, 8, 1, 1, 1, 1 };
 static int p0fc_divisors[] = { 1, 1, 1, 8, 1, 1, 1, 1 };
 static int cfc_divisors[] = { 1, 1, 4, 1, 1, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= p0fc_divisors[(ctrl_inl(FRQCR) >> 4) & 0x07];
+	return 0;
 }
 
 static struct clk_ops sh7763_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7770.c b/arch/sh/kernel/cpu/sh4a/clock-sh7770.c
index 8e23606..2d21990 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7770.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7770.c
@@ -19,9 +19,10 @@ static int ifc_divisors[] = { 1, 1, 1, 1, 1, 1, 1, 1 };
 static int bfc_divisors[] = { 1, 1, 1, 1, 1, 8,12, 1 };
 static int pfc_divisors[] = { 1, 8, 1,10,12,16, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[(ctrl_inl(FRQCR) >> 28) & 0x000f];
+	return 0;
 }
 
 static struct clk_ops sh7770_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7780.c b/arch/sh/kernel/cpu/sh4a/clock-sh7780.c
index 01f3da6..fe82df4 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7780.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7780.c
@@ -20,9 +20,10 @@ static int bfc_divisors[] = { 1, 1, 1, 8, 12, 16, 24, 1 };
 static int pfc_divisors[] = { 1, 24, 24, 1 };
 static int cfc_divisors[] = { 1, 1, 4, 1, 6, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[ctrl_inl(FRQCR) & 0x0003];
+	return 0;
 }
 
 static struct clk_ops sh7780_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7785.c b/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
index 27fa81b..ca43ad1 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
@@ -24,9 +24,10 @@ static int mfc_divisors[] = { 1, 1, 4, 6 };
 static int pfc_divisors[] = { 1, 1, 1, 1, 1, 1, 1, 18,
 			      24, 32, 36, 48, 1, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[ctrl_inl(FRQMR1) & 0x000f];
+	return 0;
 }
 
 static struct clk_ops sh7785_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7786.c b/arch/sh/kernel/cpu/sh4a/clock-sh7786.c
index f84a9c1..2d1d3e2 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7786.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7786.c
@@ -27,9 +27,10 @@ static int mfc_divisors[] = { 1, 1, 4, 1 };
 static int pfc_divisors[] = { 1, 1, 1, 1, 1, 1, 16, 1,
 			      24, 32, 1, 48, 1, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[ctrl_inl(FRQMR1) & 0x000f];
+	return 0;
 }
 
 static struct clk_ops sh7786_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-shx3.c b/arch/sh/kernel/cpu/sh4a/clock-shx3.c
index c630b29..7b0f4b8 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-shx3.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-shx3.c
@@ -31,9 +31,10 @@ static int cfc_divisors[] = { 1, 1, 4, 6 };
 #define PFC_POS		0
 #define CFC_POS		20
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[(ctrl_inl(FRQCR) >> PFC_POS) & PFC_MSK];
+	return 0;
 }
 
 static struct clk_ops shx3_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh5/clock-sh5.c b/arch/sh/kernel/cpu/sh5/clock-sh5.c
index 52c4924..89abffe 100644
--- a/arch/sh/kernel/cpu/sh5/clock-sh5.c
+++ b/arch/sh/kernel/cpu/sh5/clock-sh5.c
@@ -22,10 +22,11 @@ static int ifc_table[] = { 2, 4, 6, 8, 10, 12, 16, 24 };
 
 static unsigned long cprc_base;
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	int idx = (ctrl_inl(cprc_base + 0x00) >> 6) & 0x0007;
 	clk->rate *= ifc_table[idx];
+	return 0;
 }
 
 static struct clk_ops sh5_master_clk_ops = {
-- 
1.5.6.6

Re: [PATCH] sh: clkfwk: Changed the init function

[Francesco VIRLINZI]

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

Sorry I forgot the timers..
Here the right patch
Regards
 Francesco
Francesco VIRLINZI ha scritto:
> Hi all
>
> This patch changes the init field in the clk_ops structure.
> Moreover it changes how the init function is used.
> Now it's called during registration and if something was wrong the 
> clock isn't registered.
>
> Regards
> Francesco


[-- Attachment #2: 0002-sh-clkfwk-Changed-the-init-field-to-return-a-value.patch --]
[-- Type: text/x-patch, Size: 16759 bytes --]

From 942219bb95734329c858587806e49895bae29208 Mon Sep 17 00:00:00 2001
From: Francesco Virlinzi <francesco.virlinzi@st.com>
Date: Fri, 13 Mar 2009 09:32:45 +0100
Subject: [PATCH] sh: clkfwk: Changed the init field to return a value

This patch changes the clk_ops.init function pointer
to return a value.
Moreover it change the clk_register function to call
always the clk_ops.init function to initilize the clock
if there is an error the clock _isn't_ registered.

Signed-off-by: Francesco Virlinzi <francesco.virlinzi@st.com>
---
 arch/sh/include/asm/clock.h            |    2 +-
 arch/sh/kernel/cpu/clock.c             |   18 +++++-------------
 arch/sh/kernel/cpu/sh2/clock-sh7619.c  |    3 ++-
 arch/sh/kernel/cpu/sh2a/clock-sh7201.c |    3 ++-
 arch/sh/kernel/cpu/sh2a/clock-sh7203.c |    3 ++-
 arch/sh/kernel/cpu/sh2a/clock-sh7206.c |    3 ++-
 arch/sh/kernel/cpu/sh3/clock-sh3.c     |    3 ++-
 arch/sh/kernel/cpu/sh3/clock-sh7705.c  |    3 ++-
 arch/sh/kernel/cpu/sh3/clock-sh7706.c  |    3 ++-
 arch/sh/kernel/cpu/sh3/clock-sh7709.c  |    3 ++-
 arch/sh/kernel/cpu/sh3/clock-sh7712.c  |    3 ++-
 arch/sh/kernel/cpu/sh4/clock-sh4-202.c |    3 ++-
 arch/sh/kernel/cpu/sh4/clock-sh4.c     |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7722.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7763.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7770.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7780.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7785.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-sh7786.c |    3 ++-
 arch/sh/kernel/cpu/sh4a/clock-shx3.c   |    3 ++-
 arch/sh/kernel/cpu/sh5/clock-sh5.c     |    3 ++-
 arch/sh/kernel/timers/timer-cmt.c      |    3 ++-
 arch/sh/kernel/timers/timer-mtu2.c     |    3 ++-
 arch/sh/kernel/timers/timer-tmu.c      |    3 ++-
 24 files changed, 50 insertions(+), 36 deletions(-)

diff --git a/arch/sh/include/asm/clock.h b/arch/sh/include/asm/clock.h
index 2f6c962..f7a5899 100644
--- a/arch/sh/include/asm/clock.h
+++ b/arch/sh/include/asm/clock.h
@@ -10,7 +10,7 @@
 struct clk;
 
 struct clk_ops {
-	void (*init)(struct clk *clk);
+	int (*init)(struct clk *clk);
 	void (*enable)(struct clk *clk);
 	void (*disable)(struct clk *clk);
 	void (*recalc)(struct clk *clk);
diff --git a/arch/sh/kernel/cpu/clock.c b/arch/sh/kernel/cpu/clock.c
index e2d2451..87a54b5 100644
--- a/arch/sh/kernel/cpu/clock.c
+++ b/arch/sh/kernel/cpu/clock.c
@@ -92,17 +92,6 @@ static void propagate_rate(struct clk *clk)
 
 static int __clk_enable(struct clk *clk)
 {
-	/*
-	 * See if this is the first time we're enabling the clock, some
-	 * clocks that are always enabled still require "special"
-	 * initialization. This is especially true if the clock mode
-	 * changes and the clock needs to hunt for the proper set of
-	 * divisors to use before it can effectively recalc.
-	 */
-	if (unlikely(atomic_read(&clk->kref.refcount) == 1))
-		if (clk->ops && clk->ops->init)
-			clk->ops->init(clk);
-
 	kref_get(&clk->kref);
 
 	if (clk->flags & CLK_ALWAYS_ENABLED)
@@ -167,6 +156,11 @@ EXPORT_SYMBOL_GPL(clk_disable);
 
 int clk_register(struct clk *clk)
 {
+
+	if (clk->ops && clk->ops->init)
+		if (clk->ops->init(clk))
+			return -EINVAL;
+
 	mutex_lock(&clock_list_sem);
 
 	list_add(&clk->node, &clock_list);
@@ -176,8 +170,6 @@ int clk_register(struct clk *clk)
 
 	if (clk->flags & CLK_ALWAYS_ENABLED) {
 		pr_debug( "Clock '%s' is ALWAYS_ENABLED\n", clk->name);
-		if (clk->ops && clk->ops->init)
-			clk->ops->init(clk);
 		if (clk->ops && clk->ops->enable)
 			clk->ops->enable(clk);
 		pr_debug( "Enabled.");
diff --git a/arch/sh/kernel/cpu/sh2/clock-sh7619.c b/arch/sh/kernel/cpu/sh2/clock-sh7619.c
index d2c1579..5549534 100644
--- a/arch/sh/kernel/cpu/sh2/clock-sh7619.c
+++ b/arch/sh/kernel/cpu/sh2/clock-sh7619.c
@@ -29,9 +29,10 @@ static const int pfc_divisors[] = {1,2,0,4};
 #error "Illigal Clock Mode!"
 #endif
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= PLL2 * pll1rate[(ctrl_inw(FREQCR) >> 8) & 7];
+	return 0;
 }
 
 static struct clk_ops sh7619_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh2a/clock-sh7201.c b/arch/sh/kernel/cpu/sh2a/clock-sh7201.c
index 4a5e597..90a183a 100644
--- a/arch/sh/kernel/cpu/sh2a/clock-sh7201.c
+++ b/arch/sh/kernel/cpu/sh2a/clock-sh7201.c
@@ -32,9 +32,10 @@ static const int pfc_divisors[]={1,2,3,4,6,8,12};
 #error "Illegal Clock Mode!"
 #endif
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate = 10000000 * PLL2 * pll1rate[(ctrl_inw(FREQCR) >> 8) & 0x0007];
+	return 0;
 }
 
 static struct clk_ops sh7201_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh2a/clock-sh7203.c b/arch/sh/kernel/cpu/sh2a/clock-sh7203.c
index fb78132..2efd225 100644
--- a/arch/sh/kernel/cpu/sh2a/clock-sh7203.c
+++ b/arch/sh/kernel/cpu/sh2a/clock-sh7203.c
@@ -37,9 +37,10 @@ static const int pfc_divisors[]={1,2,3,4,6,8,12};
 #error "Illegal Clock Mode!"
 #endif
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pll1rate[(ctrl_inw(FREQCR) >> 8) & 0x0003] * PLL2 ;
+	return 0;
 }
 
 static struct clk_ops sh7203_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh2a/clock-sh7206.c b/arch/sh/kernel/cpu/sh2a/clock-sh7206.c
index 82d7f99..f374c7e 100644
--- a/arch/sh/kernel/cpu/sh2a/clock-sh7206.c
+++ b/arch/sh/kernel/cpu/sh2a/clock-sh7206.c
@@ -32,9 +32,10 @@ static const int pfc_divisors[]={1,2,3,4,6,8,12};
 #error "Illigal Clock Mode!"
 #endif
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= PLL2 * pll1rate[(ctrl_inw(FREQCR) >> 8) & 0x0007];
+	return 0;
 }
 
 static struct clk_ops sh7206_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh3/clock-sh3.c b/arch/sh/kernel/cpu/sh3/clock-sh3.c
index c3c9459..47f3591 100644
--- a/arch/sh/kernel/cpu/sh3/clock-sh3.c
+++ b/arch/sh/kernel/cpu/sh3/clock-sh3.c
@@ -26,12 +26,13 @@ static int stc_multipliers[] = { 1, 2, 3, 4, 6, 1, 1, 1 };
 static int ifc_divisors[]    = { 1, 2, 3, 4, 1, 1, 1, 1 };
 static int pfc_divisors[]    = { 1, 2, 3, 4, 6, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	int frqcr = ctrl_inw(FRQCR);
 	int idx = ((frqcr & 0x2000) >> 11) | (frqcr & 0x0003);
 
 	clk->rate *= pfc_divisors[idx];
+	return 0;
 }
 
 static struct clk_ops sh3_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh3/clock-sh7705.c b/arch/sh/kernel/cpu/sh3/clock-sh7705.c
index dfdbf32..2943c46 100644
--- a/arch/sh/kernel/cpu/sh3/clock-sh7705.c
+++ b/arch/sh/kernel/cpu/sh3/clock-sh7705.c
@@ -30,9 +30,10 @@ static int stc_multipliers[] = { 1, 2, 3, 4, 6, 1, 1, 1 };
 static int ifc_divisors[]    = { 1, 2, 3, 4, 1, 1, 1, 1 };
 static int pfc_divisors[]    = { 1, 2, 3, 4, 6, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[ctrl_inw(FRQCR) & 0x0003];
+	return 0;
 }
 
 static struct clk_ops sh7705_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh3/clock-sh7706.c b/arch/sh/kernel/cpu/sh3/clock-sh7706.c
index 0cf96f9..6397122 100644
--- a/arch/sh/kernel/cpu/sh3/clock-sh7706.c
+++ b/arch/sh/kernel/cpu/sh3/clock-sh7706.c
@@ -22,12 +22,13 @@ static int stc_multipliers[] = { 1, 2, 4, 1, 3, 6, 1, 1 };
 static int ifc_divisors[]    = { 1, 2, 4, 1, 3, 1, 1, 1 };
 static int pfc_divisors[]    = { 1, 2, 4, 1, 3, 6, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	int frqcr = ctrl_inw(FRQCR);
 	int idx = ((frqcr & 0x2000) >> 11) | (frqcr & 0x0003);
 
 	clk->rate *= pfc_divisors[idx];
+	return 0;
 }
 
 static struct clk_ops sh7706_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh3/clock-sh7709.c b/arch/sh/kernel/cpu/sh3/clock-sh7709.c
index b791a29..4cc4006 100644
--- a/arch/sh/kernel/cpu/sh3/clock-sh7709.c
+++ b/arch/sh/kernel/cpu/sh3/clock-sh7709.c
@@ -29,12 +29,13 @@ static void set_bus_parent(struct clk *clk)
 	clk_put(bus_clk);
 }
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	int frqcr = ctrl_inw(FRQCR);
 	int idx = ((frqcr & 0x2000) >> 11) | (frqcr & 0x0003);
 
 	clk->rate *= pfc_divisors[idx];
+	return 0;
 }
 
 static struct clk_ops sh7709_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh3/clock-sh7712.c b/arch/sh/kernel/cpu/sh3/clock-sh7712.c
index 54f54df..50c8033 100644
--- a/arch/sh/kernel/cpu/sh3/clock-sh7712.c
+++ b/arch/sh/kernel/cpu/sh3/clock-sh7712.c
@@ -21,12 +21,13 @@
 static int multipliers[] = { 1, 2, 3 };
 static int divisors[]    = { 1, 2, 3, 4, 6 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	int frqcr = ctrl_inw(FRQCR);
 	int idx = (frqcr & 0x0300) >> 8;
 
 	clk->rate *= multipliers[idx];
+	return 0;
 }
 
 static struct clk_ops sh7712_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4/clock-sh4-202.c b/arch/sh/kernel/cpu/sh4/clock-sh4-202.c
index a334294..2c46a51 100644
--- a/arch/sh/kernel/cpu/sh4/clock-sh4-202.c
+++ b/arch/sh/kernel/cpu/sh4/clock-sh4-202.c
@@ -66,7 +66,7 @@ static struct clk sh4202_femi_clk = {
 	.ops		= &sh4202_femi_clk_ops,
 };
 
-static void shoc_clk_init(struct clk *clk)
+static int shoc_clk_init(struct clk *clk)
 {
 	int i;
 
@@ -88,6 +88,7 @@ static void shoc_clk_init(struct clk *clk)
 	}
 
 	WARN_ON(i == ARRAY_SIZE(frqcr3_divisors));	/* Undefined clock */
+	return 0;
 }
 
 static void shoc_clk_recalc(struct clk *clk)
diff --git a/arch/sh/kernel/cpu/sh4/clock-sh4.c b/arch/sh/kernel/cpu/sh4/clock-sh4.c
index dca9f87..70794ac 100644
--- a/arch/sh/kernel/cpu/sh4/clock-sh4.c
+++ b/arch/sh/kernel/cpu/sh4/clock-sh4.c
@@ -26,9 +26,10 @@ static int ifc_divisors[] = { 1, 2, 3, 4, 6, 8, 1, 1 };
 #define bfc_divisors ifc_divisors	/* Same */
 static int pfc_divisors[] = { 2, 3, 4, 6, 8, 2, 2, 2 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[ctrl_inw(FRQCR) & 0x0007];
+	return 0;
 }
 
 static struct clk_ops sh4_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7722.c b/arch/sh/kernel/cpu/sh4a/clock-sh7722.c
index 0e174af..bbfbd6f 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7722.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7722.c
@@ -148,12 +148,13 @@ static void master_clk_recalc(struct clk *clk)
 	clk->rate = CONFIG_SH_PCLK_FREQ * (((frqcr >> 24) & 0x1f) + 1);
 }
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->parent = NULL;
 	clk->flags |= CLK_RATE_PROPAGATES;
 	clk->rate = CONFIG_SH_PCLK_FREQ;
 	master_clk_recalc(clk);
+	return 0;
 }
 
 
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7763.c b/arch/sh/kernel/cpu/sh4a/clock-sh7763.c
index 3177d0d..934fbdd 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7763.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7763.c
@@ -20,9 +20,10 @@ static int bfc_divisors[] = { 1, 1, 1, 8, 1, 1, 1, 1 };
 static int p0fc_divisors[] = { 1, 1, 1, 8, 1, 1, 1, 1 };
 static int cfc_divisors[] = { 1, 1, 4, 1, 1, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= p0fc_divisors[(ctrl_inl(FRQCR) >> 4) & 0x07];
+	return 0;
 }
 
 static struct clk_ops sh7763_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7770.c b/arch/sh/kernel/cpu/sh4a/clock-sh7770.c
index 8e23606..2d21990 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7770.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7770.c
@@ -19,9 +19,10 @@ static int ifc_divisors[] = { 1, 1, 1, 1, 1, 1, 1, 1 };
 static int bfc_divisors[] = { 1, 1, 1, 1, 1, 8,12, 1 };
 static int pfc_divisors[] = { 1, 8, 1,10,12,16, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[(ctrl_inl(FRQCR) >> 28) & 0x000f];
+	return 0;
 }
 
 static struct clk_ops sh7770_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7780.c b/arch/sh/kernel/cpu/sh4a/clock-sh7780.c
index 01f3da6..fe82df4 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7780.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7780.c
@@ -20,9 +20,10 @@ static int bfc_divisors[] = { 1, 1, 1, 8, 12, 16, 24, 1 };
 static int pfc_divisors[] = { 1, 24, 24, 1 };
 static int cfc_divisors[] = { 1, 1, 4, 1, 6, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[ctrl_inl(FRQCR) & 0x0003];
+	return 0;
 }
 
 static struct clk_ops sh7780_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7785.c b/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
index 27fa81b..ca43ad1 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7785.c
@@ -24,9 +24,10 @@ static int mfc_divisors[] = { 1, 1, 4, 6 };
 static int pfc_divisors[] = { 1, 1, 1, 1, 1, 1, 1, 18,
 			      24, 32, 36, 48, 1, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[ctrl_inl(FRQMR1) & 0x000f];
+	return 0;
 }
 
 static struct clk_ops sh7785_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-sh7786.c b/arch/sh/kernel/cpu/sh4a/clock-sh7786.c
index f84a9c1..2d1d3e2 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-sh7786.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-sh7786.c
@@ -27,9 +27,10 @@ static int mfc_divisors[] = { 1, 1, 4, 1 };
 static int pfc_divisors[] = { 1, 1, 1, 1, 1, 1, 16, 1,
 			      24, 32, 1, 48, 1, 1, 1, 1 };
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[ctrl_inl(FRQMR1) & 0x000f];
+	return 0;
 }
 
 static struct clk_ops sh7786_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh4a/clock-shx3.c b/arch/sh/kernel/cpu/sh4a/clock-shx3.c
index c630b29..7b0f4b8 100644
--- a/arch/sh/kernel/cpu/sh4a/clock-shx3.c
+++ b/arch/sh/kernel/cpu/sh4a/clock-shx3.c
@@ -31,9 +31,10 @@ static int cfc_divisors[] = { 1, 1, 4, 6 };
 #define PFC_POS		0
 #define CFC_POS		20
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	clk->rate *= pfc_divisors[(ctrl_inl(FRQCR) >> PFC_POS) & PFC_MSK];
+	return 0;
 }
 
 static struct clk_ops shx3_master_clk_ops = {
diff --git a/arch/sh/kernel/cpu/sh5/clock-sh5.c b/arch/sh/kernel/cpu/sh5/clock-sh5.c
index 52c4924..89abffe 100644
--- a/arch/sh/kernel/cpu/sh5/clock-sh5.c
+++ b/arch/sh/kernel/cpu/sh5/clock-sh5.c
@@ -22,10 +22,11 @@ static int ifc_table[] = { 2, 4, 6, 8, 10, 12, 16, 24 };
 
 static unsigned long cprc_base;
 
-static void master_clk_init(struct clk *clk)
+static int master_clk_init(struct clk *clk)
 {
 	int idx = (ctrl_inl(cprc_base + 0x00) >> 6) & 0x0007;
 	clk->rate *= ifc_table[idx];
+	return 0;
 }
 
 static struct clk_ops sh5_master_clk_ops = {
diff --git a/arch/sh/kernel/timers/timer-cmt.c b/arch/sh/kernel/timers/timer-cmt.c
index c127293..f69a1f1 100644
--- a/arch/sh/kernel/timers/timer-cmt.c
+++ b/arch/sh/kernel/timers/timer-cmt.c
@@ -112,13 +112,14 @@ static struct irqaction cmt_irq = {
 	.mask		= CPU_MASK_NONE,
 };
 
-static void cmt_clk_init(struct clk *clk)
+static int cmt_clk_init(struct clk *clk)
 {
 	u8 divisor = CMT_CMCSR_INIT & 0x3;
 	ctrl_inw(CMT_CMCSR_0);
 	ctrl_outw(CMT_CMCSR_INIT, CMT_CMCSR_0);
 	clk->parent = clk_get(NULL, "module_clk");
 	clk->rate = clk->parent->rate / (8 << (divisor << 1));
+	return 0;
 }
 
 static void cmt_clk_recalc(struct clk *clk)
diff --git a/arch/sh/kernel/timers/timer-mtu2.c b/arch/sh/kernel/timers/timer-mtu2.c
index 9a77ae8..9d56c35 100644
--- a/arch/sh/kernel/timers/timer-mtu2.c
+++ b/arch/sh/kernel/timers/timer-mtu2.c
@@ -120,7 +120,7 @@ static struct irqaction mtu2_irq = {
 
 static unsigned int divisors[] = { 1, 4, 16, 64, 1, 1, 256 };
 
-static void mtu2_clk_init(struct clk *clk)
+static int mtu2_clk_init(struct clk *clk)
 {
 	u8 idx = MTU2_TCR_INIT & 0x7;
 
@@ -128,6 +128,7 @@ static void mtu2_clk_init(struct clk *clk)
 	/* Start TCNT counting */
 	ctrl_outb(ctrl_inb(MTU2_TSTR) | MTU2_TSTR_CST1, MTU2_TSTR);
 
+	return 0;
 }
 
 static void mtu2_clk_recalc(struct clk *clk)
diff --git a/arch/sh/kernel/timers/timer-tmu.c b/arch/sh/kernel/timers/timer-tmu.c
index 10b5a6f..be439ff 100644
--- a/arch/sh/kernel/timers/timer-tmu.c
+++ b/arch/sh/kernel/timers/timer-tmu.c
@@ -165,12 +165,13 @@ static struct irqaction tmu0_irq = {
 	.mask		= CPU_MASK_NONE,
 };
 
-static void __init tmu_clk_init(struct clk *clk)
+static int __init tmu_clk_init(struct clk *clk)
 {
 	u8 divisor  = TMU_TCR_INIT & 0x7;
 	int tmu_num = clk->name[3]-'0';
 	ctrl_outw(TMU_TCR_INIT, TMU0_TCR+(tmu_num*0xC));
 	clk->rate = clk_get_rate(clk->parent) / (4 << (divisor << 1));
+	return 0;
 }
 
 static void tmu_clk_recalc(struct clk *clk)
-- 
1.5.6.6

Re: [PATCH] sh: clkfwk: Changed the init function

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:34 Fri 13 Mar     , Francesco VIRLINZI wrote:
> Sorry I forgot the timers..
> Here the right patch
> Regards
> Francesco
> Francesco VIRLINZI ha scritto:
>> Hi all
>>
>> This patch changes the init field in the clk_ops structure.
>> Moreover it changes how the init function is used.
>> Now it's called during registration and if something was wrong the  
>> clock isn't registered.
>>
>> Regards
>> Francesco
>

> >From 942219bb95734329c858587806e49895bae29208 Mon Sep 17 00:00:00 2001
> From: Francesco Virlinzi <francesco.virlinzi@st.com>
> Date: Fri, 13 Mar 2009 09:32:45 +0100
> Subject: [PATCH] sh: clkfwk: Changed the init field to return a value
> 
> This patch changes the clk_ops.init function pointer
> to return a value.
> Moreover it change the clk_register function to call
> always the clk_ops.init function to initilize the clock
> if there is an error the clock _isn't_ registered.
> 
> Signed-off-by: Francesco Virlinzi <francesco.virlinzi@st.com>
> ---
>  arch/sh/include/asm/clock.h            |    2 +-
>  arch/sh/kernel/cpu/clock.c             |   18 +++++-------------
>  arch/sh/kernel/cpu/sh2/clock-sh7619.c  |    3 ++-
>  arch/sh/kernel/cpu/sh2a/clock-sh7201.c |    3 ++-
>  arch/sh/kernel/cpu/sh2a/clock-sh7203.c |    3 ++-
>  arch/sh/kernel/cpu/sh2a/clock-sh7206.c |    3 ++-
>  arch/sh/kernel/cpu/sh3/clock-sh3.c     |    3 ++-
>  arch/sh/kernel/cpu/sh3/clock-sh7705.c  |    3 ++-
>  arch/sh/kernel/cpu/sh3/clock-sh7706.c  |    3 ++-
>  arch/sh/kernel/cpu/sh3/clock-sh7709.c  |    3 ++-
>  arch/sh/kernel/cpu/sh3/clock-sh7712.c  |    3 ++-
>  arch/sh/kernel/cpu/sh4/clock-sh4-202.c |    3 ++-
>  arch/sh/kernel/cpu/sh4/clock-sh4.c     |    3 ++-
>  arch/sh/kernel/cpu/sh4a/clock-sh7722.c |    3 ++-
>  arch/sh/kernel/cpu/sh4a/clock-sh7763.c |    3 ++-
>  arch/sh/kernel/cpu/sh4a/clock-sh7770.c |    3 ++-
>  arch/sh/kernel/cpu/sh4a/clock-sh7780.c |    3 ++-
>  arch/sh/kernel/cpu/sh4a/clock-sh7785.c |    3 ++-
>  arch/sh/kernel/cpu/sh4a/clock-sh7786.c |    3 ++-
>  arch/sh/kernel/cpu/sh4a/clock-shx3.c   |    3 ++-
>  arch/sh/kernel/cpu/sh5/clock-sh5.c     |    3 ++-
>  arch/sh/kernel/timers/timer-cmt.c      |    3 ++-
>  arch/sh/kernel/timers/timer-mtu2.c     |    3 ++-
>  arch/sh/kernel/timers/timer-tmu.c      |    3 ++-
>  24 files changed, 50 insertions(+), 36 deletions(-)
> 
> diff --git a/arch/sh/include/asm/clock.h b/arch/sh/include/asm/clock.h
> index 2f6c962..f7a5899 100644
> --- a/arch/sh/include/asm/clock.h
> +++ b/arch/sh/include/asm/clock.h
> @@ -10,7 +10,7 @@
>  struct clk;
>  
>  struct clk_ops {
> -	void (*init)(struct clk *clk);
> +	int (*init)(struct clk *clk);
>  	void (*enable)(struct clk *clk);
>  	void (*disable)(struct clk *clk);
>  	void (*recalc)(struct clk *clk);
> diff --git a/arch/sh/kernel/cpu/clock.c b/arch/sh/kernel/cpu/clock.c
> index e2d2451..87a54b5 100644
> --- a/arch/sh/kernel/cpu/clock.c
> +++ b/arch/sh/kernel/cpu/clock.c
> @@ -92,17 +92,6 @@ static void propagate_rate(struct clk *clk)
>  
>  static int __clk_enable(struct clk *clk)
>  {
> -	/*
> -	 * See if this is the first time we're enabling the clock, some
> -	 * clocks that are always enabled still require "special"
> -	 * initialization. This is especially true if the clock mode
> -	 * changes and the clock needs to hunt for the proper set of
> -	 * divisors to use before it can effectively recalc.
> -	 */
> -	if (unlikely(atomic_read(&clk->kref.refcount) = 1))
> -		if (clk->ops && clk->ops->init)
> -			clk->ops->init(clk);
> -
>  	kref_get(&clk->kref);
>  
>  	if (clk->flags & CLK_ALWAYS_ENABLED)
> @@ -167,6 +156,11 @@ EXPORT_SYMBOL_GPL(clk_disable);
>  
>  int clk_register(struct clk *clk)
>  {
> +
> +	if (clk->ops && clk->ops->init)
> +		if (clk->ops->init(clk))
> +			return -EINVAL;
> +
why do you check the return if all clk_init will always return 0; ?

IMHO if clk-ops is NULL return directly instead of recheck it later

Best Regards,
J.

Re: [PATCH] sh: clkfwk: Changed the init function

[Francesco VIRLINZI]

Hi Jean-Christophe
>> (clk->flags & CLK_ALWAYS_ENABLED)
>> @@ -167,6 +156,11 @@ EXPORT_SYMBOL_GPL(clk_disable);
>>  
>>  int clk_register(struct clk *clk)
>>  {
>> +
>> +	if (clk->ops && clk->ops->init)
>> +		if (clk->ops->init(clk))
>> +			return -EINVAL;
>> +
>>     
> why do you check the return if all clk_init will always return 0; ?
>
> IMHO if clk-ops is NULL return directly instead of recheck it later
>
> Best Regards,
> J.
>   
I'm sorry but I don't understand your issue.
You are right if you mean the current SOC-clocks implementations return 
0 in all the .init functions...

But it was currently required to maintain a working system (it's the 
only manner to change a void function with the int function).

In the future if required the .init function will be able to do extra 
check and if required it will be able to
 reject a clock registration.

Currently it could seem useless (all .init return zero) but for my point 
of view it's only a currently issue... And not really an issue.

Regards
 Francesco

Re: [PATCH] sh: clkfwk: Changed the init function

[Paul Mundt]

On Fri, Mar 13, 2009 at 09:10:45AM +0100, Francesco VIRLINZI wrote:
> This patch changes the init field in the clk_ops structure.
> Moreover it changes how the init function is used.
> Now it's called during registration and if something was wrong the clock 
> isn't registered.
> 
Ok, I'll bite. Why?

Re: [PATCH] sh: clkfwk: Changed the init function

[Francesco VIRLINZI]

Hi Paul
Paul Mundt ha scritto:
> On Fri, Mar 13, 2009 at 09:10:45AM +0100, Francesco VIRLINZI wrote:
>   
>> This patch changes the init field in the clk_ops structure.
>> Moreover it changes how the init function is used.
>> Now it's called during registration and if something was wrong the clock 
>> isn't registered.
>>
>>     
> Ok, I'll bite. Why?
>   
Than:
I think the clk_ops.init function has (mandatory) to return a value
  to notify to the clk framework if the clock initialization was ok or not.

Moreover in my point of view if a clock has an clk_ops.init function 
than the function
 has to be called during the clk_register because we must know as soon 
as possible
 if a clock is working or not... It isn't really nice discover a 
not-working clock
 after a call to clk_enable().... while the clk is perfectly registered...

i.e.: I'm thinking for example on a clock on i2c bus... ( I assume a clkfwk
 should be able to manage any kind of clock). During the clk_register
 (with the clk_ops.init) I can check if the clock can be really used
 or not instead of discover (for example) with the clk_enable()
 that the i2c-bus sees no device...

Let me know if I clarified my view.

Regards
 Francesco



> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>   

Re: [PATCH] sh: clkfwk: Changed the init function

[Paul Mundt]

On Mon, Mar 16, 2009 at 01:39:16PM +0100, Francesco VIRLINZI wrote:
> Hi Paul
> Paul Mundt ha scritto:
> >On Fri, Mar 13, 2009 at 09:10:45AM +0100, Francesco VIRLINZI wrote:
> >  
> >>This patch changes the init field in the clk_ops structure.
> >>Moreover it changes how the init function is used.
> >>Now it's called during registration and if something was wrong the clock 
> >>isn't registered.
> >>
> >>    
> >Ok, I'll bite. Why?
> >  
> Than:
> I think the clk_ops.init function has (mandatory) to return a value
>  to notify to the clk framework if the clock initialization was ok or not.
> 
> Moreover in my point of view if a clock has an clk_ops.init function 
> than the function
> has to be called during the clk_register because we must know as soon 
> as possible
> if a clock is working or not... It isn't really nice discover a 
> not-working clock
> after a call to clk_enable().... while the clk is perfectly registered...
> 
> i.e.: I'm thinking for example on a clock on i2c bus... ( I assume a clkfwk
> should be able to manage any kind of clock). During the clk_register
> (with the clk_ops.init) I can check if the clock can be really used
> or not instead of discover (for example) with the clk_enable()
> that the i2c-bus sees no device...
> 
> Let me know if I clarified my view.
> 
You are blurring the lines between what init and enable are supposed to
do. In the case of init, it is giving the clock in question a chance to
set some initial paramters and prepare itself for further use. If a clock
is registering with the clock framework, this init sequence can not fail.
This is especially true for clocks that are always enabled, as it is
simply mapping out a topology rather than anything more profound.

The issues that determine whether a clock can be enabled or not depends
entirely on the current set of constraints, constraints that are not
fixed across the lifetime of the system, this means that the one and only
place you can error out on an enable, is through the ->enable() hook
itself. The situation you discuss with the i2c clock is something that
should be erroring out through the enable path, not the init path.

The clock framework itself also specifies that no assumptions can be made
about the state of the clock, so you must call clk_enable() to enable the
clock after bumping the refcount with clk_get(), and then and only then
can you determine if there is a problem with the clock settings.

Re: [PATCH] sh: clkfwk: Changed the init function

[Francesco VIRLINZI]

Hi Paul
> The clock framework itself also specifies that no assumptions can be made
> about the state of the clock, so you must call clk_enable() to enable the
> clock after bumping the refcount with clk_get(), and then and only then
> can you determine if there is a problem with the clock settings.
I understand you point of view... and also my example was not so good!
Let me try again ;-)

Now in the clock fmwk we have also clk_set_parent/clk_get_parent this means
 we can change the topology on the fly (we have this behavior with video 
clocks).

 From my point of view the .init function should also initialize the 
clk->parent field based
 on the loaded hw setting... (I assume we can not hard-code something 
like clk_x->parent = &clk_y;
 just because something like that it's strictly based on hardware value 
on power-up).

Therefore I can trust the clk->parent value only after a call to the 
.init function.

But with the current clfwk is mandatory that a topology change can be 
done _only_ after the clock
 is enabled while in my point of view I should be able to change the 
topology without 
 this constraint.

Once a clock is registered I should be able to call clk_set_parent 
without being forced to enable it.

Moreover it could be dangerous for the devices that are using the clock 
because first of all I have to
 enable the clock.. and only after that I can change (if required) the 
clk->parent (while the clock is running).

Regards
 Francesco

Re: [PATCH] sh: clkfwk: Changed the init function

[Francesco VIRLINZI]

Hi Paul
I'm sorry if I'm boring you.
Any replay on the below issue?
Regards
 Francesco
Francesco VIRLINZI ha scritto:
> Hi Paul
>> The clock framework itself also specifies that no assumptions can be 
>> made
>> about the state of the clock, so you must call clk_enable() to enable 
>> the
>> clock after bumping the refcount with clk_get(), and then and only then
>> can you determine if there is a problem with the clock settings.
> I understand you point of view... and also my example was not so good!
> Let me try again ;-)
>
> Now in the clock fmwk we have also clk_set_parent/clk_get_parent this 
> means
> we can change the topology on the fly (we have this behavior with 
> video clocks).
>
> From my point of view the .init function should also initialize the 
> clk->parent field based
> on the loaded hw setting... (I assume we can not hard-code something 
> like clk_x->parent = &clk_y;
> just because something like that it's strictly based on hardware value 
> on power-up).
>
> Therefore I can trust the clk->parent value only after a call to the 
> .init function.
>
> But with the current clfwk is mandatory that a topology change can be 
> done _only_ after the clock
> is enabled while in my point of view I should be able to change the 
> topology without this constraint.
>
> Once a clock is registered I should be able to call clk_set_parent 
> without being forced to enable it.
>
> Moreover it could be dangerous for the devices that are using the 
> clock because first of all I have to
> enable the clock.. and only after that I can change (if required) the 
> clk->parent (while the clock is running).
>
> Regards
> Francesco

Re: [PATCH] sh: clkfwk: Changed the init function

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:24 Thu 19 Mar     , Francesco VIRLINZI wrote:
> Hi Paul
>> The clock framework itself also specifies that no assumptions can be made
>> about the state of the clock, so you must call clk_enable() to enable the
>> clock after bumping the refcount with clk_get(), and then and only then
>> can you determine if there is a problem with the clock settings.
> I understand you point of view... and also my example was not so good!
> Let me try again ;-)
>
> Now in the clock fmwk we have also clk_set_parent/clk_get_parent this means
> we can change the topology on the fly (we have this behavior with video  
> clocks).
>

IIRC the clock B can be but not the A so on the ST40 will have programmable
clock and static clock

so we must not allow people so change them

> From my point of view the .init function should also initialize the  
> clk->parent field based
> on the loaded hw setting... (I assume we can not hard-code something  
> like clk_x->parent = &clk_y;
> just because something like that it's strictly based on hardware value  
> on power-up).
why?
on some arm we hardcode because it's hardcode in the soc
>
> Therefore I can trust the clk->parent value only after a call to the  
> .init function.
after the register for non programmable clock
>
> But with the current clfwk is mandatory that a topology change can be  
> done _only_ after the clock
> is enabled while in my point of view I should be able to change the  
> topology without this constraint.
but when changing is parent we could have to disable the child before doing
it in somecase

Best Regards,
J.

Re: [PATCH] sh: clkfwk: Changed the init function

[Paul Mundt]

On Thu, Mar 19, 2009 at 09:24:38AM +0100, Francesco VIRLINZI wrote:
> >The clock framework itself also specifies that no assumptions can be made
> >about the state of the clock, so you must call clk_enable() to enable the
> >clock after bumping the refcount with clk_get(), and then and only then
> >can you determine if there is a problem with the clock settings.
> I understand you point of view... and also my example was not so good!
> Let me try again ;-)
> 
> Now in the clock fmwk we have also clk_set_parent/clk_get_parent this means
> we can change the topology on the fly (we have this behavior with video 
> clocks).
> 
> From my point of view the .init function should also initialize the 
> clk->parent field based
> on the loaded hw setting... (I assume we can not hard-code something 
> like clk_x->parent = &clk_y;
> just because something like that it's strictly based on hardware value 
> on power-up).
> 
clk->parent is meant to be hardcoded in most all cases, it is really only
the corner cases that need to change this at all. ie, in the case of root
clocks where the clkin provider can be flipped for a given clock, it
needs to be reparented accordingly. The common case there is if there are
multiple oscillators that can be switched between (also from software,
though of course the board needs to know how it is clocked).

> Therefore I can trust the clk->parent value only after a call to the 
> .init function.
> 
There is basically no point in doing that. Your CPU should have a
reasonable clock topology defined with fairly sensible defaults, which in
turn can be fixed up by the platform code. I've recently been reworking
the clock framework in the sh/clkfwk topic branch as a result of people
having a lot of confusion about how it is supposed to be used. So,
perhaps it helps to summarize the reworked initialization process a bit.

Presently the init process consists of the following steps:

	* arch_clk_init()

		- This is for the CPU to register its clock topology.
		  This is only about available clocks, mapping out
		  relationships, etc. and makes no promises that any
		  given source is used, or even possible on a given
		  board.

		- Note that this is _only_ for initializing the topology,
		  clocks do not need any rate information here if the
		  platform code is forced to set it in the case of a root
		  clock, or if the rate is established in the recalc path
		  from initial root clock propagation.

	* sh_mv.mv_clk_init()

		- This is the platform callback to step in and do any
		  fine tuning of the CPU's clock topology that is
		  necessary. This can include reparenting the root
		  clocks, setting an initial rate for root clocks (to
		  handle the case where different boards have different
		  oscillator frequencies on the same input pin and so
		  on), etc.

	* recalculate_root_clocks()

		- This calls in to ->recalc() for every root clock, to
		  handle things like topology changes made by the
		  platform code.

		- Afterwards, propagate_rate() steps in and kicks that
		  information down to all of the child clocks.

		  propagate_rate() recurses for child clocks that have
		  siblings of their own.

	* clk_enable_init_clocks()

		- With the topology mapped out and all of the rate
		  information set up and propagated, the initialization
		  code can now iterate through the clock list and call in
		  to clk_enable() for every clock that wants to be
		  enabled on initialization (via CLK_ENABLE_ON_INIT).

Now, note that at arch_clk_init()/sh_mv.mv_clk_init() time,
clk_register() is called in to for these clocks, but the enable itself is
deferred. This means that things like clk_get() and the various set
functions are accessible for manipulating and initialization the
topology, but things like clk_get_rate() and friends are not reliable
until after this stage, once clk_init() has had a chance to finish its
work.

In the case where a platform needs to set rate information on a given
clock and force propagation before being able to make other configuration
changes, both recalculate_root_clocks() and propagate_rate() are
available for use by the platform code. In the general case the common
path through clk_init() will be sufficient.

> But with the current clfwk is mandatory that a topology change can be 
> done _only_ after the clock is enabled while in my point of view I
> should be able to change the topology without this constraint.
> 
Yes, that was a bug, and is now corrected. :-)

If you see any problems with the reworked clock framework for your use
cases, now would be the time to raise concerns. I'm still busy hacking up
more generic support for CPG clocks, but you can look at the SH7785 clock
framework as an example of the direction things are heading in.

Also note that things like the frequency table are probably going to be
moved under struct clk directly, with a generic round_rate() mapped on
top of that. We will still need to tie in notifier chains to handle
things like the rate table being rebuilt on parent rate propagation and
so on, but the interdependencies there will be a lot clearer once cpuidle
starts tying in to the rate information on the same clocks as cpufreq.