On 2/13/25 04:35, Jamin Lin wrote:
The behavior of the INTC set IRQ is almost identical between INTC and INTCIO. To reduce duplicated code, introduce the "aspeed_intc_set_irq_handler" function to handle both INTC and INTCIO IRQ behavior.
It's good practice to add "No functional change". Reviewed-by: Cédric Le Goater <c...@redhat.com> Thanks, C.
Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com> --- hw/intc/aspeed_intc.c | 62 ++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c index 59c1069294..fd4f75805a 100644 --- a/hw/intc/aspeed_intc.c +++ b/hw/intc/aspeed_intc.c @@ -92,11 +92,40 @@ static void aspeed_intc_update(AspeedINTCState *s, int inpin_idx, qemu_set_irq(s->output_pins[outpin_idx], level); }+static void aspeed_intc_set_irq_handler(AspeedINTCState *s,+ const AspeedINTCIRQ *intc_irq, + uint32_t select) +{ + const char *name = object_get_typename(OBJECT(s)); + + if (s->mask[intc_irq->inpin_idx] || s->regs[intc_irq->status_addr]) { + /* + * a. mask is not 0 means in ISR mode + * sources interrupt routine are executing. + * b. status register value is not 0 means previous + * source interrupt does not be executed, yet. + * + * save source interrupt to pending variable. + */ + s->pending[intc_irq->inpin_idx] |= select; + trace_aspeed_intc_pending_irq(name, intc_irq->inpin_idx, + s->pending[intc_irq->inpin_idx]); + } else { + /* + * notify firmware which source interrupt are coming + * by setting status register + */ + s->regs[intc_irq->status_addr] = select; + trace_aspeed_intc_trigger_irq(name, intc_irq->inpin_idx, + intc_irq->outpin_idx, + s->regs[intc_irq->status_addr]); + aspeed_intc_update(s, intc_irq->inpin_idx, intc_irq->outpin_idx, 1); + } +} + /* - * The address of GICINT128 to GICINT136 are from 0x1000 to 0x1804. - * Utilize "address & 0x0f00" to get the irq and irq output pin index - * The value of irq should be 0 to num_inpins. - * The irq 0 indicates GICINT128, irq 1 indicates GICINT129 and so on. + * GICINT128 to GICINT136 map 1:1 to input and output IRQs 0 to 8. + * The value of input IRQ should be between 0 and the number of inputs. */ static void aspeed_intc_set_irq(void *opaque, int irq, int level) { @@ -135,30 +164,7 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level) }trace_aspeed_intc_select(name, select);- - if (s->mask[intc_irq->inpin_idx] || s->regs[intc_irq->status_addr]) { - /* - * a. mask is not 0 means in ISR mode - * sources interrupt routine are executing. - * b. status register value is not 0 means previous - * source interrupt does not be executed, yet. - * - * save source interrupt to pending variable. - */ - s->pending[intc_irq->inpin_idx] |= select; - trace_aspeed_intc_pending_irq(name, intc_irq->inpin_idx, - s->pending[intc_irq->inpin_idx]); - } else { - /* - * notify firmware which source interrupt are coming - * by setting status register - */ - s->regs[intc_irq->status_addr] = select; - trace_aspeed_intc_trigger_irq(name, intc_irq->inpin_idx, - intc_irq->outpin_idx, - s->regs[intc_irq->status_addr]); - aspeed_intc_update(s, intc_irq->inpin_idx, intc_irq->outpin_idx, 1); - } + aspeed_intc_set_irq_handler(s, intc_irq, select); }static void aspeed_intc_enable_handler(AspeedINTCState *s, hwaddr offset,
