Hi Colin,
Sorry about the delay, review below.
On Tue, Jan 6, 2015 at 2:26 PM, Colin Leitner
<colin.leit...@googlemail.com> wrote:
> Based on the pl061 model. This model implements all four banks with 32 I/Os
> each.
>
> The I/Os are placed in named groups:
>
> * bankX_in for the 32 inputs of each bank
> * bankX_out for the 32 outputs of each bank
>
> Basic I/O and IRQ support tested with the Zynq GPIO driver in Linux 3.12.
>
> Signed-off-by: Colin Leitner <colin.leit...@gmail.com>
> ---
> hw/gpio/Makefile.objs | 1 +
> hw/gpio/zynq-gpio.c | 402
> +++++++++++++++++++++++++++++++++++++++++++
> include/hw/gpio/zynq-gpio.h | 79 +++++++++
> 3 files changed, 482 insertions(+)
> create mode 100644 hw/gpio/zynq-gpio.c
> create mode 100644 include/hw/gpio/zynq-gpio.h
>
> diff --git a/hw/gpio/Makefile.objs b/hw/gpio/Makefile.objs
> index 1abcf17..c927c66 100644
> --- a/hw/gpio/Makefile.objs
> +++ b/hw/gpio/Makefile.objs
> @@ -5,3 +5,4 @@ common-obj-$(CONFIG_ZAURUS) += zaurus.o
> common-obj-$(CONFIG_E500) += mpc8xxx.o
>
> obj-$(CONFIG_OMAP) += omap_gpio.o
> +obj-$(CONFIG_ZYNQ) += zynq-gpio.o
> diff --git a/hw/gpio/zynq-gpio.c b/hw/gpio/zynq-gpio.c
> new file mode 100644
> index 0000000..8cf4262
> --- /dev/null
> +++ b/hw/gpio/zynq-gpio.c
> @@ -0,0 +1,402 @@
> +/*
> + * Zynq General Purpose IO
> + *
> + * Copyright (C) 2014 Colin Leitner <colin.leit...@gmail.com>
> + *
> + * Based on the PL061 model:
> + * Copyright (c) 2007 CodeSourcery.
> + * Written by Paul Brook
> + *
> + * This code is licensed under the GPL.
> + */
> +
> +/*
> + * We model all banks as if they were fully populated. MIO pins are usually
> + * limited to 54 pins, but this is probably device dependent and shouldn't
> + * cause too much trouble. One noticeable difference is the reset value of
> + * INT_TYPE_1, which is 0x003fffff according to the TRM and 0xffffffff here.
> + *
> + * The output enable pins are not modeled.
> + */
> +
> +#include "hw/gpio/zynq-gpio.h"
> +#include "qemu/bitops.h"
> +
> +#ifndef ZYNQ_GPIO_ERR_DEBUG
> +#define ZYNQ_GPIO_ERR_DEBUG 0
> +#endif
> +
> +#define DB_PRINT_L(lvl, fmt, args...) do {\
> + if (ZYNQ_GPIO_ERR_DEBUG >= lvl) {\
> + qemu_log("zynq-gpio: %s:" fmt, __func__, ## args);\
> + } \
> +} while (0);
> +
> +#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
> +
> +static void zynq_gpio_update_out(ZynqGPIOBank *bank)
> +{
> + uint32_t changed;
> + uint32_t mask;
> + uint32_t out;
> + int i;
> +
> + DB_PRINT("dir = %d, data = %d\n", bank->dir, bank->out_data);
> +
> + /*
> + * We assume non-driven (DIRM = 0) outputs float high. On real hardware
> this
> + * could be different, but here we have to decide which value to set the
> + * output IRQ to if the direction register switches the I/O to an input.
> + */
> + /* FIXME: This is board dependent. */
> + out = (bank->out_data & bank->dir) | ~bank->dir;
> +
> + changed = bank->old_out_data ^ out;
> + bank->old_out_data = out;
> +
> + for (i = 0; i < ZYNQ_GPIO_IOS_PER_BANK; i++) {
> + mask = 1 << i;
> + if (changed & mask) {
> + DB_PRINT("Set output %d = %d\n", i, (out & mask) != 0);
> + qemu_set_irq(bank->out[i], (out & mask) != 0);
> + }
> + }
> +}
> +
> +static void zynq_gpio_update_in(ZynqGPIOBank *bank)
> +{
> + uint32_t changed;
> + uint32_t mask;
> + int i;
> +
> + changed = bank->old_in_data ^ bank->in_data;
> + bank->old_in_data = bank->in_data;
> +
> + for (i = 0; i < ZYNQ_GPIO_IOS_PER_BANK; i++) {
> + mask = 1 << i;
> + if (changed & mask) {
> + DB_PRINT("Changed input %d = %d\n", i, (bank->in_data & mask) !=
> 0);
> +
> + if (bank->itype & mask) {
> + /* Edge interrupt */
> + if (bank->iany & mask) {
> + /* Any edge triggers the interrupt */
> + bank->istat |= mask;
> + } else {
> + /* Edge is selected by INT_POLARITY */
> + bank->istat |= ~(bank->in_data ^ bank->ipolarity) & mask;
> + }
> + }
> + }
> + }
> +
> + /* Level interrupt */
> + bank->istat |= ~(bank->in_data ^ bank->ipolarity) & ~bank->itype;
> +
> + DB_PRINT("istat = %08X\n", bank->istat);
> +}
> +
> +static void zynq_gpio_set_in_irq(ZynqGPIOState *s)
> +{
> + int b;
> + uint32_t istat = 0;
> +
> + for (b = 0; b < ZYNQ_GPIO_BANKS; b++) {
> + ZynqGPIOBank *bank = &s->banks[b];
> +
> + istat |= bank->istat & ~bank->imask;
> + }
> +
> + DB_PRINT("IRQ = %d\n", istat != 0);
> +
> + qemu_set_irq(s->irq, istat != 0);
> +}
> +
> +static void zynq_gpio_update(ZynqGPIOState *s)
> +{
> + int b;
> +
> + for (b = 0; b < ZYNQ_GPIO_BANKS; b++) {
> + ZynqGPIOBank *bank = &s->banks[b];
> +
> + zynq_gpio_update_out(bank);
> + zynq_gpio_update_in(bank);
> + }
> +
> + zynq_gpio_set_in_irq(s);
> +}
> +
> +static uint64_t zynq_gpio_read(void *opaque, hwaddr offset,
> + unsigned int size)
> +{
> + ZynqGPIOState *s = opaque;
> + int b;
> + int shift;
> + ZynqGPIOBank *bank;
> +
> + switch (offset) {
> + case ZYNQ_GPIO_REG_MASK_DATA_0_LSW ... ZYNQ_GPIO_REG_MASK_DATA_3_MSW:
> + b = extract32(offset - ZYNQ_GPIO_REG_MASK_DATA_0_LSW, 3, 2);
> + shift = (offset & 0x8) ? 16 : 0;
> + return extract32(s->banks[b].mask_data, shift, 16);
> +
> + case ZYNQ_GPIO_REG_DATA_0 ... ZYNQ_GPIO_REG_DATA_3:
> + b = (offset - ZYNQ_GPIO_REG_DATA_0) / 4;
> + return s->banks[b].out_data;
> +
> + case ZYNQ_GPIO_REG_DATA_0_RO ... ZYNQ_GPIO_REG_DATA_3_RO:
> + b = (offset - ZYNQ_GPIO_REG_DATA_0_RO) / 4;
> + return s->banks[b].in_data;
> + }
> +
> + if (offset < 0x204 || offset > 0x2e4) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "zynq_gpio_read: Bad offset %" HWADDR_PRIx "\n",
> offset);
> + return 0;
> + }
> +
> + b = (offset - 0x200) / 0x40;
> + bank = &s->banks[b];
> +
> + switch (offset - ZYNQ_GPIO_BANK_OFFSET(b)) {
> + case ZYNQ_GPIO_BANK_REG_DIRM:
> + return bank->dir;
> + case ZYNQ_GPIO_BANK_REG_OEN:
> + return bank->oen;
> + case ZYNQ_GPIO_BANK_REG_INT_MASK:
> + return bank->imask;
> + case ZYNQ_GPIO_BANK_REG_INT_EN:
> + return 0;
> + case ZYNQ_GPIO_BANK_REG_INT_DIS:
> + return 0;
> + case ZYNQ_GPIO_BANK_REG_INT_STAT:
> + return bank->istat;
> + case ZYNQ_GPIO_BANK_REG_INT_TYPE:
> + return bank->itype;
> + case ZYNQ_GPIO_BANK_REG_INT_POLARITY:
> + return bank->ipolarity;
> + case ZYNQ_GPIO_BANK_REG_INT_ANY:
> + return bank->iany;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "zynq_gpio_read: Bad offset %" HWADDR_PRIx "\n",
> offset);
> + return 0;
> + }
> +}
> +
> +static void zynq_gpio_mask_data(ZynqGPIOBank *bank, int bit_offset,
> + uint32_t mask_data)
> +{
> + DB_PRINT("mask data offset = %d, mask_data = %08X\n", bit_offset,
> mask_data);
> +
> + /* MASK_DATA registers are R/W on their data part */
> + bank->mask_data = deposit32(bank->mask_data, bit_offset, 16, mask_data);
> + bank->out_data = deposit32(bank->out_data, bit_offset, 16, mask_data);
> +
> + zynq_gpio_update_out(bank);
> +}
> +
> +static void zynq_gpio_data(ZynqGPIOBank *bank, uint32_t data)
> +{
> + bank->out_data = data;
> + zynq_gpio_update_out(bank);
> +}
> +
> +static void zynq_gpio_write(void *opaque, hwaddr offset,
> + uint64_t value, unsigned int size)
> +{
> + ZynqGPIOState *s = opaque;
> + int b;
> + int shift;
> + ZynqGPIOBank *bank;
> +
> + switch (offset) {
> + case ZYNQ_GPIO_REG_MASK_DATA_0_LSW ... ZYNQ_GPIO_REG_MASK_DATA_3_MSW:
> + b = extract32(offset - ZYNQ_GPIO_REG_MASK_DATA_0_LSW, 3, 2);
> + shift = (offset & 0x8) ? 16 : 0;
> + zynq_gpio_mask_data(&s->banks[b], shift, value);
> + return;
> +
> + case ZYNQ_GPIO_REG_DATA_0 ... ZYNQ_GPIO_REG_DATA_3:
> + b = (offset - ZYNQ_GPIO_REG_DATA_0) / 4;
> + zynq_gpio_data(&s->banks[b], value);
> + return;
> +
> + case ZYNQ_GPIO_REG_DATA_0_RO ... ZYNQ_GPIO_REG_DATA_3_RO:
> + return;
> + }
> +
> + if (offset < 0x204 || offset > 0x2e4) {
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "zynq_gpio_write: Bad offset %" HWADDR_PRIx "\n",
> offset);
> + return;
> + }
> +
> + b = (offset - 0x200) / 0x40;
> + bank = &s->banks[b];
> +
> + switch (offset - ZYNQ_GPIO_BANK_OFFSET(b)) {
> + case ZYNQ_GPIO_BANK_REG_DIRM:
> + bank->dir = value;
> + break;
> + case ZYNQ_GPIO_BANK_REG_OEN:
> + bank->oen = value;
> + qemu_log_mask(LOG_UNIMP,
> + "zynq_gpio_write: Output enable lines not
> implemented\n");
> + break;
> + case ZYNQ_GPIO_BANK_REG_INT_MASK:
> + return;
> + case ZYNQ_GPIO_BANK_REG_INT_EN:
> + bank->imask &= ~value;
> + break;
> + case ZYNQ_GPIO_BANK_REG_INT_DIS:
> + bank->imask |= value;
> + break;
> + case ZYNQ_GPIO_BANK_REG_INT_STAT:
> + bank->istat &= ~value;
> + break;
> + case ZYNQ_GPIO_BANK_REG_INT_TYPE:
> + bank->itype = value;
> + break;
> + case ZYNQ_GPIO_BANK_REG_INT_POLARITY:
> + bank->ipolarity = value;
> + break;
> + case ZYNQ_GPIO_BANK_REG_INT_ANY:
> + bank->iany = value;
> + break;
> + default:
> + qemu_log_mask(LOG_GUEST_ERROR,
> + "zynq_gpio_write: Bad offset %" HWADDR_PRIx "\n",
> offset);
> + return;
> + }
> +
> + zynq_gpio_update(s);
> +}
> +
> +static void zynq_gpio_reset(DeviceState *dev)
> +{
> + ZynqGPIOState *s = ZYNQ_GPIO(dev);
> + int b;
> +
> + for (b = 0; b < ZYNQ_GPIO_BANKS; b++) {
> + ZynqGPIOBank *bank = &s->banks[b];
> + int i;
> +
> + bank->mask_data = 0x00000000;
> + bank->dir = 0x00000000;
> + bank->oen = 0x00000000;
> + bank->imask = 0x00000000;
> + bank->istat = 0x00000000;
> + bank->itype = 0xffffffff;
> + bank->ipolarity = 0x00000000;
> + bank->iany = 0x00000000;
> +
> + bank->out_data = 0x00000000;
> + bank->old_out_data = 0x00000000;
> + for (i = 0; i < ZYNQ_GPIO_IOS_PER_BANK; i++) {
> + qemu_set_irq(bank->out[i], 0);
> + }
> +
> + bank->old_in_data = bank->in_data;
> + }
> +}
> +
> +static void zynq_gpio_set_irq(ZynqGPIOBank *bank, int irq, int level)
> +{
> + uint32_t mask = 1 << irq;
> +
> + bank->in_data &= ~mask;
> + if (level)
> + bank->in_data |= mask;
> +
You need braces {} around the if body. Checkpatch should catch this.
> + zynq_gpio_update_in(bank);
> +
> + zynq_gpio_set_in_irq(bank->parent);
> +}
> +
> +static void zynq_gpio_set_bank0_irq(void *opaque, int irq, int level)
> +{
> + ZynqGPIOState *s = opaque;
Blank line here between declarations and function body.
> + zynq_gpio_set_irq(&s->banks[0], irq, level);
> +}
> +
> +static void zynq_gpio_set_bank1_irq(void *opaque, int irq, int level)
> +{
> + ZynqGPIOState *s = opaque;
> + zynq_gpio_set_irq(&s->banks[1], irq, level);
> +}
> +
> +static void zynq_gpio_set_bank2_irq(void *opaque, int irq, int level)
> +{
> + ZynqGPIOState *s = opaque;
> + zynq_gpio_set_irq(&s->banks[2], irq, level);
> +}
> +
> +static void zynq_gpio_set_bank3_irq(void *opaque, int irq, int level)
> +{
> + ZynqGPIOState *s = opaque;
> + zynq_gpio_set_irq(&s->banks[3], irq, level);
> +}
> +
> +static const MemoryRegionOps zynq_gpio_ops = {
> + .read = zynq_gpio_read,
> + .write = zynq_gpio_write,
> + .endianness = DEVICE_NATIVE_ENDIAN,
> +};
> +
> +static void zynq_gpio_realize(DeviceState *dev, Error **errp)
> +{
> + ZynqGPIOState *s = ZYNQ_GPIO(dev);
> + int b;
> +
> + memory_region_init_io(&s->iomem, OBJECT(s), &zynq_gpio_ops, s,
> "zynq-gpio", 0x1000);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
> + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
> +
> + for (b = 0; b < ZYNQ_GPIO_BANKS; b++) {
> + ZynqGPIOBank *bank = &s->banks[b];
> + char name[16];
> +
> + memset(bank, 0, sizeof(*bank));
> +
Is this needed? QOM should init struct fields to 0 for you.
> + bank->parent = s;
> +
> + snprintf(name, sizeof(name), "bank%d_out", b);
> + qdev_init_gpio_out_named(dev, bank->out, name,
> ZYNQ_GPIO_IOS_PER_BANK);
> + /*
> + * TODO: it would be nice if we could pass the bank to the handler.
> This
> + * would allow us to remove the 4 callbacks and use zynq_gpio_set_irq
> + * directly.
> + */
> +#if 0
> + snprintf(name, sizeof(name), "bank%d_in", b);
> + qdev_init_gpio_in_named(dev, zynq_gpio_set_irq, name,
> ZYNQ_GPIO_IOS_PER_BANK, bank);
> +#endif
Yes I am seeing the problem here now. But this is a better solution
than V1 so I think we are good here, pending a rethink of the GPIO API
WRT this problem.
With the small style issues fixed and a checkpatch pass, please add
Reviewed-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
To the next rev.
Regards,
Peter
> + }
> + qdev_init_gpio_in_named(dev, zynq_gpio_set_bank0_irq, "bank0_in",
> ZYNQ_GPIO_IOS_PER_BANK);
> + qdev_init_gpio_in_named(dev, zynq_gpio_set_bank1_irq, "bank1_in",
> ZYNQ_GPIO_IOS_PER_BANK);
> + qdev_init_gpio_in_named(dev, zynq_gpio_set_bank2_irq, "bank2_in",
> ZYNQ_GPIO_IOS_PER_BANK);
> + qdev_init_gpio_in_named(dev, zynq_gpio_set_bank3_irq, "bank3_in",
> ZYNQ_GPIO_IOS_PER_BANK);
> +}
> +
> +static void zynq_gpio_class_init(ObjectClass *klass, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(klass);
> +
> + dc->realize = zynq_gpio_realize;
> + dc->reset = zynq_gpio_reset;
> +}
> +
> +static const TypeInfo zynq_gpio_info = {
> + .name = TYPE_ZYNQ_GPIO,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(ZynqGPIOState),
> + .class_init = zynq_gpio_class_init,
> +};
> +
> +static void zynq_gpio_register_type(void)
> +{
> + type_register_static(&zynq_gpio_info);
> +}
> +
> +type_init(zynq_gpio_register_type)
> diff --git a/include/hw/gpio/zynq-gpio.h b/include/hw/gpio/zynq-gpio.h
> new file mode 100644
> index 0000000..8146c1f
> --- /dev/null
> +++ b/include/hw/gpio/zynq-gpio.h
> @@ -0,0 +1,79 @@
> +/*
> + * Zynq General Purpose IO
> + *
> + * Copyright (C) 2014 Colin Leitner <colin.leit...@gmail.com>
> + *
> + * This code is licensed under the GPL.
> + */
> +
> +#ifndef HW_ZYNQ_GPIO_H
> +#define HW_ZYNQ_GPIO_H
> +
> +#include "hw/sysbus.h"
> +
> +#define TYPE_ZYNQ_GPIO "zynq-gpio"
> +#define ZYNQ_GPIO(obj) OBJECT_CHECK(ZynqGPIOState, (obj), TYPE_ZYNQ_GPIO)
> +
> +#define ZYNQ_GPIO_BANKS 4
> +#define ZYNQ_GPIO_IOS_PER_BANK 32
> +
> +typedef struct {
> + struct ZynqGPIOState *parent;
> +
> + uint32_t mask_data;
> + uint32_t out_data;
> + uint32_t old_out_data;
> + uint32_t in_data;
> + uint32_t old_in_data;
> + uint32_t dir;
> + uint32_t oen;
> + uint32_t imask;
> + uint32_t istat;
> + uint32_t itype;
> + uint32_t ipolarity;
> + uint32_t iany;
> +
> + qemu_irq out[ZYNQ_GPIO_IOS_PER_BANK];
> +} ZynqGPIOBank;
> +
> +typedef struct ZynqGPIOState {
> + SysBusDevice parent_obj;
> +
> + MemoryRegion iomem;
> + ZynqGPIOBank banks[ZYNQ_GPIO_BANKS];
> + qemu_irq irq;
> +} ZynqGPIOState;
> +
> +#define ZYNQ_GPIO_REG_MASK_DATA_0_LSW 0x000
> +#define ZYNQ_GPIO_REG_MASK_DATA_0_MSW 0x004
> +#define ZYNQ_GPIO_REG_MASK_DATA_1_LSW 0x008
> +#define ZYNQ_GPIO_REG_MASK_DATA_1_MSW 0x00C
> +#define ZYNQ_GPIO_REG_MASK_DATA_2_LSW 0x010
> +#define ZYNQ_GPIO_REG_MASK_DATA_2_MSW 0x014
> +#define ZYNQ_GPIO_REG_MASK_DATA_3_LSW 0x018
> +#define ZYNQ_GPIO_REG_MASK_DATA_3_MSW 0x01C
> +#define ZYNQ_GPIO_REG_DATA_0 0x040
> +#define ZYNQ_GPIO_REG_DATA_1 0x044
> +#define ZYNQ_GPIO_REG_DATA_2 0x048
> +#define ZYNQ_GPIO_REG_DATA_3 0x04C
> +#define ZYNQ_GPIO_REG_DATA_0_RO 0x060
> +#define ZYNQ_GPIO_REG_DATA_1_RO 0x064
> +#define ZYNQ_GPIO_REG_DATA_2_RO 0x068
> +#define ZYNQ_GPIO_REG_DATA_3_RO 0x06C
> +
> +/*
> + * Oddly enough these registers are neatly grouped per bank and not
> interleaved
> + * like the data registers
> + */
> +#define ZYNQ_GPIO_BANK_OFFSET(bank) (0x200 + 0x40 * (bank))
> +#define ZYNQ_GPIO_BANK_REG_DIRM 0x04
> +#define ZYNQ_GPIO_BANK_REG_OEN 0x08
> +#define ZYNQ_GPIO_BANK_REG_INT_MASK 0x0C
> +#define ZYNQ_GPIO_BANK_REG_INT_EN 0x10
> +#define ZYNQ_GPIO_BANK_REG_INT_DIS 0x14
> +#define ZYNQ_GPIO_BANK_REG_INT_STAT 0x18
> +#define ZYNQ_GPIO_BANK_REG_INT_TYPE 0x1C
> +#define ZYNQ_GPIO_BANK_REG_INT_POLARITY 0x20
> +#define ZYNQ_GPIO_BANK_REG_INT_ANY 0x24
> +
> +#endif
> --
> 1.7.10.4
>
>
