diff mbox

[v2,4/4] clocksource: dw_apb_timer_of: use clocksource_of_init

Message ID 201306041138.42639.heiko@sntech.de (mailing list archive)
State New, archived
Headers show

Commit Message

Heiko Stübner June 4, 2013, 9:38 a.m. UTC
dw_apb_timer_init used to search the devicetree for matching timer
devices, making calls to it from board files necessary.

Change the dw_apb_timer_init to work with CLOCKSOURCE_OF_DECLARE.
With this change the function gets called once for each timer node
and tracks these number of calls to attach clockevent and clocksource
devices to the nodes.

Also remove the calls to dw_apb_timer_init from all previous users, as
clocksource_of_init is the default for init_time now.

Tested on the upcoming rk3066 code.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: Rob Herring <rob.herring@calxeda.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-picoxcell/common.c      |    2 -
 arch/arm/mach-socfpga/socfpga.c       |    2 -
 drivers/clocksource/Kconfig           |    1 +
 drivers/clocksource/dw_apb_timer_of.c |   41 ++++++++++++++++-----------------
 include/linux/dw_apb_timer.h          |    1 -
 5 files changed, 21 insertions(+), 26 deletions(-)

Comments

Jamie Iles June 4, 2013, 2:21 p.m. UTC | #1
Hi Heiko,

On Tue, Jun 04, 2013 at 11:38:42AM +0200, Heiko Stübner wrote:
> dw_apb_timer_init used to search the devicetree for matching timer
> devices, making calls to it from board files necessary.
> 
> Change the dw_apb_timer_init to work with CLOCKSOURCE_OF_DECLARE.
> With this change the function gets called once for each timer node
> and tracks these number of calls to attach clockevent and clocksource
> devices to the nodes.
> 
> Also remove the calls to dw_apb_timer_init from all previous users, as
> clocksource_of_init is the default for init_time now.
> 
> Tested on the upcoming rk3066 code.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> Acked-by: Rob Herring <rob.herring@calxeda.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> ---
...
> -void __init dw_apb_timer_init(void)
> +static int num_called;
> +static void __init dw_apb_timer_init(struct device_node *timer)
>  {
> -	struct device_node *event_timer, *source_timer;
> -
> -	event_timer = of_find_matching_node(NULL, osctimer_ids);
> -	if (!event_timer)
> -		panic("No timer for clockevent");
> -	add_clockevent(event_timer);
> -
> -	source_timer = of_find_matching_node(event_timer, osctimer_ids);
> -	if (!source_timer)
> -		panic("No timer for clocksource");
> -	add_clocksource(source_timer);
> -
> -	of_node_put(source_timer);
> +	switch (num_called) {
> +	case 0:
> +		pr_debug("%s: found clockevent timer\n", __func__);
> +		add_clockevent(timer);
> +		of_node_put(timer);
> +		break;
> +	case 1:
> +		pr_debug("%s: found clocksource timer\n", __func__);
> +		add_clocksource(timer);
> +		of_node_put(timer);
> +		init_sched_clock();
> +		break;
> +	default:
> +		break;
> +	}
>  
> -	init_sched_clock();
> +	num_called++;
>  }
> +CLOCKSOURCE_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer", dw_apb_timer_init);
> +CLOCKSOURCE_OF_DECLARE(apb_timer, "snps,dw-apb-timer-osc", dw_apb_timer_init);

I think maybe we also want CLOCKSOURCE_OF_DECLARE() instances for the 
contents of sptimer_ids for completeness, otherwise looks good.

Acked-by: Jamie Iles <jamie@jamieiles.com>

Thanks,

Jamie
Heiko Stübner June 4, 2013, 3:06 p.m. UTC | #2
Am Dienstag, 4. Juni 2013, 16:21:20 schrieb Jamie Iles:
> Hi Heiko,
> 
> On Tue, Jun 04, 2013 at 11:38:42AM +0200, Heiko Stübner wrote:
> > dw_apb_timer_init used to search the devicetree for matching timer
> > devices, making calls to it from board files necessary.
> > 
> > Change the dw_apb_timer_init to work with CLOCKSOURCE_OF_DECLARE.
> > With this change the function gets called once for each timer node
> > and tracks these number of calls to attach clockevent and clocksource
> > devices to the nodes.
> > 
> > Also remove the calls to dw_apb_timer_init from all previous users, as
> > clocksource_of_init is the default for init_time now.
> > 
> > Tested on the upcoming rk3066 code.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > Acked-by: Rob Herring <rob.herring@calxeda.com>
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> 
> ...
> 
> > -void __init dw_apb_timer_init(void)
> > +static int num_called;
> > +static void __init dw_apb_timer_init(struct device_node *timer)
> > 
> >  {
> > 
> > -	struct device_node *event_timer, *source_timer;
> > -
> > -	event_timer = of_find_matching_node(NULL, osctimer_ids);
> > -	if (!event_timer)
> > -		panic("No timer for clockevent");
> > -	add_clockevent(event_timer);
> > -
> > -	source_timer = of_find_matching_node(event_timer, osctimer_ids);
> > -	if (!source_timer)
> > -		panic("No timer for clocksource");
> > -	add_clocksource(source_timer);
> > -
> > -	of_node_put(source_timer);
> > +	switch (num_called) {
> > +	case 0:
> > +		pr_debug("%s: found clockevent timer\n", __func__);
> > +		add_clockevent(timer);
> > +		of_node_put(timer);
> > +		break;
> > +	case 1:
> > +		pr_debug("%s: found clocksource timer\n", __func__);
> > +		add_clocksource(timer);
> > +		of_node_put(timer);
> > +		init_sched_clock();
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > 
> > -	init_sched_clock();
> > +	num_called++;
> > 
> >  }
> > 
> > +CLOCKSOURCE_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer",
> > dw_apb_timer_init); +CLOCKSOURCE_OF_DECLARE(apb_timer,
> > "snps,dw-apb-timer-osc", dw_apb_timer_init);
> 
> I think maybe we also want CLOCKSOURCE_OF_DECLARE() instances for the
> contents of sptimer_ids for completeness, otherwise looks good.

