Re: [Qemu-devel] [PATCH v7 5/8] irq: Add a new irq device that allows the ORing of lines

[Peter Maydell]

On 11 September 2016 at 15:54, Alistair Francis <address@hidden> wrote:
> Signed-off-by: Alistair Francis <address@hidden>
> ---
> As the migration framework is not included in user mode this needs to be a
> new file.
>
> V7:
>  - Use the standard QEMU init/realise functions
> V6:
>  - Make the OR IRQ device a TYPE_DEVICE
>  - Add vmstate
> +static void or_irq_handler(void *opaque, int n, int level)
> +{
> +    qemu_or_irq *s = OR_IRQ(opaque);
> +    int or_level = 0;
> +    int i;
> +
> +    s->levels[n] = level;
> +
> +    for (i = 0; i < s->num_lines; i++) {
> +        or_level |= s->levels[i];
> +    }
> +
> +    qemu_set_irq(s->out_irq, or_level);
> +}
> +
> +static void or_irq_reset(DeviceState *dev)
> +{
> +    qemu_or_irq *s = OR_IRQ(dev);
> +    int i;
> +
> +    for (i = 0; i < MAX_OR_LINES; i++) {
> +        s->levels[i] = false;
> +    }
> +
> +    /* Trigger an update of the out irqs. We just set the level to 0 */
> +    or_irq_handler(s, 0, 0);

Changing an output qdev IRQ line on reset() is generally
not recommended. Is it necessary here?

> +#include "hw/irq.h"
> +#include "hw/sysbus.h"
> +#include "qom/object.h"
> +
> +#define TYPE_OR_IRQ "or-irq"
> +
> +#define MAX_OR_LINES      16

This is a slightly arbitrary limit, but it's bigger than anybody's
really likely to need and there's a forward compatible path for
migration if we need to support larger sizes in future, so it's OK.
(if num_lines > 16 you would dynamically allocate a levels array
and migrate it with a migration subsection that's only present if the
num_lines property is >16)

> +typedef struct OrIRQState qemu_or_irq;
> +
> +#define OR_IRQ(obj) OBJECT_CHECK(qemu_or_irq, (obj), TYPE_OR_IRQ)
> +
> +struct OrIRQState {
> +    DeviceState parent_obj;
> +
> +    qemu_irq out_irq;
> +    qemu_irq *in_irqs;
> +    bool levels[MAX_OR_LINES];
> +    uint16_t num_lines;
> +};
> +
> +qemu_irq *qemu_get_or_irqs(DeviceState *dev);

Why this rather than defining the input lines with qdev_init_gpio_in()
so the users can use qdev_get_gpio_in() ?

thanks
-- PMM