| Message ID | 1334130487-19455-1-git-send-email-e.voevodin@samsung.com |
|---|---|
| State | New |
| Headers | show |
On 11 April 2012 08:48, 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. > > Signed-off-by: Evgeny Voevodin <e.voevodin@samsung.com> > --- > hw/exynos4210_gic.c | 60 ++++++++++++++++++++++++++++++++------------------ > 1 files changed, 38 insertions(+), 22 deletions(-) > > diff --git a/hw/exynos4210_gic.c b/hw/exynos4210_gic.c > index ec13140..1fe8225 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, > .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 field changes require a version bump. I think we can forget backcompatibility in this case, so just set all of version_id, minumum_version_id and minimum_version_id_old to 2. > 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,19 @@ 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 */ > + assert((s->n_in % s->n_out) == 0); The right way to check for validity of properties in the init function is to use hw_error() with a reasonably friendly message. > + > + 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); > + s->n_in); I think this whole function call will fit on a single line without going over the 80 column limit now. -- PMM
On 11.04.2012 19:17, Peter Maydell wrote: > On 11 April 2012 08:48, 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. >> >> Signed-off-by: Evgeny Voevodin<e.voevodin@samsung.com> >> --- >> hw/exynos4210_gic.c | 60 ++++++++++++++++++++++++++++++++------------------ >> 1 files changed, 38 insertions(+), 22 deletions(-) >> >> diff --git a/hw/exynos4210_gic.c b/hw/exynos4210_gic.c >> index ec13140..1fe8225 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, >> .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 field changes require a version bump. I think we can forget > backcompatibility in this case, so just set all of version_id, > minumum_version_id and minimum_version_id_old to 2. > Ok! >> 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,19 @@ 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 */ >> + assert((s->n_in % s->n_out) == 0); > > The right way to check for validity of properties in the init > function is to use hw_error() with a reasonably friendly message. > Ok! What's about irq handlers? Is assert() possible? >> + >> + 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); >> + s->n_in); > > I think this whole function call will fit on a single line without > going over the 80 column limit now. > Ok! > -- PMM > > Thanks for review!
On 12 April 2012 04:23, Evgeny Voevodin <e.voevodin@samsung.com> wrote: > On 11.04.2012 19:17, Peter Maydell wrote: >> >> The right way to check for validity of properties in the init >> function is to use hw_error() with a reasonably friendly message. >> > > Ok! What's about irq handlers? Is assert() possible? Yes, assert() is ok elsewhere -- it's just that qdev properties are a specifically "user" facing interface and we should be consistent about how different devices enforce validity. -- PMM
diff --git a/hw/exynos4210_gic.c b/hw/exynos4210_gic.c index ec13140..1fe8225 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, .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,19 @@ 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 */ + assert((s->n_in % s->n_out) == 0); + + 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); + 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 +456,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_gic.c | 60 ++++++++++++++++++++++++++++++++------------------ 1 files changed, 38 insertions(+), 22 deletions(-)