Re: [Qemu-devel] [PATCH 2/2] hw/exynos4210.c: Use new IRQ Gate properties.

[Evgeny Voevodin]

On 11.04.2012 19:29, Peter Maydell wrote:
> On 11 April 2012 08:18, Evgeny Voevodin<address@hidden>  wrote:
> > Signed-off-by: Evgeny Voevodin<address@hidden>
> > ---
> >   hw/exynos4210.c |    6 ++++--
> >   1 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/exynos4210.c b/hw/exynos4210.c
> > index f904370..088e96e 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++) {
> > @@ -116,7 +118,7 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
> >      busdev = sysbus_from_qdev(dev);
> >      sysbus_mmio_map(busdev, 0, EXYNOS4210_SMP_PRIVATE_BASE_ADDR);
> >      for (n = 0; n<  EXYNOS4210_NCPUS; n++) {
> > -        sysbus_connect_irq(busdev, n, gate_irq[n * 2]);
> > +        sysbus_connect_irq(busdev, n, gate_irq[n * 4]);
> >      }
> >      for (n = 0; n<  EXYNOS4210_INT_GIC_NIRQ; n++) {
> >          s->irqs.int_gic_irq[n] = qdev_get_gpio_in(dev, n);
> > @@ -135,7 +137,7 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
> >      /* Map Distributer interface */
> >      sysbus_mmio_map(busdev, 1, EXYNOS4210_EXT_GIC_DIST_BASE_ADDR);
> >      for (n = 0; n<  EXYNOS4210_NCPUS; n++) {
> > -        sysbus_connect_irq(busdev, n, gate_irq[n * 2 + 1]);
> > +        sysbus_connect_irq(busdev, n, gate_irq[n * 4 + 1]);
> >      }
> >      for (n = 0; n<  EXYNOS4210_EXT_GIC_NIRQ; n++) {
> >          s->irqs.ext_gic_irq[n] = qdev_get_gpio_in(dev, n);
>
> Why do these sysbus_connect_irq() calls have to change?
> Isn't the wiring of the gate the same before and afterwards?

Previous version of gate grouped inputs into groups of size 2. So, odd
and even IRQ numbers were a group. No matter how many inputs
(EXYNOS4210_IRQ_GATE_NINPUTS) gate had. So, to access second input in
second group we had to write gate_irq[n * 2 + 1].
New version's group size depends on amount of inputs and outputs. So,
correct access to second input of n's group is
gate_irq[n * (group_size) + 1].
Since EXYNOS4210_IRQ_GATE_NINPUTS was defined to equal 8 (we wanted
two CPU Interface outputs from Internal Gic and two from External, and
for future use space for another two CPU interfaces was allocated)
previous access method became incorrect.

> Also, since in patch 1 the default number of inputs and outputs
> for the gate is now 1, this means the exynos4 platform is broken
> between the two patches because it will be trying to wire up
> a gate with too few inputs/outputs. You might need to merge
> the two patches together to avoid that.

You're right. That was the first my though. But then I decided that
it's more clearly to divide it into two patches because if someone
would like (seems that nobody though :) I could make this device stand 
alone, something like irq_gate.c. Mistake is that gate_irq access must
be fixed in first patch and then second patch introduces new features.

> -- PMM

Thanks for review!

--
Kind regards,
Evgeny Voevodin,
Leading Software Engineer,
ASWG, Moscow R&D center, Samsung Electronics
e-mail: address@hidden