All of lore.kernel.org
 help / color / mirror / Atom feed
From: hch@infradead.org (Christoph Hellwig)
To: linux-riscv@lists.infradead.org
Subject: [RFC 3/3] RISC-V: Remove per cpu clocksource
Date: Mon, 17 Sep 2018 07:35:50 -0700	[thread overview]
Message-ID: <20180917143550.GC15588@infradead.org> (raw)
In-Reply-To: <1536962096-233842-4-git-send-email-atish.patra@wdc.com>

I think only having one clocksource is indeed the right thing.

But that makes the whole TIMER_OF_DECLARE for each hart (cpu core for
those not RISC-V savvy) even more questionable than it already is.

I think we should just initialize the clocksource directly as it is
architectually guaranteed to exist.  Below is a completely untested
(not even compiled) version of your patch that does what I think
we should be doing here.  But I'd rather hear from more timer and/or
DT savvy folks before proceeding.

diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
index 4e8b347e43e2..2fe497c67283 100644
--- a/drivers/clocksource/riscv_timer.c
+++ b/drivers/clocksource/riscv_timer.c
@@ -48,7 +48,7 @@ static unsigned long long riscv_clocksource_rdtime(struct clocksource *cs)
 	return get_cycles64();
 }
 
-static DEFINE_PER_CPU(struct clocksource, riscv_clocksource) = {
+static struct clocksource, riscv_clocksource = {
 	.name		= "riscv_clocksource",
 	.rating		= 300,
 	.mask		= CLOCKSOURCE_MASK(BITS_PER_LONG),
@@ -82,24 +82,16 @@ void riscv_timer_interrupt(void)
 	evdev->event_handler(evdev);
 }
 
-static int __init riscv_timer_init_dt(struct device_node *n)
+static int __init riscv_timer_init(void)
 {
-	int cpu_id = riscv_of_processor_hart(n), error;
-	struct clocksource *cs;
-
-	if (cpu_id != smp_processor_id())
-		return 0;
-
-	cs = per_cpu_ptr(&riscv_clocksource, cpu_id);
-	clocksource_register_hz(cs, riscv_timebase);
+	int error;
 
+	clocksource_register_hz(&riscv_clocksource, riscv_timebase);
 	error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING,
 			 "clockevents/riscv/timer:starting",
 			 riscv_timer_starting_cpu, riscv_timer_dying_cpu);
 	if (error)
-		pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
-		       error, cpu_id);
+		pr_err("RISCV timer register failed: %d.\n", error);
 	return error;
 }
-
-TIMER_OF_DECLARE(riscv_timer, "riscv", riscv_timer_init_dt);
+core_initcall(riscv_timer_init);

WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org>
To: Atish Patra <atish.patra@wdc.com>
Cc: palmer@sifive.com, linux-riscv@lists.infradead.org,
	hch@infradead.org, mark.rutland@arm.com, robh@kernel.org,
	Damien.LeMoal@wdc.com, marc.zyngier@arm.com, anup@brainfault.org,
	linux-kernel@vger.kernel.org, tglx@linutronix.de
Subject: Re: [RFC 3/3] RISC-V: Remove per cpu clocksource
Date: Mon, 17 Sep 2018 07:35:50 -0700	[thread overview]
Message-ID: <20180917143550.GC15588@infradead.org> (raw)
In-Reply-To: <1536962096-233842-4-git-send-email-atish.patra@wdc.com>

I think only having one clocksource is indeed the right thing.

But that makes the whole TIMER_OF_DECLARE for each hart (cpu core for
those not RISC-V savvy) even more questionable than it already is.

I think we should just initialize the clocksource directly as it is
architectually guaranteed to exist.  Below is a completely untested
(not even compiled) version of your patch that does what I think
we should be doing here.  But I'd rather hear from more timer and/or
DT savvy folks before proceeding.

