Hi Cedric,
>
> It's good practice to add "No functional change".
>
> Reviewed-by: Cédric Le Goater <c...@redhat.com>
>
> Thanks,
>
> C.
>
Thanks for your review and suggestion.
I will add "No functional change" in commit log.
Jamin
>
> > 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,
