|
From: | Li Guang |
Subject: | Re: [Qemu-devel] [PATCH v4 2/4] hw/intc: add sunxi interrupt controller device |
Date: | Wed, 27 Nov 2013 11:36:31 +0800 |
User-agent: | Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1.16) Gecko/20120421 Iceape/2.0.11 |
Li Guang wrote:
Peter Crosthwaite wrote:On Tue, Nov 26, 2013 at 5:22 PM, liguang<address@hidden> wrote:Signed-off-by: liguang<address@hidden> --- default-configs/arm-softmmu.mak | 1 + hw/intc/Makefile.objs | 1 +hw/intc/sunxi-pic.c | 238 +++++++++++++++++++++++++++++++++++++++include/hw/intc/sunxi-pic.h | 20 ++++ 4 files changed, 260 insertions(+), 0 deletions(-) create mode 100644 hw/intc/sunxi-pic.c create mode 100644 include/hw/intc/sunxi-pic.hdiff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.makindex 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..e84fc55 --- /dev/null +++ b/hw/intc/sunxi-pic.c @@ -0,0 +1,238 @@ +/* + * Allwinner sunxi interrupt controller device emulation + * + * Copyright (C) 2013 Li Guang + * Written by Li Guang<address@hidden> + *+ * 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;Blank line here for readability.OKthis appears to be constant 0. I cant find anywhere that sets fiq_pending.+ uint32_t vector; + uint32_t base_addr; + uint32_t protect; + uint32_t nmi; + uint32_t irq_pending[SUNXI_PIC_REG_IDX]; + uint32_t fiq_pending[SUNXI_PIC_REG_IDX];here, only NMI can generate fiq, and I did take care NMI now, so there's no real case to set fiq.+ uint32_t select[SUNXI_PIC_REG_IDX]; + uint32_t enable[SUNXI_PIC_REG_IDX]; + uint32_t mask[SUNXI_PIC_REG_IDX]; + /*priority setting here*/ +} SunxiPICState; + +static void sunxi_pic_update(SunxiPICState *s) +{ + uint8_t i = 0, j = 0;Initialisers un-needed.OK+ bool irq = false; + + for (i = 0, j = 0; i< SUNXI_PIC_REG_IDX; i++) { + for (j = 0; j< 32; j++) { + if (!test_bit(j, (void *)&s->mask[i])) {You can save on a level of indentation here with: if (test_bit(j, (void *)&s->mask[i])) { continue; }OK+ if (test_bit(j, (void *)&s->irq_pending[i])) { + qemu_set_irq(s->parent_irq, 1);qemu_irq_raise() is simpler. I can't find anywhere in the code where the interrupts are lowered. How do they come down, once they are up?+ irq = true; + } + if (test_bit(j, (void *)&s->fiq_pending[i])&& + test_bit(j, (void *)&s->select[i])) { + qemu_set_irq(s->parent_fiq, 1); + irq = true; + } + if (irq) { + goto out;What happens if two interrupts are both active, the first setting IRQ the second FIQ? Wont this escape logic cause FIQ raising to potentionally be missed?+ } + } + } + } +out: + return; +} + +static void sunxi_pic_set_irq(void *opaque, int irq, int level) +{ + SunxiPICState *s = opaque; + + if (level) { + set_bit(irq, (void *)&s->irq_pending[irq/32]);set_bit(irq % 32, ...)OK
No, it is wrong, irq/32 is right.
+ }Is this supposed to set either fiq_pending or irq_pending depending on the select bit?Yes, but, I as I stated before, maybe I will take NMI later.+ sunxi_pic_update(s); +} ++static uint64_t sunxi_pic_read(void *opaque, hwaddr offset, unsigned size)+{ + SunxiPICState *s = opaque; + uint8_t index = (offset& 0xc)/4; + + switch (offset) { + case SUNXI_PIC_VECTOR: + return s->vector; + break; + case SUNXI_PIC_BASE_ADDR: + return s->base_addr; + break; + case SUNXI_PIC_PROTECT: + return s->protect; + break; + case SUNXI_PIC_NMI: + return s->nmi; + break; + case SUNXI_PIC_IRQ_PENDING ... SUNXI_PIC_IRQ_PENDING + 8: + return s->irq_pending[index]; + break; + case SUNXI_PIC_FIQ_PENDING ... SUNXI_PIC_FIQ_PENDING + 8: + return s->fiq_pending[index]; + break; + case SUNXI_PIC_SELECT ... SUNXI_PIC_SELECT + 8: + return s->select[index]; + break; + case SUNXI_PIC_ENABLE ... SUNXI_PIC_ENABLE + 8: + return s->enable[index]; + break; + case SUNXI_PIC_MASK ... SUNXI_PIC_MASK + 8: + return s->mask[index]; + 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 = (offset& 0xc)/4; + + switch (offset) { + case SUNXI_PIC_VECTOR: + s->vector = value& ~0x3; + break; + case SUNXI_PIC_BASE_ADDR: + s->base_addr = value& ~0x3; + case SUNXI_PIC_PROTECT: + s->protect = value; + break; + case SUNXI_PIC_NMI: + s->nmi = value; + break; + case SUNXI_PIC_IRQ_PENDING ... SUNXI_PIC_IRQ_PENDING + 8: + s->irq_pending[index]&= ~value; + break; + case SUNXI_PIC_FIQ_PENDING ... SUNXI_PIC_FIQ_PENDING + 8: + s->fiq_pending[index]&= ~value; + break; + case SUNXI_PIC_SELECT ... SUNXI_PIC_SELECT + 8: + s->select[index] = value; + break; + case SUNXI_PIC_ENABLE ... SUNXI_PIC_ENABLE + 8: + s->enable[index] = value; + break; + case SUNXI_PIC_MASK ... SUNXI_PIC_MASK + 8: + s->mask[index] = value; + break; + 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 const VMStateDescription vmstate_sunxi_pic = { + .name = "sunxi.pic", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(vector, SunxiPICState), + VMSTATE_UINT32(base_addr, SunxiPICState), + VMSTATE_UINT32(protect, SunxiPICState), + VMSTATE_UINT32(nmi, SunxiPICState),+ VMSTATE_UINT32_ARRAY(irq_pending, SunxiPICState, SUNXI_PIC_REG_IDX), + VMSTATE_UINT32_ARRAY(fiq_pending, SunxiPICState, SUNXI_PIC_REG_IDX), + VMSTATE_UINT32_ARRAY(enable, SunxiPICState, SUNXI_PIC_REG_IDX), + VMSTATE_UINT32_ARRAY(select, SunxiPICState, SUNXI_PIC_REG_IDX),+ VMSTATE_UINT32_ARRAY(mask, SunxiPICState, SUNXI_PIC_REG_IDX), + VMSTATE_END_OF_LIST() + } +}; + +static void sunxi_pic_realize(DeviceState *ds, Error **errp) +{ + SunxiPICState *s = SUNXI_PIC(ds); + SysBusDevice *dev = SYS_BUS_DEVICE(ds); ++ qdev_init_gpio_in(DEVICE(dev), sunxi_pic_set_irq, SUNXI_PIC_INT_NR);+ 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); +} + +static void sunxi_pic_reset(DeviceState *d) +{ + SunxiPICState *s = SUNXI_PIC(d); + uint8_t i; + + s->base_addr = 0; + s->protect = 0; + s->nmi = 0; + s->vector = 0; + for (i = 0; i< SUNXI_PIC_REG_IDX; i++) { + s->irq_pending[i] = 0; + s->fiq_pending[i] = 0; + s->select[i] = 0; + s->enable[i] = 0; + s->mask[i] = 0; + } +} + +static void sunxi_pic_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->realize = sunxi_pic_realize; + dc->reset = sunxi_pic_reset; + dc->desc = "sunxi pic"; + dc->vmsd =&vmstate_sunxi_pic; + } + +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..d52b53c --- /dev/null +++ b/include/hw/intc/sunxi-pic.h @@ -0,0 +1,20 @@ +#ifndef SUNXI_PIC_H +#define SUNXI_PIC_H + +#define TYPE_SUNXI_PIC "sunxi-pic"+#define SUNXI_PIC(obj) OBJECT_CHECK(SunxiPICState, (obj), TYPE_SUNXI_PIC)+ +#define SUNXI_PIC_VECTOR 0 +#define SUNXI_PIC_BASE_ADDR 4 +#define SUNXI_PIC_PROTECT 8 +#define SUNXI_PIC_NMI 0xc +#define SUNXI_PIC_IRQ_PENDING 0x10 +#define SUNXI_PIC_FIQ_PENDING 0x20 +#define SUNXI_PIC_SELECT 0x30 +#define SUNXI_PIC_ENABLE 0x40 +#define SUNXI_PIC_MASK 0x50 + +#define SUNXI_PIC_INT_NR 95Is this constant or configurable? Should it perhaps be a static prop?NO, it's constant+#define SUNXI_PIC_REG_IDX (SUNXI_PIC_INT_NR / 32 + 1) +DIV_ROUND_UP is probably the intention here.right, will fix thank you so much!+#endif -- 1.7.2.5
[Prev in Thread] | Current Thread | [Next in Thread] |