From: Niek Linnenbank <nieklinnenbank@gmail.com> To: "Philippe Mathieu-Daudé" <philmd@redhat.com> Cc: b.galvani@gmail.com, Peter Maydell <peter.maydell@linaro.org>, qemu-arm@nongnu.org, qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com> Subject: Re: [PATCH 01/10] hw: arm: add Allwinner H3 System-on-Chip Date: Wed, 4 Dec 2019 21:44:00 +0100 Message-ID: <CAPan3WpeYtA249jp2iF-baFXh2YihEYMiGVtov7DLpsOVb5ZhQ@mail.gmail.com> (raw) In-Reply-To: <5d3961ca-8586-6d93-2525-fd2e29b233e1@redhat.com> [-- Attachment #1: Type: text/plain, Size: 19559 bytes --] Hello Philippe, On Wed, Dec 4, 2019 at 5:53 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > Hi Niek, > > On 12/2/19 10:09 PM, Niek Linnenbank wrote: > > The Allwinner H3 is a System on Chip containing four ARM Cortex A7 > > processor cores. Features and specifications include DDR2/DDR3 memory, > > SD/MMC storage cards, 10/100/1000Mbit ethernet, USB 2.0, HDMI and > > various I/O modules. This commit adds support for the Allwinner H3 > > System on Chip. > > > > Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com> > > --- > > MAINTAINERS | 7 ++ > > default-configs/arm-softmmu.mak | 1 + > > hw/arm/Kconfig | 8 ++ > > hw/arm/Makefile.objs | 1 + > > hw/arm/allwinner-h3.c | 215 ++++++++++++++++++++++++++++++++ > > include/hw/arm/allwinner-h3.h | 118 ++++++++++++++++++ > > 6 files changed, 350 insertions(+) > > create mode 100644 hw/arm/allwinner-h3.c > > create mode 100644 include/hw/arm/allwinner-h3.h > > Since your series changes various files, can you have a look at the > scripts/git.orderfile file and setup it for your QEMU contributions? > OK, done! I didn't know such a script existed, thanks. I ran this command in my local repository: $ git config diff.orderFile scripts/git.orderfile It seems to work, when I re-generate the patches, the order of the diff is different. > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 5e5e3e52d6..29c9936037 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -479,6 +479,13 @@ F: hw/*/allwinner* > > F: include/hw/*/allwinner* > > F: hw/arm/cubieboard.c > > > > +Allwinner-h3 > > +M: Niek Linnenbank <nieklinnenbank@gmail.com> > > +L: qemu-arm@nongnu.org > > +S: Maintained > > +F: hw/*/allwinner-h3* > > +F: include/hw/*/allwinner-h3* > > + > > ARM PrimeCell and CMSDK devices > > M: Peter Maydell <peter.maydell@linaro.org> > > L: qemu-arm@nongnu.org > > diff --git a/default-configs/arm-softmmu.mak > b/default-configs/arm-softmmu.mak > > index 1f2e0e7fde..d75a239c2c 100644 > > --- a/default-configs/arm-softmmu.mak > > +++ b/default-configs/arm-softmmu.mak > > @@ -40,3 +40,4 @@ CONFIG_FSL_IMX25=y > > CONFIG_FSL_IMX7=y > > CONFIG_FSL_IMX6UL=y > > CONFIG_SEMIHOSTING=y > > +CONFIG_ALLWINNER_H3=y > > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > > index c6e7782580..ebf8d2325f 100644 > > --- a/hw/arm/Kconfig > > +++ b/hw/arm/Kconfig > > @@ -291,6 +291,14 @@ config ALLWINNER_A10 > > select SERIAL > > select UNIMP > > > > +config ALLWINNER_H3 > > + bool > > + select ALLWINNER_A10_PIT > > + select SERIAL > > + select ARM_TIMER > > + select ARM_GIC > > + select UNIMP > > + > > config RASPI > > bool > > select FRAMEBUFFER > > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs > > index fe749f65fd..956e496052 100644 > > --- a/hw/arm/Makefile.objs > > +++ b/hw/arm/Makefile.objs > > @@ -34,6 +34,7 @@ obj-$(CONFIG_DIGIC) += digic.o > > obj-$(CONFIG_OMAP) += omap1.o omap2.o > > obj-$(CONFIG_STRONGARM) += strongarm.o > > obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o > > +obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3.o > > obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o > > obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o > > obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o > > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c > > new file mode 100644 > > index 0000000000..470fdfebef > > --- /dev/null > > +++ b/hw/arm/allwinner-h3.c > > @@ -0,0 +1,215 @@ > > +/* > > + * Allwinner H3 System on Chip emulation > > + * > > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com> > > + * > > + * This program is free software: you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation, either version 2 of the License, or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program. If not, see <http://www.gnu.org/licenses/ > >. > > + */ > > + > > +#include "qemu/osdep.h" > > +#include "exec/address-spaces.h" > > +#include "qapi/error.h" > > +#include "qemu/module.h" > > +#include "qemu/units.h" > > +#include "cpu.h" > > +#include "hw/sysbus.h" > > +#include "hw/arm/allwinner-h3.h" > > +#include "hw/misc/unimp.h" > > +#include "sysemu/sysemu.h" > > + > > +static void aw_h3_init(Object *obj) > > +{ > > + AwH3State *s = AW_H3(obj); > > + > > + sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic), > > + TYPE_ARM_GIC); > > + > > + sysbus_init_child_obj(obj, "timer", &s->timer, sizeof(s->timer), > > + TYPE_AW_A10_PIT); > > +} > > + > > +static void aw_h3_realize(DeviceState *dev, Error **errp) > > +{ > > + AwH3State *s = AW_H3(dev); > > + SysBusDevice *sysbusdev = NULL; > > + Error *err = NULL; > > + unsigned i = 0; > > + > > + /* CPUs */ > > + for (i = 0; i < AW_H3_NUM_CPUS; i++) { > > In https://www.mail-archive.com/qemu-devel@nongnu.org/msg662942.html > Markus noted some incorrect pattern, and apparently you inherited it. > You should initialize 'err' in the loop. > > > + Object *cpuobj = object_new(ARM_CPU_TYPE_NAME("cortex-a7")); > > + CPUState *cpustate = CPU(cpuobj); > > We loose access to the CPUs. Can you use an array of AW_H3_NUM_CPUS cpus > in AwH3State? > > > + > > + /* Set the proper CPU index */ > > + cpustate->cpu_index = i; > > + > > + /* Provide Power State Coordination Interface */ > > + object_property_set_int(cpuobj, QEMU_PSCI_CONDUIT_HVC, > > + "psci-conduit", &error_abort); > > Here you use the error_abort shortcut. > > > + > > + /* Disable secondary CPUs */ > > + object_property_set_bool(cpuobj, i > 0, "start-powered-off", > &err); > > + if (err != NULL) { > > + error_propagate(errp, err); > > + return; > > Here you return. > > > + } > > + > > + /* All exception levels required */ > > + object_property_set_bool(cpuobj, > > + true, "has_el3", NULL); > > + object_property_set_bool(cpuobj, > > + true, "has_el2", NULL); > > Here you don't use error. > > Cc'ing Markus who is the expert, since he might have better suggestions. > > This function is called before the machine starts, and we are not > handling with user-provided configurations, so I'd say using > &error_abort in all places is OK. > > > + > > + /* Mark realized */ > > + object_property_set_bool(cpuobj, true, "realized", &err); > > + if (err != NULL) { > > + error_propagate(errp, err); > > + return; > > + } > > + object_unref(cpuobj); > > + } > > + > > + /* Generic Interrupt Controller */ > > + qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", AW_H3_GIC_NUM_SPI + > > + GIC_INTERNAL); > > + qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2); > > + qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", AW_H3_NUM_CPUS); > > + qdev_prop_set_bit(DEVICE(&s->gic), "has-security-extensions", > false); > > + qdev_prop_set_bit(DEVICE(&s->gic), "has-virtualization-extensions", > true); > > + > > + object_property_set_bool(OBJECT(&s->gic), true, "realized", &err); > > Why change API? Can we use qdev_init_nofail() instead? > > > + if (err) { > > + error_propagate(errp, err); > > + return; > > + } > > + > > + sysbusdev = SYS_BUS_DEVICE(&s->gic); > > + sysbus_mmio_map(sysbusdev, 0, AW_H3_GIC_DIST_BASE); > > + sysbus_mmio_map(sysbusdev, 1, AW_H3_GIC_CPU_BASE); > > + sysbus_mmio_map(sysbusdev, 2, AW_H3_GIC_HYP_BASE); > > + sysbus_mmio_map(sysbusdev, 3, AW_H3_GIC_VCPU_BASE); > > + > > + /* > > + * Wire the outputs from each CPU's generic timer and the GICv3 > > + * maintenance interrupt signal to the appropriate GIC PPI inputs, > > + * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's > inputs. > > + */ > > + for (i = 0; i < AW_H3_NUM_CPUS; i++) { > > + DeviceState *cpudev = DEVICE(qemu_get_cpu(i)); > > + int ppibase = AW_H3_GIC_NUM_SPI + i * GIC_INTERNAL + > GIC_NR_SGIS; > > + int irq; > > + /* > > + * Mapping from the output timer irq lines from the CPU to the > > + * GIC PPI inputs used for this board. > > + */ > > + const int timer_irq[] = { > > + [GTIMER_PHYS] = AW_H3_GIC_PPI_ARM_PHYSTIMER, > > + [GTIMER_VIRT] = AW_H3_GIC_PPI_ARM_VIRTTIMER, > > + [GTIMER_HYP] = AW_H3_GIC_PPI_ARM_HYPTIMER, > > + [GTIMER_SEC] = AW_H3_GIC_PPI_ARM_SECTIMER, > > + }; > > + > > + /* Connect CPU timer outputs to GIC PPI inputs */ > > + for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) { > > + qdev_connect_gpio_out(cpudev, irq, > > + qdev_get_gpio_in(DEVICE(&s->gic), > > + ppibase + > timer_irq[irq])); > > + } > > + > > + /* Connect GIC outputs to CPU interrupt inputs */ > > + sysbus_connect_irq(sysbusdev, i, > > + qdev_get_gpio_in(cpudev, ARM_CPU_IRQ)); > > + sysbus_connect_irq(sysbusdev, i + AW_H3_NUM_CPUS, > > + qdev_get_gpio_in(cpudev, ARM_CPU_FIQ)); > > + sysbus_connect_irq(sysbusdev, i + (2 * AW_H3_NUM_CPUS), > > + qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ)); > > + sysbus_connect_irq(sysbusdev, i + (3 * AW_H3_NUM_CPUS), > > + qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ)); > > + > > + /* GIC maintenance signal */ > > + sysbus_connect_irq(sysbusdev, i + (4 * AW_H3_NUM_CPUS), > > + qdev_get_gpio_in(DEVICE(&s->gic), > > + ppibase + > AW_H3_GIC_PPI_MAINT)); > > + } > > + > > + for (i = 0; i < AW_H3_GIC_NUM_SPI; i++) { > > + s->irq[i] = qdev_get_gpio_in(DEVICE(&s->gic), i); > > Apparently we don't need the irq array in AwH3State, because ... > > > + } > > + > > + /* Timer */ > > + object_property_set_bool(OBJECT(&s->timer), true, "realized", &err); > > + if (err != NULL) { > > + error_propagate(errp, err); > > + return; > > + } > > + sysbusdev = SYS_BUS_DEVICE(&s->timer); > > + sysbus_mmio_map(sysbusdev, 0, AW_H3_PIT_REG_BASE); > > + sysbus_connect_irq(sysbusdev, 0, s->irq[AW_H3_GIC_SPI_TIMER0]); > > + sysbus_connect_irq(sysbusdev, 1, s->irq[AW_H3_GIC_SPI_TIMER1]); > > ... we can call qdev_get_gpio_in() here directly. > > > + > > + /* SRAM */ > > + memory_region_init_ram(&s->sram_a1, OBJECT(dev), "sram A1", > > + AW_H3_SRAM_A1_SIZE, &error_fatal); > > + memory_region_init_ram(&s->sram_a2, OBJECT(dev), "sram A2", > > + AW_H3_SRAM_A2_SIZE, &error_fatal); > > + memory_region_init_ram(&s->sram_c, OBJECT(dev), "sram C", > > + AW_H3_SRAM_C_SIZE, &error_fatal); > > + memory_region_add_subregion(get_system_memory(), AW_H3_SRAM_A1_BASE, > > + &s->sram_a1); > > + memory_region_add_subregion(get_system_memory(), AW_H3_SRAM_A2_BASE, > > + &s->sram_a2); > > + memory_region_add_subregion(get_system_memory(), AW_H3_SRAM_C_BASE, > > + &s->sram_c); > > + > > + /* UART */ > > + if (serial_hd(0)) { > > + serial_mm_init(get_system_memory(), AW_H3_UART0_REG_BASE, 2, > > + s->irq[AW_H3_GIC_SPI_UART0], 115200, > serial_hd(0), > > qdev_get_gpio_in() here too. > > > + DEVICE_NATIVE_ENDIAN); > > + } > > + > > + /* Unimplemented devices */ > > + create_unimplemented_device("display-engine", AW_H3_DE_BASE, > AW_H3_DE_SIZE); > > + create_unimplemented_device("dma", AW_H3_DMA_BASE, AW_H3_DMA_SIZE); > > + create_unimplemented_device("lcd0", AW_H3_LCD0_BASE, > AW_H3_LCD0_SIZE); > > + create_unimplemented_device("lcd1", AW_H3_LCD1_BASE, > AW_H3_LCD1_SIZE); > > + create_unimplemented_device("gpu", AW_H3_GPU_BASE, AW_H3_GPU_SIZE); > > + create_unimplemented_device("hdmi", AW_H3_HDMI_BASE, > AW_H3_HDMI_SIZE); > > + create_unimplemented_device("rtc", AW_H3_RTC_BASE, AW_H3_RTC_SIZE); > > + create_unimplemented_device("audio-codec", AW_H3_AC_BASE, > AW_H3_AC_SIZE); > > +} > > + > > +static void aw_h3_class_init(ObjectClass *oc, void *data) > > +{ > > + DeviceClass *dc = DEVICE_CLASS(oc); > > + > > + dc->realize = aw_h3_realize; > > + /* Reason: uses serial_hds and nd_table */ > > + dc->user_creatable = false; > > +} > > + > > +static const TypeInfo aw_h3_type_info = { > > + .name = TYPE_AW_H3, > > + .parent = TYPE_DEVICE, > > + .instance_size = sizeof(AwH3State), > > + .instance_init = aw_h3_init, > > + .class_init = aw_h3_class_init, > > +}; > > + > > +static void aw_h3_register_types(void) > > +{ > > + type_register_static(&aw_h3_type_info); > > +} > > + > > +type_init(aw_h3_register_types) > > diff --git a/include/hw/arm/allwinner-h3.h > b/include/hw/arm/allwinner-h3.h > > new file mode 100644 > > index 0000000000..af368c2254 > > --- /dev/null > > +++ b/include/hw/arm/allwinner-h3.h > > @@ -0,0 +1,118 @@ > > +/* > > + * Allwinner H3 System on Chip emulation > > + * > > + * Copyright (C) 2019 Niek Linnenbank <nieklinnenbank@gmail.com> > > + * > > + * This program is free software: you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation, either version 2 of the License, or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program. If not, see <http://www.gnu.org/licenses/ > >. > > + */ > > + > > +#ifndef HW_ARM_ALLWINNER_H3_H > > +#define HW_ARM_ALLWINNER_H3_H > > + > > +#include "qemu/error-report.h" > > +#include "qemu/units.h" > > +#include "hw/char/serial.h" > > +#include "hw/arm/boot.h" > > +#include "hw/timer/allwinner-a10-pit.h" > > +#include "hw/intc/arm_gic.h" > > +#include "target/arm/cpu.h" > > + > > +#define AW_H3_SRAM_A1_BASE (0x00000000) > > +#define AW_H3_SRAM_A2_BASE (0x00044000) > > +#define AW_H3_SRAM_C_BASE (0x00010000) > > +#define AW_H3_DE_BASE (0x01000000) > > +#define AW_H3_SYSCON_BASE (0x01c00000) > > +#define AW_H3_DMA_BASE (0x01c02000) > > +#define AW_H3_LCD0_BASE (0x01c0c000) > > +#define AW_H3_LCD1_BASE (0x01c0d000) > > +#define AW_H3_SID_BASE (0x01c14000) > > +#define AW_H3_CCU_BASE (0x01c20000) > > +#define AW_H3_PIC_REG_BASE (0x01c20400) > > +#define AW_H3_PIT_REG_BASE (0x01c20c00) > > +#define AW_H3_AC_BASE (0x01c22c00) > > +#define AW_H3_UART0_REG_BASE (0x01c28000) > > +#define AW_H3_EMAC_BASE (0x01c30000) > > +#define AW_H3_MMC0_BASE (0x01c0f000) > > +#define AW_H3_EHCI0_BASE (0x01c1a000) > > +#define AW_H3_OHCI0_BASE (0x01c1a400) > > +#define AW_H3_EHCI1_BASE (0x01c1b000) > > +#define AW_H3_OHCI1_BASE (0x01c1b400) > > +#define AW_H3_EHCI2_BASE (0x01c1c000) > > +#define AW_H3_OHCI2_BASE (0x01c1c400) > > +#define AW_H3_EHCI3_BASE (0x01c1d000) > > +#define AW_H3_OHCI3_BASE (0x01c1d400) > > +#define AW_H3_GPU_BASE (0x01c40000) > > +#define AW_H3_GIC_DIST_BASE (0x01c81000) > > +#define AW_H3_GIC_CPU_BASE (0x01c82000) > > +#define AW_H3_GIC_HYP_BASE (0x01c84000) > > +#define AW_H3_GIC_VCPU_BASE (0x01c86000) > > +#define AW_H3_HDMI_BASE (0x01ee0000) > > +#define AW_H3_RTC_BASE (0x01f00000) > > +#define AW_H3_CPUCFG_BASE (0x01f01c00) > > +#define AW_H3_SDRAM_BASE (0x40000000) > > + > > +#define AW_H3_SRAM_A1_SIZE (64 * KiB) > > +#define AW_H3_SRAM_A2_SIZE (32 * KiB) > > +#define AW_H3_SRAM_C_SIZE (44 * KiB) > > +#define AW_H3_DE_SIZE (4 * MiB) > > +#define AW_H3_DMA_SIZE (4 * KiB) > > +#define AW_H3_LCD0_SIZE (4 * KiB) > > +#define AW_H3_LCD1_SIZE (4 * KiB) > > +#define AW_H3_GPU_SIZE (64 * KiB) > > +#define AW_H3_HDMI_SIZE (128 * KiB) > > +#define AW_H3_RTC_SIZE (1 * KiB) > > +#define AW_H3_AC_SIZE (2 * KiB) > > + > > +#define AW_H3_GIC_PPI_MAINT (9) > > +#define AW_H3_GIC_PPI_ARM_HYPTIMER (10) > > +#define AW_H3_GIC_PPI_ARM_VIRTTIMER (11) > > +#define AW_H3_GIC_PPI_ARM_SECTIMER (13) > > +#define AW_H3_GIC_PPI_ARM_PHYSTIMER (14) > > + > > +#define AW_H3_GIC_SPI_UART0 (0) > > +#define AW_H3_GIC_SPI_TIMER0 (18) > > +#define AW_H3_GIC_SPI_TIMER1 (19) > > +#define AW_H3_GIC_SPI_MMC0 (60) > > +#define AW_H3_GIC_SPI_MMC1 (61) > > +#define AW_H3_GIC_SPI_MMC2 (62) > > +#define AW_H3_GIC_SPI_EHCI0 (72) > > +#define AW_H3_GIC_SPI_OHCI0 (73) > > +#define AW_H3_GIC_SPI_EHCI1 (74) > > +#define AW_H3_GIC_SPI_OHCI1 (75) > > +#define AW_H3_GIC_SPI_EHCI2 (76) > > +#define AW_H3_GIC_SPI_OHCI2 (77) > > +#define AW_H3_GIC_SPI_EHCI3 (78) > > +#define AW_H3_GIC_SPI_OHCI3 (79) > > +#define AW_H3_GIC_SPI_EMAC (82) > > I'd move half of the previous definitions into allwinner-h3.c, since > they are only used there. > > Indeed, you are right, I'll move them. Also, I'd use an enum for the PPI/SPI. > Thanks, I will process all of your comments above for the next patch version. > > > + > > +#define AW_H3_GIC_NUM_SPI (128) > > +#define AW_H3_NUM_CPUS (4) > > + > > +#define TYPE_AW_H3 "allwinner-h3" > > +#define AW_H3(obj) OBJECT_CHECK(AwH3State, (obj), TYPE_AW_H3) > > + > > +typedef struct AwH3State { > > + /*< private >*/ > > + DeviceState parent_obj; > > + /*< public >*/ > > + > > + qemu_irq irq[AW_H3_GIC_NUM_SPI]; > > + AwA10PITState timer; > > + GICState gic; > > + MemoryRegion sram_a1; > > + MemoryRegion sram_a2; > > + MemoryRegion sram_c; > > +} AwH3State; > > + > > +#endif > > > > Nice clean patch, for a first contribution :) > Thank you Philippe! Regards, Niek -- Niek Linnenbank [-- Attachment #2: Type: text/html, Size: 25602 bytes --] <div dir="ltr"><div>Hello Philippe,<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Dec 4, 2019 at 5:53 PM Philippe Mathieu-Daudé <<a href="mailto:philmd@redhat.com">philmd@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Niek,<br> <br> On 12/2/19 10:09 PM, Niek Linnenbank wrote:<br> > The Allwinner H3 is a System on Chip containing four ARM Cortex A7<br> > processor cores. Features and specifications include DDR2/DDR3 memory,<br> > SD/MMC storage cards, 10/100/1000Mbit ethernet, USB 2.0, HDMI and<br> > various I/O modules. This commit adds support for the Allwinner H3<br> > System on Chip.<br> > <br> > Signed-off-by: Niek Linnenbank <<a href="mailto:nieklinnenbank@gmail.com" target="_blank">nieklinnenbank@gmail.com</a>><br> > ---<br> > MAINTAINERS | 7 ++<br> > default-configs/arm-softmmu.mak | 1 +<br> > hw/arm/Kconfig | 8 ++<br> > hw/arm/Makefile.objs | 1 +<br> > hw/arm/allwinner-h3.c | 215 ++++++++++++++++++++++++++++++++<br> > include/hw/arm/allwinner-h3.h | 118 ++++++++++++++++++<br> > 6 files changed, 350 insertions(+)<br> > create mode 100644 hw/arm/allwinner-h3.c<br> > create mode 100644 include/hw/arm/allwinner-h3.h<br> <br> Since your series changes various files, can you have a look at the <br> scripts/git.orderfile file and setup it for your QEMU contributions?<br></blockquote><div><br></div><div>OK, done! I didn't know such a script existed, thanks.</div><div>I ran this command in my local repository:</div><div> $ git config diff.orderFile scripts/git.orderfile<br></div><div>It seems to work, when I re-generate the patches, the order of the diff is different.<br></div><div> </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br> > <br> > diff --git a/MAINTAINERS b/MAINTAINERS<br> > index 5e5e3e52d6..29c9936037 100644<br> > --- a/MAINTAINERS<br> > +++ b/MAINTAINERS<br> > @@ -479,6 +479,13 @@ F: hw/*/allwinner*<br> > F: include/hw/*/allwinner*<br> > F: hw/arm/cubieboard.c<br> > <br> > +Allwinner-h3<br> > +M: Niek Linnenbank <<a href="mailto:nieklinnenbank@gmail.com" target="_blank">nieklinnenbank@gmail.com</a>><br> > +L: <a href="mailto:qemu-arm@nongnu.org" target="_blank">qemu-arm@nongnu.org</a><br> > +S: Maintained<br> > +F: hw/*/allwinner-h3*<br> > +F: include/hw/*/allwinner-h3*<br> > +<br> > ARM PrimeCell and CMSDK devices<br> > M: Peter Maydell <<a href="mailto:peter.maydell@linaro.org" target="_blank">peter.maydell@linaro.org</a>><br> > L: <a href="mailto:qemu-arm@nongnu.org" target="_blank">qemu-arm@nongnu.org</a><br> > diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak<br> > index 1f2e0e7fde..d75a239c2c 100644<br> > --- a/default-configs/arm-softmmu.mak<br> > +++ b/default-configs/arm-softmmu.mak<br> > @@ -40,3 +40,4 @@ CONFIG_FSL_IMX25=y<br> > CONFIG_FSL_IMX7=y<br> > CONFIG_FSL_IMX6UL=y<br> > CONFIG_SEMIHOSTING=y<br> > +CONFIG_ALLWINNER_H3=y<br> > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig<br> > index c6e7782580..ebf8d2325f 100644<br> > --- a/hw/arm/Kconfig<br> > +++ b/hw/arm/Kconfig<br> > @@ -291,6 +291,14 @@ config ALLWINNER_A10<br> > select SERIAL<br> > select UNIMP<br> > <br> > +config ALLWINNER_H3<br> > + bool<br> > + select ALLWINNER_A10_PIT<br> > + select SERIAL<br> > + select ARM_TIMER<br> > + select ARM_GIC<br> > + select UNIMP<br> > +<br> > config RASPI<br> > bool<br> > select FRAMEBUFFER<br> > diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs<br> > index fe749f65fd..956e496052 100644<br> > --- a/hw/arm/Makefile.objs<br> > +++ b/hw/arm/Makefile.objs<br> > @@ -34,6 +34,7 @@ obj-$(CONFIG_DIGIC) += digic.o<br> > obj-$(CONFIG_OMAP) += omap1.o omap2.o<br> > obj-$(CONFIG_STRONGARM) += strongarm.o<br> > obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o<br> > +obj-$(CONFIG_ALLWINNER_H3) += allwinner-h3.o<br> > obj-$(CONFIG_RASPI) += bcm2835_peripherals.o bcm2836.o raspi.o<br> > obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o<br> > obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o<br> > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c<br> > new file mode 100644<br> > index 0000000000..470fdfebef<br> > --- /dev/null<br> > +++ b/hw/arm/allwinner-h3.c<br> > @@ -0,0 +1,215 @@<br> > +/*<br> > + * Allwinner H3 System on Chip emulation<br> > + *<br> > + * Copyright (C) 2019 Niek Linnenbank <<a href="mailto:nieklinnenbank@gmail.com" target="_blank">nieklinnenbank@gmail.com</a>><br> > + *<br> > + * This program is free software: you can redistribute it and/or modify<br> > + * it under the terms of the GNU General Public License as published by<br> > + * the Free Software Foundation, either version 2 of the License, or<br> > + * (at your option) any later version.<br> > + *<br> > + * This program is distributed in the hope that it will be useful,<br> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of<br> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the<br> > + * GNU General Public License for more details.<br> > + *<br> > + * You should have received a copy of the GNU General Public License<br> > + * along with this program. If not, see <<a href="http://www.gnu.org/licenses/" rel="noreferrer" target="_blank">http://www.gnu.org/licenses/</a>>.<br> > + */<br> > +<br> > +#include "qemu/osdep.h"<br> > +#include "exec/address-spaces.h"<br> > +#include "qapi/error.h"<br> > +#include "qemu/module.h"<br> > +#include "qemu/units.h"<br> > +#include "cpu.h"<br> > +#include "hw/sysbus.h"<br> > +#include "hw/arm/allwinner-h3.h"<br> > +#include "hw/misc/unimp.h"<br> > +#include "sysemu/sysemu.h"<br> > +<br> > +static void aw_h3_init(Object *obj)<br> > +{<br> > + AwH3State *s = AW_H3(obj);<br> > +<br> > + sysbus_init_child_obj(obj, "gic", &s->gic, sizeof(s->gic),<br> > + TYPE_ARM_GIC);<br> > +<br> > + sysbus_init_child_obj(obj, "timer", &s->timer, sizeof(s->timer),<br> > + TYPE_AW_A10_PIT);<br> > +}<br> > +<br> > +static void aw_h3_realize(DeviceState *dev, Error **errp)<br> > +{<br> > + AwH3State *s = AW_H3(dev);<br> > + SysBusDevice *sysbusdev = NULL;<br> > + Error *err = NULL;<br> > + unsigned i = 0;<br> > +<br> > + /* CPUs */<br> > + for (i = 0; i < AW_H3_NUM_CPUS; i++) {<br> <br> In <a href="https://www.mail-archive.com/qemu-devel@nongnu.org/msg662942.html" rel="noreferrer" target="_blank">https://www.mail-archive.com/qemu-devel@nongnu.org/msg662942.html</a><br> Markus noted some incorrect pattern, and apparently you inherited it.<br> You should initialize 'err' in the loop.<br> <br> > + Object *cpuobj = object_new(ARM_CPU_TYPE_NAME("cortex-a7"));<br> > + CPUState *cpustate = CPU(cpuobj);<br> <br> We loose access to the CPUs. Can you use an array of AW_H3_NUM_CPUS cpus <br> in AwH3State?<br> <br> > +<br> > + /* Set the proper CPU index */<br> > + cpustate->cpu_index = i;<br> > +<br> > + /* Provide Power State Coordination Interface */<br> > + object_property_set_int(cpuobj, QEMU_PSCI_CONDUIT_HVC,<br> > + "psci-conduit", &error_abort);<br> <br> Here you use the error_abort shortcut.<br> <br> > +<br> > + /* Disable secondary CPUs */<br> > + object_property_set_bool(cpuobj, i > 0, "start-powered-off", &err);<br> > + if (err != NULL) {<br> > + error_propagate(errp, err);<br> > + return;<br> <br> Here you return.<br> <br> > + }<br> > +<br> > + /* All exception levels required */<br> > + object_property_set_bool(cpuobj,<br> > + true, "has_el3", NULL);<br> > + object_property_set_bool(cpuobj,<br> > + true, "has_el2", NULL);<br> <br> Here you don't use error.<br> <br> Cc'ing Markus who is the expert, since he might have better suggestions.<br> <br> This function is called before the machine starts, and we are not <br> handling with user-provided configurations, so I'd say using <br> &error_abort in all places is OK.<br> <br> > +<br> > + /* Mark realized */<br> > + object_property_set_bool(cpuobj, true, "realized", &err);<br> > + if (err != NULL) {<br> > + error_propagate(errp, err);<br> > + return;<br> > + }<br> > + object_unref(cpuobj);<br> > + }<br> > +<br> > + /* Generic Interrupt Controller */<br> > + qdev_prop_set_uint32(DEVICE(&s->gic), "num-irq", AW_H3_GIC_NUM_SPI +<br> > + GIC_INTERNAL);<br> > + qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);<br> > + qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", AW_H3_NUM_CPUS);<br> > + qdev_prop_set_bit(DEVICE(&s->gic), "has-security-extensions", false);<br> > + qdev_prop_set_bit(DEVICE(&s->gic), "has-virtualization-extensions", true);<br> > +<br> > + object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);<br> <br> Why change API? Can we use qdev_init_nofail() instead?<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br> > + if (err) {<br> > + error_propagate(errp, err);<br> > + return;<br> > + }<br> > +<br> > + sysbusdev = SYS_BUS_DEVICE(&s->gic);<br> > + sysbus_mmio_map(sysbusdev, 0, AW_H3_GIC_DIST_BASE);<br> > + sysbus_mmio_map(sysbusdev, 1, AW_H3_GIC_CPU_BASE);<br> > + sysbus_mmio_map(sysbusdev, 2, AW_H3_GIC_HYP_BASE);<br> > + sysbus_mmio_map(sysbusdev, 3, AW_H3_GIC_VCPU_BASE);<br> > +<br> > + /*<br> > + * Wire the outputs from each CPU's generic timer and the GICv3<br> > + * maintenance interrupt signal to the appropriate GIC PPI inputs,<br> > + * and the GIC's IRQ/FIQ/VIRQ/VFIQ interrupt outputs to the CPU's inputs.<br> > + */<br> > + for (i = 0; i < AW_H3_NUM_CPUS; i++) {<br> > + DeviceState *cpudev = DEVICE(qemu_get_cpu(i));<br> > + int ppibase = AW_H3_GIC_NUM_SPI + i * GIC_INTERNAL + GIC_NR_SGIS;<br> > + int irq;<br> > + /*<br> > + * Mapping from the output timer irq lines from the CPU to the<br> > + * GIC PPI inputs used for this board.<br> > + */<br> > + const int timer_irq[] = {<br> > + [GTIMER_PHYS] = AW_H3_GIC_PPI_ARM_PHYSTIMER,<br> > + [GTIMER_VIRT] = AW_H3_GIC_PPI_ARM_VIRTTIMER,<br> > + [GTIMER_HYP] = AW_H3_GIC_PPI_ARM_HYPTIMER,<br> > + [GTIMER_SEC] = AW_H3_GIC_PPI_ARM_SECTIMER,<br> > + };<br> > +<br> > + /* Connect CPU timer outputs to GIC PPI inputs */<br> > + for (irq = 0; irq < ARRAY_SIZE(timer_irq); irq++) {<br> > + qdev_connect_gpio_out(cpudev, irq,<br> > + qdev_get_gpio_in(DEVICE(&s->gic),<br> > + ppibase + timer_irq[irq]));<br> > + }<br> > +<br> > + /* Connect GIC outputs to CPU interrupt inputs */<br> > + sysbus_connect_irq(sysbusdev, i,<br> > + qdev_get_gpio_in(cpudev, ARM_CPU_IRQ));<br> > + sysbus_connect_irq(sysbusdev, i + AW_H3_NUM_CPUS,<br> > + qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));<br> > + sysbus_connect_irq(sysbusdev, i + (2 * AW_H3_NUM_CPUS),<br> > + qdev_get_gpio_in(cpudev, ARM_CPU_VIRQ));<br> > + sysbus_connect_irq(sysbusdev, i + (3 * AW_H3_NUM_CPUS),<br> > + qdev_get_gpio_in(cpudev, ARM_CPU_VFIQ));<br> > +<br> > + /* GIC maintenance signal */<br> > + sysbus_connect_irq(sysbusdev, i + (4 * AW_H3_NUM_CPUS),<br> > + qdev_get_gpio_in(DEVICE(&s->gic),<br> > + ppibase + AW_H3_GIC_PPI_MAINT));<br> > + }<br> > +<br> > + for (i = 0; i < AW_H3_GIC_NUM_SPI; i++) {<br> > + s->irq[i] = qdev_get_gpio_in(DEVICE(&s->gic), i);<br> <br> Apparently we don't need the irq array in AwH3State, because ...<br> <br> > + }<br> > +<br> > + /* Timer */<br> > + object_property_set_bool(OBJECT(&s->timer), true, "realized", &err);<br> > + if (err != NULL) {<br> > + error_propagate(errp, err);<br> > + return;<br> > + }<br> > + sysbusdev = SYS_BUS_DEVICE(&s->timer);<br> > + sysbus_mmio_map(sysbusdev, 0, AW_H3_PIT_REG_BASE);<br> > + sysbus_connect_irq(sysbusdev, 0, s->irq[AW_H3_GIC_SPI_TIMER0]);<br> > + sysbus_connect_irq(sysbusdev, 1, s->irq[AW_H3_GIC_SPI_TIMER1]);<br> <br> ... we can call qdev_get_gpio_in() here directly.<br> <br> > +<br> > + /* SRAM */<br> > + memory_region_init_ram(&s->sram_a1, OBJECT(dev), "sram A1",<br> > + AW_H3_SRAM_A1_SIZE, &error_fatal);<br> > + memory_region_init_ram(&s->sram_a2, OBJECT(dev), "sram A2",<br> > + AW_H3_SRAM_A2_SIZE, &error_fatal);<br> > + memory_region_init_ram(&s->sram_c, OBJECT(dev), "sram C",<br> > + AW_H3_SRAM_C_SIZE, &error_fatal);<br> > + memory_region_add_subregion(get_system_memory(), AW_H3_SRAM_A1_BASE,<br> > + &s->sram_a1);<br> > + memory_region_add_subregion(get_system_memory(), AW_H3_SRAM_A2_BASE,<br> > + &s->sram_a2);<br> > + memory_region_add_subregion(get_system_memory(), AW_H3_SRAM_C_BASE,<br> > + &s->sram_c);<br> > +<br> > + /* UART */<br> > + if (serial_hd(0)) {<br> > + serial_mm_init(get_system_memory(), AW_H3_UART0_REG_BASE, 2,<br> > + s->irq[AW_H3_GIC_SPI_UART0], 115200, serial_hd(0),<br> <br> qdev_get_gpio_in() here too.<br> <br> > + DEVICE_NATIVE_ENDIAN);<br> > + }<br> > +<br> > + /* Unimplemented devices */<br> > + create_unimplemented_device("display-engine", AW_H3_DE_BASE, AW_H3_DE_SIZE);<br> > + create_unimplemented_device("dma", AW_H3_DMA_BASE, AW_H3_DMA_SIZE);<br> > + create_unimplemented_device("lcd0", AW_H3_LCD0_BASE, AW_H3_LCD0_SIZE);<br> > + create_unimplemented_device("lcd1", AW_H3_LCD1_BASE, AW_H3_LCD1_SIZE);<br> > + create_unimplemented_device("gpu", AW_H3_GPU_BASE, AW_H3_GPU_SIZE);<br> > + create_unimplemented_device("hdmi", AW_H3_HDMI_BASE, AW_H3_HDMI_SIZE);<br> > + create_unimplemented_device("rtc", AW_H3_RTC_BASE, AW_H3_RTC_SIZE);<br> > + create_unimplemented_device("audio-codec", AW_H3_AC_BASE, AW_H3_AC_SIZE);<br> > +}<br> > +<br> > +static void aw_h3_class_init(ObjectClass *oc, void *data)<br> > +{<br> > + DeviceClass *dc = DEVICE_CLASS(oc);<br> > +<br> > + dc->realize = aw_h3_realize;<br> > + /* Reason: uses serial_hds and nd_table */<br> > + dc->user_creatable = false;<br> > +}<br> > +<br> > +static const TypeInfo aw_h3_type_info = {<br> > + .name = TYPE_AW_H3,<br> > + .parent = TYPE_DEVICE,<br> > + .instance_size = sizeof(AwH3State),<br> > + .instance_init = aw_h3_init,<br> > + .class_init = aw_h3_class_init,<br> > +};<br> > +<br> > +static void aw_h3_register_types(void)<br> > +{<br> > + type_register_static(&aw_h3_type_info);<br> > +}<br> > +<br> > +type_init(aw_h3_register_types)<br> > diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h<br> > new file mode 100644<br> > index 0000000000..af368c2254<br> > --- /dev/null<br> > +++ b/include/hw/arm/allwinner-h3.h<br> > @@ -0,0 +1,118 @@<br> > +/*<br> > + * Allwinner H3 System on Chip emulation<br> > + *<br> > + * Copyright (C) 2019 Niek Linnenbank <<a href="mailto:nieklinnenbank@gmail.com" target="_blank">nieklinnenbank@gmail.com</a>><br> > + *<br> > + * This program is free software: you can redistribute it and/or modify<br> > + * it under the terms of the GNU General Public License as published by<br> > + * the Free Software Foundation, either version 2 of the License, or<br> > + * (at your option) any later version.<br> > + *<br> > + * This program is distributed in the hope that it will be useful,<br> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of<br> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the<br> > + * GNU General Public License for more details.<br> > + *<br> > + * You should have received a copy of the GNU General Public License<br> > + * along with this program. If not, see <<a href="http://www.gnu.org/licenses/" rel="noreferrer" target="_blank">http://www.gnu.org/licenses/</a>>.<br> > + */<br> > +<br> > +#ifndef HW_ARM_ALLWINNER_H3_H<br> > +#define HW_ARM_ALLWINNER_H3_H<br> > +<br> > +#include "qemu/error-report.h"<br> > +#include "qemu/units.h"<br> > +#include "hw/char/serial.h"<br> > +#include "hw/arm/boot.h"<br> > +#include "hw/timer/allwinner-a10-pit.h"<br> > +#include "hw/intc/arm_gic.h"<br> > +#include "target/arm/cpu.h"<br> > +<br> > +#define AW_H3_SRAM_A1_BASE (0x00000000)<br> > +#define AW_H3_SRAM_A2_BASE (0x00044000)<br> > +#define AW_H3_SRAM_C_BASE (0x00010000)<br> > +#define AW_H3_DE_BASE (0x01000000)<br> > +#define AW_H3_SYSCON_BASE (0x01c00000)<br> > +#define AW_H3_DMA_BASE (0x01c02000)<br> > +#define AW_H3_LCD0_BASE (0x01c0c000)<br> > +#define AW_H3_LCD1_BASE (0x01c0d000)<br> > +#define AW_H3_SID_BASE (0x01c14000)<br> > +#define AW_H3_CCU_BASE (0x01c20000)<br> > +#define AW_H3_PIC_REG_BASE (0x01c20400)<br> > +#define AW_H3_PIT_REG_BASE (0x01c20c00)<br> > +#define AW_H3_AC_BASE (0x01c22c00)<br> > +#define AW_H3_UART0_REG_BASE (0x01c28000)<br> > +#define AW_H3_EMAC_BASE (0x01c30000)<br> > +#define AW_H3_MMC0_BASE (0x01c0f000)<br> > +#define AW_H3_EHCI0_BASE (0x01c1a000)<br> > +#define AW_H3_OHCI0_BASE (0x01c1a400)<br> > +#define AW_H3_EHCI1_BASE (0x01c1b000)<br> > +#define AW_H3_OHCI1_BASE (0x01c1b400)<br> > +#define AW_H3_EHCI2_BASE (0x01c1c000)<br> > +#define AW_H3_OHCI2_BASE (0x01c1c400)<br> > +#define AW_H3_EHCI3_BASE (0x01c1d000)<br> > +#define AW_H3_OHCI3_BASE (0x01c1d400)<br> > +#define AW_H3_GPU_BASE (0x01c40000)<br> > +#define AW_H3_GIC_DIST_BASE (0x01c81000)<br> > +#define AW_H3_GIC_CPU_BASE (0x01c82000)<br> > +#define AW_H3_GIC_HYP_BASE (0x01c84000)<br> > +#define AW_H3_GIC_VCPU_BASE (0x01c86000)<br> > +#define AW_H3_HDMI_BASE (0x01ee0000)<br> > +#define AW_H3_RTC_BASE (0x01f00000)<br> > +#define AW_H3_CPUCFG_BASE (0x01f01c00)<br> > +#define AW_H3_SDRAM_BASE (0x40000000)<br> > +<br> > +#define AW_H3_SRAM_A1_SIZE (64 * KiB)<br> > +#define AW_H3_SRAM_A2_SIZE (32 * KiB)<br> > +#define AW_H3_SRAM_C_SIZE (44 * KiB)<br> > +#define AW_H3_DE_SIZE (4 * MiB)<br> > +#define AW_H3_DMA_SIZE (4 * KiB)<br> > +#define AW_H3_LCD0_SIZE (4 * KiB)<br> > +#define AW_H3_LCD1_SIZE (4 * KiB)<br> > +#define AW_H3_GPU_SIZE (64 * KiB)<br> > +#define AW_H3_HDMI_SIZE (128 * KiB)<br> > +#define AW_H3_RTC_SIZE (1 * KiB)<br> > +#define AW_H3_AC_SIZE (2 * KiB)<br> > +<br> > +#define AW_H3_GIC_PPI_MAINT (9)<br> > +#define AW_H3_GIC_PPI_ARM_HYPTIMER (10)<br> > +#define AW_H3_GIC_PPI_ARM_VIRTTIMER (11)<br> > +#define AW_H3_GIC_PPI_ARM_SECTIMER (13)<br> > +#define AW_H3_GIC_PPI_ARM_PHYSTIMER (14)<br> > +<br> > +#define AW_H3_GIC_SPI_UART0 (0)<br> > +#define AW_H3_GIC_SPI_TIMER0 (18)<br> > +#define AW_H3_GIC_SPI_TIMER1 (19)<br> > +#define AW_H3_GIC_SPI_MMC0 (60)<br> > +#define AW_H3_GIC_SPI_MMC1 (61)<br> > +#define AW_H3_GIC_SPI_MMC2 (62)<br> > +#define AW_H3_GIC_SPI_EHCI0 (72)<br> > +#define AW_H3_GIC_SPI_OHCI0 (73)<br> > +#define AW_H3_GIC_SPI_EHCI1 (74)<br> > +#define AW_H3_GIC_SPI_OHCI1 (75)<br> > +#define AW_H3_GIC_SPI_EHCI2 (76)<br> > +#define AW_H3_GIC_SPI_OHCI2 (77)<br> > +#define AW_H3_GIC_SPI_EHCI3 (78)<br> > +#define AW_H3_GIC_SPI_OHCI3 (79)<br> > +#define AW_H3_GIC_SPI_EMAC (82)<br> <br> I'd move half of the previous definitions into allwinner-h3.c, since <br> they are only used there.<br> <br></blockquote><div>Indeed, you are right, I'll move them.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Also, I'd use an enum for the PPI/SPI. <br></blockquote><div><br></div><div></div><div> Thanks, I will process all of your comments above for the next patch version.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> > +<br> > +#define AW_H3_GIC_NUM_SPI (128)<br> > +#define AW_H3_NUM_CPUS (4)<br> > +<br> > +#define TYPE_AW_H3 "allwinner-h3"<br> > +#define AW_H3(obj) OBJECT_CHECK(AwH3State, (obj), TYPE_AW_H3)<br> > +<br> > +typedef struct AwH3State {<br> > + /*< private >*/<br> > + DeviceState parent_obj;<br> > + /*< public >*/<br> > +<br> > + qemu_irq irq[AW_H3_GIC_NUM_SPI];<br> > + AwA10PITState timer;<br> > + GICState gic;<br> > + MemoryRegion sram_a1;<br> > + MemoryRegion sram_a2;<br> > + MemoryRegion sram_c;<br> > +} AwH3State;<br> > +<br> > +#endif<br> > <br> <br> Nice clean patch, for a first contribution :)<br></blockquote><div><br></div><div>Thank you Philippe! <br></div></div><div><br></div><div>Regards,</div><div>Niek<br></div><br>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Niek Linnenbank<br><br></div></div></div></div>
next prev parent reply index Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-12-02 21:09 [PATCH 00/10] Add Allwinner H3 SoC and Orange Pi PC Machine Niek Linnenbank 2019-12-02 21:09 ` [PATCH 01/10] hw: arm: add Allwinner H3 System-on-Chip Niek Linnenbank 2019-12-04 16:53 ` Philippe Mathieu-Daudé 2019-12-04 20:44 ` Niek Linnenbank [this message] 2019-12-10 9:02 ` Philippe Mathieu-Daudé 2019-12-10 19:17 ` Niek Linnenbank 2019-12-02 21:09 ` [PATCH 02/10] hw: arm: add Xunlong Orange Pi PC machine Niek Linnenbank 2019-12-03 9:17 ` Philippe Mathieu-Daudé 2019-12-03 19:33 ` Niek Linnenbank 2019-12-04 9:03 ` Philippe Mathieu-Daudé 2019-12-04 19:50 ` Niek Linnenbank 2019-12-05 22:15 ` Niek Linnenbank 2019-12-06 5:41 ` Philippe Mathieu-Daudé 2019-12-06 22:15 ` Niek Linnenbank 2019-12-10 8:59 ` Philippe Mathieu-Daudé 2019-12-10 19:14 ` Niek Linnenbank 2019-12-02 21:09 ` [PATCH 03/10] arm: allwinner-h3: add Clock Control Unit Niek Linnenbank 2019-12-02 21:09 ` [PATCH 04/10] arm: allwinner-h3: add USB host controller Niek Linnenbank 2019-12-04 16:11 ` Aleksandar Markovic 2019-12-04 20:20 ` Niek Linnenbank 2019-12-10 7:56 ` Philippe Mathieu-Daudé 2019-12-10 8:29 ` Gerd Hoffmann 2019-12-10 19:11 ` Niek Linnenbank 2019-12-02 21:09 ` [PATCH 05/10] arm: allwinner-h3: add System Control module Niek Linnenbank 2019-12-02 21:09 ` [PATCH 06/10] arm/arm-powerctl: set NSACR.{CP11, CP10} bits in arm_set_cpu_on() Niek Linnenbank 2019-12-06 14:24 ` Peter Maydell 2019-12-06 20:01 ` Niek Linnenbank 2019-12-02 21:09 ` [PATCH 07/10] arm: allwinner-h3: add CPU Configuration module Niek Linnenbank 2019-12-02 21:09 ` [PATCH 08/10] arm: allwinner-h3: add Security Identifier device Niek Linnenbank 2019-12-06 14:27 ` Peter Maydell 2019-12-06 16:35 ` Philippe Mathieu-Daudé 2019-12-06 20:20 ` Niek Linnenbank 2019-12-02 21:09 ` [PATCH 09/10] arm: allwinner-h3: add SD/MMC host controller Niek Linnenbank 2019-12-02 21:09 ` [PATCH 10/10] arm: allwinner-h3: add EMAC ethernet device Niek Linnenbank 2019-12-03 9:33 ` KONRAD Frederic 2019-12-03 19:41 ` Niek Linnenbank 2019-12-04 15:14 ` Philippe Mathieu-Daudé 2019-12-04 15:22 ` KONRAD Frederic 2019-12-03 8:47 ` [PATCH 00/10] Add Allwinner H3 SoC and Orange Pi PC Machine Philippe Mathieu-Daudé 2019-12-03 19:25 ` Niek Linnenbank 2019-12-10 8:40 ` Philippe Mathieu-Daudé 2019-12-09 21:37 ` Niek Linnenbank 2019-12-10 8:26 ` Philippe Mathieu-Daudé 2019-12-10 20:12 ` Niek Linnenbank 2019-12-03 9:02 ` Philippe Mathieu-Daudé 2019-12-03 19:32 ` Niek Linnenbank 2019-12-06 14:16 ` Peter Maydell 2019-12-09 22:24 ` Aleksandar Markovic 2019-12-10 10:34 ` KONRAD Frederic 2019-12-10 19:55 ` Niek Linnenbank
Reply instructions: You may reply publically to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAPan3WpeYtA249jp2iF-baFXh2YihEYMiGVtov7DLpsOVb5ZhQ@mail.gmail.com \ --to=nieklinnenbank@gmail.com \ --cc=armbru@redhat.com \ --cc=b.galvani@gmail.com \ --cc=peter.maydell@linaro.org \ --cc=philmd@redhat.com \ --cc=qemu-arm@nongnu.org \ --cc=qemu-devel@nongnu.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
QEMU-Devel Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \ qemu-devel@nongnu.org public-inbox-index qemu-devel Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel AGPL code for this site: git clone https://public-inbox.org/public-inbox.git