All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v1 0/4]  Connect the Xilinx ZynqMP IPI device
@ 2016-07-05 20:30 Alistair Francis
  2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 1/4] irq: Add opaque setter routine Alistair Francis
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Alistair Francis @ 2016-07-05 20:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: alistair.francis, crosthwaitepeter, peter.maydell,
	edgar.iglesias, edgar.iglesias

This patchset adds and connects the Xilinx ZynqmP IPI devices using the
register GPIO line.

This requires adding the register GPIO API which allows registers to be
mapped to GPIOs. This GPIO is used to propergate register reads/writes
to other devices and other registers.

This is useful to update register values in other devices when events
occur.

Alistair Francis (3):
  register: Add GPIO API
  xlnx-zynqmp-ipi: Add the IPI device
  zynqmp: Connect the IPI devices

Peter Crosthwaite (1):
  irq: Add opaque setter routine

 hw/arm/xlnx-zynqmp.c              | 102 +++++++++++++++
 hw/core/irq.c                     |   5 +
 hw/core/register.c                |  97 ++++++++++++++
 hw/intc/Makefile.objs             |   1 +
 hw/intc/xlnx-zynqmp-ipi.c         | 263 ++++++++++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynqmp.h      |   3 +
 include/hw/intc/xlnx-zynqmp-ipi.h | 116 +++++++++++++++++
 include/hw/irq.h                  |   2 +
 include/hw/register.h             |  31 ++++-
 9 files changed, 619 insertions(+), 1 deletion(-)
 create mode 100644 hw/intc/xlnx-zynqmp-ipi.c
 create mode 100644 include/hw/intc/xlnx-zynqmp-ipi.h

-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v1 1/4] irq: Add opaque setter routine
  2016-07-05 20:30 [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device Alistair Francis
@ 2016-07-05 20:30 ` Alistair Francis
  2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 2/4] register: Add GPIO API Alistair Francis
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2016-07-05 20:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: alistair.francis, crosthwaitepeter, peter.maydell,
	edgar.iglesias, edgar.iglesias

From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Add a routine to set or override the opaque data of an IRQ.

Qdev currently always initialises IRQ opaque as the device itself.
This allows you to override to a custom opaque in the case where
there is extra or different data needed.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/core/irq.c    | 5 +++++
 include/hw/irq.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/hw/core/irq.c b/hw/core/irq.c
index 49ff2e6..9d125fb 100644
--- a/hw/core/irq.c
+++ b/hw/core/irq.c
@@ -77,6 +77,11 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n)
     return irq;
 }
 
+void qemu_irq_set_opaque(qemu_irq irq, void *opaque)
+{
+    irq->opaque = opaque;
+}
+
 void qemu_free_irqs(qemu_irq *s, int n)
 {
     int i;
diff --git a/include/hw/irq.h b/include/hw/irq.h
index 4c4c2ea..edad0fc 100644
--- a/include/hw/irq.h
+++ b/include/hw/irq.h
@@ -44,6 +44,8 @@ qemu_irq qemu_allocate_irq(qemu_irq_handler handler, void *opaque, int n);
 qemu_irq *qemu_extend_irqs(qemu_irq *old, int n_old, qemu_irq_handler handler,
                                 void *opaque, int n);
 
+void qemu_irq_set_opaque(qemu_irq irq, void *opaque);
+
 void qemu_free_irqs(qemu_irq *s, int n);
 void qemu_free_irq(qemu_irq irq);
 
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v1 2/4] register: Add GPIO API
  2016-07-05 20:30 [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device Alistair Francis
  2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 1/4] irq: Add opaque setter routine Alistair Francis
@ 2016-07-05 20:30 ` Alistair Francis
  2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 3/4] xlnx-zynqmp-ipi: Add the IPI device Alistair Francis
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2016-07-05 20:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: alistair.francis, crosthwaitepeter, peter.maydell,
	edgar.iglesias, edgar.iglesias

Add GPIO functionality to the register API. This allows association
and automatic connection of GPIOs to bits in registers. GPIO inputs
will attach to handlers that automatically set read-only bits in
registers. GPIO outputs will be updated to reflect their field value
when their respective registers are written (or reset). Supports
active low GPIOs.

This is particularly effective for implementing system level
controllers, where heterogenous collections of control signals are
placed is a SoC specific peripheral then propagated all over the
system.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/core/register.c    | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/register.h | 31 +++++++++++++++-
 2 files changed, 127 insertions(+), 1 deletion(-)

diff --git a/hw/core/register.c b/hw/core/register.c
index 4bfbc50..3987af6 100644
--- a/hw/core/register.c
+++ b/hw/core/register.c
@@ -108,6 +108,7 @@ void register_write(RegisterInfo *reg, uint64_t val, uint64_t we,
     }
 
     register_write_val(reg, new_val);
