Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1464473 > unrolled thread

[PULL] : clockevents/clocksource fixes for 4.8

Back to article view | Back to linux.kernel

Contents

  [PULL] : clockevents/clocksource fixes for 4.8 Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-08-17 12:20 +0200
    [PATCH 2/3] clocksource/drivers/kona: Fix get_counter error handling Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-08-17 12:30 +0200
      [tip:timers/urgent] clocksource/drivers/kona: Fix get_counter()  error handling tip-bot for Arnd Bergmann <tipbot@zytor.com> - 2016-08-17 15:40 +0200
    [PATCH 3/3] clocksource/drivers/mips-gic-timer: Make gic_clocksource_of_init return int Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-08-17 12:30 +0200
      [tip:timers/urgent] clocksource/drivers/mips-gic-timer: Make  gic_clocksource_of_init() return int tip-bot for Paul Gortmaker <tipbot@zytor.com> - 2016-08-17 15:40 +0200
    [PATCH 1/3] clocksource/drivers/time-armada-370-xp: Fix the clock reference Daniel Lezcano <daniel.lezcano@linaro.org> - 2016-08-17 12:30 +0200
      [tip:timers/urgent] clocksource/drivers/time-armada-370-xp: Fix the  clock reference tip-bot for Gregory CLEMENT <tipbot@zytor.com> - 2016-08-17 15:40 +0200

#1464473 — [PULL] : clockevents/clocksource fixes for 4.8

Hi Ingo,

this pull request for tip/timers/urgent contains two critical fixes and
one spurious compiler warning.

 - Change the get_counter error handling function in order to prevent a
warning generated by gcc for the kona driver (Arnd Bergman)

 - Fix a wrong clock reference leading to a kernel hang and slow bootup.
The init function conversion introduced a regression by taking the wrong
clock on armada-370-xp (Gregory Clement)

 - Fix the init function prototype to return int on the mips-gic-driver
(Paul Gortmaker)

Thanks !

  -- Daniel


The following changes since commit 1f3b0f8243cb934307f59bd4d8e43b868e61d4d9:

  tick/nohz: Optimize nohz idle enter (2016-07-19 13:48:24 +0200)

are available in the git repository at:

  ssh://git@git.linaro.org/people/daniel.lezcano/linux.git
clockevents/4.8-fixes

for you to fetch changes up to 394bc60a7ce8d4aa0f5b83b7e121dcdb393739d1:

  clocksource/drivers/mips-gic-timer: Make gic_clocksource_of_init
return int (2016-08-17 07:36:32 +0200)

----------------------------------------------------------------
Arnd Bergmann (1):
      clocksource/drivers/kona: Fix get_counter error handling

Gregory CLEMENT (1):
      clocksource/drivers/time-armada-370-xp: Fix the clock reference

Paul Gortmaker (1):
      clocksource/drivers/mips-gic-timer: Make gic_clocksource_of_init
return int

 drivers/clocksource/bcm_kona_timer.c     | 16 ++++++++++------
 drivers/clocksource/mips-gic-timer.c     |  2 +-
 drivers/clocksource/time-armada-370-xp.c |  1 -
 3 files changed, 11 insertions(+), 8 deletions(-)

-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[toc] | [next] | [standalone]

#1464480 — [PATCH 2/3] clocksource/drivers/kona: Fix get_counter error handling

From: Arnd Bergmann <arnd@arndb.de>

I could not figure out why, but gcc cannot prove that the
kona_timer_init function always initializes its two outputs,
and we get a warning for the use of the 'lsw' variable later,
which is obviously correct.

drivers/clocksource/bcm_kona_timer.c: In function 'kona_timer_init':
drivers/clocksource/bcm_kona_timer.c:119:13: error: 'lsw' may be used uninitialized in this function [-Werror=maybe-uninitialized]

Slightly reordering the loop makes the warning disappear, after
it becomes more obvious to the compiler that the loop is
always entered on the first iteration.

