Re: [Qemu-arm] [Qemu-devel] [PATCH v3 4/7] bcm2835_peripherals: add rollup device for bcm2835 peripherals

[Alistair Francis]

On Thu, Dec 31, 2015 at 4:31 PM, Andrew Baumann
<address@hidden> wrote:
> This device maintains all the non-CPU peripherals on bcm2835 (Pi1)
> which are also present on bcm2836 (Pi2). It also implements the
> private address spaces used for DMA and mailboxes.
>
> Signed-off-by: Andrew Baumann <address@hidden>
> ---
>
> Notes:
>     v3:
>      * clean up raspi_platform.h
>      * s/_/-/ in type/property/child names
>      * use memory_region_init where appropriate rather than 
> memory_region_init_io
>      * pass ram as link property
>
>     v2 changes:
>      * adapted to use common SDHCI emulation
>
>  hw/arm/Makefile.objs                 |   1 +
>  hw/arm/bcm2835_peripherals.c         | 205 
> +++++++++++++++++++++++++++++++++++
>  include/hw/arm/bcm2835_peripherals.h |  42 +++++++
>  include/hw/arm/raspi_platform.h      | 128 ++++++++++++++++++++++
>  4 files changed, 376 insertions(+)
>  create mode 100644 hw/arm/bcm2835_peripherals.c
>  create mode 100644 include/hw/arm/bcm2835_peripherals.h
>  create mode 100644 include/hw/arm/raspi_platform.h
>
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 2195b60..82cc142 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -11,6 +11,7 @@ obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o 
> pxa2xx_pic.o
>  obj-$(CONFIG_DIGIC) += digic.o
>  obj-y += omap1.o omap2.o strongarm.o
>  obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
> +obj-$(CONFIG_RASPI) += bcm2835_peripherals.o
>  obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
>  obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp.o xlnx-ep108.o
>  obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> new file mode 100644
> index 0000000..879a41d
> --- /dev/null
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -0,0 +1,205 @@
> +/*
> + * Raspberry Pi emulation (c) 2012 Gregory Estrade
> + * Upstreaming code cleanup [including bcm2835_*] (c) 2013 Jan Petrous
> + *
> + * Rasperry Pi 2 emulation and refactoring Copyright (c) 2015, Microsoft
> + * Written by Andrew Baumann
> + *
> + * This code is licensed under the GNU GPLv2 and later.
> + */
> +
> +#include "hw/arm/bcm2835_peripherals.h"
> +#include "hw/misc/bcm2835_mbox_defs.h"
> +#include "hw/arm/raspi_platform.h"
> +
> +/* Peripheral base address on the VC (GPU) system bus */
> +#define BCM2835_VC_PERI_BASE 0x7e000000
> +
> +/* Capabilities for SD controller: no DMA, high-speed, default clocks etc. */
> +#define BCM2835_SDHC_CAPAREG 0x52034b4
> +
> +static void bcm2835_peripherals_init(Object *obj)
> +{
> +    BCM2835PeripheralState *s = BCM2835_PERIPHERALS(obj);
> +
> +    /* Memory region for peripheral devices, which we export to our parent */
> +    memory_region_init_io(&s->peri_mr, obj, NULL, s, "bcm2835-peripherals",
> +                          0x1000000);
> +    object_property_add_child(obj, "peripheral-io", OBJECT(&s->peri_mr), 
> NULL);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->peri_mr);
> +
> +    /* Internal memory region for peripheral bus addresses (not exported) */
> +    memory_region_init(&s->gpu_bus_mr, obj, "bcm2835-gpu", (uint64_t)1 << 
> 32);
> +    object_property_add_child(obj, "gpu-bus", OBJECT(&s->gpu_bus_mr), NULL);
> +
> +    /* Internal memory region for request/response communication with
> +     * mailbox-addressable peripherals (not exported)
> +     */
> +    memory_region_init(&s->mbox_mr, obj, "bcm2835-mbox",
> +                       MBOX_CHAN_COUNT << MBOX_AS_CHAN_SHIFT);
> +
> +    /* Interrupt Controller */
> +    object_initialize(&s->ic, sizeof(s->ic), TYPE_BCM2835_IC);
> +    object_property_add_child(obj, "ic", OBJECT(&s->ic), NULL);
> +    qdev_set_parent_bus(DEVICE(&s->ic), sysbus_get_default());
> +
> +    /* UART0 */
> +    s->uart0 = SYS_BUS_DEVICE(object_new("pl011"));
> +    object_property_add_child(obj, "uart0", OBJECT(s->uart0), NULL);
> +    qdev_set_parent_bus(DEVICE(s->uart0), sysbus_get_default());
> +
> +    /* Mailboxes */
> +    object_initialize(&s->mboxes, sizeof(s->mboxes), TYPE_BCM2835_MBOX);
> +    object_property_add_child(obj, "mbox", OBJECT(&s->mboxes), NULL);
> +    qdev_set_parent_bus(DEVICE(&s->mboxes), sysbus_get_default());
> +
> +    object_property_add_const_link(OBJECT(&s->mboxes), "mbox-mr",
> +                                   OBJECT(&s->mbox_mr), &error_abort);
> +
> +    /* Property channel */
> +    object_initialize(&s->property, sizeof(s->property), 
> TYPE_BCM2835_PROPERTY);
> +    object_property_add_child(obj, "property", OBJECT(&s->property), NULL);
> +    qdev_set_parent_bus(DEVICE(&s->property), sysbus_get_default());
> +
> +    object_property_add_const_link(OBJECT(&s->property), "dma-mr",
> +                                   OBJECT(&s->gpu_bus_mr), &error_abort);
> +
> +    /* Extended Mass Media Controller */
> +    object_initialize(&s->sdhci, sizeof(s->sdhci), TYPE_SYSBUS_SDHCI);
> +    object_property_add_child(obj, "sdhci", OBJECT(&s->sdhci), NULL);
> +    qdev_set_parent_bus(DEVICE(&s->sdhci), sysbus_get_default());
> +}
> +
> +static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
> +{
> +    BCM2835PeripheralState *s = BCM2835_PERIPHERALS(dev);
> +    Object *obj;
> +    MemoryRegion *ram;
> +    Error *err = NULL;
> +    uint32_t ram_size;
> +    int n;
> +
> +    obj = object_property_get_link(OBJECT(dev), "ram", &err);
> +    if (obj == NULL) {
> +        error_setg(errp, "%s: required ram link not found: %s",
> +                   __func__, error_get_pretty(err));
> +        return;
> +    }

I only had a quick read of this patch, but this RAM linking looks fine
to me. Out of curiosity is there a reason you use
object_property_get_link() instead of object_property_add_link() in
the init?

Thanks,

Alistair