Hmm ... difficult.

As you can see, currently we call init_sched_clock when the second timer is 
found, which then tries to find the sptimer or uses the clocksource as 
sched_clock.

So to use CLOCKSOURCE_OF_DECLARE for the sptimer the init_sched_clock call 
would need to move to its init function. So on system without sptimer (like 
mine) this third call would never happen, and the system would be missing a 
sched_clock.

Or I just don't see the solution currently. In any case this could probably be 
handled by someone with the relevant hardware to test ;-) .


> Acked-by: Jamie Iles <jamie@jamieiles.com>

thanks
diff mbox

Patch

diff --git a/arch/arm/mach-picoxcell/common.c b/arch/arm/mach-picoxcell/common.c
index 70b441a..7cde042 100644
--- a/arch/arm/mach-picoxcell/common.c
+++ b/arch/arm/mach-picoxcell/common.c
@@ -15,7 +15,6 @@ 
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
-#include <linux/dw_apb_timer.h>
 
 #include <asm/mach/arch.h>
 #include <asm/mach/map.h>
@@ -88,7 +87,6 @@  DT_MACHINE_START(PICOXCELL, "Picochip picoXcell")
 	.map_io		= picoxcell_map_io,
 	.nr_irqs	= NR_IRQS_LEGACY,
 	.init_irq	= irqchip_init,
-	.init_time	= dw_apb_timer_init,
 	.init_machine	= picoxcell_init_machine,
 	.dt_compat	= picoxcell_dt_match,
 	.restart	= picoxcell_wdt_restart,
diff --git a/arch/arm/mach-socfpga/socfpga.c b/arch/arm/mach-socfpga/socfpga.c
index 46a0513..8ea11b47 100644
--- a/arch/arm/mach-socfpga/socfpga.c
+++ b/arch/arm/mach-socfpga/socfpga.c
@@ -14,7 +14,6 @@ 
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
-#include <linux/dw_apb_timer.h>
 #include <linux/clk-provider.h>
 #include <linux/irqchip.h>
 #include <linux/of_address.h>
@@ -120,7 +119,6 @@  DT_MACHINE_START(SOCFPGA, "Altera SOCFPGA")
 	.smp		= smp_ops(socfpga_smp_ops),
 	.map_io		= socfpga_map_io,
 	.init_irq	= socfpga_init_irq,
-	.init_time	= dw_apb_timer_init,
 	.init_machine	= socfpga_cyclone5_init,
 	.restart	= socfpga_cyclone5_restart,
 	.dt_compat	= altera_dt_match,
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 48a0f2e..5871933 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -22,6 +22,7 @@  config DW_APB_TIMER
 config DW_APB_TIMER_OF
 	bool
 	select DW_APB_TIMER
+	select CLKSRC_OF
 
 config ARMADA_370_XP_TIMER
 	bool
diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index 1964f87..cef5544 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -133,27 +133,26 @@  static void init_sched_clock(void)
 	setup_sched_clock(read_sched_clock, 32, sched_rate);
 }
 
-static const struct of_device_id osctimer_ids[] __initconst = {
-	{ .compatible = "picochip,pc3x2-timer" },
-	{ .compatible = "snps,dw-apb-timer-osc" },
-	{},
-};
-
-void __init dw_apb_timer_init(void)
+static int num_called;
+static void __init dw_apb_timer_init(struct device_node *timer)
 {
-	struct device_node *event_timer, *source_timer;
-
-	event_timer = of_find_matching_node(NULL, osctimer_ids);
-	if (!event_timer)
-		panic("No timer for clockevent");
-	add_clockevent(event_timer);
-
-	source_timer = of_find_matching_node(event_timer, osctimer_ids);
-	if (!source_timer)
-		panic("No timer for clocksource");
-	add_clocksource(source_timer);
-
-	of_node_put(source_timer);
+	switch (num_called) {
+	case 0:
+		pr_debug("%s: found clockevent timer\n", __func__);
+		add_clockevent(timer);
+		of_node_put(timer);
+		break;
+	case 1:
+		pr_debug("%s: found clocksource timer\n", __func__);
+		add_clocksource(timer);
+		of_node_put(timer);
+		init_sched_clock();
+		break;
+	default:
+		break;
+	}
 
-	init_sched_clock();
+	num_called++;
 }
+CLOCKSOURCE_OF_DECLARE(pc3x2_timer, "picochip,pc3x2-timer", dw_apb_timer_init);
+CLOCKSOURCE_OF_DECLARE(apb_timer, "snps,dw-apb-timer-osc", dw_apb_timer_init);
diff --git a/include/linux/dw_apb_timer.h b/include/linux/dw_apb_timer.h
index dd755ce..07261d5 100644
--- a/include/linux/dw_apb_timer.h
+++ b/include/linux/dw_apb_timer.h
@@ -53,5 +53,4 @@  void dw_apb_clocksource_start(struct dw_apb_clocksource *dw_cs);
 cycle_t dw_apb_clocksource_read(struct dw_apb_clocksource *dw_cs);
 void dw_apb_clocksource_unregister(struct dw_apb_clocksource *dw_cs);
 
-extern void dw_apb_timer_init(void);
 #endif /* __DW_APB_TIMER_H__ */