On Mon, Nov 25, 2013 at 5:41 PM, liguang <lig.f...@cn.fujitsu.com> wrote: > Signed-off-by: liguang <lig.f...@cn.fujitsu.com> > --- > default-configs/arm-softmmu.mak | 1 + > hw/intc/Makefile.objs | 1 + > hw/intc/sunxi-pic.c | 301 > +++++++++++++++++++++++++++++++++++++++ > include/hw/intc/sunxi-pic.h | 27 ++++ > 4 files changed, 330 insertions(+), 0 deletions(-) > create mode 100644 hw/intc/sunxi-pic.c > create mode 100644 include/hw/intc/sunxi-pic.h > > diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak > index 7bf5ad0..bbe00e4 100644 > --- a/default-configs/arm-softmmu.mak > +++ b/default-configs/arm-softmmu.mak > @@ -83,3 +83,4 @@ CONFIG_SDHCI=y > CONFIG_INTEGRATOR_DEBUG=y > > CONFIG_SUNXI_PIT=y > +CONFIG_SUNXI_PIC=y > diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs > index 47ac442..dad8c43 100644 > --- a/hw/intc/Makefile.objs > +++ b/hw/intc/Makefile.objs > @@ -12,6 +12,7 @@ common-obj-$(CONFIG_IOAPIC) += ioapic_common.o > common-obj-$(CONFIG_ARM_GIC) += arm_gic_common.o > common-obj-$(CONFIG_ARM_GIC) += arm_gic.o > common-obj-$(CONFIG_OPENPIC) += openpic.o > +common-obj-$(CONFIG_SUNXI_PIC) += sunxi-pic.o > > obj-$(CONFIG_APIC) += apic.o apic_common.o > obj-$(CONFIG_ARM_GIC_KVM) += arm_gic_kvm.o > diff --git a/hw/intc/sunxi-pic.c b/hw/intc/sunxi-pic.c > new file mode 100644 > index 0000000..09a3d09 > --- /dev/null > +++ b/hw/intc/sunxi-pic.c > @@ -0,0 +1,301 @@ > +/* > + * Allwinner sunxi interrupt controller device emulation > + * > + * Copyright (C) 2013 Li Guang > + * Written by Li Guang <lig.f...@cn.fujitsu.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. > + */ > + > +#include "hw/sysbus.h" > +#include "hw/devices.h" > +#include "sysemu/sysemu.h" > +#include "hw/intc/sunxi-pic.h" > + > + > +typedef struct SunxiPICState { > + /*< private >*/ > + SysBusDevice parent_obj; > + /*< public >*/ > + MemoryRegion iomem; > + qemu_irq parent_fiq; > + qemu_irq parent_irq; > + uint32_t vector; > + uint32_t base_addr; > + uint32_t protect; > + uint32_t nmi; > + uint32_t irq_pending0; > + uint32_t irq_pending1; > + uint32_t irq_pending2; > + uint32_t select0; > + uint32_t select1; > + uint32_t select2; > + uint32_t enable0; > + uint32_t enable1; > + uint32_t enable2; > + uint32_t mask0; > + uint32_t mask1; > + uint32_t mask2; > + /*priority setting here*/ > +} SunxiPICState; > + > +static void sunxi_pic_update(SunxiPICState *s) > +{ > + uint32_t flags = 0;
initializer to 0 not needed. > + > + flags = s->irq_pending0 | s->irq_pending1 | s->irq_pending0; > + qemu_set_irq(s->parent_irq, flags != 0); > + flags &= s->select0 | s->select1 | s->select2; > + qemu_set_irq(s->parent_fiq, flags != 0); This logic seems strange to me, in that that behavior of individual interrupts is not self contained. e.g. if interrupt 0 (bit 0 of irq_pending0) is pending and interrupt 32 is selected (bit 0 of select1) then flags is non-zero regardless of state of bit 0 of s->select0). Whats the intended behavior of the select bits? If an interrupt is "selected" it will also route to both IRQ and FIQ it seems. > +} > + > +static void sunxi_pic_set_irq(void *opaque, int irq, int level) > +{ > + SunxiPICState *s = opaque; > + bool allow_irq = false; > + > + if (level) { > + if (irq < 32) { > + set_bit(irq, (void *)&s->irq_pending0); > + if (test_bit(irq, (void *)&s->enable0) && > + !test_bit(irq, (void *)&s->mask0)) { > + allow_irq = true; this check inhibits immediate interrupt generation of masked intr, but it doesnt stop a future event from triggering the IRQ. Should the enable and mask checks be in the interrupt generation code? For example, your bus write handler calls sunxi_pic_update() which means these pending-but-masked interrupts will potentially cause the irq to raise at bus write time. > + } > + } else if (irq < 64) { > + irq -= 32; > + set_bit(irq, (void *)&s->irq_pending1); > + if (test_bit(irq, (void *)&s->enable1) && > + !test_bit(irq, (void *)&s->mask1)) { > + allow_irq = true; > + } > + } else if (irq < 95) { > + irq -= 64; > + set_bit(irq, (void *)&s->irq_pending2); > + if (test_bit(irq, (void *)&s->enable2) && > + !test_bit(irq, (void *)&s->mask2)) { > + allow_irq = true; > + } > + } > + } else { > + if (irq < 32) { > + clear_bit(irq, (void *)&s->irq_pending0); > + } else if (irq < 64) { > + irq -= 32; > + clear_bit(irq, (void *)&s->irq_pending1); > + } else if (irq < 95) { > + irq -= 64; > + clear_bit(irq, (void *)&s->irq_pending2); > + } > + } > + if (allow_irq) { > + sunxi_pic_update(s); > + } > +} > + > +static uint64_t sunxi_pic_read(void *opaque, hwaddr offset, unsigned size) > +{ > + SunxiPICState *s = opaque; > + > + switch (offset) { > + case PIC_VECTOR: > + return s->vector; > + break; > + case PIC_BASE_ADDR: > + return s->base_addr; > + break; > + case PIC_PROTECT: > + return s->protect; > + break; > + case PIC_NMI: > + return s->nmi; > + break; > + case PIC_IRQ_PENDING0: > + case PIC_FIQ_PENDING0: > + return s->irq_pending0; > + break; > + case PIC_IRQ_PENDING1: > + case PIC_FIQ_PENDING1: > + return s->irq_pending1; > + break; > + case PIC_IRQ_PENDING2: > + case PIC_FIQ_PENDING2: > + return s->irq_pending2; > + break; > + case PIC_SELECT0: > + return s->select0; > + break; > + case PIC_SELECT1: > + return s->select1; > + break; > + case PIC_SELECT2: > + return s->select2; > + break; > + case PIC_ENABLE0: > + return s->enable0; > + break; > + case PIC_ENABLE1: > + return s->enable1; > + break; > + case PIC_ENABLE2: > + return s->enable2; > + break; > + case PIC_MASK0: > + return s->mask0; > + break; > + case PIC_MASK1: > + return s->mask1; > + break; > + case PIC_MASK2: > + return s->mask2; > + break; > + default: > + break; > + } > + > + return 0; > +} > + > +static void sunxi_pic_write(void *opaque, hwaddr offset, uint64_t value, > + unsigned size) > +{ > + SunxiPICState *s = opaque; > + uint8_t index = 0; > + > + switch (offset) { > + case PIC_VECTOR: > + s->vector = value & ~0x3; > + break; > + case PIC_BASE_ADDR: > + s->base_addr = value & ~0x3; > + case PIC_PROTECT: > + case PIC_NMI: > + break; > + case PIC_IRQ_PENDING0: > + case PIC_FIQ_PENDING0: > + for (index = 0; index < 32; index++) { > + if (test_bit(index, (void *)&value)) { > + clear_bit(index, (void *)&s->irq_pending0); > + } &= ~foo to implement write to clear. > + } > + break; > + case PIC_IRQ_PENDING1: > + case PIC_FIQ_PENDING1: > + for (index = 0; index < 32; index++) { > + if (test_bit(index, (void *)&value)) { > + clear_bit(index, (void *)&s->irq_pending1); > + } > + } > + break; > + case PIC_IRQ_PENDING2: > + case PIC_FIQ_PENDING2: > + for (index = 0; index < 32; index++) { > + if (test_bit(index, (void *)&value)) { > + clear_bit(index, (void *)&s->irq_pending2); > + } > + } > + break; > + case PIC_SELECT0: > + s->select0 = value; > + break; > + case PIC_SELECT1: > + s->select1 = value; > + break; > + case PIC_SELECT2: > + s->select2 = value; > + break; > + case PIC_ENABLE0: > + s->enable0 = value; > + break; > + case PIC_ENABLE1: > + s->enable1 = value; > + break; > + case PIC_ENABLE2: > + s->enable2 = value; > + break; > + case PIC_MASK0: > + s->mask0 = value; > + break; > + case PIC_MASK1: > + s->mask1 = value; > + break; > + case PIC_MASK2: > + s->mask2 = value; You can make this much shorter by making the reigsters an array, and indexing into it using offset for the default case. > + default: > + break; > + } > + > + sunxi_pic_update(s); > +} > + > +static const MemoryRegionOps sunxi_pic_ops = { > + .read = sunxi_pic_read, > + .write = sunxi_pic_write, > + .endianness = DEVICE_NATIVE_ENDIAN, > +}; > + > +static int sunxi_pic_init(SysBusDevice *dev) > +{ > + SunxiPICState *s = SUNXI_PIC(dev); > + > + qdev_init_gpio_in(DEVICE(dev), sunxi_pic_set_irq, 95); > + sysbus_init_irq(dev, &s->parent_irq); > + sysbus_init_irq(dev, &s->parent_fiq); > + memory_region_init_io(&s->iomem, OBJECT(s), &sunxi_pic_ops, s, > + "sunxi-pic", 0x400); > + sysbus_init_mmio(dev, &s->iomem); > + > + return 0; > +} > + > +static void sunxi_pic_reset(DeviceState *d) > +{ > + SunxiPICState *s = SUNXI_PIC(d); > + > + s->base_addr = 0; > + s->protect = 0; > + s->nmi = 0; > + s->vector = 0; > + s->irq_pending0 = 0; > + s->irq_pending1 = 0; > + s->irq_pending2 = 0; > + s->select0 = 0; > + s->select1 = 0; > + s->select2 = 0; > + s->enable0 = 0; > + s->enable1 = 0; > + s->enable2 = 0; > + s->mask0 = 0; > + s->mask1 = 0; > + s->mask2 = 0; > +} > + > +static void sunxi_pic_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); > + > + k->init = sunxi_pic_init; Use of SysBusDevice::init is depracated. Please use Device::realize or the object init instead. With no properties deps, I think the latter is more correct. > + dc->reset = sunxi_pic_reset; missing VMSD support. > + } > + > +static const TypeInfo sunxi_pic_info = { > + .name = TYPE_SUNXI_PIC, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(SunxiPICState), > + .class_init = sunxi_pic_class_init, > +}; > + > +static void sunxi_register_types(void) > +{ > + type_register_static(&sunxi_pic_info); > +} > + > +type_init(sunxi_register_types); > diff --git a/include/hw/intc/sunxi-pic.h b/include/hw/intc/sunxi-pic.h > new file mode 100644 > index 0000000..fd07b33 > --- /dev/null > +++ b/include/hw/intc/sunxi-pic.h > @@ -0,0 +1,27 @@ > +#ifndef SUNXI_PIC_H > +#define SUNXI_PIC_H > + > +#define TYPE_SUNXI_PIC "sunxi_PIC" I think -'s are encouraged over _'s. The casing should be made consistent as well. Regards, Peter > +#define SUNXI_PIC(obj) OBJECT_CHECK(SunxiPICState, (obj), TYPE_SUNXI_PIC) > + > +#define PIC_VECTOR 0 > +#define PIC_BASE_ADDR 4 > +#define PIC_PROTECT 8 > +#define PIC_NMI 0xc > +#define PIC_IRQ_PENDING0 0x10 > +#define PIC_IRQ_PENDING1 0x14 > +#define PIC_IRQ_PENDING2 0x18 > +#define PIC_FIQ_PENDING0 0x20 > +#define PIC_FIQ_PENDING1 0x24 > +#define PIC_FIQ_PENDING2 0x28 > +#define PIC_SELECT0 0x30 > +#define PIC_SELECT1 0x34 > +#define PIC_SELECT2 0x38 > +#define PIC_ENABLE0 0x40 > +#define PIC_ENABLE1 0x44 > +#define PIC_ENABLE2 0x48 > +#define PIC_MASK0 0x50 > +#define PIC_MASK1 0x54 > +#define PIC_MASK2 0x58 > + > +#endif > -- > 1.7.2.5 > >