diff --git a/drivers/clocksource/riscv_timer.c b/drivers/clocksource/riscv_timer.c
index 4e8b347e43e2..2fe497c67283 100644
--- a/drivers/clocksource/riscv_timer.c
+++ b/drivers/clocksource/riscv_timer.c
@@ -48,7 +48,7 @@ static unsigned long long riscv_clocksource_rdtime(struct clocksource *cs)
 	return get_cycles64();
 }
 
-static DEFINE_PER_CPU(struct clocksource, riscv_clocksource) = {
+static struct clocksource, riscv_clocksource = {
 	.name		= "riscv_clocksource",
 	.rating		= 300,
 	.mask		= CLOCKSOURCE_MASK(BITS_PER_LONG),
@@ -82,24 +82,16 @@ void riscv_timer_interrupt(void)
 	evdev->event_handler(evdev);
 }
 
-static int __init riscv_timer_init_dt(struct device_node *n)
+static int __init riscv_timer_init(void)
 {
-	int cpu_id = riscv_of_processor_hart(n), error;
-	struct clocksource *cs;
-
-	if (cpu_id != smp_processor_id())
-		return 0;
-
-	cs = per_cpu_ptr(&riscv_clocksource, cpu_id);
-	clocksource_register_hz(cs, riscv_timebase);
+	int error;
 
+	clocksource_register_hz(&riscv_clocksource, riscv_timebase);
 	error = cpuhp_setup_state(CPUHP_AP_RISCV_TIMER_STARTING,
 			 "clockevents/riscv/timer:starting",
 			 riscv_timer_starting_cpu, riscv_timer_dying_cpu);
 	if (error)
-		pr_err("RISCV timer register failed [%d] for cpu = [%d]\n",
-		       error, cpu_id);
+		pr_err("RISCV timer register failed: %d.\n", error);
 	return error;
 }
-
-TIMER_OF_DECLARE(riscv_timer, "riscv", riscv_timer_init_dt);
+core_initcall(riscv_timer_init);

  reply	other threads:[~2018-09-17 14:35 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-14 21:54 [RFC 0/3] Timer code cleanup Atish Patra
2018-09-14 21:54 ` Atish Patra
2018-09-14 21:54 ` [RFC 1/3] dt-bindings: Correct RISC-V's timebase-frequency Atish Patra
2018-09-14 21:54   ` Atish Patra
2018-09-17 14:20   ` Christoph Hellwig
2018-09-17 14:20     ` Christoph Hellwig
2018-09-18  2:26     ` Atish Patra
2018-09-18  2:26       ` Atish Patra
2018-09-14 21:54 ` [RFC 2/3] RISC-V:Support per-hart timebase-frequency Atish Patra
2018-09-14 21:54   ` Atish Patra
2018-09-17 14:23   ` Christoph Hellwig
2018-09-17 14:23     ` Christoph Hellwig
2018-09-18  2:23     ` Atish Patra
2018-09-18  2:23       ` Atish Patra
2018-09-29  0:20     ` Palmer Dabbelt
2018-09-29  0:20       ` Palmer Dabbelt
2018-09-29  0:20       ` Palmer Dabbelt
2018-09-14 21:54 ` [RFC 3/3] RISC-V: Remove per cpu clocksource Atish Patra
2018-09-14 21:54   ` Atish Patra
2018-09-17 14:35   ` Christoph Hellwig [this message]
2018-09-17 14:35     ` Christoph Hellwig
2018-09-17 14:52     ` Thomas Gleixner
2018-09-17 14:52       ` Thomas Gleixner
2018-09-17 15:01       ` Christoph Hellwig
2018-09-17 15:01         ` Christoph Hellwig
2018-09-17 15:04         ` Thomas Gleixner
2018-09-17 15:04           ` Thomas Gleixner
2018-09-17 15:54           ` Anup Patel
2018-09-17 15:54             ` Anup Patel
2018-09-17 18:32         ` Atish Patra
2018-09-17 18:32           ` Atish Patra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180917143550.GC15588@infradead.org \
    --to=hch@infradead.org \
    --cc=linux-riscv@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.