diff mbox

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

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

Commit Message

liguang Nov. 25, 2013, 7:41 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             |  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

Comments

Peter Crosthwaite Nov. 25, 2013, 10:42 a.m. UTC | #1
On Mon, Nov 25, 2013 at 5:41 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             |  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.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_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
>
>
liguang Nov. 26, 2013, 12:45 a.m. UTC | #2
Peter Crosthwaite wrote:
> On Mon, Nov 25, 2013 at 5:41 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             |  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.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_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.
>
>    

All are reasonable, will fix.

Thanks a lot!

>    
>> +#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
>>
>>
>>      
>
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..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.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_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;
+
+    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);
+}
+
+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;
+            }
+        } 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);
+            }
+        }
+        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;
+    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;
+    dc->reset = sunxi_pic_reset;
+ }
+
+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"
+#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