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

[Cédric Le Goater]

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_...@aspeedtech.com>

Reviewed-by: Cédric Le Goater <c...@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;
      }