Re: gemini timer change

Actually thinking on it some more, enable_interrupts is the wrong thing. If you entered that function without interrupts enabled, it's now hard coded to turn them on, rather than return to the previous state.

Given it's an init routine it might actually mean that interrupts are on before the system has enabled them. (and in fact that maybe the reason it wasn't noticed before, because the timer init happens before enable_interrupt is called)

Regards,

Chris

On 17 March 2014 00:43, Chris Gilbert <chris%dokein.co.uk@localhost> wrote:

While I've not looked at bsd code for some time and just lurked on the list, generally the save = disable_interrupts(I32_bit) is paired with restore_interrupts(save) otherwise the f32_bit won't be restored (not that I think netbsd ever used FIQs)

IE enable_interrupts() tended to not be the right thing to use other than in the depths of interrupt handling.

Regards,
Chris

On 9 March 2014 10:35, Martin Husemann <martin%duskware.de@localhost> wrote:
I just commited the patch below to fix the build.

However, I am not sure about the last part (changing disable_interrupts to
enable_interrupts) - it looked like a copy & paste bug to me, but there
may be a deeper reasoning.

Please feel free to fix differently.

Martin

Index: gemini_timer.c
===================================================================
RCS file: /cvsroot/src/sys/arch/arm/gemini/gemini_timer.c,v
retrieving revision 1.5
diff -u -p -r1.5 gemini_timer.c
--- gemini_timer.c 1 Jul 2011 19:32:28 -0000 1.5
+++ gemini_timer.c 9 Mar 2014 10:31:13 -0000
@@ -257,9 +257,7 @@ statintr(void *frame)
static void
timer_init(geminitmr_softc_t *sc, int schz, boolean_t autoload, boolean_t intr)
{
- int psw;
-
- psw = disable_interrupts(I32_bit);
+ disable_interrupts(I32_bit);
timer_factors(sc, schz, autoload);
_timer_stop(sc);
_timer_intr_dis(sc);
@@ -267,7 +265,7 @@ timer_init(geminitmr_softc_t *sc, int sc
if (intr)
_timer_intr_enb(sc);
_timer_start(sc);
- psw = disable_interrupts(I32_bit);
+ enable_interrupts(I32_bit);
}

void