Re: [PATCH v3 05/16] hw/arm/bcm2836: Make the SoC code modular

[Alistair Francis]

On Sat, Oct 19, 2019 at 4:50 PM Philippe Mathieu-Daudé <address@hidden> wrote:
>
> This file creates the BCM2836/BCM2837 blocks.
> The biggest differences with the BCM2838 we are going to add, are
> the base addresses of the interrupt controller and the peripherals.
> Add these addresses in the BCM283XInfo structure to make this
> block more modular. Remove the MCORE_OFFSET offset as it is
> not useful and rather confusing.
>
> Reviewed-by: Esteban Bosse <address@hidden>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>

Reviewed-by: Alistair Francis <address@hidden>

Alistair

> ---
>  hw/arm/bcm2836.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
> index 723aef6bf5..019e67b906 100644
> --- a/hw/arm/bcm2836.c
> +++ b/hw/arm/bcm2836.c
> @@ -16,15 +16,11 @@
>  #include "hw/arm/raspi_platform.h"
>  #include "hw/sysbus.h"
>
> -/* Peripheral base address seen by the CPU */
> -#define BCM2836_PERI_BASE       0x3F000000
> -
> -/* "QA7" (Pi2) interrupt controller and mailboxes etc. */
> -#define BCM2836_CONTROL_BASE    0x40000000
> -
>  struct BCM283XInfo {
>      const char *name;
>      const char *cpu_type;
> +    hwaddr peri_base; /* Peripheral base address seen by the CPU */
> +    hwaddr ctrl_base; /* Interrupt controller and mailboxes etc. */
>      int clusterid;
>  };
>
> @@ -32,12 +28,16 @@ static const BCM283XInfo bcm283x_socs[] = {
>      {
>          .name = TYPE_BCM2836,
>          .cpu_type = ARM_CPU_TYPE_NAME("cortex-a7"),
> +        .peri_base = 0x3f000000,
> +        .ctrl_base = 0x40000000,
>          .clusterid = 0xf,
>      },
>  #ifdef TARGET_AARCH64
>      {
>          .name = TYPE_BCM2837,
>          .cpu_type = ARM_CPU_TYPE_NAME("cortex-a53"),
> +        .peri_base = 0x3f000000,
> +        .ctrl_base = 0x40000000,
>          .clusterid = 0x0,
>      },
>  #endif
> @@ -104,7 +104,7 @@ static void bcm2836_realize(DeviceState *dev, Error 
> **errp)
>      }
>
>      sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->peripherals), 0,
> -                            BCM2836_PERI_BASE, 1);
> +                            info->peri_base, 1);
>
>      /* bcm2836 interrupt controller (and mailboxes, etc.) */
>      object_property_set_bool(OBJECT(&s->control), true, "realized", &err);
> @@ -113,7 +113,7 @@ static void bcm2836_realize(DeviceState *dev, Error 
> **errp)
>          return;
>      }
>
> -    sysbus_mmio_map(SYS_BUS_DEVICE(&s->control), 0, BCM2836_CONTROL_BASE);
> +    sysbus_mmio_map(SYS_BUS_DEVICE(&s->control), 0, info->ctrl_base);
>
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->peripherals), 0,
>          qdev_get_gpio_in_named(DEVICE(&s->control), "gpu-irq", 0));
> @@ -126,7 +126,7 @@ static void bcm2836_realize(DeviceState *dev, Error 
> **errp)
>
>          /* set periphbase/CBAR value for CPU-local registers */
>          object_property_set_int(OBJECT(&s->cpus[n]),
> -                                BCM2836_PERI_BASE + MSYNC_OFFSET,
> +                                info->peri_base,
>                                  "reset-cbar", &err);
>          if (err) {
>              error_propagate(errp, err);
> --
> 2.21.0
>
>