* [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping for the interrupt space
2019-05-06 14:20 [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions Cédric Le Goater
@ 2019-05-06 14:20 ` Cédric Le Goater
2019-05-06 14:47 ` Philippe Mathieu-Daudé
2019-05-07 5:18 ` Joel Stanley
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 2/3] aspeed: add a per SoC mapping for the memory space Cédric Le Goater
` (2 subsequent siblings)
3 siblings, 2 replies; 21+ messages in thread
From: Cédric Le Goater @ 2019-05-06 14:20 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrew Jeffery, Cédric Le Goater, qemu-arm, qemu-devel,
Joel Stanley
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
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping for the interrupt space
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping for the interrupt space Cédric Le Goater
@ 2019-05-06 14:47 ` Philippe Mathieu-Daudé
2019-05-06 14:50 ` Philippe Mathieu-Daudé
2019-05-07 7:25 ` Cédric Le Goater
2019-05-07 5:18 ` Joel Stanley
1 sibling, 2 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-06 14:47 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell
Cc: Andrew Jeffery, qemu-arm, qemu-devel, Joel Stanley
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>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping for the interrupt space
2019-05-06 14:47 ` Philippe Mathieu-Daudé
@ 2019-05-06 14:50 ` Philippe Mathieu-Daudé
2019-05-07 7:25 ` Cédric Le Goater
1 sibling, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-06 14:50 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell
Cc: Andrew Jeffery, qemu-arm, QEMU Developers, Joel Stanley
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>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping for the interrupt space
2019-05-06 14:47 ` Philippe Mathieu-Daudé
2019-05-06 14:50 ` Philippe Mathieu-Daudé
@ 2019-05-07 7:25 ` Cédric Le Goater
1 sibling, 0 replies; 21+ messages in thread
From: Cédric Le Goater @ 2019-05-07 7:25 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell
Cc: Andrew Jeffery, qemu-arm, qemu-devel, Joel Stanley
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>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping for the interrupt space
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping for the interrupt space Cédric Le Goater
2019-05-06 14:47 ` Philippe Mathieu-Daudé
@ 2019-05-07 5:18 ` Joel Stanley
1 sibling, 0 replies; 21+ messages in thread
From: Joel Stanley @ 2019-05-07 5:18 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, QEMU Developers
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>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v2 2/3] aspeed: add a per SoC mapping for the memory space
2019-05-06 14:20 [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions Cédric Le Goater
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping for the interrupt space Cédric Le Goater
@ 2019-05-06 14:20 ` Cédric Le Goater
2019-05-06 14:52 ` Philippe Mathieu-Daudé
2019-05-07 5:19 ` Joel Stanley
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 3/3] aspeed: use sysbus_init_child_obj() to initialize children Cédric Le Goater
2019-05-20 7:47 ` [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions Cédric Le Goater
3 siblings, 2 replies; 21+ messages in thread
From: Cédric Le Goater @ 2019-05-06 14:20 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrew Jeffery, Cédric Le Goater, qemu-arm, qemu-devel,
Joel Stanley
This will simplify the definition of new SoCs, like the AST2600 which
should use a slightly different address space and have a different set
of controllers.
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
Changes since v1:
- removed AspeedSoCInfo 'sdram_base' field
- fixed clang compilation
include/hw/arm/aspeed_soc.h | 3 +-
hw/arm/aspeed.c | 8 +--
hw/arm/aspeed_soc.c | 117 ++++++++++++++++++++++--------------
3 files changed, 77 insertions(+), 51 deletions(-)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 2dd968092c69..70fa1c5a3887 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -50,14 +50,13 @@ typedef struct AspeedSoCInfo {
const char *name;
const char *cpu_type;
uint32_t silicon_rev;
- hwaddr sdram_base;
uint64_t sram_size;
int spis_num;
- const hwaddr *spi_bases;
const char *fmc_typename;
const char **spi_typename;
int wdts_num;
const int *irqmap;
+ const hwaddr *memmap;
} AspeedSoCInfo;
typedef struct AspeedSoCClass {
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index eb434d9bae27..ef92eb2a7028 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -191,8 +191,8 @@ static void aspeed_board_init(MachineState *machine,
&error_abort);
memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size);
- memory_region_add_subregion(get_system_memory(), sc->info->sdram_base,
- &bmc->ram);
+ memory_region_add_subregion(get_system_memory(),
+ sc->info->memmap[ASPEED_SDRAM], &bmc->ram);
object_property_add_const_link(OBJECT(&bmc->soc), "ram", OBJECT(&bmc->ram),
&error_abort);
@@ -201,7 +201,7 @@ static void aspeed_board_init(MachineState *machine,
memory_region_init_io(&bmc->max_ram, NULL, &max_ram_ops, NULL,
"max_ram", max_ram_size - ram_size);
memory_region_add_subregion(get_system_memory(),
- sc->info->sdram_base + ram_size,
+ sc->info->memmap[ASPEED_SDRAM] + ram_size,
&bmc->max_ram);
aspeed_board_init_flashes(&bmc->soc.fmc, cfg->fmc_model, &error_abort);
@@ -236,7 +236,7 @@ static void aspeed_board_init(MachineState *machine,
aspeed_board_binfo.initrd_filename = machine->initrd_filename;
aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline;
aspeed_board_binfo.ram_size = ram_size;
- aspeed_board_binfo.loader_start = sc->info->sdram_base;
+ aspeed_board_binfo.loader_start = sc->info->memmap[ASPEED_SDRAM];
if (cfg->i2c_init) {
cfg->i2c_init(bmc);
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 29bce5c9188c..bd83618ceba9 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -22,21 +22,58 @@
#include "hw/i2c/aspeed_i2c.h"
#include "net/net.h"
-#define ASPEED_SOC_UART_5_BASE 0x00184000
#define ASPEED_SOC_IOMEM_SIZE 0x00200000
-#define ASPEED_SOC_IOMEM_BASE 0x1E600000
-#define ASPEED_SOC_FMC_BASE 0x1E620000
-#define ASPEED_SOC_SPI_BASE 0x1E630000
-#define ASPEED_SOC_SPI2_BASE 0x1E631000
-#define ASPEED_SOC_VIC_BASE 0x1E6C0000
-#define ASPEED_SOC_SDMC_BASE 0x1E6E0000
-#define ASPEED_SOC_SCU_BASE 0x1E6E2000
-#define ASPEED_SOC_SRAM_BASE 0x1E720000
-#define ASPEED_SOC_TIMER_BASE 0x1E782000
-#define ASPEED_SOC_WDT_BASE 0x1E785000
-#define ASPEED_SOC_I2C_BASE 0x1E78A000
-#define ASPEED_SOC_ETH1_BASE 0x1E660000
-#define ASPEED_SOC_ETH2_BASE 0x1E680000
+
+static const hwaddr aspeed_soc_ast2400_memmap[] = {
+ [ASPEED_IOMEM] = 0x1E600000,
+ [ASPEED_FMC] = 0x1E620000,
+ [ASPEED_SPI1] = 0x1E630000,
+ [ASPEED_VIC] = 0x1E6C0000,
+ [ASPEED_SDMC] = 0x1E6E0000,
+ [ASPEED_SCU] = 0x1E6E2000,
+ [ASPEED_ADC] = 0x1E6E9000,
+ [ASPEED_SRAM] = 0x1E720000,
+ [ASPEED_GPIO] = 0x1E780000,
+ [ASPEED_RTC] = 0x1E781000,
+ [ASPEED_TIMER1] = 0x1E782000,
+ [ASPEED_WDT] = 0x1E785000,
+ [ASPEED_PWM] = 0x1E786000,
+ [ASPEED_LPC] = 0x1E789000,
+ [ASPEED_IBT] = 0x1E789140,
+ [ASPEED_I2C] = 0x1E78A000,
+ [ASPEED_ETH1] = 0x1E660000,
+ [ASPEED_ETH2] = 0x1E680000,
+ [ASPEED_UART1] = 0x1E783000,
+ [ASPEED_UART5] = 0x1E784000,
+ [ASPEED_VUART] = 0x1E787000,
+ [ASPEED_SDRAM] = 0x40000000,
+};
+
+static const hwaddr aspeed_soc_ast2500_memmap[] = {
+ [ASPEED_IOMEM] = 0x1E600000,
+ [ASPEED_FMC] = 0x1E620000,
+ [ASPEED_SPI1] = 0x1E630000,
+ [ASPEED_SPI2] = 0x1E631000,
+ [ASPEED_VIC] = 0x1E6C0000,
+ [ASPEED_SDMC] = 0x1E6E0000,
+ [ASPEED_SCU] = 0x1E6E2000,
+ [ASPEED_ADC] = 0x1E6E9000,
+ [ASPEED_SRAM] = 0x1E720000,
+ [ASPEED_GPIO] = 0x1E780000,
+ [ASPEED_RTC] = 0x1E781000,
+ [ASPEED_TIMER1] = 0x1E782000,
+ [ASPEED_WDT] = 0x1E785000,
+ [ASPEED_PWM] = 0x1E786000,
+ [ASPEED_LPC] = 0x1E789000,
+ [ASPEED_IBT] = 0x1E789140,
+ [ASPEED_I2C] = 0x1E78A000,
+ [ASPEED_ETH1] = 0x1E660000,
+ [ASPEED_ETH2] = 0x1E680000,
+ [ASPEED_UART1] = 0x1E783000,
+ [ASPEED_UART5] = 0x1E784000,
+ [ASPEED_VUART] = 0x1E787000,
+ [ASPEED_SDRAM] = 0x80000000,
+};
static const int aspeed_soc_ast2400_irqmap[] = {
[ASPEED_UART1] = 9,
@@ -68,17 +105,9 @@ static const int aspeed_soc_ast2400_irqmap[] = {
[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" };
-
-static const hwaddr aspeed_soc_ast2500_spi_bases[] = { ASPEED_SOC_SPI_BASE,
- ASPEED_SOC_SPI2_BASE};
static const char *aspeed_soc_ast2500_typenames[] = {
"aspeed.smc.ast2500-spi1", "aspeed.smc.ast2500-spi2" };
@@ -87,50 +116,46 @@ static const AspeedSoCInfo aspeed_socs[] = {
.name = "ast2400-a0",
.cpu_type = ARM_CPU_TYPE_NAME("arm926"),
.silicon_rev = AST2400_A0_SILICON_REV,
- .sdram_base = AST2400_SDRAM_BASE,
.sram_size = 0x8000,
.spis_num = 1,
- .spi_bases = aspeed_soc_ast2400_spi_bases,
.fmc_typename = "aspeed.smc.fmc",
.spi_typename = aspeed_soc_ast2400_typenames,
.wdts_num = 2,
.irqmap = aspeed_soc_ast2400_irqmap,
+ .memmap = aspeed_soc_ast2400_memmap,
}, {
.name = "ast2400-a1",
.cpu_type = ARM_CPU_TYPE_NAME("arm926"),
.silicon_rev = AST2400_A1_SILICON_REV,
- .sdram_base = AST2400_SDRAM_BASE,
.sram_size = 0x8000,
.spis_num = 1,
- .spi_bases = aspeed_soc_ast2400_spi_bases,
.fmc_typename = "aspeed.smc.fmc",
.spi_typename = aspeed_soc_ast2400_typenames,
.wdts_num = 2,
.irqmap = aspeed_soc_ast2400_irqmap,
+ .memmap = aspeed_soc_ast2400_memmap,
}, {
.name = "ast2400",
.cpu_type = ARM_CPU_TYPE_NAME("arm926"),
.silicon_rev = AST2400_A0_SILICON_REV,
- .sdram_base = AST2400_SDRAM_BASE,
.sram_size = 0x8000,
.spis_num = 1,
- .spi_bases = aspeed_soc_ast2400_spi_bases,
.fmc_typename = "aspeed.smc.fmc",
.spi_typename = aspeed_soc_ast2400_typenames,
.wdts_num = 2,
.irqmap = aspeed_soc_ast2400_irqmap,
+ .memmap = aspeed_soc_ast2400_memmap,
}, {
.name = "ast2500-a1",
.cpu_type = ARM_CPU_TYPE_NAME("arm1176"),
.silicon_rev = AST2500_A1_SILICON_REV,
- .sdram_base = AST2500_SDRAM_BASE,
.sram_size = 0x9000,
.spis_num = 2,
- .spi_bases = aspeed_soc_ast2500_spi_bases,
.fmc_typename = "aspeed.smc.ast2500-fmc",
.spi_typename = aspeed_soc_ast2500_typenames,
.wdts_num = 3,
.irqmap = aspeed_soc_ast2500_irqmap,
+ .memmap = aspeed_soc_ast2500_memmap,
},
};
@@ -220,8 +245,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
Error *err = NULL, *local_err = NULL;
/* IO space */
- create_unimplemented_device("aspeed_soc.io",
- ASPEED_SOC_IOMEM_BASE, ASPEED_SOC_IOMEM_SIZE);
+ create_unimplemented_device("aspeed_soc.io", sc->info->memmap[ASPEED_IOMEM],
+ ASPEED_SOC_IOMEM_SIZE);
/* CPU */
object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
@@ -237,8 +262,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
error_propagate(errp, err);
return;
}
- memory_region_add_subregion(get_system_memory(), ASPEED_SOC_SRAM_BASE,
- &s->sram);
+ memory_region_add_subregion(get_system_memory(),
+ sc->info->memmap[ASPEED_SRAM], &s->sram);
/* SCU */
object_property_set_bool(OBJECT(&s->scu), true, "realized", &err);
@@ -246,7 +271,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
error_propagate(errp, err);
return;
}
- sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, ASPEED_SOC_SCU_BASE);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, sc->info->memmap[ASPEED_SCU]);
/* VIC */
object_property_set_bool(OBJECT(&s->vic), true, "realized", &err);
@@ -254,7 +279,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
error_propagate(errp, err);
return;
}
- sysbus_mmio_map(SYS_BUS_DEVICE(&s->vic), 0, ASPEED_SOC_VIC_BASE);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->vic), 0, sc->info->memmap[ASPEED_VIC]);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->vic), 0,
qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ));
sysbus_connect_irq(SYS_BUS_DEVICE(&s->vic), 1,
@@ -266,7 +291,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
error_propagate(errp, err);
return;
}
- sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0, ASPEED_SOC_TIMER_BASE);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0,
+ sc->info->memmap[ASPEED_TIMER1]);
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);
@@ -275,8 +301,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
/* UART - attach an 8250 to the IO space as our UART5 */
if (serial_hd(0)) {
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,
+ serial_mm_init(get_system_memory(), sc->info->memmap[ASPEED_UART5], 2,
uart5, 38400, serial_hd(0), DEVICE_LITTLE_ENDIAN);
}
@@ -286,7 +311,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
error_propagate(errp, err);
return;
}
- sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, ASPEED_SOC_I2C_BASE);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, sc->info->memmap[ASPEED_I2C]);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
aspeed_soc_get_irq(s, ASPEED_I2C));
@@ -296,7 +321,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
error_propagate(errp, err);
return;
}
- sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, ASPEED_SOC_FMC_BASE);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, sc->info->memmap[ASPEED_FMC]);
sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
s->fmc.ctrl->flash_window_base);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
@@ -312,7 +337,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
error_propagate(errp, err);
return;
}
- sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, sc->info->spi_bases[i]);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0,
+ sc->info->memmap[ASPEED_SPI1 + i]);
sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 1,
s->spi[i].ctrl->flash_window_base);
}
@@ -323,7 +349,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
error_propagate(errp, err);
return;
}
- sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdmc), 0, ASPEED_SOC_SDMC_BASE);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdmc), 0, sc->info->memmap[ASPEED_SDMC]);
/* Watch dog */
for (i = 0; i < sc->info->wdts_num; i++) {
@@ -333,7 +359,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
return;
}
sysbus_mmio_map(SYS_BUS_DEVICE(&s->wdt[i]), 0,
- ASPEED_SOC_WDT_BASE + i * 0x20);
+ sc->info->memmap[ASPEED_WDT] + i * 0x20);
}
/* Net */
@@ -346,7 +372,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
error_propagate(errp, err);
return;
}
- sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0,
+ sc->info->memmap[ASPEED_ETH1]);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
aspeed_soc_get_irq(s, ASPEED_ETH1));
}
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/3] aspeed: add a per SoC mapping for the memory space
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 2/3] aspeed: add a per SoC mapping for the memory space Cédric Le Goater
@ 2019-05-06 14:52 ` Philippe Mathieu-Daudé
2019-05-07 5:19 ` Joel Stanley
1 sibling, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-06 14:52 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell
Cc: Andrew Jeffery, qemu-arm, qemu-devel, Joel Stanley
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 slightly different address space and have a different set
> of controllers.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> Changes since v1:
>
> - removed AspeedSoCInfo 'sdram_base' field
> - fixed clang compilation
>
> include/hw/arm/aspeed_soc.h | 3 +-
> hw/arm/aspeed.c | 8 +--
> hw/arm/aspeed_soc.c | 117 ++++++++++++++++++++++--------------
> 3 files changed, 77 insertions(+), 51 deletions(-)
>
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 2dd968092c69..70fa1c5a3887 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -50,14 +50,13 @@ typedef struct AspeedSoCInfo {
> const char *name;
> const char *cpu_type;
> uint32_t silicon_rev;
> - hwaddr sdram_base;
> uint64_t sram_size;
> int spis_num;
> - const hwaddr *spi_bases;
> const char *fmc_typename;
> const char **spi_typename;
> int wdts_num;
> const int *irqmap;
> + const hwaddr *memmap;
> } AspeedSoCInfo;
>
> typedef struct AspeedSoCClass {
> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> index eb434d9bae27..ef92eb2a7028 100644
> --- a/hw/arm/aspeed.c
> +++ b/hw/arm/aspeed.c
> @@ -191,8 +191,8 @@ static void aspeed_board_init(MachineState *machine,
> &error_abort);
>
> memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size);
> - memory_region_add_subregion(get_system_memory(), sc->info->sdram_base,
> - &bmc->ram);
> + memory_region_add_subregion(get_system_memory(),
> + sc->info->memmap[ASPEED_SDRAM], &bmc->ram);
> object_property_add_const_link(OBJECT(&bmc->soc), "ram", OBJECT(&bmc->ram),
> &error_abort);
>
> @@ -201,7 +201,7 @@ static void aspeed_board_init(MachineState *machine,
> memory_region_init_io(&bmc->max_ram, NULL, &max_ram_ops, NULL,
> "max_ram", max_ram_size - ram_size);
> memory_region_add_subregion(get_system_memory(),
> - sc->info->sdram_base + ram_size,
> + sc->info->memmap[ASPEED_SDRAM] + ram_size,
> &bmc->max_ram);
>
> aspeed_board_init_flashes(&bmc->soc.fmc, cfg->fmc_model, &error_abort);
> @@ -236,7 +236,7 @@ static void aspeed_board_init(MachineState *machine,
> aspeed_board_binfo.initrd_filename = machine->initrd_filename;
> aspeed_board_binfo.kernel_cmdline = machine->kernel_cmdline;
> aspeed_board_binfo.ram_size = ram_size;
> - aspeed_board_binfo.loader_start = sc->info->sdram_base;
> + aspeed_board_binfo.loader_start = sc->info->memmap[ASPEED_SDRAM];
>
> if (cfg->i2c_init) {
> cfg->i2c_init(bmc);
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 29bce5c9188c..bd83618ceba9 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -22,21 +22,58 @@
> #include "hw/i2c/aspeed_i2c.h"
> #include "net/net.h"
>
> -#define ASPEED_SOC_UART_5_BASE 0x00184000
> #define ASPEED_SOC_IOMEM_SIZE 0x00200000
> -#define ASPEED_SOC_IOMEM_BASE 0x1E600000
> -#define ASPEED_SOC_FMC_BASE 0x1E620000
> -#define ASPEED_SOC_SPI_BASE 0x1E630000
> -#define ASPEED_SOC_SPI2_BASE 0x1E631000
> -#define ASPEED_SOC_VIC_BASE 0x1E6C0000
> -#define ASPEED_SOC_SDMC_BASE 0x1E6E0000
> -#define ASPEED_SOC_SCU_BASE 0x1E6E2000
> -#define ASPEED_SOC_SRAM_BASE 0x1E720000
> -#define ASPEED_SOC_TIMER_BASE 0x1E782000
> -#define ASPEED_SOC_WDT_BASE 0x1E785000
> -#define ASPEED_SOC_I2C_BASE 0x1E78A000
> -#define ASPEED_SOC_ETH1_BASE 0x1E660000
> -#define ASPEED_SOC_ETH2_BASE 0x1E680000
> +
> +static const hwaddr aspeed_soc_ast2400_memmap[] = {
static const hwaddr aspeed_soc_ast2400_memmap[ASPEED_MODULE_COUNT] = {...
> + [ASPEED_IOMEM] = 0x1E600000,
> + [ASPEED_FMC] = 0x1E620000,
> + [ASPEED_SPI1] = 0x1E630000,
> + [ASPEED_VIC] = 0x1E6C0000,
> + [ASPEED_SDMC] = 0x1E6E0000,
> + [ASPEED_SCU] = 0x1E6E2000,
> + [ASPEED_ADC] = 0x1E6E9000,
> + [ASPEED_SRAM] = 0x1E720000,
> + [ASPEED_GPIO] = 0x1E780000,
> + [ASPEED_RTC] = 0x1E781000,
> + [ASPEED_TIMER1] = 0x1E782000,
> + [ASPEED_WDT] = 0x1E785000,
> + [ASPEED_PWM] = 0x1E786000,
> + [ASPEED_LPC] = 0x1E789000,
> + [ASPEED_IBT] = 0x1E789140,
> + [ASPEED_I2C] = 0x1E78A000,
> + [ASPEED_ETH1] = 0x1E660000,
> + [ASPEED_ETH2] = 0x1E680000,
> + [ASPEED_UART1] = 0x1E783000,
> + [ASPEED_UART5] = 0x1E784000,
> + [ASPEED_VUART] = 0x1E787000,
> + [ASPEED_SDRAM] = 0x40000000,
> +};
> +
> +static const hwaddr aspeed_soc_ast2500_memmap[] = {
Ditto.
> + [ASPEED_IOMEM] = 0x1E600000,
> + [ASPEED_FMC] = 0x1E620000,
> + [ASPEED_SPI1] = 0x1E630000,
> + [ASPEED_SPI2] = 0x1E631000,
> + [ASPEED_VIC] = 0x1E6C0000,
> + [ASPEED_SDMC] = 0x1E6E0000,
> + [ASPEED_SCU] = 0x1E6E2000,
> + [ASPEED_ADC] = 0x1E6E9000,
> + [ASPEED_SRAM] = 0x1E720000,
> + [ASPEED_GPIO] = 0x1E780000,
> + [ASPEED_RTC] = 0x1E781000,
> + [ASPEED_TIMER1] = 0x1E782000,
> + [ASPEED_WDT] = 0x1E785000,
> + [ASPEED_PWM] = 0x1E786000,
> + [ASPEED_LPC] = 0x1E789000,
> + [ASPEED_IBT] = 0x1E789140,
> + [ASPEED_I2C] = 0x1E78A000,
> + [ASPEED_ETH1] = 0x1E660000,
> + [ASPEED_ETH2] = 0x1E680000,
> + [ASPEED_UART1] = 0x1E783000,
> + [ASPEED_UART5] = 0x1E784000,
> + [ASPEED_VUART] = 0x1E787000,
> + [ASPEED_SDRAM] = 0x80000000,
> +};
>
> static const int aspeed_soc_ast2400_irqmap[] = {
> [ASPEED_UART1] = 9,
> @@ -68,17 +105,9 @@ static const int aspeed_soc_ast2400_irqmap[] = {
> [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" };
> -
> -static const hwaddr aspeed_soc_ast2500_spi_bases[] = { ASPEED_SOC_SPI_BASE,
> - ASPEED_SOC_SPI2_BASE};
> static const char *aspeed_soc_ast2500_typenames[] = {
> "aspeed.smc.ast2500-spi1", "aspeed.smc.ast2500-spi2" };
>
> @@ -87,50 +116,46 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .name = "ast2400-a0",
> .cpu_type = ARM_CPU_TYPE_NAME("arm926"),
> .silicon_rev = AST2400_A0_SILICON_REV,
> - .sdram_base = AST2400_SDRAM_BASE,
> .sram_size = 0x8000,
> .spis_num = 1,
> - .spi_bases = aspeed_soc_ast2400_spi_bases,
> .fmc_typename = "aspeed.smc.fmc",
> .spi_typename = aspeed_soc_ast2400_typenames,
> .wdts_num = 2,
> .irqmap = aspeed_soc_ast2400_irqmap,
> + .memmap = aspeed_soc_ast2400_memmap,
> }, {
> .name = "ast2400-a1",
> .cpu_type = ARM_CPU_TYPE_NAME("arm926"),
> .silicon_rev = AST2400_A1_SILICON_REV,
> - .sdram_base = AST2400_SDRAM_BASE,
> .sram_size = 0x8000,
> .spis_num = 1,
> - .spi_bases = aspeed_soc_ast2400_spi_bases,
> .fmc_typename = "aspeed.smc.fmc",
> .spi_typename = aspeed_soc_ast2400_typenames,
> .wdts_num = 2,
> .irqmap = aspeed_soc_ast2400_irqmap,
> + .memmap = aspeed_soc_ast2400_memmap,
> }, {
> .name = "ast2400",
> .cpu_type = ARM_CPU_TYPE_NAME("arm926"),
> .silicon_rev = AST2400_A0_SILICON_REV,
> - .sdram_base = AST2400_SDRAM_BASE,
> .sram_size = 0x8000,
> .spis_num = 1,
> - .spi_bases = aspeed_soc_ast2400_spi_bases,
> .fmc_typename = "aspeed.smc.fmc",
> .spi_typename = aspeed_soc_ast2400_typenames,
> .wdts_num = 2,
> .irqmap = aspeed_soc_ast2400_irqmap,
> + .memmap = aspeed_soc_ast2400_memmap,
> }, {
> .name = "ast2500-a1",
> .cpu_type = ARM_CPU_TYPE_NAME("arm1176"),
> .silicon_rev = AST2500_A1_SILICON_REV,
> - .sdram_base = AST2500_SDRAM_BASE,
> .sram_size = 0x9000,
> .spis_num = 2,
> - .spi_bases = aspeed_soc_ast2500_spi_bases,
> .fmc_typename = "aspeed.smc.ast2500-fmc",
> .spi_typename = aspeed_soc_ast2500_typenames,
> .wdts_num = 3,
> .irqmap = aspeed_soc_ast2500_irqmap,
> + .memmap = aspeed_soc_ast2500_memmap,
> },
> };
>
> @@ -220,8 +245,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> Error *err = NULL, *local_err = NULL;
>
> /* IO space */
> - create_unimplemented_device("aspeed_soc.io",
> - ASPEED_SOC_IOMEM_BASE, ASPEED_SOC_IOMEM_SIZE);
> + create_unimplemented_device("aspeed_soc.io", sc->info->memmap[ASPEED_IOMEM],
> + ASPEED_SOC_IOMEM_SIZE);
>
> /* CPU */
> object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
> @@ -237,8 +262,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, err);
> return;
> }
> - memory_region_add_subregion(get_system_memory(), ASPEED_SOC_SRAM_BASE,
> - &s->sram);
> + memory_region_add_subregion(get_system_memory(),
> + sc->info->memmap[ASPEED_SRAM], &s->sram);
>
> /* SCU */
> object_property_set_bool(OBJECT(&s->scu), true, "realized", &err);
> @@ -246,7 +271,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, ASPEED_SOC_SCU_BASE);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->scu), 0, sc->info->memmap[ASPEED_SCU]);
>
> /* VIC */
> object_property_set_bool(OBJECT(&s->vic), true, "realized", &err);
> @@ -254,7 +279,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(&s->vic), 0, ASPEED_SOC_VIC_BASE);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->vic), 0, sc->info->memmap[ASPEED_VIC]);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->vic), 0,
> qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ));
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->vic), 1,
> @@ -266,7 +291,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0, ASPEED_SOC_TIMER_BASE);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->timerctrl), 0,
> + sc->info->memmap[ASPEED_TIMER1]);
> 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);
> @@ -275,8 +301,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> /* UART - attach an 8250 to the IO space as our UART5 */
> if (serial_hd(0)) {
> 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,
> + serial_mm_init(get_system_memory(), sc->info->memmap[ASPEED_UART5], 2,
> uart5, 38400, serial_hd(0), DEVICE_LITTLE_ENDIAN);
> }
>
> @@ -286,7 +311,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, ASPEED_SOC_I2C_BASE);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c), 0, sc->info->memmap[ASPEED_I2C]);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c), 0,
> aspeed_soc_get_irq(s, ASPEED_I2C));
>
> @@ -296,7 +321,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, ASPEED_SOC_FMC_BASE);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 0, sc->info->memmap[ASPEED_FMC]);
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->fmc), 1,
> s->fmc.ctrl->flash_window_base);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->fmc), 0,
> @@ -312,7 +337,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0, sc->info->spi_bases[i]);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 0,
> + sc->info->memmap[ASPEED_SPI1 + i]);
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->spi[i]), 1,
> s->spi[i].ctrl->flash_window_base);
> }
> @@ -323,7 +349,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdmc), 0, ASPEED_SOC_SDMC_BASE);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdmc), 0, sc->info->memmap[ASPEED_SDMC]);
>
> /* Watch dog */
> for (i = 0; i < sc->info->wdts_num; i++) {
> @@ -333,7 +359,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> return;
> }
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->wdt[i]), 0,
> - ASPEED_SOC_WDT_BASE + i * 0x20);
> + sc->info->memmap[ASPEED_WDT] + i * 0x20);
> }
>
> /* Net */
> @@ -346,7 +372,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> error_propagate(errp, err);
> return;
> }
> - sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0, ASPEED_SOC_ETH1_BASE);
> + sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100), 0,
> + sc->info->memmap[ASPEED_ETH1]);
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->ftgmac100), 0,
> aspeed_soc_get_irq(s, ASPEED_ETH1));
> }
>
Using ASPEED_MODULE_COUNT:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/3] aspeed: add a per SoC mapping for the memory space
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 2/3] aspeed: add a per SoC mapping for the memory space Cédric Le Goater
2019-05-06 14:52 ` Philippe Mathieu-Daudé
@ 2019-05-07 5:19 ` Joel Stanley
1 sibling, 0 replies; 21+ messages in thread
From: Joel Stanley @ 2019-05-07 5:19 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, QEMU Developers
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 slightly different address space and have a different set
> of controllers.
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v2 3/3] aspeed: use sysbus_init_child_obj() to initialize children
2019-05-06 14:20 [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions Cédric Le Goater
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping for the interrupt space Cédric Le Goater
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 2/3] aspeed: add a per SoC mapping for the memory space Cédric Le Goater
@ 2019-05-06 14:20 ` Cédric Le Goater
2019-05-07 5:20 ` Joel Stanley
2019-05-07 5:46 ` Philippe Mathieu-Daudé
2019-05-20 7:47 ` [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions Cédric Le Goater
3 siblings, 2 replies; 21+ messages in thread
From: Cédric Le Goater @ 2019-05-06 14:20 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrew Jeffery, Cédric Le Goater, qemu-arm, qemu-devel,
Joel Stanley
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
hw/arm/aspeed_soc.c | 50 ++++++++++++++++++---------------------------
1 file changed, 20 insertions(+), 30 deletions(-)
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index bd83618ceba9..4b705afd096a 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -172,12 +172,11 @@ static void aspeed_soc_init(Object *obj)
AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
int i;
- object_initialize(&s->cpu, sizeof(s->cpu), sc->info->cpu_type);
- object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
+ object_initialize_child(obj, "cpu", OBJECT(&s->cpu), sizeof(s->cpu),
+ sc->info->cpu_type, &error_abort, NULL);
- object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
- object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
- qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
+ sysbus_init_child_obj(obj, "scu", OBJECT(&s->scu), sizeof(s->scu),
+ TYPE_ASPEED_SCU);
qdev_prop_set_uint32(DEVICE(&s->scu), "silicon-rev",
sc->info->silicon_rev);
object_property_add_alias(obj, "hw-strap1", OBJECT(&s->scu),
@@ -187,36 +186,29 @@ static void aspeed_soc_init(Object *obj)
object_property_add_alias(obj, "hw-prot-key", OBJECT(&s->scu),
"hw-prot-key", &error_abort);
- object_initialize(&s->vic, sizeof(s->vic), TYPE_ASPEED_VIC);
- object_property_add_child(obj, "vic", OBJECT(&s->vic), NULL);
- qdev_set_parent_bus(DEVICE(&s->vic), sysbus_get_default());
+ sysbus_init_child_obj(obj, "vic", OBJECT(&s->vic), sizeof(s->vic),
+ TYPE_ASPEED_VIC);
- object_initialize(&s->timerctrl, sizeof(s->timerctrl), TYPE_ASPEED_TIMER);
- object_property_add_child(obj, "timerctrl", OBJECT(&s->timerctrl), NULL);
+ sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
+ sizeof(s->timerctrl), TYPE_ASPEED_TIMER);
object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
OBJECT(&s->scu), &error_abort);
- qdev_set_parent_bus(DEVICE(&s->timerctrl), sysbus_get_default());
- object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C);
- object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL);
- qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default());
+ sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
+ TYPE_ASPEED_I2C);
- object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename);
- object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
- qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default());
+ sysbus_init_child_obj(obj, "fmc", OBJECT(&s->fmc), sizeof(s->fmc),
+ sc->info->fmc_typename);
object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs",
&error_abort);
for (i = 0; i < sc->info->spis_num; i++) {
- object_initialize(&s->spi[i], sizeof(s->spi[i]),
- sc->info->spi_typename[i]);
- object_property_add_child(obj, "spi[*]", OBJECT(&s->spi[i]), NULL);
- qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
+ sysbus_init_child_obj(obj, "spi[*]", OBJECT(&s->spi[i]),
+ sizeof(s->spi[i]), sc->info->spi_typename[i]);
}
- object_initialize(&s->sdmc, sizeof(s->sdmc), TYPE_ASPEED_SDMC);
- object_property_add_child(obj, "sdmc", OBJECT(&s->sdmc), NULL);
- qdev_set_parent_bus(DEVICE(&s->sdmc), sysbus_get_default());
+ sysbus_init_child_obj(obj, "sdmc", OBJECT(&s->sdmc), sizeof(s->sdmc),
+ TYPE_ASPEED_SDMC);
qdev_prop_set_uint32(DEVICE(&s->sdmc), "silicon-rev",
sc->info->silicon_rev);
object_property_add_alias(obj, "ram-size", OBJECT(&s->sdmc),
@@ -225,16 +217,14 @@ static void aspeed_soc_init(Object *obj)
"max-ram-size", &error_abort);
for (i = 0; i < sc->info->wdts_num; i++) {
- object_initialize(&s->wdt[i], sizeof(s->wdt[i]), TYPE_ASPEED_WDT);
- object_property_add_child(obj, "wdt[*]", OBJECT(&s->wdt[i]), NULL);
- qdev_set_parent_bus(DEVICE(&s->wdt[i]), sysbus_get_default());
+ sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
+ sizeof(s->wdt[i]), TYPE_ASPEED_WDT);
qdev_prop_set_uint32(DEVICE(&s->wdt[i]), "silicon-rev",
sc->info->silicon_rev);
}
- object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100);
- object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL);
- qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default());
+ sysbus_init_child_obj(obj, "ftgmac100", OBJECT(&s->ftgmac100),
+ sizeof(s->ftgmac100), TYPE_FTGMAC100);
}
static void aspeed_soc_realize(DeviceState *dev, Error **errp)
--
2.20.1
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/3] aspeed: use sysbus_init_child_obj() to initialize children
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 3/3] aspeed: use sysbus_init_child_obj() to initialize children Cédric Le Goater
@ 2019-05-07 5:20 ` Joel Stanley
2019-05-07 5:46 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 21+ messages in thread
From: Joel Stanley @ 2019-05-07 5:20 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Andrew Jeffery, Peter Maydell, qemu-arm, QEMU Developers
On Mon, 6 May 2019 at 14:21, Cédric Le Goater <clg@kaod.org> wrote:
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/3] aspeed: use sysbus_init_child_obj() to initialize children
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 3/3] aspeed: use sysbus_init_child_obj() to initialize children Cédric Le Goater
2019-05-07 5:20 ` Joel Stanley
@ 2019-05-07 5:46 ` Philippe Mathieu-Daudé
2019-05-17 10:11 ` Cédric Le Goater
1 sibling, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-07 5:46 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell
Cc: Andrew Jeffery, qemu-arm, qemu-devel, Joel Stanley
Hi Cédric,
On 5/6/19 4:20 PM, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/arm/aspeed_soc.c | 50 ++++++++++++++++++---------------------------
> 1 file changed, 20 insertions(+), 30 deletions(-)
>
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index bd83618ceba9..4b705afd096a 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -172,12 +172,11 @@ static void aspeed_soc_init(Object *obj)
> AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
> int i;
>
> - object_initialize(&s->cpu, sizeof(s->cpu), sc->info->cpu_type);
> - object_property_add_child(obj, "cpu", OBJECT(&s->cpu), NULL);
> + object_initialize_child(obj, "cpu", OBJECT(&s->cpu), sizeof(s->cpu),
> + sc->info->cpu_type, &error_abort, NULL);
>
> - object_initialize(&s->scu, sizeof(s->scu), TYPE_ASPEED_SCU);
> - object_property_add_child(obj, "scu", OBJECT(&s->scu), NULL);
> - qdev_set_parent_bus(DEVICE(&s->scu), sysbus_get_default());
> + sysbus_init_child_obj(obj, "scu", OBJECT(&s->scu), sizeof(s->scu),
> + TYPE_ASPEED_SCU);
A similar patch (although better described IMO) was posted here:
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05931.html
However you improved it by using sysbus_init_child_obj(). If you don't
mind I'll respin addressing Markus comment and using your improvement.
> qdev_prop_set_uint32(DEVICE(&s->scu), "silicon-rev",
> sc->info->silicon_rev);
> object_property_add_alias(obj, "hw-strap1", OBJECT(&s->scu),
> @@ -187,36 +186,29 @@ static void aspeed_soc_init(Object *obj)
> object_property_add_alias(obj, "hw-prot-key", OBJECT(&s->scu),
> "hw-prot-key", &error_abort);
>
> - object_initialize(&s->vic, sizeof(s->vic), TYPE_ASPEED_VIC);
> - object_property_add_child(obj, "vic", OBJECT(&s->vic), NULL);
> - qdev_set_parent_bus(DEVICE(&s->vic), sysbus_get_default());
> + sysbus_init_child_obj(obj, "vic", OBJECT(&s->vic), sizeof(s->vic),
> + TYPE_ASPEED_VIC);
>
> - object_initialize(&s->timerctrl, sizeof(s->timerctrl), TYPE_ASPEED_TIMER);
> - object_property_add_child(obj, "timerctrl", OBJECT(&s->timerctrl), NULL);
> + sysbus_init_child_obj(obj, "timerctrl", OBJECT(&s->timerctrl),
> + sizeof(s->timerctrl), TYPE_ASPEED_TIMER);
> object_property_add_const_link(OBJECT(&s->timerctrl), "scu",
> OBJECT(&s->scu), &error_abort);
> - qdev_set_parent_bus(DEVICE(&s->timerctrl), sysbus_get_default());
>
> - object_initialize(&s->i2c, sizeof(s->i2c), TYPE_ASPEED_I2C);
> - object_property_add_child(obj, "i2c", OBJECT(&s->i2c), NULL);
> - qdev_set_parent_bus(DEVICE(&s->i2c), sysbus_get_default());
> + sysbus_init_child_obj(obj, "i2c", OBJECT(&s->i2c), sizeof(s->i2c),
> + TYPE_ASPEED_I2C);
>
> - object_initialize(&s->fmc, sizeof(s->fmc), sc->info->fmc_typename);
> - object_property_add_child(obj, "fmc", OBJECT(&s->fmc), NULL);
> - qdev_set_parent_bus(DEVICE(&s->fmc), sysbus_get_default());
> + sysbus_init_child_obj(obj, "fmc", OBJECT(&s->fmc), sizeof(s->fmc),
> + sc->info->fmc_typename);
> object_property_add_alias(obj, "num-cs", OBJECT(&s->fmc), "num-cs",
> &error_abort);
>
> for (i = 0; i < sc->info->spis_num; i++) {
> - object_initialize(&s->spi[i], sizeof(s->spi[i]),
> - sc->info->spi_typename[i]);
> - object_property_add_child(obj, "spi[*]", OBJECT(&s->spi[i]), NULL);
> - qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
> + sysbus_init_child_obj(obj, "spi[*]", OBJECT(&s->spi[i]),
> + sizeof(s->spi[i]), sc->info->spi_typename[i]);
> }
>
> - object_initialize(&s->sdmc, sizeof(s->sdmc), TYPE_ASPEED_SDMC);
> - object_property_add_child(obj, "sdmc", OBJECT(&s->sdmc), NULL);
> - qdev_set_parent_bus(DEVICE(&s->sdmc), sysbus_get_default());
> + sysbus_init_child_obj(obj, "sdmc", OBJECT(&s->sdmc), sizeof(s->sdmc),
> + TYPE_ASPEED_SDMC);
> qdev_prop_set_uint32(DEVICE(&s->sdmc), "silicon-rev",
> sc->info->silicon_rev);
> object_property_add_alias(obj, "ram-size", OBJECT(&s->sdmc),
> @@ -225,16 +217,14 @@ static void aspeed_soc_init(Object *obj)
> "max-ram-size", &error_abort);
>
> for (i = 0; i < sc->info->wdts_num; i++) {
> - object_initialize(&s->wdt[i], sizeof(s->wdt[i]), TYPE_ASPEED_WDT);
> - object_property_add_child(obj, "wdt[*]", OBJECT(&s->wdt[i]), NULL);
> - qdev_set_parent_bus(DEVICE(&s->wdt[i]), sysbus_get_default());
> + sysbus_init_child_obj(obj, "wdt[*]", OBJECT(&s->wdt[i]),
> + sizeof(s->wdt[i]), TYPE_ASPEED_WDT);
> qdev_prop_set_uint32(DEVICE(&s->wdt[i]), "silicon-rev",
> sc->info->silicon_rev);
> }
>
> - object_initialize(&s->ftgmac100, sizeof(s->ftgmac100), TYPE_FTGMAC100);
> - object_property_add_child(obj, "ftgmac100", OBJECT(&s->ftgmac100), NULL);
> - qdev_set_parent_bus(DEVICE(&s->ftgmac100), sysbus_get_default());
> + sysbus_init_child_obj(obj, "ftgmac100", OBJECT(&s->ftgmac100),
> + sizeof(s->ftgmac100), TYPE_FTGMAC100);
> }
>
> static void aspeed_soc_realize(DeviceState *dev, Error **errp)
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/3] aspeed: use sysbus_init_child_obj() to initialize children
2019-05-07 5:46 ` Philippe Mathieu-Daudé
@ 2019-05-17 10:11 ` Cédric Le Goater
0 siblings, 0 replies; 21+ messages in thread
From: Cédric Le Goater @ 2019-05-17 10:11 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell
Cc: Andrew Jeffery, qemu-arm, qemu-devel, Joel Stanley
Hello Philippe,
[ ... ]
> A similar patch (although better described IMO) was posted here:
> https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05931.html
>
> However you improved it by using sysbus_init_child_obj(). If you don't
> mind I'll respin addressing Markus comment and using your improvement.
Sure, np.
When is this patchset expected to be merged ? AFAICT, It's blocking
this current patchset, which is blocking Joel's RTC one and others
are on the way.
Thanks,
C.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
2019-05-06 14:20 [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions Cédric Le Goater
` (2 preceding siblings ...)
2019-05-06 14:20 ` [Qemu-devel] [PATCH v2 3/3] aspeed: use sysbus_init_child_obj() to initialize children Cédric Le Goater
@ 2019-05-20 7:47 ` Cédric Le Goater
2019-05-20 11:09 ` Philippe Mathieu-Daudé
3 siblings, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2019-05-20 7:47 UTC (permalink / raw)
To: Peter Maydell
Cc: Andrew Jeffery, qemu-arm, Philippe Mathieu-Daudé,
qemu-devel, Joel Stanley
Hello,
On 5/6/19 4:20 PM, Cédric Le Goater wrote:
> Hello,
>
> Here is a series adding a couple of cleanups to the Aspeed SoCs to
> prepare ground for extensions and new SoCs.
>
> Thanks,
>
> C.
>
> Changes since v1:
>
> - moved enum defining the Aspeed controller names under aspeed_soc.h
> - removed AspeedSoCInfo 'sdram_base' field
> - fixed clang compilation
>
> Cédric Le Goater (3):
> aspeed: add a per SoC mapping for the interrupt space
> aspeed: add a per SoC mapping for the memory space
I think these two patches are fine to go even if Philippe's comments
are not addressed. There are valid but not a blocker to me.
> aspeed: use sysbus_init_child_obj() to initialize children
Philippe has taken over this patch in a larger series which will go
through Eduardo's tree, if I understood well the emails. When merged,
we can try to re-merge the RTC patchset from Joel. I think we made
things a little more complex than they should have been.
Thanks,
C.
> include/hw/arm/aspeed_soc.h | 40 ++++++-
> hw/arm/aspeed.c | 8 +-
> hw/arm/aspeed_soc.c | 226 ++++++++++++++++++++++--------------
> 3 files changed, 184 insertions(+), 90 deletions(-)
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
2019-05-20 7:47 ` [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions Cédric Le Goater
@ 2019-05-20 11:09 ` Philippe Mathieu-Daudé
2019-05-20 13:11 ` Cédric Le Goater
0 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-20 11:09 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell
Cc: Andrew Jeffery, qemu-arm, qemu-devel, Joel Stanley
On 5/20/19 9:47 AM, Cédric Le Goater wrote:
> Hello,
>
> On 5/6/19 4:20 PM, Cédric Le Goater wrote:
>> Hello,
>>
>> Here is a series adding a couple of cleanups to the Aspeed SoCs to
>> prepare ground for extensions and new SoCs.
>>
>> Thanks,
>>
>> C.
>>
>> Changes since v1:
>>
>> - moved enum defining the Aspeed controller names under aspeed_soc.h
>> - removed AspeedSoCInfo 'sdram_base' field
>> - fixed clang compilation
>>
>> Cédric Le Goater (3):
>> aspeed: add a per SoC mapping for the interrupt space
>> aspeed: add a per SoC mapping for the memory space
>
> I think these two patches are fine to go even if Philippe's comments
> are not addressed. There are valid but not a blocker to me.
OK, so:
patches 1 & 2:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Peter, can you apply them?
>
>> aspeed: use sysbus_init_child_obj() to initialize children
>
> Philippe has taken over this patch in a larger series which will go
> through Eduardo's tree, if I understood well the emails. When merged,
> we can try to re-merge the RTC patchset from Joel. I think we made
> things a little more complex than they should have been.
Sorry if I made things more complex. I went on PTO after sending
"hw/arm: Use object_initialize_child for correct reference counting" [*]
then was slow to address Thomas/Markus comments.
Then maybe I should start pinging maintainer more aggressively when my
series are reviewed but not merged, to not delay further developments.
I took note of your comment and will try to keep things simple the next
time.
Regards,
Phil.
>
> Thanks,
>
> C.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
2019-05-20 11:09 ` Philippe Mathieu-Daudé
@ 2019-05-20 13:11 ` Cédric Le Goater
2019-05-20 13:32 ` Cédric Le Goater
0 siblings, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2019-05-20 13:11 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell
Cc: Andrew Jeffery, qemu-arm, qemu-devel, Joel Stanley
Hello,
On 5/20/19 1:09 PM, Philippe Mathieu-Daudé wrote:
> On 5/20/19 9:47 AM, Cédric Le Goater wrote:
>> Hello,
>>
>> On 5/6/19 4:20 PM, Cédric Le Goater wrote:
>>> Hello,
>>>
>>> Here is a series adding a couple of cleanups to the Aspeed SoCs to
>>> prepare ground for extensions and new SoCs.
>>>
>>> Thanks,
>>>
>>> C.
>>>
>>> Changes since v1:
>>>
>>> - moved enum defining the Aspeed controller names under aspeed_soc.h
>>> - removed AspeedSoCInfo 'sdram_base' field
>>> - fixed clang compilation
>>>
>>> Cédric Le Goater (3):
>>> aspeed: add a per SoC mapping for the interrupt space
>>> aspeed: add a per SoC mapping for the memory space
>>
>> I think these two patches are fine to go even if Philippe's comments
>> are not addressed. There are valid but not a blocker to me.
>
> OK, so:
>
> patches 1 & 2:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Peter, can you apply them?
>
>>
>>> aspeed: use sysbus_init_child_obj() to initialize children
>>
>> Philippe has taken over this patch in a larger series which will go
>> through Eduardo's tree, if I understood well the emails. When merged,
>> we can try to re-merge the RTC patchset from Joel. I think we made
>> things a little more complex than they should have been.
>
> Sorry if I made things more complex. I went on PTO after sending
PTO ?
> "hw/arm: Use object_initialize_child for correct reference counting" [*]
> then was slow to address Thomas/Markus comments.
> Then maybe I should start pinging maintainer more aggressively when my
> series are reviewed but not merged, to not delay further developments.
Well, I don't know if there is a good method for transversal patchsets
like this one. I guess it depends on the area.
The overall merging process became more complex that expected after our
three simple patchsets (Yours, Joel's and mine) collided.
> I took note of your comment and will try to keep things simple the next
> time.
It's not a big issue. We have time to provide fixes before 4.1 is out.
Let's put some energy to move on and get code merged.
Peter,
do you want me to resend with only the two first patches and include
Joel's in the same series ? I would leave out the part Philippe is
covering in his object_initialize_child() patchset.
Thanks,
C.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
2019-05-20 13:11 ` Cédric Le Goater
@ 2019-05-20 13:32 ` Cédric Le Goater
2019-05-20 16:32 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2019-05-20 13:32 UTC (permalink / raw)
To: Cédric Le Goater, Philippe Mathieu-Daudé, Peter Maydell
Cc: Andrew Jeffery, qemu-arm, qemu-devel, Joel Stanley
> Peter,
>
> do you want me to resend with only the two first patches and include
> Joel's in the same series ? I would leave out the part Philippe is
> covering in his object_initialize_child() patchset.
Nope, we can not do that, conflicts arise. I suppose the easier is wait
for Philippe's patchset to be merged and then rebase.
C.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
2019-05-20 13:32 ` Cédric Le Goater
@ 2019-05-20 16:32 ` Philippe Mathieu-Daudé
2019-05-23 12:52 ` Peter Maydell
0 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-05-20 16:32 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell
Cc: Andrew Jeffery, qemu-arm, qemu-devel, Eduardo Habkost, Joel Stanley
On 5/20/19 3:32 PM, Cédric Le Goater wrote:
>> Peter,
>>
>> do you want me to resend with only the two first patches and include
>> Joel's in the same series ? I would leave out the part Philippe is
>> covering in his object_initialize_child() patchset.
>
> Nope, we can not do that, conflicts arise. I suppose the easier is wait
> for Philippe's patchset to be merged and then rebase.
Eduardo said he'll send a pull request during the week.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
2019-05-20 16:32 ` Philippe Mathieu-Daudé
@ 2019-05-23 12:52 ` Peter Maydell
2019-05-23 13:03 ` Cédric Le Goater
0 siblings, 1 reply; 21+ messages in thread
From: Peter Maydell @ 2019-05-23 12:52 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Eduardo Habkost, Andrew Jeffery, QEMU Developers, qemu-arm,
Joel Stanley, Cédric Le Goater
On Mon, 20 May 2019 at 17:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 5/20/19 3:32 PM, Cédric Le Goater wrote:
> >> Peter,
> >>
> >> do you want me to resend with only the two first patches and include
> >> Joel's in the same series ? I would leave out the part Philippe is
> >> covering in his object_initialize_child() patchset.
> >
> > Nope, we can not do that, conflicts arise. I suppose the easier is wait
> > for Philippe's patchset to be merged and then rebase.
>
> Eduardo said he'll send a pull request during the week.
I am now completely lost about the status of these patches,
so I'm just dropping this series from my to-review queue.
Please send a fresh patchset once any dependencies have
got into master.
thanks
-- PMM
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
2019-05-23 12:52 ` Peter Maydell
@ 2019-05-23 13:03 ` Cédric Le Goater
2019-05-24 19:09 ` Eduardo Habkost
0 siblings, 1 reply; 21+ messages in thread
From: Cédric Le Goater @ 2019-05-23 13:03 UTC (permalink / raw)
To: Peter Maydell, Philippe Mathieu-Daudé
Cc: Andrew Jeffery, qemu-arm, QEMU Developers, Eduardo Habkost, Joel Stanley
On 5/23/19 2:52 PM, Peter Maydell wrote:
> On Mon, 20 May 2019 at 17:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> On 5/20/19 3:32 PM, Cédric Le Goater wrote:
>>>> Peter,
>>>>
>>>> do you want me to resend with only the two first patches and include
>>>> Joel's in the same series ? I would leave out the part Philippe is
>>>> covering in his object_initialize_child() patchset.
>>>
>>> Nope, we can not do that, conflicts arise. I suppose the easier is wait
>>> for Philippe's patchset to be merged and then rebase.
>>
>> Eduardo said he'll send a pull request during the week.
>
> I am now completely lost about the status of these patches,
> so I'm just dropping this series from my to-review queue.
yes. It's ok.
> Please send a fresh patchset once any dependencies have
> got into master.
I plan to send a larger one once Eduardo's patchset is merged.
Thanks,
C.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/3] aspeed: cleanups and extensions
2019-05-23 13:03 ` Cédric Le Goater
@ 2019-05-24 19:09 ` Eduardo Habkost
0 siblings, 0 replies; 21+ messages in thread
From: Eduardo Habkost @ 2019-05-24 19:09 UTC (permalink / raw)
To: Cédric Le Goater
Cc: Peter Maydell, Andrew Jeffery, QEMU Developers, qemu-arm,
Joel Stanley, Philippe Mathieu-Daudé
On Thu, May 23, 2019 at 03:03:11PM +0200, Cédric Le Goater wrote:
> On 5/23/19 2:52 PM, Peter Maydell wrote:
> > On Mon, 20 May 2019 at 17:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >>
> >> On 5/20/19 3:32 PM, Cédric Le Goater wrote:
> >>>> Peter,
> >>>>
> >>>> do you want me to resend with only the two first patches and include
> >>>> Joel's in the same series ? I would leave out the part Philippe is
> >>>> covering in his object_initialize_child() patchset.
> >>>
> >>> Nope, we can not do that, conflicts arise. I suppose the easier is wait
> >>> for Philippe's patchset to be merged and then rebase.
> >>
> >> Eduardo said he'll send a pull request during the week.
> >
> > I am now completely lost about the status of these patches,
> > so I'm just dropping this series from my to-review queue.
>
> yes. It's ok.
>
> > Please send a fresh patchset once any dependencies have
> > got into master.
>
> I plan to send a larger one once Eduardo's patchset is merged.
I've just submitted a pull request including the
object_initialize_child() patches from Philippe:
[PULL 00/17] Machine Core queue, 2019-05-24
Sorry for the delay.
--
Eduardo
^ permalink raw reply [flat|nested] 21+ messages in thread