Re: [PATCH v6 05/29] hw/intc/aspeed: Reduce regs array size by adding a register sub-region

[Cédric Le Goater]

On 3/7/25 04:59, Jamin Lin wrote:

Currently, the size of the "regs" array is 0x2000, which is too large. So far,
it only uses "GICINT128 to `GICINT134", and the offsets from 0 to 0x1000 are
unused. To save code size and avoid mapping large unused gaps, update to only
map the useful set of registers:

INTC register [0x1000 – 0x1804]

Update "reg_size" to 0x808. Introduce a new class attribute "reg_offset" to set
the start offset of a "INTC" sub-region. Set the "reg_offset" to 0x1000 for INTC
registers.

Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>

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

Thanks,

C.

---
  include/hw/intc/aspeed_intc.h |  1 +
  hw/intc/aspeed_intc.c         | 50 ++++++++++++++++++++---------------
  2 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/include/hw/intc/aspeed_intc.h b/include/hw/intc/aspeed_intc.h
index ec4936b3f4..208e764c4a 100644
--- a/include/hw/intc/aspeed_intc.h
+++ b/include/hw/intc/aspeed_intc.h
@@ -42,6 +42,7 @@ struct AspeedINTCClass {
      uint32_t num_ints;
      uint64_t mem_size;
      uint64_t nr_regs;
+    uint64_t reg_offset;
  };

#endif /* ASPEED_INTC_H */

diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index 134922e46f..d684b4bb4f 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -14,25 +14,31 @@
  #include "hw/registerfields.h"
  #include "qapi/error.h"

-/* INTC Registers */

-REG32(GICINT128_EN,         0x1000)
-REG32(GICINT128_STATUS,     0x1004)
-REG32(GICINT129_EN,         0x1100)
-REG32(GICINT129_STATUS,     0x1104)
-REG32(GICINT130_EN,         0x1200)
-REG32(GICINT130_STATUS,     0x1204)
-REG32(GICINT131_EN,         0x1300)
-REG32(GICINT131_STATUS,     0x1304)
-REG32(GICINT132_EN,         0x1400)
-REG32(GICINT132_STATUS,     0x1404)
-REG32(GICINT133_EN,         0x1500)
-REG32(GICINT133_STATUS,     0x1504)
-REG32(GICINT134_EN,         0x1600)
-REG32(GICINT134_STATUS,     0x1604)
-REG32(GICINT135_EN,         0x1700)
-REG32(GICINT135_STATUS,     0x1704)
-REG32(GICINT136_EN,         0x1800)
-REG32(GICINT136_STATUS,     0x1804)
+/*
+ * INTC Registers
+ *
+ * values below are offset by - 0x1000 from datasheet
+ * because its memory region is start at 0x1000
+ *
+ */
+REG32(GICINT128_EN,         0x000)
+REG32(GICINT128_STATUS,     0x004)
+REG32(GICINT129_EN,         0x100)
+REG32(GICINT129_STATUS,     0x104)
+REG32(GICINT130_EN,         0x200)
+REG32(GICINT130_STATUS,     0x204)
+REG32(GICINT131_EN,         0x300)
+REG32(GICINT131_STATUS,     0x304)
+REG32(GICINT132_EN,         0x400)
+REG32(GICINT132_STATUS,     0x404)
+REG32(GICINT133_EN,         0x500)
+REG32(GICINT133_STATUS,     0x504)
+REG32(GICINT134_EN,         0x600)
+REG32(GICINT134_STATUS,     0x604)
+REG32(GICINT135_EN,         0x700)
+REG32(GICINT135_STATUS,     0x704)
+REG32(GICINT136_EN,         0x800)
+REG32(GICINT136_STATUS,     0x804)

#define GICINT_STATUS_BASE R_GICINT128_STATUS @@ -298,7 +304,8 @@ static void aspeed_intc_realize(DeviceState *dev, Error **errp)

      memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_intc_ops, s,
                            TYPE_ASPEED_INTC ".regs", aic->nr_regs << 2);

- memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);

+    memory_region_add_subregion(&s->iomem_container, aic->reg_offset,
+                                &s->iomem);

qdev_init_gpio_in(dev, aspeed_intc_set_irq, aic->num_ints); @@ -348,7 +355,8 @@ static void aspeed_2700_intc_class_init(ObjectClass *klass, void *data)

      aic->num_lines = 32;
      aic->num_ints = 9;
      aic->mem_size = 0x4000;
-    aic->nr_regs = 0x2000 >> 2;
+    aic->nr_regs = 0x808 >> 2;
+    aic->reg_offset = 0x1000;
  }

static const TypeInfo aspeed_2700_intc_info = {