Re: [Qemu-devel] [PATCH v2 2/2] hw/exynos4210_gic.c: Introduce n_in and n_out propeties.

[Peter Maydell]

On 12 April 2012 09:13, Evgeny Voevodin <address@hidden> 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