Bug report from Jiri Gaisler <jgais@ws.estec.esa.nl>:

> > 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 ...
This commit is contained in:
Joel Sherrill
1999-01-13 14:13:47 +00:00
parent 97503aa5e3
commit 16a9ee1d06
2 changed files with 2 additions and 2 deletions

View File

@@ -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 ) \

View File

@@ -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 ) \