Message ID | 1334128700-10002-3-git-send-email-e.voevodin@samsung.com |
---|---|
State | New |
Headers | show |
On 11 April 2012 08:18, Evgeny Voevodin <e.voevodin@samsung.com> wrote: > Signed-off-by: Evgeny Voevodin <e.voevodin@samsung.com> > --- > 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? 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. -- PMM
On 11.04.2012 19:29, Peter Maydell wrote: > On 11 April 2012 08:18, Evgeny Voevodin<e.voevodin@samsung.com> wrote: >> Signed-off-by: Evgeny Voevodin<e.voevodin@samsung.com> >> --- >> 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!
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);
Signed-off-by: Evgeny Voevodin <e.voevodin@samsung.com> --- hw/exynos4210.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)