Skip to contentSkip to conversation

sh: clkfw: Moved the .init callback in the clk_register

PATCH In review Francesco VIRLINZI 2 participants 4 messages May 11, 2009 SuperH

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sh: clkfw: Moved the .init callback in the clk_register

Hi Magnus
Here there is the patch to move the .init callback during the clock registration. Currently the __clk_init returns always zero...
in the next step if the __clk_init fails the clock registration will be rejected.

Let me know.
Ciao
Francesco

#1 lore raw

Re: [Proposal][PATCH] sh: clkfw: Moved the .init callback in the

Sorry... THe attached was broken.
Here the right patch,
Ciao
Francesco
Francesco VIRLINZI ha scritto:

Quoted 8 lines

Hi Magnus
Here there is the patch to move the .init callback during the clock registration. Currently the __clk_init returns always zero...
in the next step if the __clk_init fails the clock registration will be rejected.

Let me know.
Ciao
Francesco

From 8b11a6b0fac5d0bbd85b43c1f5e51c006e32ffbb Mon Sep 17 00:00:00 2001
From: Francesco Virlinzi <[email protected]>
Date: Mon, 11 May 2009 11:04:11 +0200
Subject: [PATCH] sh: clkfw: Moved the .init callback in the clk_register function

This patch moves the .init callback function in the clk_register

---
 arch/sh/include/asm/clock.h |    3 +--
 arch/sh/kernel/cpu/clock.c  |   20 ++++++++++----------
 2 files changed, 11 insertions(+), 12 deletions(-)
arch/sh/include/asm/clock.h +1 -2
index b1f2919..f8d253b 100644
@@ -9,7 +9,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);
@@ -36,7 +36,6 @@ struct clk {
 
 #define CLK_ALWAYS_ENABLED	(1 << 0)
 #define CLK_RATE_PROPAGATES	(1 << 1)
-#define CLK_NEEDS_INIT		(1 << 2)
 
 /* Should be defined by processor-specific code */
 void arch_init_clk_ops(struct clk_ops **, int type);
arch/sh/kernel/cpu/clock.c +10 -10
index 133dbe4..16b5cca 100644
@@ -89,8 +89,9 @@ static void propagate_rate(struct clk *clk)
 	}
 }
 
-static void __clk_init(struct clk *clk)
+static int __clk_init(struct clk *clk)
 {
+	int ret = 0;
 	/*
 	 * See if this is the first time we're enabling the clock, some
 	 * clocks that are always enabled still require "special"
@@ -99,12 +100,11 @@ static void __clk_init(struct clk *clk)
 	 * divisors to use before it can effectively recalc.
 	 */
 
-	if (clk->flags & CLK_NEEDS_INIT) {
-		if (clk->ops && clk->ops->init)
-			clk->ops->init(clk);
+	if (clk->ops && clk->ops->init)
+		clk->ops->init(clk);	/* First step... */
+/*		ret = clk->ops->init(clk); ... Next step...*/
 
-		clk->flags &= ~CLK_NEEDS_INIT;
-	}
+	return ret;
 }
 
 static int __clk_enable(struct clk *clk)
@@ -119,8 +119,6 @@ static int __clk_enable(struct clk *clk)
 		return 0;
 
 	if (clk->usecount == 1) {
-		__clk_init(clk);
-
 		__clk_enable(clk->parent);
 
 		if (clk->ops && clk->ops->enable)
@@ -175,16 +173,18 @@ EXPORT_SYMBOL_GPL(clk_disable);
 
 int clk_register(struct clk *clk)
 {
+
+	if (__clk_init(clk))	/* it does every hardware initialization */
+		return -EPERM;	/* and checks they were ok */
+
 	mutex_lock(&clock_list_sem);
 
 	list_add(&clk->node, &clock_list);
 	clk->usecount = 0;
-	clk->flags |= CLK_NEEDS_INIT;
 
 	mutex_unlock(&clock_list_sem);
 
 	if (clk->flags & CLK_ALWAYS_ENABLED) {
-		__clk_init(clk);
 		pr_debug( "Clock '%s' is ALWAYS_ENABLED\n", clk->name);
 		if (clk->ops && clk->ops->enable)
 			clk->ops->enable(clk);
Signature · 1 line
1.6.0.6
#2 lore raw

Re: [Proposal][PATCH] sh: clkfw: Moved the .init callback in the

In reply to Francesco VIRLINZI

Hi Francesco,

I had a look at your patch. The idea behind the patch is fine with me
- still untested - but I'd be happy to test the next version if you can improve it a bit. Here's some feedback:

Changing init() to return an int is ok with me. Not sure what Paul thinks about this. Anyway, to do so you need to fix your patch so all init() callbacks are changed. Applying this patch as-is will break many things.

No need for the __clk_init() function any more. And please use the return value from the init() callback.

You probably want to call init() from clk_register() after adding the clock to the list. Also, return the value from the init() callback, no need to always return -EPERM.

And if possible, please send the patch inline. That makes reviewing much easier.

Cheers,

/ magnus - waiting for a V3

Quoted 18 lines · Francesco VIRLINZI

Sorry... THe attached was broken.
Here the right patch,
Ciao
Francesco
Francesco VIRLINZI ha scritto:

Quoted 11 lines

Hi Magnus
Here there is the patch to move the .init callback during the clock registration.
Currently the __clk_init returns always zero...
in the next step if the __clk_init fails the clock registration will be rejected.

Let me know.
Ciao
Francesco

#3 lore raw

Re: [Proposal][PATCH] sh: clkfw: Moved the .init callback in the

In reply to Francesco VIRLINZI

Hi Magnus

Quoted 5 lines

Hi Francesco,

I had a look at your patch. The idea behind the patch is fine with me
- still untested - but I'd be happy to test the next version if you can improve it a bit.

No problems I can do that... but.

Paul: I remember you had some issue on this design.

Do you agree now? Do you think the v3 is commit-able?

Let me know.

Regards
Francesco

Quoted 47 lines

Here's some feedback:

Changing init() to return an int is ok with me. Not sure what Paul thinks about this. Anyway, to do so you need to fix your patch so all init() callbacks are changed. Applying this patch as-is will break many things.

No need for the __clk_init() function any more. And please use the return value from the init() callback.

You probably want to call init() from clk_register() after adding the clock to the list. Also, return the value from the init() callback, no need to always return -EPERM.

And if possible, please send the patch inline. That makes reviewing much easier.

Cheers,

/ magnus - waiting for a V3

Quoted 18 lines · Francesco VIRLINZI

Sorry... THe attached was broken.
Here the right patch,
Ciao
Francesco
Francesco VIRLINZI ha scritto:

Quoted 11 lines

Hi Magnus
Here there is the patch to move the .init callback during the clock registration.
Currently the __clk_init returns always zero...
in the next step if the __clk_init fails the clock registration will be rejected.

Let me know.
Ciao
Francesco

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