sh: clkfw: Moved the .init callback in the clk_register
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
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_initreturns always zero...
in the next step if the__clk_initfails 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
@@ -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
@@ -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
Re: [Proposal][PATCH] sh: clkfw: Moved the .init callback in the
In reply to Francesco VIRLINZIHi 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_initreturns always zero...
in the next step if the__clk_initfails the clock registration will be rejected.Let me know.
Ciao
Francesco
Re: [Proposal][PATCH] sh: clkfw: Moved the .init callback in the
In reply to Francesco VIRLINZIHi 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 allinit()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 theinit()callback.You probably want to call
init()fromclk_register()after adding the clock to the list. Also, return the value from theinit()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_initreturns always zero...
in the next step if the__clk_initfails 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