| Message ID | 1334218381-4208-3-git-send-email-e.voevodin@samsung.com |
|---|---|
| State | New |
| Headers | show |
On 12 April 2012 09:13, Evgeny Voevodin <e.voevodin@samsung.com> wrote: > With these properties irq gate could be tuned to mux up to > QDEV_MAX_IRQ inputs and ouputs. Gate will group inputs > into groups of size n_in/n_out each. > @@ -420,14 +430,20 @@ static int exynos4210_irq_gate_init(SysBusDevice *dev) > Exynos4210IRQGateState *s = > FROM_SYSBUS(Exynos4210IRQGateState, dev); > > + /* Gate will make each input group of size n_in / n_out */ I think it would be useful to expand this comment to describe how the device uses its gpio_in and sysbus irq outputs. (This is necessary at the moment because we don't have the ability to have nicely named arrays of gpio inputs/outputs, so often the single gpio input array is arbitrarily divided up and you just have to document in a comment how it works; see for instance arm_gic.c and arm_mptimer.c. Something like: /* This device models a collection of OR gates. There are n_out * separate gates, and output sysbus IRQ line N is the output of * gate N. The input qdev gpio lines are the inputs to each gate * in order: * [0.. n_in/n_out - 1] : inputs to gate 0 * [n_in/n_out .. 2*n_in/n_out - 1] : inputs to gate 1 * and so on. */ The other approach to consider would be to make each OR gate a separate device, and just instantiate N of them. That would probably look cleaner in the device implementation but be a little more code in exynos4210.c. I don't have a preference either way -- just a thought. > + if ((s->n_in % s->n_out) != 0) { > + hw_error("n_in is not multiple of n_out in Irq Gate"); "exynos4210.irq_gate: n_in must be a multiple of n_out". -- PMM
On 16.04.2012 15:01, Peter Maydell wrote: > On 12 April 2012 09:13, Evgeny Voevodin<e.voevodin@samsung.com> wrote: >> With these properties irq gate could be tuned to mux up to >> QDEV_MAX_IRQ inputs and ouputs. Gate will group inputs >> into groups of size n_in/n_out each. >> @@ -420,14 +430,20 @@ static int exynos4210_irq_gate_init(SysBusDevice *dev) >> Exynos4210IRQGateState *s = >> FROM_SYSBUS(Exynos4210IRQGateState, dev); >> >> + /* Gate will make each input group of size n_in / n_out */ > I think it would be useful to expand this comment to describe how the > device uses its gpio_in and sysbus irq outputs. (This is necessary at > the moment because we don't have the ability to have nicely named > arrays of gpio inputs/outputs, so often the single gpio input array > is arbitrarily divided up and you just have to document in a comment > how it works; see for instance arm_gic.c and arm_mptimer.c. Something > like: > > /* This device models a collection of OR gates. There are n_out > * separate gates, and output sysbus IRQ line N is the output of > * gate N. The input qdev gpio lines are the inputs to each gate > * in order: > * [0.. n_in/n_out - 1] : inputs to gate 0 > * [n_in/n_out .. 2*n_in/n_out - 1] : inputs to gate 1 > * and so on. > */ Ok. > The other approach to consider would be to make each OR gate a separate > device, and just instantiate N of them. That would probably look > cleaner in the device implementation but be a little more code in > exynos4210.c. I don't have a preference either way -- just a thought. Do you mean to have each OR gate with only two inputs? Then if we want to mux n inputs into one output, we'll have to place a number of gates after a gate output... As for me it's better to have one configurable gate with multiple inputs. >> + if ((s->n_in % s->n_out) != 0) { >> + hw_error("n_in is not multiple of n_out in Irq Gate"); > "exynos4210.irq_gate: n_in must be a multiple of n_out". Ok. > -- PMM > Thanks!
On 16 April 2012 12:14, Evgeny Voevodin <e.voevodin@samsung.com> wrote: > On 16.04.2012 15:01, Peter Maydell wrote: >> The other approach to consider would be to make each OR gate a separate >> device, and just instantiate N of them. That would probably look >> cleaner in the device implementation but be a little more code in >> exynos4210.c. I don't have a preference either way -- just a thought. > > Do you mean to have each OR gate with only two inputs? No, I meant that you could have a qdev device which is a single OR gate with N inputs and one output. (As opposed to now, where you have a qdev device which is X OR gates all lumped together, with N * X inputs and X outputs, but each OR gate is actually not interacting with any of the others.) -- PMM
On 16.04.2012 15:17, Peter Maydell wrote: > On 16 April 2012 12:14, Evgeny Voevodin<e.voevodin@samsung.com> wrote: >> On 16.04.2012 15:01, Peter Maydell wrote: >>> The other approach to consider would be to make each OR gate a separate >>> device, and just instantiate N of them. That would probably look >>> cleaner in the device implementation but be a little more code in >>> exynos4210.c. I don't have a preference either way -- just a thought. >> Do you mean to have each OR gate with only two inputs? > No, I meant that you could have a qdev device which is a single OR > gate with N inputs and one output. (As opposed to now, where > you have a qdev device which is X OR gates all lumped together, > with N * X inputs and X outputs, but each OR gate is actually > not interacting with any of the others.) > > -- PMM > Hmmm. Having one output per gate we can configure resulting multiplexer easier then now in the case of different inputs per gate. Seems you're right. Again :)
diff --git a/hw/exynos4210.c b/hw/exynos4210.c index 5e30387..2cb7890 100644 --- a/hw/exynos4210.c +++ b/hw/exynos4210.c @@ -98,6 +98,8 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem, /* IRQ Gate */ dev = qdev_create(NULL, "exynos4210.irq_gate"); + qdev_prop_set_uint32(dev, "n_out", EXYNOS4210_NCPUS); + qdev_prop_set_uint32(dev, "n_in", EXYNOS4210_IRQ_GATE_NINPUTS); qdev_init_nofail(dev); /* Get IRQ Gate input in gate_irq */ for (n = 0; n < EXYNOS4210_IRQ_GATE_NINPUTS; n++) { diff --git a/hw/exynos4210_gic.c b/hw/exynos4210_gic.c index ec13140..be95fe4 100644 --- a/hw/exynos4210_gic.c +++ b/hw/exynos4210_gic.c @@ -361,18 +361,29 @@ type_init(exynos4210_gic_register_types) typedef struct { SysBusDevice busdev; - qemu_irq pic_irq[NCPU]; /* output IRQs to PICs */ - uint32_t gpio_level[EXYNOS4210_IRQ_GATE_NINPUTS]; /* Input levels */ + qemu_irq out[QDEV_MAX_IRQ]; /* output IRQs */ + uint32_t n_out; /* outputs amount */ + uint32_t gpio_level[QDEV_MAX_IRQ]; /* Input levels */ + uint32_t n_in; /* inputs amount */ + uint32_t group_size; /* input group size */ } Exynos4210IRQGateState; +static Property exynos4210_irq_gate_properties[] = { + DEFINE_PROP_UINT32("n_out", Exynos4210IRQGateState, n_out, 1), + DEFINE_PROP_UINT32("n_in", Exynos4210IRQGateState, n_in, 1), + DEFINE_PROP_END_OF_LIST(), +}; + static const VMStateDescription vmstate_exynos4210_irq_gate = { .name = "exynos4210.irq_gate", - .version_id = 1, - .minimum_version_id = 1, - .minimum_version_id_old = 1, + .version_id = 2, + .minimum_version_id = 2, + .minimum_version_id_old = 2, .fields = (VMStateField[]) { - VMSTATE_UINT32_ARRAY(gpio_level, Exynos4210IRQGateState, - EXYNOS4210_IRQ_GATE_NINPUTS), + VMSTATE_UINT32(n_out, Exynos4210IRQGateState), + VMSTATE_UINT32_ARRAY(gpio_level, Exynos4210IRQGateState, QDEV_MAX_IRQ), + VMSTATE_UINT32(n_in, Exynos4210IRQGateState), + VMSTATE_UINT32(group_size, Exynos4210IRQGateState), VMSTATE_END_OF_LIST() } }; @@ -382,25 +393,24 @@ static void exynos4210_irq_gate_handler(void *opaque, int irq, int level) { Exynos4210IRQGateState *s = (Exynos4210IRQGateState *)opaque; - uint32_t odd, even; - - if (irq & 1) { - odd = irq; - even = irq & ~1; - } else { - even = irq; - odd = irq | 1; - } + uint32_t n_out, n_group, i; + + assert(irq < s->n_in); + + n_out = irq / s->group_size; + n_group = n_out * s->group_size; - assert(irq < EXYNOS4210_IRQ_GATE_NINPUTS); s->gpio_level[irq] = level; - if (s->gpio_level[odd] >= 1 || s->gpio_level[even] >= 1) { - qemu_irq_raise(s->pic_irq[even >> 1]); - } else { - qemu_irq_lower(s->pic_irq[even >> 1]); + for (i = 0; i < s->group_size; i++) { + if (s->gpio_level[n_group + i] >= 1) { + qemu_irq_raise(s->out[n_out]); + return; + } } + qemu_irq_lower(s->out[n_out]); + return; } @@ -420,14 +430,20 @@ static int exynos4210_irq_gate_init(SysBusDevice *dev) Exynos4210IRQGateState *s = FROM_SYSBUS(Exynos4210IRQGateState, dev); + /* Gate will make each input group of size n_in / n_out */ + if ((s->n_in % s->n_out) != 0) { + hw_error("n_in is not multiple of n_out in Irq Gate"); + } + + s->group_size = s->n_in / s->n_out; + /* Allocate general purpose input signals and connect a handler to each of * them */ - qdev_init_gpio_in(&s->busdev.qdev, exynos4210_irq_gate_handler, - EXYNOS4210_IRQ_GATE_NINPUTS); + qdev_init_gpio_in(&s->busdev.qdev, exynos4210_irq_gate_handler, s->n_in); - /* Connect SysBusDev irqs to device specific irqs */ - for (i = 0; i < NCPU; i++) { - sysbus_init_irq(dev, &s->pic_irq[i]); + /* Pass gate outs to SysBusDev */ + for (i = 0; i < s->n_out; i++) { + sysbus_init_irq(dev, &s->out[i]); } return 0; @@ -441,6 +457,7 @@ static void exynos4210_irq_gate_class_init(ObjectClass *klass, void *data) k->init = exynos4210_irq_gate_init; dc->reset = exynos4210_irq_gate_reset; dc->vmsd = &vmstate_exynos4210_irq_gate; + dc->props = exynos4210_irq_gate_properties; } static TypeInfo exynos4210_irq_gate_info = {
With these properties irq gate could be tuned to mux up to QDEV_MAX_IRQ inputs and ouputs. Gate will group inputs into groups of size n_in/n_out each. Signed-off-by: Evgeny Voevodin <e.voevodin@samsung.com> --- hw/exynos4210.c | 2 + hw/exynos4210_gic.c | 69 +++++++++++++++++++++++++++++++------------------- 2 files changed, 45 insertions(+), 26 deletions(-)