diff mbox series

[v5,2/3] aspeed: add a GPIO controller to the SoC

Message ID 20190816073229.22787-3-rashmica.g@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add Aspeed GPIO controller model | expand

Commit Message

Rashmica Gupta Aug. 16, 2019, 7:32 a.m. UTC 
Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
---
 include/hw/arm/aspeed_soc.h |  3 +++
 hw/arm/aspeed_soc.c         | 17 +++++++++++++++++
 2 files changed, 20 insertions(+)

Comments

Rashmica Gupta Aug. 16, 2019, 7:40 a.m. UTC | #1 
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 <rashmica.g@gmail.com>
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(&s->xdma), sizeof(s-
>xdma),
                           TYPE_ASPEED_XDMA);
 
-    snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
-    sysbus_init_child_obj(obj, "gpio", OBJECT(&s->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(&s->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(&s->gpio), true, "realized",
&err);
-    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(&s->gpio[i]), true,
"realized", &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0, addr);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio[i]), 0,
+                           aspeed_soc_get_irq(s, ASPEED_GPIO));
     }
-    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, sc->info-
>memmap[ASPEED_GPIO]);
-    sysbus_connect_irq(SYS_BUS_DEVICE(&s->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(),
Cédric Le Goater Aug. 16, 2019, 4 p.m. UTC | #2 
On 16/08/2019 09:32, Rashmica Gupta wrote:
> Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>



Reviewed-by: Cédric Le Goater <clg@kaod.org>

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(&s->xdma), sizeof(s->xdma),
>                            TYPE_ASPEED_XDMA);
> +
> +    sysbus_init_child_obj(obj, "gpio", OBJECT(&s->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(&s->xdma), 0,
>                         aspeed_soc_get_irq(s, ASPEED_XDMA));
> +
> +    /* GPIO */
> +    object_property_set_bool(OBJECT(&s->gpio), true, "realized", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, sc->info->memmap[ASPEED_GPIO]);
> +    sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
> +                       aspeed_soc_get_irq(s, ASPEED_GPIO));
>  }
>  static Property aspeed_soc_properties[] = {
>      DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0),
>
Cédric Le Goater Aug. 16, 2019, 4:07 p.m. UTC | #3 
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 <rashmica.g@gmail.com>
> 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(&s->xdma), sizeof(s-
>> xdma),
>                            TYPE_ASPEED_XDMA);
>  
> -    snprintf(typename, sizeof(typename), "aspeed.gpio-%s", socname);
> -    sysbus_init_child_obj(obj, "gpio", OBJECT(&s->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(&s->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(&s->gpio), true, "realized",
> &err);
> -    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(&s->gpio[i]), true,
> "realized", &err);
> +        if (err) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +        sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0, addr);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio[i]), 0,
> +                           aspeed_soc_get_irq(s, ASPEED_GPIO));

The interrupt is different.

C. 

>      }
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, sc->info-
>> memmap[ASPEED_GPIO]);
> -    sysbus_connect_irq(SYS_BUS_DEVICE(&s->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(),
>
diff mbox series

Patch

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(&s->xdma), sizeof(s->xdma),
                           TYPE_ASPEED_XDMA);
+
+    sysbus_init_child_obj(obj, "gpio", OBJECT(&s->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(&s->xdma), 0,
                        aspeed_soc_get_irq(s, ASPEED_XDMA));
+
+    /* GPIO */
+    object_property_set_bool(OBJECT(&s->gpio), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, sc->info->memmap[ASPEED_GPIO]);
+    sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpio), 0,
+                       aspeed_soc_get_irq(s, ASPEED_GPIO));
 }
 static Property aspeed_soc_properties[] = {
     DEFINE_PROP_UINT32("num-cpus", AspeedSoCState, num_cpus, 0),