linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12
@ 2017-03-30 21:01 John Stultz
  2017-03-30 21:01 ` [PATCH 1/9] clocksource: sh_cmt: Compute rate before registration again John Stultz
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: John Stultz @ 2017-03-30 21:01 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Ingo Molnar, Thomas Gleixner, Daniel Lezcano,
	Richard Cochran, Prarit Bhargava, Stephen Boyd

Hey Thomas, Ingo,
  Here are the changes I've got queued so far for 4.12.
Main changes are the initial steps of Nicoli's work to make the
clockevent timers be corrected for NTP adjustments. Then a few
smaller fixes that I've queued, and adding Stephen Boyd to
the maintainers list for timekeeping.

Please let me know if you have any objections or thoughts.

thanks
-john

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>

The following changes since commit 97da3854c526d3a6ee05c849c96e48d21527606c:

  Linux 4.11-rc3 (2017-03-19 19:09:39 -0700)

are available in the git repository at:

  https://git.linaro.org/people/john.stultz/linux.git fortglx/4.12/time

for you to fetch changes up to 0107042768658fea9f5f5a9c00b1c90f5dab6a06:

  sysrq: Reset the watchdog timers while displaying high-resolution timers (2017-03-23 12:46:53 -0700)

----------------------------------------------------------------
David Engraf (1):
  timers, sched_clock: Update timeout for clock wrap

John Stultz (1):
  MAINTAINERS: Add Stephen Boyd as timekeeping reviewer

Nicolai Stange (6):
  clocksource: sh_cmt: Compute rate before registration again
  clocksource: sh_tmu: Compute rate before registration again
  clocksource: em_sti: Split clock prepare and enable steps
  clocksource: em_sti: Compute rate before registration
  clocksource: h8300_timer8: Don't reset rate in ->set_state_oneshot()
  clockevents: Make clockevents_config() static

Tom Hromatka (1):
  sysrq: Reset the watchdog timers while displaying high-resolution
    timers

 MAINTAINERS                        |  1 +
 drivers/clocksource/em_sti.c       | 46 +++++++++++++++++++++-----------------
 drivers/clocksource/h8300_timer8.c |  8 -------
 drivers/clocksource/sh_cmt.c       | 45 ++++++++++++++++++++-----------------
 drivers/clocksource/sh_tmu.c       | 26 ++++++++++-----------
 include/linux/clockchips.h         |  1 -
 kernel/time/clockevents.c          |  2 +-
 kernel/time/sched_clock.c          |  5 +++++
 kernel/time/timer_list.c           |  6 +++++
 9 files changed, 77 insertions(+), 63 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/9] clocksource: sh_cmt: Compute rate before registration again
  2017-03-30 21:01 [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12 John Stultz
@ 2017-03-30 21:01 ` John Stultz
  2017-03-30 21:01 ` [PATCH 2/9] clocksource: sh_tmu: " John Stultz
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2017-03-30 21:01 UTC (permalink / raw)
  To: lkml
  Cc: Nicolai Stange, Ingo Molnar, Thomas Gleixner, Daniel Lezcano,
	Richard Cochran, Prarit Bhargava, Stephen Boyd, John Stultz

From: Nicolai Stange <nicstange@gmail.com>

With the upcoming NTP correction related rate adjustments to be implemented
in the clockevents core, the latter needs to get informed about every rate
change of a clockevent device made after its registration.

Currently, sh_cmt violates this requirement in that it registers its
clockevent device with a dummy rate and sets its final ->mult and ->shift
values from its ->set_state_oneshot() and ->set_state_periodic() functions
respectively.

This patch moves the setting of the clockevent device's ->mult and ->shift
values to before its registration.

Note that there has been some back and forth regarding this question with
respect to the clocksource also provided by this driver:
  commit f4d7c3565c16 ("clocksource: sh_cmt: compute mult and shift before
                        registration")
moves the rate determination from the clocksource's ->enable() function to
before its registration. OTOH, the later
  commit 3593f5fe40a1 ("clocksource: sh_cmt: __clocksource_updatefreq_hz()
                        update")
basically reverts this, saying
  "Without this patch the old code uses clocksource_register() together
   with a hack that assumes a never changing clock rate."

However, I checked all current sh_cmt users in arch/sh as well as in
arch/arm/mach-shmobile carefully and right now, none of them changes any
rate in any clock tree relevant to sh_cmt after their respective
time_init(). Since all sh_cmt instances are created after time_init(), none
of them should ever observe any clock rate changes.

What's more, both, a clocksource as well as a clockevent device, can
immediately get selected for use at their registration and thus, enabled
at this point already. So it's probably safer to assume a "never changing
clock rate" here.

- Move the struct sh_cmt_channel's ->rate member to struct sh_cmt_device:
  it's a property of the underlying clock which is in turn specific to
  the sh_cmt_device.
- Determine the ->rate value in sh_cmt_setup() at device probing rather
  than at first usage.
- Set the clockevent device's ->mult and ->shift values right before its
  registration.
- Although not strictly necessary for the upcoming clockevent core changes,
  set the clocksource's rate at its registration for consistency.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/clocksource/sh_cmt.c | 45 ++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)

diff --git a/drivers/clocksource/sh_cmt.c b/drivers/clocksource/sh_cmt.c
index 28757ed..e3bf3ba 100644
--- a/drivers/clocksource/sh_cmt.c
+++ b/drivers/clocksource/sh_cmt.c
@@ -103,7 +103,6 @@ struct sh_cmt_channel {
 	unsigned long match_value;
 	unsigned long next_match_value;
 	unsigned long max_match_value;
-	unsigned long rate;
 	raw_spinlock_t lock;
 	struct clock_event_device ced;
 	struct clocksource cs;
@@ -118,6 +117,7 @@ struct sh_cmt_device {
 
 	void __iomem *mapbase;
 	struct clk *clk;
+	unsigned long rate;
 
 	raw_spinlock_t lock; /* Protect the shared start/stop register */
 
@@ -320,7 +320,7 @@ static void sh_cmt_start_stop_ch(struct sh_cmt_channel *ch, int start)
 	raw_spin_unlock_irqrestore(&ch->cmt->lock, flags);
 }
 
-static int sh_cmt_enable(struct sh_cmt_channel *ch, unsigned long *rate)
+static int sh_cmt_enable(struct sh_cmt_channel *ch)
 {
 	int k, ret;
 
@@ -340,11 +340,9 @@ static int sh_cmt_enable(struct sh_cmt_channel *ch, unsigned long *rate)
 
 	/* configure channel, periodic mode and maximum timeout */
 	if (ch->cmt->info->width == 16) {
-		*rate = clk_get_rate(ch->cmt->clk) / 512;
 		sh_cmt_write_cmcsr(ch, SH_CMT16_CMCSR_CMIE |
 				   SH_CMT16_CMCSR_CKS512);
 	} else {
-		*rate = clk_get_rate(ch->cmt->clk) / 8;
 		sh_cmt_write_cmcsr(ch, SH_CMT32_CMCSR_CMM |
 				   SH_CMT32_CMCSR_CMTOUT_IE |
 				   SH_CMT32_CMCSR_CMR_IRQ |
@@ -572,7 +570,7 @@ static int sh_cmt_start(struct sh_cmt_channel *ch, unsigned long flag)
 	raw_spin_lock_irqsave(&ch->lock, flags);
 
 	if (!(ch->flags & (FLAG_CLOCKEVENT | FLAG_CLOCKSOURCE)))
-		ret = sh_cmt_enable(ch, &ch->rate);
+		ret = sh_cmt_enable(ch);
 
 	if (ret)
 		goto out;
@@ -640,10 +638,9 @@ static int sh_cmt_clocksource_enable(struct clocksource *cs)
 	ch->total_cycles = 0;
 
 	ret = sh_cmt_start(ch, FLAG_CLOCKSOURCE);
-	if (!ret) {
-		__clocksource_update_freq_hz(cs, ch->rate);
+	if (!ret)
 		ch->cs_enabled = true;
-	}
+
 	return ret;
 }
 
@@ -697,8 +694,7 @@ static int sh_cmt_register_clocksource(struct sh_cmt_channel *ch,
 	dev_info(&ch->cmt->pdev->dev, "ch%u: used as clock source\n",
 		 ch->index);
 
-	/* Register with dummy 1 Hz value, gets updated in ->enable() */
-	clocksource_register_hz(cs, 1);
+	clocksource_register_hz(cs, ch->cmt->rate);
 	return 0;
 }
 
@@ -709,19 +705,10 @@ static struct sh_cmt_channel *ced_to_sh_cmt(struct clock_event_device *ced)
 
 static void sh_cmt_clock_event_start(struct sh_cmt_channel *ch, int periodic)
 {
-	struct clock_event_device *ced = &ch->ced;
-
 	sh_cmt_start(ch, FLAG_CLOCKEVENT);
 
-	/* TODO: calculate good shift from rate and counter bit width */
-
-	ced->shift = 32;
-	ced->mult = div_sc(ch->rate, NSEC_PER_SEC, ced->shift);
-	ced->max_delta_ns = clockevent_delta2ns(ch->max_match_value, ced);
-	ced->min_delta_ns = clockevent_delta2ns(0x1f, ced);
-
 	if (periodic)
-		sh_cmt_set_next(ch, ((ch->rate + HZ/2) / HZ) - 1);
+		sh_cmt_set_next(ch, ((ch->cmt->rate + HZ/2) / HZ) - 1);
 	else
 		sh_cmt_set_next(ch, ch->max_match_value);
 }
@@ -824,6 +811,12 @@ static int sh_cmt_register_clockevent(struct sh_cmt_channel *ch,
 	ced->suspend = sh_cmt_clock_event_suspend;
 	ced->resume = sh_cmt_clock_event_resume;
 
+	/* TODO: calculate good shift from rate and counter bit width */
+	ced->shift = 32;
+	ced->mult = div_sc(ch->cmt->rate, NSEC_PER_SEC, ced->shift);
+	ced->max_delta_ns = clockevent_delta2ns(ch->max_match_value, ced);
+	ced->min_delta_ns = clockevent_delta2ns(0x1f, ced);
+
 	dev_info(&ch->cmt->pdev->dev, "ch%u: used for clock events\n",
 		 ch->index);
 	clockevents_register_device(ced);
@@ -996,6 +989,18 @@ static int sh_cmt_setup(struct sh_cmt_device *cmt, struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk_put;
 
+	/* Determine clock rate. */
+	ret = clk_enable(cmt->clk);
+	if (ret < 0)
+		goto err_clk_unprepare;
+
+	if (cmt->info->width == 16)
+		cmt->rate = clk_get_rate(cmt->clk) / 512;
+	else
+		cmt->rate = clk_get_rate(cmt->clk) / 8;
+
+	clk_disable(cmt->clk);
+
 	/* Map the memory resource(s). */
 	ret = sh_cmt_map_memory(cmt);
 	if (ret < 0)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/9] clocksource: sh_tmu: Compute rate before registration again
  2017-03-30 21:01 [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12 John Stultz
  2017-03-30 21:01 ` [PATCH 1/9] clocksource: sh_cmt: Compute rate before registration again John Stultz