+    register_refresh_gpios(reg, old_val, debug);
 
     if (ac->post_write) {
         ac->post_write(reg, new_val);
@@ -151,23 +152,119 @@ uint64_t register_read(RegisterInfo *reg, uint64_t re, const char* prefix,
 void register_reset(RegisterInfo *reg)
 {
     g_assert(reg);
+    uint64_t old_val;
 
     if (!reg->data || !reg->access) {
         return;
     }
 
+    old_val = register_read_val(reg);
+
     register_write_val(reg, reg->access->reset);
+    register_refresh_gpios(reg, old_val, false);
+}
+
+void register_refresh_gpios(RegisterInfo *reg, uint64_t old_value, bool debug)
+{
+    const RegisterAccessInfo *ac;
+    const RegisterGPIOMapping *gpio;
+
+    ac = reg->access;
+    for (gpio = ac->gpios; gpio && gpio->name; gpio++) {
+        int i;
+
+        if (gpio->input) {
+            continue;
+        }
+
+        for (i = 0; i < gpio->num; ++i) {
+            uint64_t gpio_value, gpio_value_old;
+
+            qemu_irq gpo = qdev_get_gpio_out_connector(DEVICE(reg),
+                                                       gpio->name, i);
+            gpio_value_old = extract64(old_value,
+                                       gpio->bit_pos + i * gpio->width,
+                                       gpio->width) ^ gpio->polarity;
+            gpio_value = extract64(register_read_val(reg),
+                                   gpio->bit_pos + i * gpio->width,
+                                   gpio->width) ^ gpio->polarity;
+            if (!(gpio_value_old ^ gpio_value)) {
+                continue;
+            }
+            if (debug && gpo) {
+                qemu_log("refreshing gpio out %s to %" PRIx64 "\n",
+                         gpio->name, gpio_value);
+            }
+            qemu_set_irq(gpo, gpio_value);
+        }
+    }
+}
+
+typedef struct DeviceNamedGPIOHandlerOpaque {
+    DeviceState *dev;
+    const char *name;
+} DeviceNamedGPIOHandlerOpaque;
+
+static void register_gpio_handler(void *opaque, int n, int level)
+{
+    DeviceNamedGPIOHandlerOpaque *gho = opaque;
+    RegisterInfo *reg = REGISTER(gho->dev);
+
+    const RegisterAccessInfo *ac;
+    const RegisterGPIOMapping *gpio;
+
+    ac = reg->access;
+    for (gpio = ac->gpios; gpio && gpio->name; gpio++) {
+        if (gpio->input && !strcmp(gho->name, gpio->name)) {
+            register_write_val(reg, deposit64(register_read_val(reg),
+                                              gpio->bit_pos + n * gpio->width,
+                                              gpio->width,
+                                              level ^ gpio->polarity));
+            return;
+        }
+    }
+
+    abort();
 }
 
 void register_init(RegisterInfo *reg)
 {
     assert(reg);
+    const RegisterAccessInfo *ac;
+    const RegisterGPIOMapping *gpio;
 
     if (!reg->data || !reg->access) {
         return;
     }
 
     object_initialize((void *)reg, sizeof(*reg), TYPE_REGISTER);
+
+    ac = reg->access;
+    for (gpio = ac->gpios; gpio && gpio->name; gpio++) {
+        if (!gpio->num) {
+            ((RegisterGPIOMapping *)gpio)->num = 1;
+        }
+        if (!gpio->width) {
+            ((RegisterGPIOMapping *)gpio)->width = 1;
+        }
+        if (gpio->input) {
+            DeviceNamedGPIOHandlerOpaque gho = {
+                .name = gpio->name,
+                .dev = DEVICE(reg),
+            };
+            qemu_irq irq;
+
+            qdev_init_gpio_in_named(DEVICE(reg), register_gpio_handler,
+                                    gpio->name, gpio->num);
+            irq = qdev_get_gpio_in_named(DEVICE(reg), gpio->name, gpio->num);
+            qemu_irq_set_opaque(irq, g_memdup(&gho, sizeof(gho)));
+        } else {
+            qemu_irq *gpos = g_new0(qemu_irq, gpio->num);
+
+            qdev_init_gpio_out_named(DEVICE(reg), gpos, gpio->name, gpio->num);
+            qdev_pass_gpios(DEVICE(reg), reg->opaque, gpio->name);
+        }
+    }
 }
 
 void register_write_memory(void *opaque, hwaddr addr,
diff --git a/include/hw/register.h b/include/hw/register.h
index 8c12233..94967a4 100644
--- a/include/hw/register.h
+++ b/include/hw/register.h
@@ -13,11 +13,24 @@
 
 #include "hw/qdev-core.h"
 #include "exec/memory.h"
+#include "hw/irq.h"
 
 typedef struct RegisterInfo RegisterInfo;
 typedef struct RegisterAccessInfo RegisterAccessInfo;
 typedef struct RegisterInfoArray RegisterInfoArray;
 
+#define REG_GPIO_POL_HIGH 0
+#define REG_GPIO_POL_LOW  1
+
+typedef struct RegisterGPIOMapping {
+    const char *name;
+    uint8_t bit_pos;
+    bool input;
+    bool polarity;
+    uint8_t num;
+    uint8_t width;
+} RegisterGPIOMapping;
+
 /**
  * Access description for a register that is part of guest accessible device
  * state.
@@ -55,6 +68,8 @@ struct RegisterAccessInfo {
     uint64_t (*post_read)(RegisterInfo *reg, uint64_t val);
 
     hwaddr addr;
+
+    const RegisterGPIOMapping *gpios;
 };
 
 /**
@@ -141,13 +156,27 @@ uint64_t register_read(RegisterInfo *reg, uint64_t re, const char* prefix,
 void register_reset(RegisterInfo *reg);
 
 /**
- * Initialize a register.
+ * Initialize a register. This will also setup any GPIO links which are used
+ * to connect register updates in one device to other devices. Generally this
+ * is useful for interrupt propagation.
  * @reg: Register to initialize
  */
 
 void register_init(RegisterInfo *reg);
 
 /**
+ * Refresh GPIO outputs based on diff between old value register current value.
+ * GPIOs are refreshed for fields where the old value differs to the current
+ * value.
+ *
+ * @reg: Register to refresh GPIO outs
+ * @old_value: previous value of register
+ * @debug: Should the read operation debug information be printed?
+ */
+
+void register_refresh_gpios(RegisterInfo *reg, uint64_t old_value, bool debug);
+
+/**
  * Memory API MMIO write handler that will write to a Register API register.
  * @opaque: RegisterInfo to write to
  * @addr: Address to write
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v1 3/4] xlnx-zynqmp-ipi: Add the IPI device
  2016-07-05 20:30 [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device Alistair Francis
  2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 1/4] irq: Add opaque setter routine Alistair Francis
  2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 2/4] register: Add GPIO API Alistair Francis
@ 2016-07-05 20:30 ` Alistair Francis
  2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 4/4] zynqmp: Connect the IPI devices Alistair Francis
  2016-07-05 20:37 ` [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device Peter Maydell
  4 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2016-07-05 20:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: alistair.francis, crosthwaitepeter, peter.maydell,
	edgar.iglesias, edgar.iglesias

Add the Xilinx ZynqMP Inter Processor Interrupt device.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/intc/Makefile.objs             |   1 +
 hw/intc/xlnx-zynqmp-ipi.c         | 263 ++++++++++++++++++++++++++++++++++++++
 include/hw/intc/xlnx-zynqmp-ipi.h | 116 +++++++++++++++++
 3 files changed, 380 insertions(+)
 create mode 100644 hw/intc/xlnx-zynqmp-ipi.c
 create mode 100644 include/hw/intc/xlnx-zynqmp-ipi.h

diff --git a/hw/intc/Makefile.objs b/hw/intc/Makefile.objs
index 530df2e..2f63bd7 100644
--- a/hw/intc/Makefile.objs
+++ b/hw/intc/Makefile.objs
@@ -37,3 +37,4 @@ obj-$(CONFIG_S390_FLIC) += s390_flic.o
 obj-$(CONFIG_S390_FLIC_KVM) += s390_flic_kvm.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_vic.o
 obj-$(CONFIG_ARM_GIC) += arm_gicv3_cpuif.o
+obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp-ipi.o
diff --git a/hw/intc/xlnx-zynqmp-ipi.c b/hw/intc/xlnx-zynqmp-ipi.c
new file mode 100644
index 0000000..f5f9c97
--- /dev/null
+++ b/hw/intc/xlnx-zynqmp-ipi.c
@@ -0,0 +1,263 @@
+/*
+ * QEMU model of the Xilinx ZynqMP Inter Processor Interrupt block
+ *
+ * Copyright (c) 2016 Xilinx Inc.
+ * Written by Edgar E. Iglesias <edgar.iglesias@xilinx.com>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sysbus.h"
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+#include "hw/intc/xlnx-zynqmp-ipi.h"
+
+#ifndef XLNX_ZYNQMP_IPI_ERR_DEBUG
+#define XLNX_ZYNQMP_IPI_ERR_DEBUG 0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do {\
+    if (XLNX_ZYNQMP_IPI_ERR_DEBUG >= lvl) {\
+        qemu_log(TYPE_XLNX_ZYNQMP_IPI ": %s:" fmt, __func__, ## args);\
+    } \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+static void xlnx_zynqmp_ipi_update_irq(XlnxZynqMPIPI *s)
+{
+    bool pending = s->regs[R_IPI_ISR] & ~s->regs[R_IPI_IMR];
+
+    DB_PRINT("%s: irq=%d isr=%x mask=%x\n",
+             object_get_canonical_path(OBJECT(s)),
+             pending, s->regs[R_IPI_ISR], s->regs[R_IPI_IMR]);
+
+    qemu_set_irq(s->irq, pending);
+}
+
+static void xlnx_zynqmp_ipi_isr_postw(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPIPI *s = XLNX_ZYNQMP_IPI(reg->opaque);
+
+    xlnx_zynqmp_ipi_update_irq(s);
+}
+
+static uint64_t xlnx_zynqmp_ipi_ier_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPIPI *s = XLNX_ZYNQMP_IPI(reg->opaque);
+    uint32_t val = val64;
+
+    s->regs[R_IPI_IMR] &= ~val;
+    xlnx_zynqmp_ipi_update_irq(s);
+
+    return 0;
+}
+
+static uint64_t xlnx_zynqmp_ipi_idr_prew(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPIPI *s = XLNX_ZYNQMP_IPI(reg->opaque);
+    uint32_t val = val64;
+
+    s->regs[R_IPI_IMR] |= val;
+    xlnx_zynqmp_ipi_update_irq(s);
+
+    return 0;
+}
+
+static void xlnx_zynqmp_ipi_trig_postw(RegisterInfo *reg, uint64_t val64)
+{
+    XlnxZynqMPIPI *s = XLNX_ZYNQMP_IPI(reg->opaque);
+    uint64_t old_value = s->regs[R_IPI_TRIG];
+
+    /* TRIG generates a pulse on the outbound signals. We use the
+     * post-write callback to bring the signal back-down.
+     */
+    s->regs[R_IPI_TRIG] = 0;
+    register_refresh_gpios(reg, old_value, XLNX_ZYNQMP_IPI_ERR_DEBUG);
+}
+
+static RegisterAccessInfo xlnx_zynqmp_ipi_regs_info[] = {
+    {   .name = "IPI_TRIG", .addr = A_IPI_TRIG,
+        .rsvd = 0xf0f0fcfe, .ro = 0xf0f0fcfe,
+        .post_write = xlnx_zynqmp_ipi_trig_postw,
+        .gpios = (RegisterGPIOMapping[]) {
+            { .name = "APU", .bit_pos = R_IPI_TRIG_APU_SHIFT, .width = 1 },
+            { .name = "RPU_0", .bit_pos = R_IPI_TRIG_RPU_0_SHIFT, .width = 1 },
+            { .name = "RPU_1", .bit_pos = R_IPI_TRIG_RPU_1_SHIFT, .width = 1 },
+            { .name = "PMU_0", .bit_pos = R_IPI_TRIG_PMU_0_SHIFT, .width = 1 },
+            { .name = "PMU_1", .bit_pos = R_IPI_TRIG_PMU_1_SHIFT, .width = 1 },
+            { .name = "PMU_2", .bit_pos = R_IPI_TRIG_PMU_2_SHIFT, .width = 1 },
+            { .name = "PMU_3", .bit_pos = R_IPI_TRIG_PMU_3_SHIFT, .width = 1 },
+            { .name = "PL_0", .bit_pos = R_IPI_TRIG_PL_0_SHIFT, .width = 1 },
+            { .name = "PL_1", .bit_pos = R_IPI_TRIG_PL_1_SHIFT, .width = 1 },
+            { .name = "PL_2", .bit_pos = R_IPI_TRIG_PL_2_SHIFT, .width = 1 },
+            { .name = "PL_3", .bit_pos = R_IPI_TRIG_PL_3_SHIFT, .width = 1 },
+            { },
+        }
+    },{ .name = "IPI_OBS", .addr = A_IPI_OBS,
+        .rsvd = 0xf0f0fcfe, .ro = 0xffffffff,
+    },{ .name = "IPI_ISR", .addr = A_IPI_ISR,
+        .rsvd = 0xf0f0fcfe, .ro = 0xf0f0fcfe, .w1c = 0xf0f0301,
+        .post_write = xlnx_zynqmp_ipi_isr_postw,
+        .gpios = (RegisterGPIOMapping[]) {
+            { .name = "OBS_APU", .bit_pos = R_IPI_ISR_APU_SHIFT, .width = 1 },
+            { .name = "OBS_RPU_0", .bit_pos = R_IPI_ISR_RPU_0_SHIFT,
+              .width = 1 },
+            { .name = "OBS_RPU_1", .bit_pos = R_IPI_ISR_RPU_1_SHIFT,
+              .width = 1 },
+            { .name = "OBS_PMU_0", .bit_pos = R_IPI_ISR_PMU_0_SHIFT,
+              .width = 1 },
+            { .name = "OBS_PMU_1", .bit_pos = R_IPI_ISR_PMU_1_SHIFT,
+              .width = 1 },
+            { .name = "OBS_PMU_2", .bit_pos = R_IPI_ISR_PMU_2_SHIFT,
+              .width = 1 },
+            { .name = "OBS_PMU_3", .bit_pos = R_IPI_ISR_PMU_3_SHIFT,
+              .width = 1 },
+            { .name = "OBS_PL_0", .bit_pos = R_IPI_ISR_PL_0_SHIFT,
+              .width = 1 },
+            { .name = "OBS_PL_1", .bit_pos = R_IPI_ISR_PL_1_SHIFT,
+              .width = 1 },
+            { .name = "OBS_PL_2", .bit_pos = R_IPI_ISR_PL_2_SHIFT,
+              .width = 1 },
+            { .name = "OBS_PL_3", .bit_pos = R_IPI_ISR_PL_3_SHIFT,
+              .width = 1 },
+            { },
+        }
+    },{ .name = "IPI_IMR", .addr = A_IPI_IMR,
+        .reset = 0xf0f0301, .rsvd = 0xf0f0fcfe,
+        .ro = 0xffffffff,
+    },{ .name = "IPI_IER", .addr = A_IPI_IER,
+        .rsvd = 0xf0f0fcfe, .ro = 0xf0f0fcfe,
+        .pre_write = xlnx_zynqmp_ipi_ier_prew,
+    },{ .name = "IPI_IDR", .addr = A_IPI_IDR,
+        .rsvd = 0xf0f0fcfe, .ro = 0xf0f0fcfe,
+        .pre_write = xlnx_zynqmp_ipi_idr_prew,
+    }
+};
+
+static void xlnx_zynqmp_ipi_reset(DeviceState *dev)
+{
+    XlnxZynqMPIPI *s = XLNX_ZYNQMP_IPI(dev);
+    unsigned int i;
+
+    for (i = 0; i < ARRAY_SIZE(s->regs_info); ++i) {
+        register_reset(&s->regs_info[i]);
+    }
+
+    xlnx_zynqmp_ipi_update_irq(s);
+}
+
+static void xlnx_zynqmp_ipi_handler(void *opaque, int n, int level)
+{
+    XlnxZynqMPIPI *s = XLNX_ZYNQMP_IPI(opaque);
+    RegisterInfo *r_isr = &s->regs_info[A_IPI_ISR / 4];
+
+    uint32_t val = (!!level) << n;
+    uint64_t old_value = s->regs[R_IPI_ISR];
+
+    DB_PRINT("%s: %s: irq[%d]=%d\n", __func__,
+             object_get_canonical_path(OBJECT(s)), n, level);
+
+    s->regs[R_IPI_ISR] |= val;
+
+    xlnx_zynqmp_ipi_update_irq(s);
+    register_refresh_gpios(r_isr, old_value, XLNX_ZYNQMP_IPI_ERR_DEBUG);
+}
+
+static void xlnx_zynqmp_obs_handler(void *opaque, int n, int level)
+{
+    XlnxZynqMPIPI *s = XLNX_ZYNQMP_IPI(opaque);
+
+    s->regs[R_IPI_OBS] &= ~(1ULL << n);
+    s->regs[R_IPI_OBS] |= (level << n);
+}
+
+static const MemoryRegionOps xlnx_zynqmp_ipi_ops = {
+    .read = register_read_memory,
+    .write = register_write_memory,
+    .endianness = DEVICE_LITTLE_ENDIAN,
+    .valid = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
+};
+
+static void xlnx_zynqmp_ipi_realize(DeviceState *dev, Error **errp)
+{
+    qdev_init_gpio_in_named(dev, xlnx_zynqmp_ipi_handler, "IPI_INPUTS", 32);
+    qdev_init_gpio_in_named(dev, xlnx_zynqmp_obs_handler, "OBS_INPUTS", 32);
+}
+
+static void xlnx_zynqmp_ipi_init(Object *obj)
+{
+    XlnxZynqMPIPI *s = XLNX_ZYNQMP_IPI(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    RegisterInfoArray *reg_array;
+
+    memory_region_init_io(&s->iomem, obj, &xlnx_zynqmp_ipi_ops, s,
+                          TYPE_XLNX_ZYNQMP_IPI, R_MAX * 4);
+    reg_array =
+        register_init_block32(DEVICE(obj), xlnx_zynqmp_ipi_regs_info,
+                              ARRAY_SIZE(xlnx_zynqmp_ipi_regs_info),
+                              s->regs_info, s->regs,
+                              &xlnx_zynqmp_ipi_ops, XLNX_ZYNQMP_IPI_ERR_DEBUG,
+                              R_MAX);
+    memory_region_add_subregion(&s->iomem,
+                                A_IPI_IDR,
+                                &reg_array->mem);
+
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq);
+}
+
+static const VMStateDescription vmstate_ipi = {
+    .name = TYPE_XLNX_ZYNQMP_IPI,
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32_ARRAY(regs, XlnxZynqMPIPI, R_MAX),
+        VMSTATE_END_OF_LIST(),
+    }
+};
+
+static void xlnx_zynqmp_ipi_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->reset = xlnx_zynqmp_ipi_reset;
+    dc->realize = xlnx_zynqmp_ipi_realize;
+    dc->vmsd = &vmstate_ipi;
+}
+
+static const TypeInfo xlnx_zynqmp_ipi_info = {
+    .name          = TYPE_XLNX_ZYNQMP_IPI,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(XlnxZynqMPIPI),
+    .class_init    = xlnx_zynqmp_ipi_class_init,
+    .instance_init = xlnx_zynqmp_ipi_init,
+};
+
+static void xlnx_zynqmp_ipi_register_types(void)
+{
+    type_register_static(&xlnx_zynqmp_ipi_info);
+}
+
+type_init(xlnx_zynqmp_ipi_register_types)
diff --git a/include/hw/intc/xlnx-zynqmp-ipi.h b/include/hw/intc/xlnx-zynqmp-ipi.h
new file mode 100644
index 0000000..065d130
--- /dev/null
+++ b/include/hw/intc/xlnx-zynqmp-ipi.h
@@ -0,0 +1,116 @@
+/*
+ * QEMU model of the Xilinx ZynqMP Inter Processor Interrupt block
+ *
+ * Copyright (c) 2016 Xilinx Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/register.h"
+
+#define TYPE_XLNX_ZYNQMP_IPI "xlnx.zynqmp.ipi"
+
+#define XLNX_ZYNQMP_IPI(obj) \
+     OBJECT_CHECK(XlnxZynqMPIPI, (obj), TYPE_XLNX_ZYNQMP_IPI)
+
+REG32(IPI_TRIG, 0x0)
+    FIELD(IPI_TRIG, PL_3, 27, 1)
+    FIELD(IPI_TRIG, PL_2, 26, 1)
+    FIELD(IPI_TRIG, PL_1, 25, 1)
+    FIELD(IPI_TRIG, PL_0, 24, 1)
+    FIELD(IPI_TRIG, PMU_3, 19, 1)
+    FIELD(IPI_TRIG, PMU_2, 18, 1)
+    FIELD(IPI_TRIG, PMU_1, 17, 1)
+    FIELD(IPI_TRIG, PMU_0, 16, 1)
+    FIELD(IPI_TRIG, RPU_1, 9, 1)
+    FIELD(IPI_TRIG, RPU_0, 8, 1)
+    FIELD(IPI_TRIG, APU, 0, 1)
+REG32(IPI_OBS, 0x4)
+    FIELD(IPI_OBS, PL_3, 27, 1)
+    FIELD(IPI_OBS, PL_2, 26, 1)
+    FIELD(IPI_OBS, PL_1, 25, 1)
+    FIELD(IPI_OBS, PL_0, 24, 1)
+    FIELD(IPI_OBS, PMU_3, 19, 1)
+    FIELD(IPI_OBS, PMU_2, 18, 1)
+    FIELD(IPI_OBS, PMU_1, 17, 1)
+    FIELD(IPI_OBS, PMU_0, 16, 1)
+    FIELD(IPI_OBS, RPU_1, 9, 1)
+    FIELD(IPI_OBS, RPU_0, 8, 1)
+    FIELD(IPI_OBS, APU, 0, 1)
+REG32(IPI_ISR, 0x10)
+    FIELD(IPI_ISR, PL_3, 27, 1)
+    FIELD(IPI_ISR, PL_2, 26, 1)
+    FIELD(IPI_ISR, PL_1, 25, 1)
+    FIELD(IPI_ISR, PL_0, 24, 1)
+    FIELD(IPI_ISR, PMU_3, 19, 1)
+    FIELD(IPI_ISR, PMU_2, 18, 1)
+    FIELD(IPI_ISR, PMU_1, 17, 1)
+    FIELD(IPI_ISR, PMU_0, 16, 1)
+    FIELD(IPI_ISR, RPU_1, 9, 1)
+    FIELD(IPI_ISR, RPU_0, 8, 1)
+    FIELD(IPI_ISR, APU, 0, 1)
+REG32(IPI_IMR, 0x14)
+    FIELD(IPI_IMR, PL_3, 27, 1)
+    FIELD(IPI_IMR, PL_2, 26, 1)
+    FIELD(IPI_IMR, PL_1, 25, 1)
+    FIELD(IPI_IMR, PL_0, 24, 1)
+    FIELD(IPI_IMR, PMU_3, 19, 1)
+    FIELD(IPI_IMR, PMU_2, 18, 1)
+    FIELD(IPI_IMR, PMU_1, 17, 1)
+    FIELD(IPI_IMR, PMU_0, 16, 1)
+    FIELD(IPI_IMR, RPU_1, 9, 1)
+    FIELD(IPI_IMR, RPU_0, 8, 1)
+    FIELD(IPI_IMR, APU, 0, 1)
+REG32(IPI_IER, 0x18)
+    FIELD(IPI_IER, PL_3, 27, 1)
+    FIELD(IPI_IER, PL_2, 26, 1)
+    FIELD(IPI_IER, PL_1, 25, 1)
+    FIELD(IPI_IER, PL_0, 24, 1)
+    FIELD(IPI_IER, PMU_3, 19, 1)
+    FIELD(IPI_IER, PMU_2, 18, 1)
+    FIELD(IPI_IER, PMU_1, 17, 1)
+    FIELD(IPI_IER, PMU_0, 16, 1)
+    FIELD(IPI_IER, RPU_1, 9, 1)
+    FIELD(IPI_IER, RPU_0, 8, 1)
+    FIELD(IPI_IER, APU, 0, 1)
+REG32(IPI_IDR, 0x1c)
+    FIELD(IPI_IDR, PL_3, 27, 1)
+    FIELD(IPI_IDR, PL_2, 26, 1)
+    FIELD(IPI_IDR, PL_1, 25, 1)
+    FIELD(IPI_IDR, PL_0, 24, 1)
+    FIELD(IPI_IDR, PMU_3, 19, 1)
+    FIELD(IPI_IDR, PMU_2, 18, 1)
+    FIELD(IPI_IDR, PMU_1, 17, 1)
+    FIELD(IPI_IDR, PMU_0, 16, 1)
+    FIELD(IPI_IDR, RPU_1, 9, 1)
+    FIELD(IPI_IDR, RPU_0, 8, 1)
+    FIELD(IPI_IDR, APU, 0, 1)
+
+#define R_MAX (R_IPI_IDR + 1)
+
+typedef struct XlnxZynqMPIPI {
+    SysBusDevice parent_obj;
+
+    MemoryRegion iomem;
+    qemu_irq irq;
+
+    uint32_t regs[R_MAX];
+    RegisterInfo regs_info[R_MAX];
+} XlnxZynqMPIPI;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH v1 4/4] zynqmp: Connect the IPI devices
  2016-07-05 20:30 [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device Alistair Francis
                   ` (2 preceding siblings ...)
  2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 3/4] xlnx-zynqmp-ipi: Add the IPI device Alistair Francis
@ 2016-07-05 20:30 ` Alistair Francis
  2016-07-05 20:37 ` [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device Peter Maydell
  4 siblings, 0 replies; 8+ messages in thread
From: Alistair Francis @ 2016-07-05 20:30 UTC (permalink / raw)
  To: qemu-devel
  Cc: alistair.francis, crosthwaitepeter, peter.maydell,
	edgar.iglesias, edgar.iglesias

Connect the Xilinx ZynqMP Inter Processor Interrupt (IPI) devices to the
ZynqMP SoC. This includes connecting the devices to each other.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 hw/arm/xlnx-zynqmp.c         | 102 +++++++++++++++++++++++++++++++++++++++++++
 include/hw/arm/xlnx-zynqmp.h |   3 ++
 2 files changed, 105 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 23c7199..efa05bf 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -64,6 +64,15 @@ static const uint64_t sdhci_addr[XLNX_ZYNQMP_NUM_SDHCI] = {
     0xFF160000, 0xFF170000,
 };
 
+/* The IPI devices: APU, RPU_0, RPU_1, PMU_0, PMU_1, PMU_2, PMU_3, PL_0, PL_1,
+ * PL_2 and PL_3
+ */
+static const uint64_t ipi_addr[XLNX_ZYNQMP_NUM_IPIS] = {
+    0xFF300000, 0xFF310000, 0xFF320000, 0xFF330000, 0xFF331000,
+    0xFF332000, 0xFF333000, 0xFF340000, 0xFF350000, 0xFF360000,
+    0xFF370000,
+};
+
 static const int sdhci_intr[XLNX_ZYNQMP_NUM_SDHCI] = {
     48, 49,
 };
