| Message ID | 1356142552-13453-10-git-send-email-scottwood@freescale.com |
|---|---|
| State | New |
| Headers | show |
On 22.12.2012, at 03:15, Scott Wood wrote: > Previously the code relied on the queue's "next" field getting > set to -1 sometime between an update to the bitmap, and the next > call to IRQ_get_next. Sometimes this happened after the update. > Sometimes it happened before the check. Sometimes it didn't happen > at all. > > Signed-off-by: Scott Wood <scottwood@freescale.com> Have you verified that we don't run the check too often then? It's quite costly, no? Applied nevertheless to ppc-next. Alex
On 01/03/2013 12:42:09 PM, Alexander Graf wrote: > > On 22.12.2012, at 03:15, Scott Wood wrote: > > > Previously the code relied on the queue's "next" field getting > > set to -1 sometime between an update to the bitmap, and the next > > call to IRQ_get_next. Sometimes this happened after the update. > > Sometimes it happened before the check. Sometimes it didn't happen > > at all. > > > > Signed-off-by: Scott Wood <scottwood@freescale.com> > > Have you verified that we don't run the check too often then? It's > quite costly, no? Correctness takes precedence over speed, as does readability/maintainability if the difference is minor. In any case, the check gets faster later in the patchset. -Scott
diff --git a/hw/openpic.c b/hw/openpic.c index 268f312..e2e7079 100644 --- a/hw/openpic.c +++ b/hw/openpic.c @@ -316,10 +316,8 @@ out: static int IRQ_get_next(OpenPICState *opp, IRQ_queue_t *q) { - if (q->next == -1) { - /* XXX: optimize */ - IRQ_check(opp, q); - } + /* XXX: optimize */ + IRQ_check(opp, q); return q->next; } @@ -366,7 +364,7 @@ static void IRQ_local_pipe(OpenPICState *opp, int n_CPU, int n_IRQ) __func__, n_IRQ, dst->raised.next, n_CPU); return; } - IRQ_get_next(opp, &dst->raised); + IRQ_check(opp, &dst->raised); if (IRQ_get_next(opp, &dst->servicing) != -1 && priority <= dst->servicing.priority) { DPRINTF("%s: IRQ %d is hidden by servicing IRQ %d on CPU %d\n", @@ -937,7 +935,6 @@ static void openpic_cpu_write_internal(void *opaque, hwaddr addr, DPRINTF("EOI\n"); s_IRQ = IRQ_get_next(opp, &dst->servicing); IRQ_resetbit(&dst->servicing, s_IRQ); - dst->servicing.next = -1; /* Set up next servicing IRQ */ s_IRQ = IRQ_get_next(opp, &dst->servicing); /* Check queued interrupts. */ @@ -1013,7 +1010,6 @@ static uint32_t openpic_cpu_read_internal(void *opaque, hwaddr addr, retval = IVPR_VECTOR(opp, src->ivpr); } IRQ_resetbit(&dst->raised, n_IRQ); - dst->raised.next = -1; if (!src->level) { /* edge-sensitive IRQ */ src->ivpr &= ~IVPR_ACTIVITY_MASK;
Previously the code relied on the queue's "next" field getting set to -1 sometime between an update to the bitmap, and the next call to IRQ_get_next. Sometimes this happened after the update. Sometimes it happened before the check. Sometimes it didn't happen at all. Signed-off-by: Scott Wood <scottwood@freescale.com> --- hw/openpic.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)