From 16a9ee1d0644c573fd78e690c43dfd4e32748463 Mon Sep 17 00:00:00 2001 From: Joel Sherrill Date: Wed, 13 Jan 1999 14:13:47 +0000 Subject: Bug report from Jiri Gaisler : > > I think I have found a bug in src/exec/scor/sparc/cpu/erc32.h in: > > > > #define ERC32_Disable_interrupt( _source, _previous ) \ > > do { \ > > unsigned32 _level; \ > > unsigned32 _mask = 1 << (_source); \ > > \ > > sparc_disable_interrupts( _level ); \ > > (_previous) = ERC32_MEC.Interrupt_Mask; \ > > ERC32_MEC.Interrupt_Mask = _previous | _mask; \ > > sparc_enable_interrupts( _level ); \ > > (_previous) &= ~_mask; \ <- IS THIS CORRECT...? > > } while (0) > > > > The previous interrupt mask is returned after first clearing the > > bit to be disabled, regardless whether the bit was set before or > > not. If the bit was set (interrupt masked), subsequent call to > > ERC32_Restore_interrupt() will enable the interrupt even though > > it was supposed to be masked. This is indeed what happens in > > DEBUG_puts when polled console I/O is used. In my opinion, the > > last statement in the macro should be removed - what is your opinion? > > I think the "~" shouldn't be there. I recall that the intent of that line > is to only return the state of the interrupts you were concerned with. > Removing the line returns entire state. Given that the value returned > shuold only be used in conjunction with the map, I suppose either removing > the ~ or the entire line is correct? I can go either way. Just let me > know which you think is more correct and the source will change. :) Hmmm, just removing the '~' should be OK. DEBUG_puts() seems to be the only user of ERC32_Restore_interrupt() anyway ... --- c/src/exec/score/cpu/sparc/erc32.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'c/src/exec') diff --git a/c/src/exec/score/cpu/sparc/erc32.h b/c/src/exec/score/cpu/sparc/erc32.h index 50df21267f..aa0eef05d9 100644 --- a/c/src/exec/score/cpu/sparc/erc32.h +++ b/c/src/exec/score/cpu/sparc/erc32.h @@ -384,7 +384,7 @@ extern ERC32_Register_Map ERC32_MEC; (_previous) = ERC32_MEC.Interrupt_Mask; \ ERC32_MEC.Interrupt_Mask = _previous | _mask; \ sparc_enable_interrupts( _level ); \ - (_previous) &= ~_mask; \ + (_previous) &= _mask; \ } while (0) #define ERC32_Restore_interrupt( _source, _previous ) \ -- cgit v1.2.3