@@ -76,6 +85,10 @@ static const int spi_intr[XLNX_ZYNQMP_NUM_SPIS] = {
     19, 20,
 };
 
+static const int ipi_intr[XLNX_ZYNQMP_NUM_IPIS] = {
+    35, 33, 34, 19, 20, 21, 22, 29, 30, 31, 32
+};
+
 typedef struct XlnxZynqMPGICRegion {
     int region_index;
     uint32_t address;
@@ -177,6 +190,12 @@ static void xlnx_zynqmp_init(Object *obj)
 
     object_initialize(&s->dpdma, sizeof(s->dpdma), TYPE_XLNX_DPDMA);
     qdev_set_parent_bus(DEVICE(&s->dpdma), sysbus_get_default());
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_IPIS; i++) {
+        object_initialize(&s->ipi[i], sizeof(s->ipi[i]),
+                          TYPE_XLNX_ZYNQMP_IPI);
+        qdev_set_parent_bus(DEVICE(&s->ipi[i]), sysbus_get_default());
+    }
 }
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -420,6 +439,89 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
                              &error_abort);
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->dpdma), 0, DPDMA_ADDR);
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->dpdma), 0, gic_spi[DPDMA_IRQ]);
+
+    for (i = 0; i < XLNX_ZYNQMP_NUM_IPIS; i++) {
+        object_property_set_bool(OBJECT(&s->ipi[i]), true, "realized", &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ipi[i]), 0, ipi_addr[i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ipi[i]), 0,
+                           gic_spi[ipi_intr[i]]);
+        /* TODO: Connect this to the other GICs */
+    }
+
+    /* Connect the GPIO lines between the IPI devices */
+    for (i = 0; i < XLNX_ZYNQMP_NUM_IPIS; i++) {
+        qemu_irq irq;
+
+        /* Connec the APU line for each device to the APU IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[0]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "APU", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[0]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_APU", 0, irq);
+
+        /* Connec the RPU_0 line for each device to the RPU_0 IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[1]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "RPU_0", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[1]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_RPU_0", 0, irq);
+
+        /* Connec the RPU_1 line for each device to the RPU_1 IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[2]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "RPU_1", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[2]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_RPU_1", 0, irq);
+
+        /* Connec the PMU_0 line for each device to the PMU_0 IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[3]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "PMU_0", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[3]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_PMU_0", 0, irq);
+
+        /* Connec the PMU_1 line for each device to the PMU_1 IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[4]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "PMU_1", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[4]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_PMU_1", 0, irq);
+
+        /* Connec the PMU_2 line for each device to the PMU_2 IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[5]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "PMU_2", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[5]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_PMU_2", 0, irq);
+
+        /* Connec the PMU_3 line for each device to the PMU_3 IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[6]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "PMU_3", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[6]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_PMU_3", 0, irq);
+
+        /* Connec the PL_0 line for each device to the PL_0 IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[7]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "PL_0", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[7]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_PL_0", 0, irq);
+
+        /* Connec the PL_1 line for each device to the PL_1 IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[8]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "PL_1", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[8]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_PL_1", 0, irq);
+
+        /* Connec the PL_2 line for each device to the PL_2 IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[9]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "PL_2", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[9]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_PL_2", 0, irq);
+
+        /* Connec the PL_3 line for each device to the PL_3 IPI deivice */
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[10]), "IPI_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "PL_3", 0, irq);
+        irq = qdev_get_gpio_in_named(DEVICE(&s->ipi[10]), "OBS_INPUTS", i);
+        qdev_connect_gpio_out_named(DEVICE(&s->ipi[i]), "OBS_PL_3", 0, irq);
+    }
 }
 
 static Property xlnx_zynqmp_props[] = {
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index c2931bf..9a7adf8 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -28,6 +28,7 @@
 #include "hw/ssi/xilinx_spips.h"
 #include "hw/dma/xlnx_dpdma.h"
 #include "hw/display/xlnx_dp.h"
+#include "hw/intc/xlnx-zynqmp-ipi.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
@@ -39,6 +40,7 @@
 #define XLNX_ZYNQMP_NUM_UARTS 2
 #define XLNX_ZYNQMP_NUM_SDHCI 2
 #define XLNX_ZYNQMP_NUM_SPIS 2
+#define XLNX_ZYNQMP_NUM_IPIS 11
 
 #define XLNX_ZYNQMP_NUM_OCM_BANKS 4
 #define XLNX_ZYNQMP_OCM_RAM_0_ADDRESS 0xFFFC0000
@@ -85,6 +87,7 @@ typedef struct XlnxZynqMPState {
     XilinxSPIPS spi[XLNX_ZYNQMP_NUM_SPIS];
     XlnxDPState dp;
     XlnxDPDMAState dpdma;
+    XlnxZynqMPIPI ipi[XLNX_ZYNQMP_NUM_IPIS];
 
     char *boot_cpu;
     ARMCPU *boot_cpu_ptr;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device
  2016-07-05 20:30 [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device Alistair Francis
                   ` (3 preceding siblings ...)
  2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 4/4] zynqmp: Connect the IPI devices Alistair Francis
@ 2016-07-05 20:37 ` Peter Maydell
  2016-07-11 21:56   ` Alistair Francis
  4 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2016-07-05 20:37 UTC (permalink / raw)
  To: Alistair Francis
  Cc: QEMU Developers, Peter Crosthwaite, Edgar Iglesias, Edgar E. Iglesias

On 5 July 2016 at 21:30, Alistair Francis <alistair.francis@xilinx.com> wrote:
> This patchset adds and connects the Xilinx ZynqmP IPI devices using the
> register GPIO line.
>
> This requires adding the register GPIO API which allows registers to be
> mapped to GPIOs. This GPIO is used to propergate register reads/writes
> to other devices and other registers.
>
> This is useful to update register values in other devices when events
> occur.

I (still) don't think this is useful enough to be worth it.
In fact you can see in patch 4 the problems it causes,
because the register API has left you with named GPIOs
like "PMU_0", "PMU_1", etc rather than a single array
of GPIOs named "PMU", which then means you can't loop
through wiring it up.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device
  2016-07-05 20:37 ` [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device Peter Maydell
@ 2016-07-11 21:56   ` Alistair Francis
  2016-07-11 22:25     ` Peter Maydell
  0 siblings, 1 reply; 8+ messages in thread
From: Alistair Francis @ 2016-07-11 21:56 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Alistair Francis, Edgar Iglesias, Edgar E. Iglesias,
	Peter Crosthwaite, QEMU Developers

On Tue, Jul 5, 2016 at 1:37 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 5 July 2016 at 21:30, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> This patchset adds and connects the Xilinx ZynqmP IPI devices using the
>> register GPIO line.
>>
>> This requires adding the register GPIO API which allows registers to be
>> mapped to GPIOs. This GPIO is used to propergate register reads/writes
>> to other devices and other registers.
>>
>> This is useful to update register values in other devices when events
>> occur.
>
> I (still) don't think this is useful enough to be worth it.
> In fact you can see in patch 4 the problems it causes,
> because the register API has left you with named GPIOs
> like "PMU_0", "PMU_1", etc rather than a single array
> of GPIOs named "PMU", which then means you can't loop
> through wiring it up.

I think a single array would be very confusing to connect and
understand what is going on as there are so many connections here. But
I see your point.

I still think this is helpful as there are a large number of cases
where setting a bit in a register propagates through the system to
somewhere else. We use this functionality to control the VINITI pin
for the R5 in our tree and many other use cases where it is very
helpful to directly map a bit to the GPIO line.

Is there any chance I can convince you that this is useful and get it merged?

Thanks,

Alistair

>
> thanks
> -- PMM
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device
  2016-07-11 21:56   ` Alistair Francis
@ 2016-07-11 22:25     ` Peter Maydell
  0 siblings, 0 replies; 8+ messages in thread
From: Peter Maydell @ 2016-07-11 22:25 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Edgar Iglesias, Edgar E. Iglesias, Peter Crosthwaite, QEMU Developers

On 11 July 2016 at 22:56, Alistair Francis <alistair.francis@xilinx.com> wrote:
> I still think this is helpful as there are a large number of cases
> where setting a bit in a register propagates through the system to
> somewhere else. We use this functionality to control the VINITI pin
> for the R5 in our tree and many other use cases where it is very
> helpful to directly map a bit to the GPIO line.

Well, there are some devices like that. But I think:
 * most devices aren't like that
 * most GPIO lines are more complicated than "just follows
   a register bit"
 * most devices that do have GPIO lines don't have very
   many and it's not very complicated to just implement
   them by open coding them
 * a special case facility that only gets used for the
   rare situation is going to make those devices harder
   to understand and maintain because they do things in
   a different way to everything else

> Is there any chance I can convince you that this is useful and
> get it merged?

It doesn't seem very likely, I'm afraid.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-07-11 22:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-05 20:30 [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device Alistair Francis
2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 1/4] irq: Add opaque setter routine Alistair Francis
2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 2/4] register: Add GPIO API Alistair Francis
2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 3/4] xlnx-zynqmp-ipi: Add the IPI device Alistair Francis
2016-07-05 20:30 ` [Qemu-devel] [PATCH v1 4/4] zynqmp: Connect the IPI devices Alistair Francis
2016-07-05 20:37 ` [Qemu-devel] [PATCH v1 0/4] Connect the Xilinx ZynqMP IPI device Peter Maydell
2016-07-11 21:56   ` Alistair Francis
2016-07-11 22:25     ` Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.