All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Matthias Kaehlcke <mka@chromium.org>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>
Cc: Fu Wei <fu.wei@linaro.org>, Marc Zyngier <marc.zyngier@arm.com>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Doug Anderson <dianders@chromium.org>,
	ard.biesheuvel@linaro.org
Subject: Re: [PATCH] clocksource: arm_arch_timer: Fix mem frame loop initialization
Date: Tue, 1 Aug 2017 12:12:23 +0100	[thread overview]
Message-ID: <20170801111222.GC9347@leverpostej> (raw)
In-Reply-To: <20170731183728.61087-1-mka@chromium.org>

On Mon, Jul 31, 2017 at 11:37:28AM -0700, Matthias Kaehlcke wrote:
> The loop to find the best memory frame in arch_timer_mem_acpi_init()
> initializes the loop counter with itself ('i = i'), which is suspicious
> in the first place and pointed out by clang. The loop condition is
> 'i < timer_count' and a prior for loop exits when 'i' reaches
> 'timer_count', therefore the second loop is never executed.
> 
> Initialize the loop counter with 0 to iterate over all timers, which
> supposedly was the intention before the typo monster attacked.
> 
> Fixes: c2743a36765d3 ("clocksource: arm_arch_timer: add GTDT support for memory-mapped timer")
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>

Ard also spotted this issue [1], along with another issue we still need
to solve. In the mean time it would be nice to have this fixed, so:

Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> 
Acked-by: Mark Rutland <mark.rutland@arm.com>

Daniel, Thomas,

Could one of you please pick this up as a fix, with the above tags?

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-July/520779.html

> ---
>  drivers/clocksource/arm_arch_timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index aae87c4c546e..72bbfccef113 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -1440,7 +1440,7 @@ static int __init arch_timer_mem_acpi_init(int platform_timer_count)
>  	 * While unlikely, it's theoretically possible that none of the frames
>  	 * in a timer expose the combination of feature we want.
>  	 */
> -	for (i = i; i < timer_count; i++) {
> +	for (i = 0; i < timer_count; i++) {
>  		timer = &timers[i];
>  
>  		frame = arch_timer_mem_find_best_frame(timer);
> -- 
> 2.14.0.rc0.400.g1c36432dff-goog
> 

WARNING: multiple messages have this Message-ID (diff)
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clocksource: arm_arch_timer: Fix mem frame loop initialization
Date: Tue, 1 Aug 2017 12:12:23 +0100	[thread overview]
Message-ID: <20170801111222.GC9347@leverpostej> (raw)
In-Reply-To: <20170731183728.61087-1-mka@chromium.org>

On Mon, Jul 31, 2017 at 11:37:28AM -0700, Matthias Kaehlcke wrote:
> The loop to find the best memory frame in arch_timer_mem_acpi_init()
> initializes the loop counter with itself ('i = i'), which is suspicious
> in the first place and pointed out by clang. The loop condition is
> 'i < timer_count' and a prior for loop exits when 'i' reaches
> 'timer_count', therefore the second loop is never executed.
> 
> Initialize the loop counter with 0 to iterate over all timers, which
> supposedly was the intention before the typo monster attacked.
> 
> Fixes: c2743a36765d3 ("clocksource: arm_arch_timer: add GTDT support for memory-mapped timer")
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>

Ard also spotted this issue [1], along with another issue we still need
to solve. In the mean time it would be nice to have this fixed, so:

Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> 
Acked-by: Mark Rutland <mark.rutland@arm.com>

Daniel, Thomas,

Could one of you please pick this up as a fix, with the above tags?

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-July/520779.html

> ---
>  drivers/clocksource/arm_arch_timer.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index aae87c4c546e..72bbfccef113 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -1440,7 +1440,7 @@ static int __init arch_timer_mem_acpi_init(int platform_timer_count)
>  	 * While unlikely, it's theoretically possible that none of the frames
>  	 * in a timer expose the combination of feature we want.
>  	 */
> -	for (i = i; i < timer_count; i++) {
> +	for (i = 0; i < timer_count; i++) {
>  		timer = &timers[i];
>  
>  		frame = arch_timer_mem_find_best_frame(timer);
> -- 
> 2.14.0.rc0.400.g1c36432dff-goog
> 

  reply	other threads:[~2017-08-01 11:13 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-31 18:37 [PATCH] clocksource: arm_arch_timer: Fix mem frame loop initialization Matthias Kaehlcke
2017-07-31 18:37 ` Matthias Kaehlcke
2017-08-01 11:12 ` Mark Rutland [this message]
2017-08-01 11:12   ` Mark Rutland
2017-08-01 11:34   ` Daniel Lezcano
2017-08-01 11:34     ` Daniel Lezcano

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=20170801111222.GC9347@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=dianders@chromium.org \
    --cc=fu.wei@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=mka@chromium.org \
    --cc=tglx@linutronix.de \
    /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.