As pointed out by Ray Jui, there is a related problem in the
way we deal with the loop running into the limit, as we just
keep going there with an invalid counter data, so instead we
now propagate a -ETIMEDOUT result to the caller.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Link: https://patchwork.kernel.org/patch/9174261/
Acked-by: Ray Jui <ray.jui@broadcom.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/bcm_kona_timer.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/bcm_kona_timer.c b/drivers/clocksource/bcm_kona_timer.c
index 7e3fd37..92f6e4d 100644
--- a/drivers/clocksource/bcm_kona_timer.c
+++ b/drivers/clocksource/bcm_kona_timer.c
@@ -66,10 +66,10 @@ static void kona_timer_disable_and_clear(void __iomem *base)
 
 }
 
-static void
+static int
 kona_timer_get_counter(void __iomem *timer_base, uint32_t *msw, uint32_t *lsw)
 {
-	int loop_limit = 4;
+	int loop_limit = 3;
 
 	/*
 	 * Read 64-bit free running counter
@@ -83,18 +83,19 @@ kona_timer_get_counter(void __iomem *timer_base, uint32_t *msw, uint32_t *lsw)
 	 *      if new hi-word is equal to previously read hi-word then stop.
 	 */
 
-	while (--loop_limit) {
+	do {
 		*msw = readl(timer_base + KONA_GPTIMER_STCHI_OFFSET);
 		*lsw = readl(timer_base + KONA_GPTIMER_STCLO_OFFSET);
 		if (*msw == readl(timer_base + KONA_GPTIMER_STCHI_OFFSET))
 			break;
-	}
+	} while (--loop_limit);
 	if (!loop_limit) {
 		pr_err("bcm_kona_timer: getting counter failed.\n");
 		pr_err(" Timer will be impacted\n");
+		return -ETIMEDOUT;
 	}
 
-	return;
+	return 0;
 }
 
 static int kona_timer_set_next_event(unsigned long clc,
@@ -112,8 +113,11 @@ static int kona_timer_set_next_event(unsigned long clc,
 
 	uint32_t lsw, msw;
 	uint32_t reg;
+	int ret;
 
-	kona_timer_get_counter(timers.tmr_regs, &msw, &lsw);
+	ret = kona_timer_get_counter(timers.tmr_regs, &msw, &lsw);
+	if (ret)
+		return ret;
 
 	/* Load the "next" event tick value */
 	writel(lsw + clc, timers.tmr_regs + KONA_GPTIMER_STCM0_OFFSET);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1464584 — [tip:timers/urgent] clocksource/drivers/kona: Fix get_counter() error handling

Commit-ID:  16c8eba0fe01e03317f48868105103a8f5938e85
Gitweb:     http://git.kernel.org/tip/16c8eba0fe01e03317f48868105103a8f5938e85
Author:     Arnd Bergmann <arnd@arndb.de>
AuthorDate: Wed, 17 Aug 2016 12:21:34 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 17 Aug 2016 13:08:31 +0200

clocksource/drivers/kona: Fix get_counter() error handling

I could not figure out why, but GCC cannot prove that the
kona_timer_init() function always initializes its two outputs,
and we get a warning for the use of the 'lsw' variable later,
which is obviously correct.

  drivers/clocksource/bcm_kona_timer.c: In function 'kona_timer_init':
  drivers/clocksource/bcm_kona_timer.c:119:13: error: 'lsw' may be used uninitialized in this function [-Werror=maybe-uninitialized]

Slightly reordering the loop makes the warning disappear, after
it becomes more obvious to the compiler that the loop is
always entered on the first iteration.

As pointed out by Ray Jui, there is a related problem in the
way we deal with the loop running into the limit, as we just
keep going there with an invalid counter data, so instead we
now propagate a -ETIMEDOUT result to the caller.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Acked-by: Ray Jui <ray.jui@broadcom.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: bcm-kernel-feedback-list@broadcom.com
Link: http://lkml.kernel.org/r/1471429296-9053-2-git-send-email-daniel.lezcano@linaro.org
Link: https://patchwork.kernel.org/patch/9174261/
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/clocksource/bcm_kona_timer.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/clocksource/bcm_kona_timer.c b/drivers/clocksource/bcm_kona_timer.c
index 7e3fd37..92f6e4d 100644
--- a/drivers/clocksource/bcm_kona_timer.c
+++ b/drivers/clocksource/bcm_kona_timer.c
@@ -66,10 +66,10 @@ static void kona_timer_disable_and_clear(void __iomem *base)
 
 }
 
-static void
+static int
 kona_timer_get_counter(void __iomem *timer_base, uint32_t *msw, uint32_t *lsw)
 {
-	int loop_limit = 4;
+	int loop_limit = 3;
 
 	/*
 	 * Read 64-bit free running counter
@@ -83,18 +83,19 @@ kona_timer_get_counter(void __iomem *timer_base, uint32_t *msw, uint32_t *lsw)
 	 *      if new hi-word is equal to previously read hi-word then stop.
 	 */
 
-	while (--loop_limit) {
+	do {
 		*msw = readl(timer_base + KONA_GPTIMER_STCHI_OFFSET);
 		*lsw = readl(timer_base + KONA_GPTIMER_STCLO_OFFSET);
 		if (*msw == readl(timer_base + KONA_GPTIMER_STCHI_OFFSET))
 			break;
-	}
+	} while (--loop_limit);
 	if (!loop_limit) {
 		pr_err("bcm_kona_timer: getting counter failed.\n");
 		pr_err(" Timer will be impacted\n");
+		return -ETIMEDOUT;
 	}
 
-	return;
+	return 0;
 }
 
 static int kona_timer_set_next_event(unsigned long clc,
@@ -112,8 +113,11 @@ static int kona_timer_set_next_event(unsigned long clc,
 
 	uint32_t lsw, msw;
 	uint32_t reg;
+	int ret;
 
-	kona_timer_get_counter(timers.tmr_regs, &msw, &lsw);
+	ret = kona_timer_get_counter(timers.tmr_regs, &msw, &lsw);
+	if (ret)
+		return ret;
 
 	/* Load the "next" event tick value */
 	writel(lsw + clc, timers.tmr_regs + KONA_GPTIMER_STCM0_OFFSET);

[toc] | [prev] | [next] | [standalone]

#1464481 — [PATCH 3/3] clocksource/drivers/mips-gic-timer: Make gic_clocksource_of_init return int

From: Paul Gortmaker <paul.gortmaker@windriver.com>

In commit d8152bf85d2c057fc39c3e20a4d623f524d9f09c:
  ("clocksource/drivers/mips-gic-timer: Convert init function to return error")

several return values were added to a void function resulting in:

 clocksource/mips-gic-timer.c: In function 'gic_clocksource_of_init':
 clocksource/mips-gic-timer.c:175:3: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c:183:4: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c:190:3: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c:195:3: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c:200:3: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c:211:2: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c: At top level:
 clocksource/mips-gic-timer.c:213:1: warning: comparison of distinct pointer types lacks a cast [enabled by default]
 clocksource/mips-gic-timer.c: In function 'gic_clocksource_of_init':
 clocksource/mips-gic-timer.c:183:18: warning: ignoring return value of 'PTR_ERR', declared with attribute warn_unused_result [-Wunused-result]

Given that the addition of the return values was intentional, it seems
that the conversion of the containing function from void to int was
simply overlooked.

Fixes: d8152bf85d2c ("clocksource/drivers/mips-gic-timer: Convert init function to return error")
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: linux-mips@linux-mips.org
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/mips-gic-timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index 1572c7a..2577a2f 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -176,7 +176,7 @@ void __init gic_clocksource_init(unsigned int frequency)
 	gic_start_count();
 }
 
-static void __init gic_clocksource_of_init(struct device_node *node)
+static int __init gic_clocksource_of_init(struct device_node *node)
 {
 	struct clk *clk;
 	int ret;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1464583 — [tip:timers/urgent] clocksource/drivers/mips-gic-timer: Make gic_clocksource_of_init() return int

Commit-ID:  be5769e2061ac40b32daa83e28e1c4aac7133511
Gitweb:     http://git.kernel.org/tip/be5769e2061ac40b32daa83e28e1c4aac7133511
Author:     Paul Gortmaker <paul.gortmaker@windriver.com>
AuthorDate: Wed, 17 Aug 2016 12:21:35 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 17 Aug 2016 13:08:32 +0200

clocksource/drivers/mips-gic-timer: Make gic_clocksource_of_init() return int

In commit:

  d8152bf85d2c0 ("clocksource/drivers/mips-gic-timer: Convert init function to return error")

several return values were added to a void function resulting in the following warnings:

 clocksource/mips-gic-timer.c: In function 'gic_clocksource_of_init':
 clocksource/mips-gic-timer.c:175:3: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c:183:4: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c:190:3: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c:195:3: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c:200:3: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c:211:2: warning: 'return' with a value, in function returning void [enabled by default]
 clocksource/mips-gic-timer.c: At top level:
 clocksource/mips-gic-timer.c:213:1: warning: comparison of distinct pointer types lacks a cast [enabled by default]
 clocksource/mips-gic-timer.c: In function 'gic_clocksource_of_init':
 clocksource/mips-gic-timer.c:183:18: warning: ignoring return value of 'PTR_ERR', declared with attribute warn_unused_result [-Wunused-result]

Given that the addition of the return values was intentional, it seems
that the conversion of the containing function from void to int was
simply overlooked.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-mips@linux-mips.org
Fixes: d8152bf85d2c ("clocksource/drivers/mips-gic-timer: Convert init function to return error")
Link: http://lkml.kernel.org/r/1471429296-9053-3-git-send-email-daniel.lezcano@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/clocksource/mips-gic-timer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index d91e872..b4b3ab5 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -164,7 +164,7 @@ void __init gic_clocksource_init(unsigned int frequency)
 	gic_start_count();
 }
 
-static void __init gic_clocksource_of_init(struct device_node *node)
+static int __init gic_clocksource_of_init(struct device_node *node)
 {
 	struct clk *clk;
 	int ret;

[toc] | [prev] | [next] | [standalone]

#1464484 — [PATCH 1/3] clocksource/drivers/time-armada-370-xp: Fix the clock reference

From: Gregory CLEMENT <gregory.clement@free-electrons.com>

While converting the init function to return an error, the wrong clock
was get. This lead to wrong clock rate and slow down the kernel. For
example, before the patch a typical boot was around 15s after it was 1
minute slower.

Fixes: 12549e27c63c ("clocksource/drivers/time-armada-370-xp: Convert init function to return error")

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Tested-by: Stefan Roese <sr@denx.de>
Tested-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
---
 drivers/clocksource/time-armada-370-xp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index 20ec066..9872992 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -357,7 +357,6 @@ static int __init armada_xp_timer_init(struct device_node *np)
 	struct clk *clk = of_clk_get_by_name(np, "fixed");
 	int ret;
 
-	clk = of_clk_get(np, 0);
 	if (IS_ERR(clk)) {
 		pr_err("Failed to get clock");
 		return PTR_ERR(clk);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1464587 — [tip:timers/urgent] clocksource/drivers/time-armada-370-xp: Fix the clock reference

Commit-ID:  0e62fd836e4c2908cc1e32c68806529b4f859955
Gitweb:     http://git.kernel.org/tip/0e62fd836e4c2908cc1e32c68806529b4f859955
Author:     Gregory CLEMENT <gregory.clement@free-electrons.com>
AuthorDate: Wed, 17 Aug 2016 12:21:33 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 17 Aug 2016 13:08:31 +0200

clocksource/drivers/time-armada-370-xp: Fix the clock reference

While converting the init function to return an error, the wrong clock
was get. This leads to the wrong clock rate and slows down the kernel.
For example, it affects typical boot time:

 - without fix: over 1 minute
 - with    fix: 15 seconds

Tested-by: Stefan Roese <sr@denx.de>
Tested-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 12549e27c63c ("clocksource/drivers/time-armada-370-xp: Convert init function to return error")
Link: http://lkml.kernel.org/r/1471429296-9053-1-git-send-email-daniel.lezcano@linaro.org
[ Refined the changelog. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/clocksource/time-armada-370-xp.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index 719b478..3c39e6f 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -338,7 +338,6 @@ static int __init armada_xp_timer_init(struct device_node *np)
 	struct clk *clk = of_clk_get_by_name(np, "fixed");
 	int ret;
 
-	clk = of_clk_get(np, 0);
 	if (IS_ERR(clk)) {
 		pr_err("Failed to get clock");
 		return PTR_ERR(clk);

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web