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 >
next prev parent 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: linkBe 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.