Hi Peter, On 02/05/2018 07:57 AM, Peter Maydell wrote: > In many of the NVIC registers relating to interrupts, we > have to convert from a byte offset within a register set > into the number of the first interrupt which is affected. > We were getting this wrong for: > * reads of NVIC_ISPR<n>, NVIC_ISER<n>, NVIC_ICPR<n>, NVIC_ICER<n>, > NVIC_IABR<n> -- in all these cases we were missing the "* 8" > needed to convert from the byte offset to the interrupt number > (since all these registers use one bit per interrupt) > * writes of NVIC_IPR<n> had the opposite problem of a spurious > "* 8" (since these registers use one byte per interrupt)
What about using inline function (suggested) with those comments to ease
code review, since this code is kinda confusing at first.
>
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
at any rate:
Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> ---
> hw/intc/armv7m_nvic.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
> index 8726be796e..9433efd1b8 100644
> --- a/hw/intc/armv7m_nvic.c
> +++ b/hw/intc/armv7m_nvic.c
> @@ -1721,7 +1721,7 @@ static MemTxResult nvic_sysreg_read(void *opaque,
> hwaddr addr,
> /* fall through */
> case 0x180 ... 0x1bf: /* NVIC Clear enable */
> val = 0;
> - startvec = offset - 0x180 + NVIC_FIRST_IRQ; /* vector # */
> + startvec = 8 * (offset - 0x180) + NVIC_FIRST_IRQ; /* vector # */
static uint32_t off2vec_rd(uint32_t offset, uint32_t base)
{
return 8 * (offset - base) + NVIC_FIRST_IRQ;
}
>
> for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq;
> i++) {
> if (s->vectors[startvec + i].enabled &&
> @@ -1735,7 +1735,7 @@ static MemTxResult nvic_sysreg_read(void *opaque,
> hwaddr addr,
> /* fall through */
> case 0x280 ... 0x2bf: /* NVIC Clear pend */
> val = 0;
> - startvec = offset - 0x280 + NVIC_FIRST_IRQ; /* vector # */
> + startvec = 8 * (offset - 0x280) + NVIC_FIRST_IRQ; /* vector # */
> for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq;
> i++) {
> if (s->vectors[startvec + i].pending &&
> (attrs.secure || s->itns[startvec + i])) {
> @@ -1745,7 +1745,7 @@ static MemTxResult nvic_sysreg_read(void *opaque,
> hwaddr addr,
> break;
> case 0x300 ... 0x33f: /* NVIC Active */
> val = 0;
> - startvec = offset - 0x300 + NVIC_FIRST_IRQ; /* vector # */
> + startvec = 8 * (offset - 0x300) + NVIC_FIRST_IRQ; /* vector # */
>
> for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq;
> i++) {
> if (s->vectors[startvec + i].active &&
> @@ -1860,7 +1860,7 @@ static MemTxResult nvic_sysreg_write(void *opaque,
> hwaddr addr,
> case 0x300 ... 0x33f: /* NVIC Active */
> return MEMTX_OK; /* R/O */
> case 0x400 ... 0x5ef: /* NVIC Priority */
> - startvec = 8 * (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */
> + startvec = (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */
static uint32_t off2vec_wr(uint32_t offset, uint32_t base)
{
return (offset - base) + NVIC_FIRST_IRQ;
}
>
> for (i = 0; i < size && startvec + i < s->num_irq; i++) {
> if (attrs.secure || s->itns[startvec + i]) {
>
signature.asc
Description: OpenPGP digital signature
