diff mbox

[v4,2/4] hw/intc: add sunxi interrupt controller device

Message ID 1385450531-3170-3-git-send-email-lig.fnst@cn.fujitsu.com
State New
Headers show

Commit Message

liguang Nov. 26, 2013, 7:22 a.m. UTC
Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 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.h

Comments

Peter Crosthwaite Nov. 26, 2013, 9:02 a.m. UTC | #1
On Tue, Nov 26, 2013 at 5:22 PM, liguang <lig.fnst@cn.fujitsu.com> wrote:
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  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.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..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 <lig.fnst@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;

Blank line here for readability.

> +    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];

this appears to be constant 0. I cant find anywhere that sets fiq_pending.

> +    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.

> +    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;
}

> +                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, ...)

> +    }

Is this supposed to set either fiq_pending or irq_pending depending on
the select bit?

> +    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       95

Is this constant or configurable? Should it perhaps be a static prop?

> +#define SUNXI_PIC_REG_IDX      (SUNXI_PIC_INT_NR / 32 + 1)
> +

DIV_ROUND_UP is probably the intention here.

Regards,
Peter

> +#endif
> --
> 1.7.2.5
>
>
liguang Nov. 26, 2013, 9:11 a.m. UTC | #2
Peter Crosthwaite wrote:
> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com>  wrote:
>    
>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>> ---
>>   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.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..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<lig.fnst@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;
>>      
> Blank line here for readability.
>
>    

OK
>> +    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];
>>      
> this appears to be constant 0. I cant find anywhere that sets fiq_pending.
>
>    

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

>> +    }
>>      
> 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       95
>>      
> Is 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
>>
>>
>>      
>
liguang Nov. 27, 2013, 3:36 a.m. UTC | #3
Li Guang wrote:
> Peter Crosthwaite wrote:
>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com>  
>> wrote:
>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>> ---
>>>   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.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..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<lig.fnst@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;
>> Blank line here for readability.
>>
>
> OK
>>> +    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];
>> this appears to be constant 0. I cant find anywhere that sets 
>> fiq_pending.
>>
>
> 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       95
>> Is 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
>>>
>>>
>
Peter Crosthwaite Nov. 27, 2013, 5:31 a.m. UTC | #4
On Wed, Nov 27, 2013 at 1:36 PM, Li Guang <lig.fnst@cn.fujitsu.com> wrote:
> Li Guang wrote:
>>
>> Peter Crosthwaite wrote:
>>>
>>> On Tue, Nov 26, 2013 at 5:22 PM, liguang<lig.fnst@cn.fujitsu.com>  wrote:
>>>>
>>>> Signed-off-by: liguang<lig.fnst@cn.fujitsu.com>
>>>> ---
>>>>   default-configs/arm-softmmu.mak |    1 +
>>>>   hw/intc/Makefile.objs           |    1 +
>>>>   hw/intc/sunxi-pic.c             |  238
>>>> +++++++++++++++++++++++++++++++++++++++
>>>>   include/hw/intc/sunxi-pic.h     |   20 ++++

>>>> +
>>>> +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.
>

The irq/32 is right I agree. This issue is the first arugment.
Shouln't the whole thing be:

set_bit(irq%32, (void *)&s->irq_pending[irq/32]);

Regards,
Peter
diff mbox

Patch

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..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 <lig.fnst@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_pending[SUNXI_PIC_REG_IDX];
+    uint32_t fiq_pending[SUNXI_PIC_REG_IDX];
+    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;
+    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])) {
+                if (test_bit(j, (void *)&s->irq_pending[i])) {
+                    qemu_set_irq(s->parent_irq, 1);
+                    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;
+                }
+            }
+        }
+    }
+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]);
+    }
+    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	95
+#define SUNXI_PIC_REG_IDX	(SUNXI_PIC_INT_NR / 32 + 1)
+
+#endif