This will simplify the definition of new SoCs, like the AST2600 which
should use a different CPU and a different IRQ number layout.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Changes since v1:
- moved enum defining the Aspeed controller names under aspeed_soc.h
include/hw/arm/aspeed_soc.h | 37 ++++++++++++++++++++++++
hw/arm/aspeed_soc.c | 57 +++++++++++++++++++++++++++++++------
2 files changed, 86 insertions(+), 8 deletions(-)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 11ec0179db50..2dd968092c69 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -57,6 +57,7 @@ typedef struct AspeedSoCInfo {
const char *fmc_typename;
const char **spi_typename;
int wdts_num;
+ const int *irqmap;
} AspeedSoCInfo;
typedef struct AspeedSoCClass {
@@ -69,4 +70,40 @@ typedef struct AspeedSoCClass {
#define ASPEED_SOC_GET_CLASS(obj) \
OBJECT_GET_CLASS(AspeedSoCClass, (obj), TYPE_ASPEED_SOC)
+enum {
+ ASPEED_IOMEM,
+ ASPEED_UART1,
+ ASPEED_UART2,
+ ASPEED_UART3,
+ ASPEED_UART4,
+ ASPEED_UART5,
+ ASPEED_VUART,
+ ASPEED_FMC,
+ ASPEED_SPI1,
+ ASPEED_SPI2,
+ ASPEED_VIC,
+ ASPEED_SDMC,
+ ASPEED_SCU,
+ ASPEED_ADC,
+ ASPEED_SRAM,
+ ASPEED_GPIO,
+ ASPEED_RTC,
+ ASPEED_TIMER1,
+ ASPEED_TIMER2,
+ ASPEED_TIMER3,
+ ASPEED_TIMER4,
+ ASPEED_TIMER5,
+ ASPEED_TIMER6,
+ ASPEED_TIMER7,
+ ASPEED_TIMER8,
+ ASPEED_WDT,
+ ASPEED_PWM,
+ ASPEED_LPC,
+ ASPEED_IBT,
+ ASPEED_I2C,
+ ASPEED_ETH1,
+ ASPEED_ETH2,
+ ASPEED_SDRAM,
+};
+
#endif /* ASPEED_SOC_H */
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index a27233d4876b..29bce5c9188c 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -38,12 +38,42 @@
#define ASPEED_SOC_ETH1_BASE 0x1E660000
#define ASPEED_SOC_ETH2_BASE 0x1E680000
-static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
-static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
+static const int aspeed_soc_ast2400_irqmap[] = {
+ [ASPEED_UART1] = 9,
+ [ASPEED_UART2] = 32,
+ [ASPEED_UART3] = 33,
+ [ASPEED_UART4] = 34,
+ [ASPEED_UART5] = 10,
+ [ASPEED_VUART] = 8,
+ [ASPEED_FMC] = 19,
+ [ASPEED_SDMC] = 0,
+ [ASPEED_SCU] = 21,
+ [ASPEED_ADC] = 31,
+ [ASPEED_GPIO] = 20,
+ [ASPEED_RTC] = 22,
+ [ASPEED_TIMER1] = 16,
+ [ASPEED_TIMER2] = 17,
+ [ASPEED_TIMER3] = 18,
+ [ASPEED_TIMER4] = 35,
+ [ASPEED_TIMER5] = 36,
+ [ASPEED_TIMER6] = 37,
+ [ASPEED_TIMER7] = 38,
+ [ASPEED_TIMER8] = 39,
+ [ASPEED_WDT] = 27,
+ [ASPEED_PWM] = 28,
+ [ASPEED_LPC] = 8,
+ [ASPEED_IBT] = 8, /* LPC */
+ [ASPEED_I2C] = 12,
+ [ASPEED_ETH1] = 2,
+ [ASPEED_ETH2] = 3,
+};
#define AST2400_SDRAM_BASE 0x40000000
#define AST2500_SDRAM_BASE 0x80000000
+/* AST2500 uses the same IRQs as the AST2400 */
+#define aspeed_soc_ast2500_irqmap aspeed_soc_ast2400_irqmap
+
static const hwaddr aspeed_soc_ast2400_spi_bases[] = { ASPEED_SOC_SPI_BASE };
static const char *aspeed_soc_ast2400_typenames[] = { "aspeed.smc.spi" };
@@ -64,6 +94,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
.fmc_typename = "aspeed.smc.fmc",
.spi_typename = aspeed_soc_ast2400_typenames,
.wdts_num = 2,
+ .irqmap = aspeed_soc_ast2400_irqmap,
}, {
.name = "ast2400-a1",
.cpu_type = ARM_CPU_TYPE_NAME("arm926"),
@@ -75,6 +106,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
.fmc_typename = "aspeed.smc.fmc",
.spi_typename = aspeed_soc_ast2400_typenames,
.wdts_num = 2,
+ .irqmap = aspeed_soc_ast2400_irqmap,
}, {
.name = "ast2400",
.cpu_type = ARM_CPU_TYPE_NAME("arm926"),
@@ -86,6 +118,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
.fmc_typename = "aspeed.smc.fmc",
.spi_typename = aspeed_soc_ast2400_typenames,
.wdts_num = 2,
+ .irqmap = aspeed_soc_ast2400_irqmap,
}, {
.name = "ast2500-a1",
.cpu_type = ARM_CPU_TYPE_NAME("arm1176"),
@@ -97,9 +130,17 @@ static const AspeedSoCInfo aspeed_socs[] = {
.fmc_typename = "aspeed.smc.ast2500-fmc",
.spi_typename = aspeed_soc_ast2500_typenames,
.wdts_num = 3,
+ .irqmap = aspeed_soc_ast2500_irqmap,
},
};
+static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
+{
+ AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
+
+ return qdev_get_gpio_in(DEVICE(&s->vic), sc->info->irqmap[ctrl]);
+}
+
static void aspeed_soc_init(Object *obj)
{
AspeedSoCState *s = ASPEED_SOC(obj);
@@ -226,14 +267,14 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
return;
}
sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0, ASPEED_SOC_TIMER_BASE);
- for (i = 0; i < ARRAY_SIZE(timer_irqs); i++) {
- qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->vic), timer_irqs[i]);
+ for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
+ qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_TIMER1 + i);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq);
}
/* UART - attach an 8250 to the IO space as our UART5 */
if (serial_hd(0)) {
- qemu_irq uart5 = qdev_get_gpio_in(DEVICE(&s->vic), uart_irqs[4]);
+ qemu_irq uart5 = aspeed_soc_get_irq(s, ASPEED_UART5);
serial_mm_init(get_system_memory(),
ASPEED_SOC_IOMEM_BASE + ASPEED_SOC_UART_5_BASE, 2,
uart5, 38400, serial_hd(0), DEVICE_LITTLE_ENDIAN);
@@ -247,7 +288,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
}
sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, ASPEED_SOC_I2C_BASE);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
- qdev_get_gpio_in(DEVICE(&s->vic), 12));
+ aspeed_soc_get_irq(s, ASPEED_I2C));
/* FMC, The number of CS is set at the board level */
object_property_set_bool(OBJECT(&s->fmc), true, "realized", &err);
@@ -259,7 +300,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
s->fmc.ctrl->flash_window_base);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
- qdev_get_gpio_in(DEVICE(&s->vic), 19));
+ aspeed_soc_get_irq(s, ASPEED_FMC));
/* SPI */
for (i = 0; i < sc->info->spis_num; i++) {
@@ -307,7 +348,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
}
sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
- qdev_get_gpio_in(DEVICE(&s->vic), 2));
+ aspeed_soc_get_irq(s, ASPEED_ETH1));
}
static void aspeed_soc_class_init(ObjectClass *oc, void *data)
--
2.20.1
On Mon, 6 May 2019 at 14:21, Cédric Le Goater <clg@kaod.org> wrote: > > This will simplify the definition of new SoCs, like the AST2600 which > should use a different CPU and a different IRQ number layout. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> Reviewed-by: Joel Stanley <joel@jms.id.au>
Hi Cédric,
On 5/6/19 4:20 PM, Cédric Le Goater wrote:
> This will simplify the definition of new SoCs, like the AST2600 which
> should use a different CPU and a different IRQ number layout.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>
> Changes since v1:
>
> - moved enum defining the Aspeed controller names under aspeed_soc.h
>
> include/hw/arm/aspeed_soc.h | 37 ++++++++++++++++++++++++
> hw/arm/aspeed_soc.c | 57 +++++++++++++++++++++++++++++++------
> 2 files changed, 86 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 11ec0179db50..2dd968092c69 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -57,6 +57,7 @@ typedef struct AspeedSoCInfo {
> const char *fmc_typename;
> const char **spi_typename;
> int wdts_num;
> + const int *irqmap;
> } AspeedSoCInfo;
>
> typedef struct AspeedSoCClass {
> @@ -69,4 +70,40 @@ typedef struct AspeedSoCClass {
> #define ASPEED_SOC_GET_CLASS(obj) \
> OBJECT_GET_CLASS(AspeedSoCClass, (obj), TYPE_ASPEED_SOC)
>
> +enum {
> + ASPEED_IOMEM,
> + ASPEED_UART1,
> + ASPEED_UART2,
> + ASPEED_UART3,
> + ASPEED_UART4,
> + ASPEED_UART5,
> + ASPEED_VUART,
> + ASPEED_FMC,
> + ASPEED_SPI1,
> + ASPEED_SPI2,
> + ASPEED_VIC,
> + ASPEED_SDMC,
> + ASPEED_SCU,
> + ASPEED_ADC,
> + ASPEED_SRAM,
> + ASPEED_GPIO,
> + ASPEED_RTC,
> + ASPEED_TIMER1,
> + ASPEED_TIMER2,
> + ASPEED_TIMER3,
> + ASPEED_TIMER4,
> + ASPEED_TIMER5,
> + ASPEED_TIMER6,
> + ASPEED_TIMER7,
> + ASPEED_TIMER8,
> + ASPEED_WDT,
> + ASPEED_PWM,
> + ASPEED_LPC,
> + ASPEED_IBT,
> + ASPEED_I2C,
> + ASPEED_ETH1,
> + ASPEED_ETH2,
> + ASPEED_SDRAM,
I'd add ...:
ASPEED_IRQMAP_COUNT /* keep last */
> +};
> +
> #endif /* ASPEED_SOC_H */
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index a27233d4876b..29bce5c9188c 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -38,12 +38,42 @@
> #define ASPEED_SOC_ETH1_BASE 0x1E660000
> #define ASPEED_SOC_ETH2_BASE 0x1E680000
>
> -static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
> -static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
> +static const int aspeed_soc_ast2400_irqmap[] = {
... and use it here:
static const int aspeed_soc_ast2400_irqmap[ASPEED_IRQMAP_COUNT] = {
because you define ASPEED_SDRAM, and if some code access
aspeed_soc_ast2400_irqmap[ASPEED_SDRAM] it would be uninitialized.
Specifying the array size, the uninitialized entries are
zero-initialized (not sure this is the default you expect although).
> + [ASPEED_UART1] = 9,
> + [ASPEED_UART2] = 32,
> + [ASPEED_UART3] = 33,
> + [ASPEED_UART4] = 34,
> + [ASPEED_UART5] = 10,
> + [ASPEED_VUART] = 8,
> + [ASPEED_FMC] = 19,
> + [ASPEED_SDMC] = 0,
> + [ASPEED_SCU] = 21,
> + [ASPEED_ADC] = 31,
> + [ASPEED_GPIO] = 20,
> + [ASPEED_RTC] = 22,
> + [ASPEED_TIMER1] = 16,
> + [ASPEED_TIMER2] = 17,
> + [ASPEED_TIMER3] = 18,
> + [ASPEED_TIMER4] = 35,
> + [ASPEED_TIMER5] = 36,
> + [ASPEED_TIMER6] = 37,
> + [ASPEED_TIMER7] = 38,
> + [ASPEED_TIMER8] = 39,
> + [ASPEED_WDT] = 27,
> + [ASPEED_PWM] = 28,
> + [ASPEED_LPC] = 8,
> + [ASPEED_IBT] = 8, /* LPC */
> + [ASPEED_I2C] = 12,
> + [ASPEED_ETH1] = 2,
> + [ASPEED_ETH2] = 3,
> +};
>
> #define AST2400_SDRAM_BASE 0x40000000
> #define AST2500_SDRAM_BASE 0x80000000
>
> +/* AST2500 uses the same IRQs as the AST2400 */
> +#define aspeed_soc_ast2500_irqmap aspeed_soc_ast2400_irqmap
> +
> static const hwaddr aspeed_soc_ast2400_spi_bases[] = { ASPEED_SOC_SPI_BASE };
> static const char *aspeed_soc_ast2400_typenames[] = { "aspeed.smc.spi" };
>
> @@ -64,6 +94,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .fmc_typename = "aspeed.smc.fmc",
> .spi_typename = aspeed_soc_ast2400_typenames,
> .wdts_num = 2,
> + .irqmap = aspeed_soc_ast2400_irqmap,
> }, {
> .name = "ast2400-a1",
> .cpu_type = ARM_CPU_TYPE_NAME("arm926"),
> @@ -75,6 +106,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .fmc_typename = "aspeed.smc.fmc",
> .spi_typename = aspeed_soc_ast2400_typenames,
> .wdts_num = 2,
> + .irqmap = aspeed_soc_ast2400_irqmap,
> }, {
> .name = "ast2400",
> .cpu_type = ARM_CPU_TYPE_NAME("arm926"),
> @@ -86,6 +118,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .fmc_typename = "aspeed.smc.fmc",
> .spi_typename = aspeed_soc_ast2400_typenames,
> .wdts_num = 2,
> + .irqmap = aspeed_soc_ast2400_irqmap,
> }, {
> .name = "ast2500-a1",
> .cpu_type = ARM_CPU_TYPE_NAME("arm1176"),
> @@ -97,9 +130,17 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .fmc_typename = "aspeed.smc.ast2500-fmc",
> .spi_typename = aspeed_soc_ast2500_typenames,
> .wdts_num = 3,
> + .irqmap = aspeed_soc_ast2500_irqmap,
> },
> };
>
> +static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
> +{
> + AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> +
> + return qdev_get_gpio_in(DEVICE(&s->vic), sc->info->irqmap[ctrl]);
> +}
> +
> static void aspeed_soc_init(Object *obj)
> {
> AspeedSoCState *s = ASPEED_SOC(obj);
> @@ -226,14 +267,14 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> return;
> }
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0, ASPEED_SOC_TIMER_BASE);
> - for (i = 0; i < ARRAY_SIZE(timer_irqs); i++) {
> - qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->vic), timer_irqs[i]);
> + for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
> + qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_TIMER1 + i);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq);
> }
>
> /* UART - attach an 8250 to the IO space as our UART5 */
> if (serial_hd(0)) {
> - qemu_irq uart5 = qdev_get_gpio_in(DEVICE(&s->vic), uart_irqs[4]);
> + qemu_irq uart5 = aspeed_soc_get_irq(s, ASPEED_UART5);
> serial_mm_init(get_system_memory(),
> ASPEED_SOC_IOMEM_BASE + ASPEED_SOC_UART_5_BASE, 2,
> uart5, 38400, serial_hd(0), DEVICE_LITTLE_ENDIAN);
> @@ -247,7 +288,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> }
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, ASPEED_SOC_I2C_BASE);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
> - qdev_get_gpio_in(DEVICE(&s->vic), 12));
> + aspeed_soc_get_irq(s, ASPEED_I2C));
>
> /* FMC, The number of CS is set at the board level */
> object_property_set_bool(OBJECT(&s->fmc), true, "realized", &err);
> @@ -259,7 +300,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
> s->fmc.ctrl->flash_window_base);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
> - qdev_get_gpio_in(DEVICE(&s->vic), 19));
> + aspeed_soc_get_irq(s, ASPEED_FMC));
>
> /* SPI */
> for (i = 0; i < sc->info->spis_num; i++) {
> @@ -307,7 +348,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> }
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
> - qdev_get_gpio_in(DEVICE(&s->vic), 2));
> + aspeed_soc_get_irq(s, ASPEED_ETH1));
> }
>
> static void aspeed_soc_class_init(ObjectClass *oc, void *data)
>
With the array explicit size:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Mon, May 6, 2019 at 4:48 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 5/6/19 4:20 PM, Cédric Le Goater wrote:
> > This will simplify the definition of new SoCs, like the AST2600 which
> > should use a different CPU and a different IRQ number layout.
> >
> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > ---
> >
> > Changes since v1:
> >
> > - moved enum defining the Aspeed controller names under aspeed_soc.h
> >
> > include/hw/arm/aspeed_soc.h | 37 ++++++++++++++++++++++++
> > hw/arm/aspeed_soc.c | 57 +++++++++++++++++++++++++++++++------
> > 2 files changed, 86 insertions(+), 8 deletions(-)
> >
> > diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> > index 11ec0179db50..2dd968092c69 100644
> > --- a/include/hw/arm/aspeed_soc.h
> > +++ b/include/hw/arm/aspeed_soc.h
> > @@ -57,6 +57,7 @@ typedef struct AspeedSoCInfo {
> > const char *fmc_typename;
> > const char **spi_typename;
> > int wdts_num;
> > + const int *irqmap;
> > } AspeedSoCInfo;
> >
> > typedef struct AspeedSoCClass {
> > @@ -69,4 +70,40 @@ typedef struct AspeedSoCClass {
> > #define ASPEED_SOC_GET_CLASS(obj) \
> > OBJECT_GET_CLASS(AspeedSoCClass, (obj), TYPE_ASPEED_SOC)
> >
> > +enum {
> > + ASPEED_IOMEM,
> > + ASPEED_UART1,
> > + ASPEED_UART2,
> > + ASPEED_UART3,
> > + ASPEED_UART4,
> > + ASPEED_UART5,
> > + ASPEED_VUART,
> > + ASPEED_FMC,
> > + ASPEED_SPI1,
> > + ASPEED_SPI2,
> > + ASPEED_VIC,
> > + ASPEED_SDMC,
> > + ASPEED_SCU,
> > + ASPEED_ADC,
> > + ASPEED_SRAM,
> > + ASPEED_GPIO,
> > + ASPEED_RTC,
> > + ASPEED_TIMER1,
> > + ASPEED_TIMER2,
> > + ASPEED_TIMER3,
> > + ASPEED_TIMER4,
> > + ASPEED_TIMER5,
> > + ASPEED_TIMER6,
> > + ASPEED_TIMER7,
> > + ASPEED_TIMER8,
> > + ASPEED_WDT,
> > + ASPEED_PWM,
> > + ASPEED_LPC,
> > + ASPEED_IBT,
> > + ASPEED_I2C,
> > + ASPEED_ETH1,
> > + ASPEED_ETH2,
> > + ASPEED_SDRAM,
>
> I'd add ...:
>
> ASPEED_IRQMAP_COUNT /* keep last */
Better name after reading patch 2/3: ASPEED_MODULE_COUNT?
> > +};
> > +
> > #endif /* ASPEED_SOC_H */
> > diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> > index a27233d4876b..29bce5c9188c 100644
> > --- a/hw/arm/aspeed_soc.c
> > +++ b/hw/arm/aspeed_soc.c
> > @@ -38,12 +38,42 @@
> > #define ASPEED_SOC_ETH1_BASE 0x1E660000
> > #define ASPEED_SOC_ETH2_BASE 0x1E680000
> >
> > -static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
> > -static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
> > +static const int aspeed_soc_ast2400_irqmap[] = {
>
> ... and use it here:
>
> static const int aspeed_soc_ast2400_irqmap[ASPEED_IRQMAP_COUNT] = {
>
> because you define ASPEED_SDRAM, and if some code access
> aspeed_soc_ast2400_irqmap[ASPEED_SDRAM] it would be uninitialized.
> Specifying the array size, the uninitialized entries are
> zero-initialized (not sure this is the default you expect although).
>
> > + [ASPEED_UART1] = 9,
> > + [ASPEED_UART2] = 32,
> > + [ASPEED_UART3] = 33,
> > + [ASPEED_UART4] = 34,
> > + [ASPEED_UART5] = 10,
> > + [ASPEED_VUART] = 8,
> > + [ASPEED_FMC] = 19,
> > + [ASPEED_SDMC] = 0,
> > + [ASPEED_SCU] = 21,
> > + [ASPEED_ADC] = 31,
> > + [ASPEED_GPIO] = 20,
> > + [ASPEED_RTC] = 22,
> > + [ASPEED_TIMER1] = 16,
> > + [ASPEED_TIMER2] = 17,
> > + [ASPEED_TIMER3] = 18,
> > + [ASPEED_TIMER4] = 35,
> > + [ASPEED_TIMER5] = 36,
> > + [ASPEED_TIMER6] = 37,
> > + [ASPEED_TIMER7] = 38,
> > + [ASPEED_TIMER8] = 39,
> > + [ASPEED_WDT] = 27,
> > + [ASPEED_PWM] = 28,
> > + [ASPEED_LPC] = 8,
> > + [ASPEED_IBT] = 8, /* LPC */
> > + [ASPEED_I2C] = 12,
> > + [ASPEED_ETH1] = 2,
> > + [ASPEED_ETH2] = 3,
> > +};
> >
> > #define AST2400_SDRAM_BASE 0x40000000
> > #define AST2500_SDRAM_BASE 0x80000000
> >
> > +/* AST2500 uses the same IRQs as the AST2400 */
> > +#define aspeed_soc_ast2500_irqmap aspeed_soc_ast2400_irqmap
> > +
> > static const hwaddr aspeed_soc_ast2400_spi_bases[] = { ASPEED_SOC_SPI_BASE };
> > static const char *aspeed_soc_ast2400_typenames[] = { "aspeed.smc.spi" };
> >
> > @@ -64,6 +94,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> > .fmc_typename = "aspeed.smc.fmc",
> > .spi_typename = aspeed_soc_ast2400_typenames,
> > .wdts_num = 2,
> > + .irqmap = aspeed_soc_ast2400_irqmap,
> > }, {
> > .name = "ast2400-a1",
> > .cpu_type = ARM_CPU_TYPE_NAME("arm926"),
> > @@ -75,6 +106,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> > .fmc_typename = "aspeed.smc.fmc",
> > .spi_typename = aspeed_soc_ast2400_typenames,
> > .wdts_num = 2,
> > + .irqmap = aspeed_soc_ast2400_irqmap,
> > }, {
> > .name = "ast2400",
> > .cpu_type = ARM_CPU_TYPE_NAME("arm926"),
> > @@ -86,6 +118,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> > .fmc_typename = "aspeed.smc.fmc",
> > .spi_typename = aspeed_soc_ast2400_typenames,
> > .wdts_num = 2,
> > + .irqmap = aspeed_soc_ast2400_irqmap,
> > }, {
> > .name = "ast2500-a1",
> > .cpu_type = ARM_CPU_TYPE_NAME("arm1176"),
> > @@ -97,9 +130,17 @@ static const AspeedSoCInfo aspeed_socs[] = {
> > .fmc_typename = "aspeed.smc.ast2500-fmc",
> > .spi_typename = aspeed_soc_ast2500_typenames,
> > .wdts_num = 3,
> > + .irqmap = aspeed_soc_ast2500_irqmap,
> > },
> > };
> >
> > +static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
> > +{
> > + AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> > +
> > + return qdev_get_gpio_in(DEVICE(&s->vic), sc->info->irqmap[ctrl]);
> > +}
> > +
> > static void aspeed_soc_init(Object *obj)
> > {
> > AspeedSoCState *s = ASPEED_SOC(obj);
> > @@ -226,14 +267,14 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> > return;
> > }
> > sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0, ASPEED_SOC_TIMER_BASE);
> > - for (i = 0; i < ARRAY_SIZE(timer_irqs); i++) {
> > - qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->vic), timer_irqs[i]);
> > + for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
> > + qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_TIMER1 + i);
> > sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq);
> > }
> >
> > /* UART - attach an 8250 to the IO space as our UART5 */
> > if (serial_hd(0)) {
> > - qemu_irq uart5 = qdev_get_gpio_in(DEVICE(&s->vic), uart_irqs[4]);
> > + qemu_irq uart5 = aspeed_soc_get_irq(s, ASPEED_UART5);
> > serial_mm_init(get_system_memory(),
> > ASPEED_SOC_IOMEM_BASE + ASPEED_SOC_UART_5_BASE, 2,
> > uart5, 38400, serial_hd(0), DEVICE_LITTLE_ENDIAN);
> > @@ -247,7 +288,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> > }
> > sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, ASPEED_SOC_I2C_BASE);
> > sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
> > - qdev_get_gpio_in(DEVICE(&s->vic), 12));
> > + aspeed_soc_get_irq(s, ASPEED_I2C));
> >
> > /* FMC, The number of CS is set at the board level */
> > object_property_set_bool(OBJECT(&s->fmc), true, "realized", &err);
> > @@ -259,7 +300,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> > sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
> > s->fmc.ctrl->flash_window_base);
> > sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
> > - qdev_get_gpio_in(DEVICE(&s->vic), 19));
> > + aspeed_soc_get_irq(s, ASPEED_FMC));
> >
> > /* SPI */
> > for (i = 0; i < sc->info->spis_num; i++) {
> > @@ -307,7 +348,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> > }
> > sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
> > sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
> > - qdev_get_gpio_in(DEVICE(&s->vic), 2));
> > + aspeed_soc_get_irq(s, ASPEED_ETH1));
> > }
> >
> > static void aspeed_soc_class_init(ObjectClass *oc, void *data)
> >
>
> With the array explicit size:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On 5/6/19 4:47 PM, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
>
> On 5/6/19 4:20 PM, Cédric Le Goater wrote:
>> This will simplify the definition of new SoCs, like the AST2600 which
>> should use a different CPU and a different IRQ number layout.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>
>> Changes since v1:
>>
>> - moved enum defining the Aspeed controller names under aspeed_soc.h
>>
>> include/hw/arm/aspeed_soc.h | 37 ++++++++++++++++++++++++
>> hw/arm/aspeed_soc.c | 57 +++++++++++++++++++++++++++++++------
>> 2 files changed, 86 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
>> index 11ec0179db50..2dd968092c69 100644
>> --- a/include/hw/arm/aspeed_soc.h
>> +++ b/include/hw/arm/aspeed_soc.h
>> @@ -57,6 +57,7 @@ typedef struct AspeedSoCInfo {
>> const char *fmc_typename;
>> const char **spi_typename;
>> int wdts_num;
>> + const int *irqmap;
>> } AspeedSoCInfo;
>>
>> typedef struct AspeedSoCClass {
>> @@ -69,4 +70,40 @@ typedef struct AspeedSoCClass {
>> #define ASPEED_SOC_GET_CLASS(obj) \
>> OBJECT_GET_CLASS(AspeedSoCClass, (obj), TYPE_ASPEED_SOC)
>>
>> +enum {
>> + ASPEED_IOMEM,
>> + ASPEED_UART1,
>> + ASPEED_UART2,
>> + ASPEED_UART3,
>> + ASPEED_UART4,
>> + ASPEED_UART5,
>> + ASPEED_VUART,
>> + ASPEED_FMC,
>> + ASPEED_SPI1,
>> + ASPEED_SPI2,
>> + ASPEED_VIC,
>> + ASPEED_SDMC,
>> + ASPEED_SCU,
>> + ASPEED_ADC,
>> + ASPEED_SRAM,
>> + ASPEED_GPIO,
>> + ASPEED_RTC,
>> + ASPEED_TIMER1,
>> + ASPEED_TIMER2,
>> + ASPEED_TIMER3,
>> + ASPEED_TIMER4,
>> + ASPEED_TIMER5,
>> + ASPEED_TIMER6,
>> + ASPEED_TIMER7,
>> + ASPEED_TIMER8,
>> + ASPEED_WDT,
>> + ASPEED_PWM,
>> + ASPEED_LPC,
>> + ASPEED_IBT,
>> + ASPEED_I2C,
>> + ASPEED_ETH1,
>> + ASPEED_ETH2,
>> + ASPEED_SDRAM,
>
> I'd add ...:
>
> ASPEED_IRQMAP_COUNT /* keep last */
>
>> +};
>> +
>> #endif /* ASPEED_SOC_H */
>> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
>> index a27233d4876b..29bce5c9188c 100644
>> --- a/hw/arm/aspeed_soc.c
>> +++ b/hw/arm/aspeed_soc.c
>> @@ -38,12 +38,42 @@
>> #define ASPEED_SOC_ETH1_BASE 0x1E660000
>> #define ASPEED_SOC_ETH2_BASE 0x1E680000
>>
>> -static const int uart_irqs[] = { 9, 32, 33, 34, 10 };
>> -static const int timer_irqs[] = { 16, 17, 18, 35, 36, 37, 38, 39, };
>> +static const int aspeed_soc_ast2400_irqmap[] = {
>
> ... and use it here:
>
> static const int aspeed_soc_ast2400_irqmap[ASPEED_IRQMAP_COUNT] = {
>
> because you define ASPEED_SDRAM, and if some code access
> aspeed_soc_ast2400_irqmap[ASPEED_SDRAM] it would be uninitialized.
The ASPEED_SDRAM should be initialized in the next patch.
> Specifying the array size, the uninitialized entries are
> zero-initialized (not sure this is the default you expect although).
I am not sure to understand the problem you are describing. I think
the compiler should catch any severe issues. no ?
Thanks,
C.
>
>> + [ASPEED_UART1] = 9,
>> + [ASPEED_UART2] = 32,
>> + [ASPEED_UART3] = 33,
>> + [ASPEED_UART4] = 34,
>> + [ASPEED_UART5] = 10,
>> + [ASPEED_VUART] = 8,
>> + [ASPEED_FMC] = 19,
>> + [ASPEED_SDMC] = 0,
>> + [ASPEED_SCU] = 21,
>> + [ASPEED_ADC] = 31,
>> + [ASPEED_GPIO] = 20,
>> + [ASPEED_RTC] = 22,
>> + [ASPEED_TIMER1] = 16,
>> + [ASPEED_TIMER2] = 17,
>> + [ASPEED_TIMER3] = 18,
>> + [ASPEED_TIMER4] = 35,
>> + [ASPEED_TIMER5] = 36,
>> + [ASPEED_TIMER6] = 37,
>> + [ASPEED_TIMER7] = 38,
>> + [ASPEED_TIMER8] = 39,
>> + [ASPEED_WDT] = 27,
>> + [ASPEED_PWM] = 28,
>> + [ASPEED_LPC] = 8,
>> + [ASPEED_IBT] = 8, /* LPC */
>> + [ASPEED_I2C] = 12,
>> + [ASPEED_ETH1] = 2,
>> + [ASPEED_ETH2] = 3,
>> +};
>>
>> #define AST2400_SDRAM_BASE 0x40000000
>> #define AST2500_SDRAM_BASE 0x80000000
>>
>> +/* AST2500 uses the same IRQs as the AST2400 */
>> +#define aspeed_soc_ast2500_irqmap aspeed_soc_ast2400_irqmap
>> +
>> static const hwaddr aspeed_soc_ast2400_spi_bases[] = { ASPEED_SOC_SPI_BASE };
>> static const char *aspeed_soc_ast2400_typenames[] = { "aspeed.smc.spi" };
>>
>> @@ -64,6 +94,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
>> .fmc_typename = "aspeed.smc.fmc",
>> .spi_typename = aspeed_soc_ast2400_typenames,
>> .wdts_num = 2,
>> + .irqmap = aspeed_soc_ast2400_irqmap,
>> }, {
>> .name = "ast2400-a1",
>> .cpu_type = ARM_CPU_TYPE_NAME("arm926"),
>> @@ -75,6 +106,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
>> .fmc_typename = "aspeed.smc.fmc",
>> .spi_typename = aspeed_soc_ast2400_typenames,
>> .wdts_num = 2,
>> + .irqmap = aspeed_soc_ast2400_irqmap,
>> }, {
>> .name = "ast2400",
>> .cpu_type = ARM_CPU_TYPE_NAME("arm926"),
>> @@ -86,6 +118,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
>> .fmc_typename = "aspeed.smc.fmc",
>> .spi_typename = aspeed_soc_ast2400_typenames,
>> .wdts_num = 2,
>> + .irqmap = aspeed_soc_ast2400_irqmap,
>> }, {
>> .name = "ast2500-a1",
>> .cpu_type = ARM_CPU_TYPE_NAME("arm1176"),
>> @@ -97,9 +130,17 @@ static const AspeedSoCInfo aspeed_socs[] = {
>> .fmc_typename = "aspeed.smc.ast2500-fmc",
>> .spi_typename = aspeed_soc_ast2500_typenames,
>> .wdts_num = 3,
>> + .irqmap = aspeed_soc_ast2500_irqmap,
>> },
>> };
>>
>> +static qemu_irq aspeed_soc_get_irq(AspeedSoCState *s, int ctrl)
>> +{
>> + AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
>> +
>> + return qdev_get_gpio_in(DEVICE(&s->vic), sc->info->irqmap[ctrl]);
>> +}
>> +
>> static void aspeed_soc_init(Object *obj)
>> {
>> AspeedSoCState *s = ASPEED_SOC(obj);
>> @@ -226,14 +267,14 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>> return;
>> }
>> sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0, ASPEED_SOC_TIMER_BASE);
>> - for (i = 0; i < ARRAY_SIZE(timer_irqs); i++) {
>> - qemu_irq irq = qdev_get_gpio_in(DEVICE(&s->vic), timer_irqs[i]);
>> + for (i = 0; i < ASPEED_TIMER_NR_TIMERS; i++) {
>> + qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_TIMER1 + i);
>> sysbus_connect_irq(SYS_BUS_DEVICE(&s->timerctrl), i, irq);
>> }
>>
>> /* UART - attach an 8250 to the IO space as our UART5 */
>> if (serial_hd(0)) {
>> - qemu_irq uart5 = qdev_get_gpio_in(DEVICE(&s->vic), uart_irqs[4]);
>> + qemu_irq uart5 = aspeed_soc_get_irq(s, ASPEED_UART5);
>> serial_mm_init(get_system_memory(),
>> ASPEED_SOC_IOMEM_BASE + ASPEED_SOC_UART_5_BASE, 2,
>> uart5, 38400, serial_hd(0), DEVICE_LITTLE_ENDIAN);
>> @@ -247,7 +288,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>> }
>> sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, ASPEED_SOC_I2C_BASE);
>> sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
>> - qdev_get_gpio_in(DEVICE(&s->vic), 12));
>> + aspeed_soc_get_irq(s, ASPEED_I2C));
>>
>> /* FMC, The number of CS is set at the board level */
>> object_property_set_bool(OBJECT(&s->fmc), true, "realized", &err);
>> @@ -259,7 +300,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>> sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
>> s->fmc.ctrl->flash_window_base);
>> sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
>> - qdev_get_gpio_in(DEVICE(&s->vic), 19));
>> + aspeed_soc_get_irq(s, ASPEED_FMC));
>>
>> /* SPI */
>> for (i = 0; i < sc->info->spis_num; i++) {
>> @@ -307,7 +348,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>> }
>> sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
>> sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
>> - qdev_get_gpio_in(DEVICE(&s->vic), 2));
>> + aspeed_soc_get_irq(s, ASPEED_ETH1));
>> }
>>
>> static void aspeed_soc_class_init(ObjectClass *oc, void *data)
>>
>
> With the array explicit size:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
© 2016 - 2026 Red Hat, Inc.