Re: [PATCH v5 02/29] hw/intc/aspeed: Rename status_addr and addr to status_reg and reg for clarity

["Cédric Le Goater" <clg@kaod.org>]

On 3/6/25 11:38, Jamin Lin wrote:
> Rename the variables "status_addr" to "status_reg" and "addr" to "reg" because
> they are used as register index. This change makes the code more appropriate
> and improves readability.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/intc/aspeed_intc.c | 38 +++++++++++++++++++-------------------
>   1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
> index 033b574c1e..465f41e4fd 100644
> --- a/hw/intc/aspeed_intc.c
> +++ b/hw/intc/aspeed_intc.c
> @@ -60,7 +60,7 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level)
>   {
>       AspeedINTCState *s = (AspeedINTCState *)opaque;
>       AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
> -    uint32_t status_addr = GICINT_STATUS_BASE + ((0x100 * irq) >> 2);
> +    uint32_t status_reg = GICINT_STATUS_BASE + ((0x100 * irq) >> 2);
>       uint32_t select = 0;
>       uint32_t enable;
>       int i;
> @@ -92,7 +92,7 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level)
>   
>       trace_aspeed_intc_select(select);
>   
> -    if (s->mask[irq] || s->regs[status_addr]) {
> +    if (s->mask[irq] || s->regs[status_reg]) {
>           /*
>            * a. mask is not 0 means in ISR mode
>            * sources interrupt routine are executing.
> @@ -108,8 +108,8 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level)
>            * notify firmware which source interrupt are coming
>            * by setting status register
>            */
> -        s->regs[status_addr] = select;
> -        trace_aspeed_intc_trigger_irq(irq, s->regs[status_addr]);
> +        s->regs[status_reg] = select;
> +        trace_aspeed_intc_trigger_irq(irq, s->regs[status_reg]);
>           aspeed_intc_update(s, irq, 1);
>       }
>   }
> @@ -117,17 +117,17 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level)
>   static uint64_t aspeed_intc_read(void *opaque, hwaddr offset, unsigned int size)
>   {
>       AspeedINTCState *s = ASPEED_INTC(opaque);
> -    uint32_t addr = offset >> 2;
> +    uint32_t reg = offset >> 2;
>       uint32_t value = 0;
>   
> -    if (addr >= ASPEED_INTC_NR_REGS) {
> +    if (reg >= ASPEED_INTC_NR_REGS) {
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
>                         __func__, offset);
>           return 0;
>       }
>   
> -    value = s->regs[addr];
> +    value = s->regs[reg];
>       trace_aspeed_intc_read(offset, size, value);
>   
>       return value;
> @@ -138,12 +138,12 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
>   {
>       AspeedINTCState *s = ASPEED_INTC(opaque);
>       AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
> -    uint32_t addr = offset >> 2;
> +    uint32_t reg = offset >> 2;
>       uint32_t old_enable;
>       uint32_t change;
>       uint32_t irq;
>   
> -    if (addr >= ASPEED_INTC_NR_REGS) {
> +    if (reg >= ASPEED_INTC_NR_REGS) {
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n",
>                         __func__, offset);
> @@ -152,7 +152,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
>   
>       trace_aspeed_intc_write(offset, size, data);
>   
> -    switch (addr) {
> +    switch (reg) {
>       case R_GICINT128_EN:
>       case R_GICINT129_EN:
>       case R_GICINT130_EN:
> @@ -177,7 +177,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
>   
>           /* disable all source interrupt */
>           if (!data && !s->enable[irq]) {
> -            s->regs[addr] = data;
> +            s->regs[reg] = data;
>               return;
>           }
>   
> @@ -187,12 +187,12 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
>           /* enable new source interrupt */
>           if (old_enable != s->enable[irq]) {
>               trace_aspeed_intc_enable(s->enable[irq]);
> -            s->regs[addr] = data;
> +            s->regs[reg] = data;
>               return;
>           }
>   
>           /* mask and unmask source interrupt */
> -        change = s->regs[addr] ^ data;
> +        change = s->regs[reg] ^ data;
>           if (change & data) {
>               s->mask[irq] &= ~change;
>               trace_aspeed_intc_unmask(change, s->mask[irq]);
> @@ -200,7 +200,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
>               s->mask[irq] |= change;
>               trace_aspeed_intc_mask(change, s->mask[irq]);
>           }
> -        s->regs[addr] = data;
> +        s->regs[reg] = data;
>           break;
>       case R_GICINT128_STATUS:
>       case R_GICINT129_STATUS:
> @@ -220,7 +220,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
>           }
>   
>           /* clear status */
> -        s->regs[addr] &= ~data;
> +        s->regs[reg] &= ~data;
>   
>           /*
>            * These status registers are used for notify sources ISR are executed.
> @@ -233,7 +233,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
>           }
>   
>           /* All source ISR execution are done */
> -        if (!s->regs[addr]) {
> +        if (!s->regs[reg]) {
>               trace_aspeed_intc_all_isr_done(irq);
>               if (s->pending[irq]) {
>                   /*
> @@ -241,9 +241,9 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
>                    * notify firmware which source interrupt are pending
>                    * by setting status register
>                    */
> -                s->regs[addr] = s->pending[irq];
> +                s->regs[reg] = s->pending[irq];
>                   s->pending[irq] = 0;
> -                trace_aspeed_intc_trigger_irq(irq, s->regs[addr]);
> +                trace_aspeed_intc_trigger_irq(irq, s->regs[reg]);
>                   aspeed_intc_update(s, irq, 1);
>               } else {
>                   /* clear irq */
> @@ -253,7 +253,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
>           }
>           break;
>       default:
> -        s->regs[addr] = data;
> +        s->regs[reg] = data;
>           break;
>       }
>