qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-arm] [Qemu-devel] [PATCH v2 1/3] aspeed: add a per SoC mapping for the interrupt space
Date: Mon, 6 May 2019 16:47:54 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1

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 <address@hidden>
> ---
> 
>  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é <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]