@ 2017-03-30 21:01 ` John Stultz
  2017-03-30 21:01 ` [PATCH 3/9] clocksource: em_sti: Split clock prepare and enable steps John Stultz
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2017-03-30 21:01 UTC (permalink / raw)
  To: lkml
  Cc: Nicolai Stange, Ingo Molnar, Thomas Gleixner, Daniel Lezcano,
	Richard Cochran, Prarit Bhargava, Stephen Boyd, John Stultz

From: Nicolai Stange <nicstange@gmail.com>

With the upcoming NTP correction related rate adjustments to be implemented
in the clockevents core, the latter needs to get informed about every rate
change of a clockevent device made after its registration.

Currently, sh_tmu violates this requirement in that it registers its
clockevent device with a dummy rate and sets its final rate through
clockevents_config() called from its ->set_state_oneshot() and
->set_state_periodic() functions respectively.

This patch moves the setting of the clockevent device's rate to its
registration.

Note that there has been some back and forth regarding this question with
respect to the clocksource also provided by this driver:
  commit 66f49121ffa4 ("clocksource: sh_tmu: compute mult and shift before
                        registration")
moves the rate determination from the clocksource's ->enable() function to
before its registration. OTOH, the later
  commit 0aeac458d9eb ("clocksource: sh_tmu: __clocksource_updatefreq_hz()
                        update")
basically reverts this, saying
  "Without this patch the old code uses clocksource_register() together
   with a hack that assumes a never changing clock rate."

However, I checked all current sh_tmu users in arch/sh as well as in
arch/arm/mach-shmobile carefully and right now, none of them changes any
rate in any clock tree relevant to sh_tmu after their respective
time_init(). Since all sh_tmu instances are created after time_init(), none
of them should ever observe any clock rate changes.

What's more, both, a clocksource as well as a clockevent device, can
immediately get selected for use at their registration and thus, enabled
at this point already. So it's probably safer to assume a "never changing
clock rate" here.

- Move the struct sh_tmu_channel's ->rate member to struct sh_tmu_device:
  it's a property of the underlying clock which is in turn specific to
  the sh_tmu_device.
- Determine the ->rate value in sh_tmu_setup() at device probing rather
  than at first usage.
- Set the clockevent device's rate at its registration.
- Although not strictly necessary for the upcoming clockevent core changes,
  set the clocksource's rate at its registration for consistency.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/clocksource/sh_tmu.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/sh_tmu.c b/drivers/clocksource/sh_tmu.c
index 1fbf2aa..31d8816 100644
--- a/drivers/clocksource/sh_tmu.c
+++ b/drivers/clocksource/sh_tmu.c
@@ -46,7 +46,6 @@ struct sh_tmu_channel {
 	void __iomem *base;
 	int irq;
 
-	unsigned long rate;
 	unsigned long periodic;
 	struct clock_event_device ced;
 	struct clocksource cs;
@@ -59,6 +58,7 @@ struct sh_tmu_device {
 
 	void __iomem *mapbase;
 	struct clk *clk;
+	unsigned long rate;
 
 	enum sh_tmu_model model;
 
@@ -165,7 +165,6 @@ static int __sh_tmu_enable(struct sh_tmu_channel *ch)
 	sh_tmu_write(ch, TCNT, 0xffffffff);
 
 	/* configure channel to parent clock / 4, irq off */
-	ch->rate = clk_get_rate(ch->tmu->clk) / 4;
 	sh_tmu_write(ch, TCR, TCR_TPSC_CLK4);
 
 	/* enable channel */
@@ -271,10 +270,8 @@ static int sh_tmu_clocksource_enable(struct clocksource *cs)
 		return 0;
 
 	ret = sh_tmu_enable(ch);
-	if (!ret) {
-		__clocksource_update_freq_hz(cs, ch->rate);
+	if (!ret)
 		ch->cs_enabled = true;
-	}
 
 	return ret;
 }
@@ -334,8 +331,7 @@ static int sh_tmu_register_clocksource(struct sh_tmu_channel *ch,
 	dev_info(&ch->tmu->pdev->dev, "ch%u: used as clock source\n",
 		 ch->index);
 
-	/* Register with dummy 1 Hz value, gets updated in ->enable() */
-	clocksource_register_hz(cs, 1);
+	clocksource_register_hz(cs, ch->tmu->rate);
 	return 0;
 }
 
@@ -346,14 +342,10 @@ static struct sh_tmu_channel *ced_to_sh_tmu(struct clock_event_device *ced)
 
 static void sh_tmu_clock_event_start(struct sh_tmu_channel *ch, int periodic)
 {
-	struct clock_event_device *ced = &ch->ced;
-
 	sh_tmu_enable(ch);
 
-	clockevents_config(ced, ch->rate);
-
 	if (periodic) {
-		ch->periodic = (ch->rate + HZ/2) / HZ;
+		ch->periodic = (ch->tmu->rate + HZ/2) / HZ;
 		sh_tmu_set_next(ch, ch->periodic, 1);
 	}
 }
@@ -435,7 +427,7 @@ static void sh_tmu_register_clockevent(struct sh_tmu_channel *ch,
 	dev_info(&ch->tmu->pdev->dev, "ch%u: used for clock events\n",
 		 ch->index);
 
-	clockevents_config_and_register(ced, 1, 0x300, 0xffffffff);
+	clockevents_config_and_register(ced, ch->tmu->rate, 0x300, 0xffffffff);
 
 	ret = request_irq(ch->irq, sh_tmu_interrupt,
 			  IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
@@ -561,6 +553,14 @@ static int sh_tmu_setup(struct sh_tmu_device *tmu, struct platform_device *pdev)
 	if (ret < 0)
 		goto err_clk_put;
 
+	/* Determine clock rate. */
+	ret = clk_enable(tmu->clk);
+	if (ret < 0)
+		goto err_clk_unprepare;
+
+	tmu->rate = clk_get_rate(tmu->clk) / 4;
+	clk_disable(tmu->clk);
+
 	/* Map the memory resource. */
 	ret = sh_tmu_map_memory(tmu);
 	if (ret < 0) {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/9] clocksource: em_sti: Split clock prepare and enable steps
  2017-03-30 21:01 [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12 John Stultz
  2017-03-30 21:01 ` [PATCH 1/9] clocksource: sh_cmt: Compute rate before registration again John Stultz
  2017-03-30 21:01 ` [PATCH 2/9] clocksource: sh_tmu: " John Stultz
@ 2017-03-30 21:01 ` John Stultz
  2017-03-30 21:01 ` [PATCH 4/9] clocksource: em_sti: Compute rate before registration John Stultz
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2017-03-30 21:01 UTC (permalink / raw)
  To: lkml
  Cc: Nicolai Stange, Ingo Molnar, Thomas Gleixner, Daniel Lezcano,
	Richard Cochran, Prarit Bhargava, Stephen Boyd, John Stultz

From: Nicolai Stange <nicstange@gmail.com>

Currently, the em_sti driver prepares and enables the needed clock in
em_sti_enable(), potentially called through its clockevent device's
->set_state_oneshot().

However, the clk_prepare() step may sleep whereas tick_program_event() and
thus, ->set_state_oneshot(), can be called in atomic context.

Split the clk_prepare_enable() in em_sti_enable() into two steps:
- prepare the clock at device probing via clk_prepare()
- and enable it in em_sti_enable() via clk_enable().
Slightly reorder resource initialization in em_sti_probe() in order to
facilitate error handling in later patches.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/clocksource/em_sti.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
index aff87df..6c0955a 100644
--- a/drivers/clocksource/em_sti.c
+++ b/drivers/clocksource/em_sti.c
@@ -78,7 +78,7 @@ static int em_sti_enable(struct em_sti_priv *p)
 	int ret;
 
 	/* enable clock */
-	ret = clk_prepare_enable(p->clk);
+	ret = clk_enable(p->clk);
 	if (ret) {
 		dev_err(&p->pdev->dev, "cannot enable clock\n");
 		return ret;
@@ -107,7 +107,7 @@ static void em_sti_disable(struct em_sti_priv *p)
 	em_sti_write(p, STI_INTENCLR, 3);
 
 	/* stop clock */
-	clk_disable_unprepare(p->clk);
+	clk_disable(p->clk);
 }
 
 static u64 em_sti_count(struct em_sti_priv *p)
@@ -303,6 +303,7 @@ static int em_sti_probe(struct platform_device *pdev)
 	struct em_sti_priv *p;
 	struct resource *res;
 	int irq;
+	int ret;
 
 	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
 	if (p == NULL)
@@ -323,6 +324,13 @@ static int em_sti_probe(struct platform_device *pdev)
 	if (IS_ERR(p->base))
 		return PTR_ERR(p->base);
 
+	if (devm_request_irq(&pdev->dev, irq, em_sti_interrupt,
+			     IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
+			     dev_name(&pdev->dev), p)) {
+		dev_err(&pdev->dev, "failed to request low IRQ\n");
+		return -ENOENT;
+	}
+
 	/* get hold of clock */
 	p->clk = devm_clk_get(&pdev->dev, "sclk");
 	if (IS_ERR(p->clk)) {
@@ -330,11 +338,10 @@ static int em_sti_probe(struct platform_device *pdev)
 		return PTR_ERR(p->clk);
 	}
 
-	if (devm_request_irq(&pdev->dev, irq, em_sti_interrupt,
-			     IRQF_TIMER | IRQF_IRQPOLL | IRQF_NOBALANCING,
-			     dev_name(&pdev->dev), p)) {
-		dev_err(&pdev->dev, "failed to request low IRQ\n");
-		return -ENOENT;
+	ret = clk_prepare(p->clk);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "cannot prepare clock\n");
+		return ret;
 	}
 
 	raw_spin_lock_init(&p->lock);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/9] clocksource: em_sti: Compute rate before registration
  2017-03-30 21:01 [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12 John Stultz
                   ` (2 preceding siblings ...)
  2017-03-30 21:01 ` [PATCH 3/9] clocksource: em_sti: Split clock prepare and enable steps John Stultz
@ 2017-03-30 21:01 ` John Stultz
  2017-03-30 21:01 ` [PATCH 5/9] clocksource: h8300_timer8: Don't reset rate in ->set_state_oneshot() John Stultz
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2017-03-30 21:01 UTC (permalink / raw)
  To: lkml
  Cc: Nicolai Stange, Ingo Molnar, Thomas Gleixner, Daniel Lezcano,
	Richard Cochran, Prarit Bhargava, Stephen Boyd, John Stultz

From: Nicolai Stange <nicstange@gmail.com>

With the upcoming NTP correction related rate adjustments to be implemented
in the clockevents core, the latter needs to get informed about every rate
change of a clockevent device made after its registration.

Currently, em_sti violates this requirement in that it registers its
clockevent device with a dummy rate and sets its final rate through
clockevents_config() called from its ->set_state_oneshot().

This patch moves the setting of the clockevent device's rate to its
registration.

I checked all current em_sti users in arch/arm/mach-shmobile and right now,
none of them changes any rate in any clock tree relevant to em_sti after
their respective time_init(). Since all em_sti instances are created after
time_init(), none of them should ever observe any clock rate changes.

- Determine the ->rate value in em_sti_probe() at device probing rather
  than at first usage.
- Set the clockevent device's rate at its registration.
- Although not strictly necessary for the upcoming clockevent core changes,
  set the clocksource's rate at its registration for consistency.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/clocksource/em_sti.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
index 6c0955a..bc48cbf 100644
--- a/drivers/clocksource/em_sti.c
+++ b/drivers/clocksource/em_sti.c
@@ -84,9 +84,6 @@ static int em_sti_enable(struct em_sti_priv *p)
 		return ret;
 	}
 
-	/* configure channel, periodic mode and maximum timeout */
-	p->rate = clk_get_rate(p->clk);
-
 	/* reset the counter */
 	em_sti_write(p, STI_SET_H, 0x40000000);
 	em_sti_write(p, STI_SET_L, 0x00000000);
@@ -205,13 +202,9 @@ static u64 em_sti_clocksource_read(struct clocksource *cs)
 
 static int em_sti_clocksource_enable(struct clocksource *cs)
 {
-	int ret;
 	struct em_sti_priv *p = cs_to_em_sti(cs);
 
-	ret = em_sti_start(p, USER_CLOCKSOURCE);
-	if (!ret)
-		__clocksource_update_freq_hz(cs, p->rate);
-	return ret;
+	return em_sti_start(p, USER_CLOCKSOURCE);
 }
 
 static void em_sti_clocksource_disable(struct clocksource *cs)
@@ -240,8 +233,7 @@ static int em_sti_register_clocksource(struct em_sti_priv *p)
 
 	dev_info(&p->pdev->dev, "used as clock source\n");
 
-	/* Register with dummy 1 Hz value, gets updated in ->enable() */
-	clocksource_register_hz(cs, 1);
+	clocksource_register_hz(cs, p->rate);
 	return 0;
 }
 
@@ -263,7 +255,6 @@ static int em_sti_clock_event_set_oneshot(struct clock_event_device *ced)
 
 	dev_info(&p->pdev->dev, "used for oneshot clock events\n");
 	em_sti_start(p, USER_CLOCKEVENT);
-	clockevents_config(&p->ced, p->rate);
 	return 0;
 }
 
@@ -294,8 +285,7 @@ static void em_sti_register_clockevent(struct em_sti_priv *p)
 
 	dev_info(&p->pdev->dev, "used for clock events\n");
 
-	/* Register with dummy 1 Hz value, gets updated in ->set_state_oneshot() */
-	clockevents_config_and_register(ced, 1, 2, 0xffffffff);
+	clockevents_config_and_register(ced, p->rate, 2, 0xffffffff);
 }
 
 static int em_sti_probe(struct platform_device *pdev)
@@ -344,6 +334,15 @@ static int em_sti_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	ret = clk_enable(p->clk);
+	if (ret < 0) {
+		dev_err(&p->pdev->dev, "cannot enable clock\n");
+		clk_unprepare(p->clk);
+		return ret;
+	}
+	p->rate = clk_get_rate(p->clk);
+	clk_disable(p->clk);
+
 	raw_spin_lock_init(&p->lock);
 	em_sti_register_clockevent(p);
 	em_sti_register_clocksource(p);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/9] clocksource: h8300_timer8: Don't reset rate in ->set_state_oneshot()
  2017-03-30 21:01 [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12 John Stultz
                   ` (3 preceding siblings ...)
  2017-03-30 21:01 ` [PATCH 4/9] clocksource: em_sti: Compute rate before registration John Stultz
@ 2017-03-30 21:01 ` John Stultz
  2017-03-30 21:01 ` [PATCH 6/9] clockevents: Make clockevents_config() static John Stultz
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2017-03-30 21:01 UTC (permalink / raw)
  To: lkml
  Cc: Nicolai Stange, Ingo Molnar, Thomas Gleixner, Daniel Lezcano,
	Richard Cochran, Prarit Bhargava, Stephen Boyd, John Stultz

From: Nicolai Stange <nicstange@gmail.com>

With the upcoming NTP correction related rate adjustments to be implemented
in the clockevents core, the latter needs to get informed about every rate
change of a clockevent device made after its registration.

Currently, h8300_timer8 violates this requirement in that it registers its
clockevent device with the correct rate, but resets its ->mult and ->rate
values in timer8_clock_event_start(), called from its ->set_state_oneshot()
function.

It seems like
  commit 4633f4cac85a ("clocksource/drivers/h8300: Cleanup startup and
                        remove module code."),
which introduced the rate initialization at registration, missed to remove
the manual setting of ->mult and ->shift from timer8_clock_event_start().

Purge the setting of ->mult, ->shift, ->min_delta_ns and ->max_delta_ns
from timer8_clock_event_start().

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 drivers/clocksource/h8300_timer8.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index 546bb18..804c489 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -101,15 +101,7 @@ static inline struct timer8_priv *ced_to_priv(struct clock_event_device *ced)
 
 static void timer8_clock_event_start(struct timer8_priv *p, unsigned long delta)
 {
-	struct clock_event_device *ced = &p->ced;
-
 	timer8_start(p);
-
-	ced->shift = 32;
-	ced->mult = div_sc(p->rate, NSEC_PER_SEC, ced->shift);
-	ced->max_delta_ns = clockevent_delta2ns(0xffff, ced);
-	ced->min_delta_ns = clockevent_delta2ns(0x0001, ced);
-
 	timer8_set_next(p, delta);
 }
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/9] clockevents: Make clockevents_config() static
  2017-03-30 21:01 [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12 John Stultz
                   ` (4 preceding siblings ...)
  2017-03-30 21:01 ` [PATCH 5/9] clocksource: h8300_timer8: Don't reset rate in ->set_state_oneshot() John Stultz
@ 2017-03-30 21:01 ` John Stultz
  2017-03-30 21:01 ` [PATCH 7/9] MAINTAINERS: Add Stephen Boyd as timekeeping reviewer John Stultz
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2017-03-30 21:01 UTC (permalink / raw)
  To: lkml
  Cc: Nicolai Stange, Ingo Molnar, Thomas Gleixner, Daniel Lezcano,
	Richard Cochran, Prarit Bhargava, Stephen Boyd, John Stultz

From: Nicolai Stange <nicstange@gmail.com>

A clockevent device's rate should be configured before or at registration
and changed afterwards through clockevents_update_freq() only.

For the configuration at registration, we already have
clockevents_config_and_register().

Right now, there are no clockevents_config() users outside of the
clockevents core.

To mitigiate the risk of drivers errorneously reconfiguring their rates
through clockevents_config() *after* device registration, make
clockevents_config() static.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Nicolai Stange <nicstange@gmail.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 include/linux/clockchips.h | 1 -
 kernel/time/clockevents.c  | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index 5d3053c..eef1569 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -182,7 +182,6 @@ extern u64 clockevent_delta2ns(unsigned long latch, struct clock_event_device *e
 extern void clockevents_register_device(struct clock_event_device *dev);
 extern int clockevents_unbind_device(struct clock_event_device *ced, int cpu);
 
-extern void clockevents_config(struct clock_event_device *dev, u32 freq);
 extern void clockevents_config_and_register(struct clock_event_device *dev,
 					    u32 freq, unsigned long min_delta,
 					    unsigned long max_delta);
diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 97ac095..4237e07 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -468,7 +468,7 @@ void clockevents_register_device(struct clock_event_device *dev)
 }
 EXPORT_SYMBOL_GPL(clockevents_register_device);
 
-void clockevents_config(struct clock_event_device *dev, u32 freq)
+static void clockevents_config(struct clock_event_device *dev, u32 freq)
 {
 	u64 sec;
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/9] MAINTAINERS: Add Stephen Boyd as timekeeping reviewer
  2017-03-30 21:01 [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12 John Stultz
                   ` (5 preceding siblings ...)
  2017-03-30 21:01 ` [PATCH 6/9] clockevents: Make clockevents_config() static John Stultz
@ 2017-03-30 21:01 ` John Stultz
  2017-03-30 21:01 ` [PATCH 8/9] timers, sched_clock: Update timeout for clock wrap John Stultz
  2017-03-30 21:01 ` [PATCH 9/9] sysrq: Reset the watchdog timers while displaying high-resolution timers John Stultz
  8 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2017-03-30 21:01 UTC (permalink / raw)
  To: lkml
  Cc: John Stultz, Ingo Molnar, Thomas Gleixner, Daniel Lezcano,
	Richard Cochran, Prarit Bhargava, Stephen Boyd

After showing expertise and presenting on the timekeeping
subsystem at ELC[1], Stephen clearly should be included in
the maintainer list.

[1] https://www.youtube.com/watch?v=Puv4mW55bF8

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Acked-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c776906..68b1a14 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11099,6 +11099,7 @@ F:	drivers/power/supply/bq27xxx_battery_i2c.c
 TIMEKEEPING, CLOCKSOURCE CORE, NTP, ALARMTIMER
 M:	John Stultz <john.stultz@linaro.org>
 M:	Thomas Gleixner <tglx@linutronix.de>
+R:	Stephen Boyd <sboyd@codeaurora.org>
 L:	linux-kernel@vger.kernel.org
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git timers/core
 S:	Supported
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/9] timers, sched_clock: Update timeout for clock wrap
  2017-03-30 21:01 [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12 John Stultz
                   ` (6 preceding siblings ...)
  2017-03-30 21:01 ` [PATCH 7/9] MAINTAINERS: Add Stephen Boyd as timekeeping reviewer John Stultz
@ 2017-03-30 21:01 ` John Stultz
  2017-03-30 21:01 ` [PATCH 9/9] sysrq: Reset the watchdog timers while displaying high-resolution timers John Stultz
  8 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2017-03-30 21:01 UTC (permalink / raw)
  To: lkml
  Cc: David Engraf, Ingo Molnar, Thomas Gleixner, Daniel Lezcano,
	Richard Cochran, Prarit Bhargava, Stephen Boyd, John Stultz

From: David Engraf <david.engraf@sysgo.com>

The scheduler clock framework may not use the correct timeout for the clock
wrap. This happens when a new clock driver calls sched_clock_register()
after the kernel called sched_clock_postinit(). In this case the clock wrap
timeout is too long thus sched_clock_poll() is called too late and the clock
already wrapped.

On my ARM system the scheduler was no longer scheduling any other task than
the idle task because the sched_clock() wrapped.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: David Engraf <david.engraf@sysgo.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/sched_clock.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index ea6b610..2d8f05a 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -206,6 +206,11 @@ sched_clock_register(u64 (*read)(void), int bits, unsigned long rate)
 
 	update_clock_read_data(&rd);
 
+	if (sched_clock_timer.function != NULL) {
+		/* update timeout for clock wrap */
+		hrtimer_start(&sched_clock_timer, cd.wrap_kt, HRTIMER_MODE_REL);
+	}
+
 	r = rate;
 	if (r >= 4000000) {
 		r /= 1000000;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 9/9] sysrq: Reset the watchdog timers while displaying high-resolution timers
  2017-03-30 21:01 [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12 John Stultz
                   ` (7 preceding siblings ...)
  2017-03-30 21:01 ` [PATCH 8/9] timers, sched_clock: Update timeout for clock wrap John Stultz
@ 2017-03-30 21:01 ` John Stultz
  8 siblings, 0 replies; 10+ messages in thread
From: John Stultz @ 2017-03-30 21:01 UTC (permalink / raw)
  To: lkml
  Cc: Tom Hromatka, Ingo Molnar, Thomas Gleixner, Daniel Lezcano,
	Richard Cochran, Prarit Bhargava, Stephen Boyd, John Stultz

From: Tom Hromatka <tom.hromatka@oracle.com>

On systems with a large number of CPUs, running sysrq-<q> can cause
watchdog timeouts.  There are two slow sections of code in the sysrq-<q>
path in timer_list.c.

1. print_active_timers() - This function is called by print_cpu() and
   contains a slow goto loop.  On a machine with hundreds of CPUs, this
   loop took approximately 100ms for the first CPU in a NUMA node.
   (Subsequent CPUs in the same node ran much quicker.)  The total time
   to print all of the CPUs is ultimately long enough to trigger the
   soft lockup watchdog.

2. print_tickdevice() - This function outputs a large amount of textual
   information.  This function also took approximately 100ms per CPU.

Since sysrq-<q> is not a performance critical path, there should be no
harm in touching the nmi watchdog in both slow sections above.  Touching
it in just one location was insufficient on systems with hundreds of
CPUs as occasional timeouts were still observed during testing.

This issue was observed on an Oracle T7 machine with 128 CPUs, but I
anticipate it may affect other systems with similarly large numbers of
CPUs.

Cc: Ingo Molnar <mingo@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Prarit Bhargava <prarit@redhat.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
Reviewed-by: Rob Gardner <rob.gardner@oracle.com>
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
 kernel/time/timer_list.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c
index ff8d5c1..0e7f542 100644
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -16,6 +16,7 @@
 #include <linux/sched.h>
 #include <linux/seq_file.h>
 #include <linux/kallsyms.h>
+#include <linux/nmi.h>
 
 #include <linux/uaccess.h>
 
@@ -86,6 +87,9 @@ print_active_timers(struct seq_file *m, struct hrtimer_clock_base *base,
 
 next_one:
 	i = 0;
+
+	touch_nmi_watchdog();
+
 	raw_spin_lock_irqsave(&base->cpu_base->lock, flags);
 
 	curr = timerqueue_getnext(&base->active);
@@ -197,6 +201,8 @@ print_tickdevice(struct seq_file *m, struct tick_device *td, int cpu)
 {
 	struct clock_event_device *dev = td->evtdev;
 
+	touch_nmi_watchdog();
+
 	SEQ_printf(m, "Tick Device: mode:     %d\n", td->mode);
 	if (cpu < 0)
 		SEQ_printf(m, "Broadcast device\n");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-03-30 21:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-30 21:01 [GIT PULL][PATCH 0/9] Timekeeping changes for 4.12 John Stultz
2017-03-30 21:01 ` [PATCH 1/9] clocksource: sh_cmt: Compute rate before registration again John Stultz
2017-03-30 21:01 ` [PATCH 2/9] clocksource: sh_tmu: " John Stultz
2017-03-30 21:01 ` [PATCH 3/9] clocksource: em_sti: Split clock prepare and enable steps John Stultz
2017-03-30 21:01 ` [PATCH 4/9] clocksource: em_sti: Compute rate before registration John Stultz
2017-03-30 21:01 ` [PATCH 5/9] clocksource: h8300_timer8: Don't reset rate in ->set_state_oneshot() John Stultz
2017-03-30 21:01 ` [PATCH 6/9] clockevents: Make clockevents_config() static John Stultz
2017-03-30 21:01 ` [PATCH 7/9] MAINTAINERS: Add Stephen Boyd as timekeeping reviewer John Stultz
2017-03-30 21:01 ` [PATCH 8/9] timers, sched_clock: Update timeout for clock wrap John Stultz
2017-03-30 21:01 ` [PATCH 9/9] sysrq: Reset the watchdog timers while displaying high-resolution timers John Stultz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).