Re: [Qemu-devel] [PATCH v5 2/3] aspeed: add a GPIO controller to the SoC
On 16/08/2019 09:40, Rashmica Gupta wrote:
> Cédric, this is how I thought changes to the SOC for your aspeed-4.1
> branch would look
Some comments,
> From 13a07834476fa266c352d9a075b341c483b2edf9 Mon Sep 17 00:00:00 2001
> From: Rashmica Gupta
> Date: Fri, 16 Aug 2019 15:18:22 +1000
> Subject: [PATCH] Aspeed SOC changes
>
> ---
> include/hw/arm/aspeed_soc.h | 4 +++-
> hw/arm/aspeed_soc.c | 32 ++--
> 2 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index 8673661de8..f375271d5a 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -28,6 +28,7 @@
> #define ASPEED_WDTS_NUM 3
> #define ASPEED_CPUS_NUM 2
> #define ASPEED_MACS_NUM 2
> +#define ASPEED_GPIOS_NUM 2
>
>
> typedef struct AspeedSoCState {
> /*< private >*/
> @@ -48,7 +49,7 @@ typedef struct AspeedSoCState {
> AspeedSDMCState sdmc;
> AspeedWDTState wdt[ASPEED_WDTS_NUM];
> FTGMAC100State ftgmac100[ASPEED_MACS_NUM];
> -AspeedGPIOState gpio;
> +AspeedGPIOState gpio[ASPEED_GPIOS_NUM];
Even if they look the same, I think these are two different controllers
and not multiple instances of the same. So I would rather introduce a new
field 'gpio_1_8v' for the AST2600.
> } AspeedSoCState;
>
> #define TYPE_ASPEED_SOC "aspeed-soc"
> @@ -61,6 +62,7 @@ typedef struct AspeedSoCInfo {
> uint64_t sram_size;
> int spis_num;
> int wdts_num;
> +int gpios_num;
> const int *irqmap;
> const hwaddr *memmap;
> uint32_t num_cpus;
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 7d47647016..414b99c4f3 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -119,6 +119,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .sram_size= 0x8000,
> .spis_num = 1,
> .wdts_num = 2,
> +.gpios_num= 1,
> .irqmap = aspeed_soc_ast2400_irqmap,
> .memmap = aspeed_soc_ast2400_memmap,
> .num_cpus = 1,
> @@ -132,6 +133,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .irqmap = aspeed_soc_ast2500_irqmap,
> .memmap = aspeed_soc_ast2500_memmap,
> .num_cpus = 1,
> +.gpios_num= 1,
> },
> };
>
> @@ -226,9 +228,15 @@ static void aspeed_soc_init(Object *obj)
> sysbus_init_child_obj(obj, "xdma", OBJECT(>xdma), sizeof(s-
>> xdma),
>TYPE_ASPEED_XDMA);
>
> -snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
> -sysbus_init_child_obj(obj, "gpio", OBJECT(>gpio), sizeof(s-
>> gpio),
> - typename);
> +for (i = 0; i < sc->info->gpios_num; i++) {
> +if (ASPEED_IS_AST2600(sc->info->silicon_rev)) {
> +snprintf(typename, sizeof(typename), "aspeed.gpio%d-%s",
> i, socname);
> +} else {
> +snprintf(typename, sizeof(typename), "aspeed.gpio-%s",
> socname);
> +}
> +sysbus_init_child_obj(obj, "gpio[*]", OBJECT(>gpio[i]),
> sizeof(s->gpio[i]),
> + typename);
> +}
> }
>
> static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> @@ -410,15 +418,19 @@ static void aspeed_soc_realize(DeviceState *dev,
> Error **errp)
> aspeed_soc_get_irq(s, ASPEED_XDMA));
>
> /* GPIO */
> -object_property_set_bool(OBJECT(>gpio), true, "realized",
> );
> -if (err) {
> -error_propagate(errp, err);
> -return;
> +for (i = 0; i < sc->info->gpios_num; i++) {
> +hwaddr addr = sc->info->memmap[ASPEED_GPIO] + i * 0x800;
I would introduce ASPEED_GPIO_V1_8V instead.
> +object_property_set_bool(OBJECT(>gpio[i]), true,
> "realized", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
> +sysbus_mmio_map(SYS_BUS_DEVICE(>gpio[i]), 0, addr);
> +sysbus_connect_irq(SYS_BUS_DEVICE(>gpio[i]), 0,
> + aspeed_soc_get_irq(s, ASPEED_GPIO));
The interrupt is different.
C.
> }
> -sysbus_mmio_map(SYS_BUS_DEVICE(>gpio), 0, sc->info-
>> memmap[ASPEED_GPIO]);
> -sysbus_connect_irq(SYS_BUS_DEVICE(>gpio), 0,
> - aspeed_soc_get_irq(s, ASPEED_GPIO));
> }
> +
> static Property aspeed_soc_properties[] = {
> DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0),
> DEFINE_PROP_END_OF_LIST(),
>
Re: [Qemu-devel] [PATCH v5 2/3] aspeed: add a GPIO controller to the SoC
On 16/08/2019 09:32, Rashmica Gupta wrote:
> Signed-off-by: Rashmica Gupta
Reviewed-by: Cédric Le Goater
Thanks,
C.
> ---
> include/hw/arm/aspeed_soc.h | 3 +++
> hw/arm/aspeed_soc.c | 17 +
> 2 files changed, 20 insertions(+)
>
> diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
> index cef605ad6b..fa04abddd8 100644
> --- a/include/hw/arm/aspeed_soc.h
> +++ b/include/hw/arm/aspeed_soc.h
> @@ -22,6 +22,7 @@
> #include "hw/ssi/aspeed_smc.h"
> #include "hw/watchdog/wdt_aspeed.h"
> #include "hw/net/ftgmac100.h"
> +#include "hw/gpio/aspeed_gpio.h"
>
> #define ASPEED_SPIS_NUM 2
> #define ASPEED_WDTS_NUM 3
> @@ -47,6 +48,7 @@ typedef struct AspeedSoCState {
> AspeedSDMCState sdmc;
> AspeedWDTState wdt[ASPEED_WDTS_NUM];
> FTGMAC100State ftgmac100[ASPEED_MACS_NUM];
> +AspeedGPIOState gpio;
> } AspeedSoCState;
>
> #define TYPE_ASPEED_SOC "aspeed-soc"
> @@ -60,6 +62,7 @@ typedef struct AspeedSoCInfo {
> int spis_num;
> const char *fmc_typename;
> const char **spi_typename;
> +const char *gpio_typename;
> int wdts_num;
> const int *irqmap;
> const hwaddr *memmap;
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index c6fb3700f2..ff422c8ad1 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -124,6 +124,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .spis_num = 1,
> .fmc_typename = "aspeed.smc.fmc",
> .spi_typename = aspeed_soc_ast2400_typenames,
> +.gpio_typename = "aspeed.gpio-ast2400",
> .wdts_num = 2,
> .irqmap = aspeed_soc_ast2400_irqmap,
> .memmap = aspeed_soc_ast2400_memmap,
> @@ -136,6 +137,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .spis_num = 1,
> .fmc_typename = "aspeed.smc.fmc",
> .spi_typename = aspeed_soc_ast2400_typenames,
> +.gpio_typename = "aspeed.gpio-ast2400",
> .wdts_num = 2,
> .irqmap = aspeed_soc_ast2400_irqmap,
> .memmap = aspeed_soc_ast2400_memmap,
> @@ -148,6 +150,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .spis_num = 1,
> .fmc_typename = "aspeed.smc.fmc",
> .spi_typename = aspeed_soc_ast2400_typenames,
> +.gpio_typename = "aspeed.gpio-ast2400",
> .wdts_num = 2,
> .irqmap = aspeed_soc_ast2400_irqmap,
> .memmap = aspeed_soc_ast2400_memmap,
> @@ -160,6 +163,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
> .spis_num = 2,
> .fmc_typename = "aspeed.smc.ast2500-fmc",
> .spi_typename = aspeed_soc_ast2500_typenames,
> +.gpio_typename = "aspeed.gpio-ast2500",
> .wdts_num = 3,
> .irqmap = aspeed_soc_ast2500_irqmap,
> .memmap = aspeed_soc_ast2500_memmap,
> @@ -246,6 +250,9 @@ static void aspeed_soc_init(Object *obj)
>
> sysbus_init_child_obj(obj, "xdma", OBJECT(>xdma), sizeof(s->xdma),
>TYPE_ASPEED_XDMA);
> +
> +sysbus_init_child_obj(obj, "gpio", OBJECT(>gpio), sizeof(s->gpio),
> + sc->info->gpio_typename);
> }
>
> static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> @@ -425,6 +432,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error
> **errp)
> sc->info->memmap[ASPEED_XDMA]);
> sysbus_connect_irq(SYS_BUS_DEVICE(>xdma), 0,
> aspeed_soc_get_irq(s, ASPEED_XDMA));
> +
> +/* GPIO */
> +object_property_set_bool(OBJECT(>gpio), true, "realized", );
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
> +sysbus_mmio_map(SYS_BUS_DEVICE(>gpio), 0,
> sc->info->memmap[ASPEED_GPIO]);
> +sysbus_connect_irq(SYS_BUS_DEVICE(>gpio), 0,
> + aspeed_soc_get_irq(s, ASPEED_GPIO));
> }
> static Property aspeed_soc_properties[] = {
> DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0),
>
Re: [Qemu-devel] [PATCH v5 2/3] aspeed: add a GPIO controller to the SoC
Cédric, this is how I thought changes to the SOC for your aspeed-4.1
branch would look
>From 13a07834476fa266c352d9a075b341c483b2edf9 Mon Sep 17 00:00:00 2001
From: Rashmica Gupta
Date: Fri, 16 Aug 2019 15:18:22 +1000
Subject: [PATCH] Aspeed SOC changes
---
include/hw/arm/aspeed_soc.h | 4 +++-
hw/arm/aspeed_soc.c | 32 ++--
2 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index 8673661de8..f375271d5a 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -28,6 +28,7 @@
#define ASPEED_WDTS_NUM 3
#define ASPEED_CPUS_NUM 2
#define ASPEED_MACS_NUM 2
+#define ASPEED_GPIOS_NUM 2
typedef struct AspeedSoCState {
/*< private >*/
@@ -48,7 +49,7 @@ typedef struct AspeedSoCState {
AspeedSDMCState sdmc;
AspeedWDTState wdt[ASPEED_WDTS_NUM];
FTGMAC100State ftgmac100[ASPEED_MACS_NUM];
-AspeedGPIOState gpio;
+AspeedGPIOState gpio[ASPEED_GPIOS_NUM];
} AspeedSoCState;
#define TYPE_ASPEED_SOC "aspeed-soc"
@@ -61,6 +62,7 @@ typedef struct AspeedSoCInfo {
uint64_t sram_size;
int spis_num;
int wdts_num;
+int gpios_num;
const int *irqmap;
const hwaddr *memmap;
uint32_t num_cpus;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 7d47647016..414b99c4f3 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -119,6 +119,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
.sram_size= 0x8000,
.spis_num = 1,
.wdts_num = 2,
+.gpios_num= 1,
.irqmap = aspeed_soc_ast2400_irqmap,
.memmap = aspeed_soc_ast2400_memmap,
.num_cpus = 1,
@@ -132,6 +133,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
.irqmap = aspeed_soc_ast2500_irqmap,
.memmap = aspeed_soc_ast2500_memmap,
.num_cpus = 1,
+.gpios_num= 1,
},
};
@@ -226,9 +228,15 @@ static void aspeed_soc_init(Object *obj)
sysbus_init_child_obj(obj, "xdma", OBJECT(>xdma), sizeof(s-
>xdma),
TYPE_ASPEED_XDMA);
-snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
-sysbus_init_child_obj(obj, "gpio", OBJECT(>gpio), sizeof(s-
>gpio),
- typename);
+for (i = 0; i < sc->info->gpios_num; i++) {
+if (ASPEED_IS_AST2600(sc->info->silicon_rev)) {
+snprintf(typename, sizeof(typename), "aspeed.gpio%d-%s",
i, socname);
+} else {
+snprintf(typename, sizeof(typename), "aspeed.gpio-%s",
socname);
+}
+sysbus_init_child_obj(obj, "gpio[*]", OBJECT(>gpio[i]),
sizeof(s->gpio[i]),
+ typename);
+}
}
static void aspeed_soc_realize(DeviceState *dev, Error **errp)
@@ -410,15 +418,19 @@ static void aspeed_soc_realize(DeviceState *dev,
Error **errp)
aspeed_soc_get_irq(s, ASPEED_XDMA));
/* GPIO */
-object_property_set_bool(OBJECT(>gpio), true, "realized",
);
-if (err) {
-error_propagate(errp, err);
-return;
+for (i = 0; i < sc->info->gpios_num; i++) {
+hwaddr addr = sc->info->memmap[ASPEED_GPIO] + i * 0x800;
+object_property_set_bool(OBJECT(>gpio[i]), true,
"realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>gpio[i]), 0, addr);
+sysbus_connect_irq(SYS_BUS_DEVICE(>gpio[i]), 0,
+ aspeed_soc_get_irq(s, ASPEED_GPIO));
}
-sysbus_mmio_map(SYS_BUS_DEVICE(>gpio), 0, sc->info-
>memmap[ASPEED_GPIO]);
-sysbus_connect_irq(SYS_BUS_DEVICE(>gpio), 0,
- aspeed_soc_get_irq(s, ASPEED_GPIO));
}
+
static Property aspeed_soc_properties[] = {
DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0),
DEFINE_PROP_END_OF_LIST(),
--
2.20.1
[Qemu-devel] [PATCH v5 2/3] aspeed: add a GPIO controller to the SoC
Signed-off-by: Rashmica Gupta
---
include/hw/arm/aspeed_soc.h | 3 +++
hw/arm/aspeed_soc.c | 17 +
2 files changed, 20 insertions(+)
diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index cef605ad6b..fa04abddd8 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -22,6 +22,7 @@
#include "hw/ssi/aspeed_smc.h"
#include "hw/watchdog/wdt_aspeed.h"
#include "hw/net/ftgmac100.h"
+#include "hw/gpio/aspeed_gpio.h"
#define ASPEED_SPIS_NUM 2
#define ASPEED_WDTS_NUM 3
@@ -47,6 +48,7 @@ typedef struct AspeedSoCState {
AspeedSDMCState sdmc;
AspeedWDTState wdt[ASPEED_WDTS_NUM];
FTGMAC100State ftgmac100[ASPEED_MACS_NUM];
+AspeedGPIOState gpio;
} AspeedSoCState;
#define TYPE_ASPEED_SOC "aspeed-soc"
@@ -60,6 +62,7 @@ typedef struct AspeedSoCInfo {
int spis_num;
const char *fmc_typename;
const char **spi_typename;
+const char *gpio_typename;
int wdts_num;
const int *irqmap;
const hwaddr *memmap;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index c6fb3700f2..ff422c8ad1 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -124,6 +124,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
.spis_num = 1,
.fmc_typename = "aspeed.smc.fmc",
.spi_typename = aspeed_soc_ast2400_typenames,
+.gpio_typename = "aspeed.gpio-ast2400",
.wdts_num = 2,
.irqmap = aspeed_soc_ast2400_irqmap,
.memmap = aspeed_soc_ast2400_memmap,
@@ -136,6 +137,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
.spis_num = 1,
.fmc_typename = "aspeed.smc.fmc",
.spi_typename = aspeed_soc_ast2400_typenames,
+.gpio_typename = "aspeed.gpio-ast2400",
.wdts_num = 2,
.irqmap = aspeed_soc_ast2400_irqmap,
.memmap = aspeed_soc_ast2400_memmap,
@@ -148,6 +150,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
.spis_num = 1,
.fmc_typename = "aspeed.smc.fmc",
.spi_typename = aspeed_soc_ast2400_typenames,
+.gpio_typename = "aspeed.gpio-ast2400",
.wdts_num = 2,
.irqmap = aspeed_soc_ast2400_irqmap,
.memmap = aspeed_soc_ast2400_memmap,
@@ -160,6 +163,7 @@ static const AspeedSoCInfo aspeed_socs[] = {
.spis_num = 2,
.fmc_typename = "aspeed.smc.ast2500-fmc",
.spi_typename = aspeed_soc_ast2500_typenames,
+.gpio_typename = "aspeed.gpio-ast2500",
.wdts_num = 3,
.irqmap = aspeed_soc_ast2500_irqmap,
.memmap = aspeed_soc_ast2500_memmap,
@@ -246,6 +250,9 @@ static void aspeed_soc_init(Object *obj)
sysbus_init_child_obj(obj, "xdma", OBJECT(>xdma), sizeof(s->xdma),
TYPE_ASPEED_XDMA);
+
+sysbus_init_child_obj(obj, "gpio", OBJECT(>gpio), sizeof(s->gpio),
+ sc->info->gpio_typename);
}
static void aspeed_soc_realize(DeviceState *dev, Error **errp)
@@ -425,6 +432,16 @@ static void aspeed_soc_realize(DeviceState *dev, Error
**errp)
sc->info->memmap[ASPEED_XDMA]);
sysbus_connect_irq(SYS_BUS_DEVICE(>xdma), 0,
aspeed_soc_get_irq(s, ASPEED_XDMA));
+
+/* GPIO */
+object_property_set_bool(OBJECT(>gpio), true, "realized", );
+if (err) {
+error_propagate(errp, err);
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>gpio), 0,
sc->info->memmap[ASPEED_GPIO]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>gpio), 0,
+ aspeed_soc_get_irq(s, ASPEED_GPIO));
}
static Property aspeed_soc_properties[] = {
DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0),
--
2.20.1
