| Message ID | 20170525102849.22754-2-drjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 25/05/2017 12:28, Andrew Jones wrote: > +int gic_init(void) > +{ > + if (gicv2_init()) > + gic_common_ops = &gicv2_common_ops; > + else if (gicv3_init()) > + gic_common_ops = &gicv3_common_ops; > + return gic_version(); > +} > + Of the other uses of gic_version(), this one is the ugliest: if (gic_version() == 2) base = gicv2_dist_base(); else base = gicv3_redist_base(); writel(val, base + GICD_ICACTIVER); Could you change this writel to a new op gicv[23]_eoi or something like that? Thanks, Paolo
On Thu, May 25, 2017 at 03:53:10PM +0200, Paolo Bonzini wrote: > > > On 25/05/2017 12:28, Andrew Jones wrote: > > +int gic_init(void) > > +{ > > + if (gicv2_init()) > > + gic_common_ops = &gicv2_common_ops; > > + else if (gicv3_init()) > > + gic_common_ops = &gicv3_common_ops; > > + return gic_version(); > > +} > > + > > Of the other uses of gic_version(), this one is the ugliest: > > if (gic_version() == 2) > base = gicv2_dist_base(); > else > base = gicv3_redist_base(); > > writel(val, base + GICD_ICACTIVER); > > Could you change this writel to a new op gicv[23]_eoi or something like > that? That use is just for an irq handler specific to a single unit test, and at this time I don't think we need an op for that writel, as that's the only place we currently write that register. I do have some plans for irq handler refactoring in arm/gic.c, in order to prepare for more tests. I'll clean this up a bit when I do that. Thanks, drew
diff --git a/lib/arm/gic.c b/lib/arm/gic.c index 3ed539727f8c..59273b1716d6 100644 --- a/lib/arm/gic.c +++ b/lib/arm/gic.c @@ -11,7 +11,6 @@ struct gicv2_data gicv2_data; struct gicv3_data gicv3_data; struct gic_common_ops { - int gic_version; void (*enable_defaults)(void); u32 (*read_iar)(void); u32 (*iar_irqnr)(u32 iar); @@ -23,7 +22,6 @@ struct gic_common_ops { static const struct gic_common_ops *gic_common_ops; static const struct gic_common_ops gicv2_common_ops = { - .gic_version = 2, .enable_defaults = gicv2_enable_defaults, .read_iar = gicv2_read_iar, .iar_irqnr = gicv2_iar_irqnr, @@ -33,7 +31,6 @@ static const struct gic_common_ops gicv2_common_ops = { }; static const struct gic_common_ops gicv3_common_ops = { - .gic_version = 3, .enable_defaults = gicv3_enable_defaults, .read_iar = gicv3_read_iar, .iar_irqnr = gicv3_iar_irqnr, @@ -88,18 +85,24 @@ int gicv3_init(void) &gicv3_data.redist_base[0]); } -int gic_init(void) +int gic_version(void) { - if (gicv2_init()) { - gic_common_ops = &gicv2_common_ops; + if (gic_common_ops == &gicv2_common_ops) return 2; - } else if (gicv3_init()) { - gic_common_ops = &gicv3_common_ops; + else if (gic_common_ops == &gicv3_common_ops) return 3; - } return 0; } +int gic_init(void) +{ + if (gicv2_init()) + gic_common_ops = &gicv2_common_ops; + else if (gicv3_init()) + gic_common_ops = &gicv3_common_ops; + return gic_version(); +} + void gic_enable_defaults(void) { if (!gic_common_ops) { @@ -110,12 +113,6 @@ void gic_enable_defaults(void) gic_common_ops->enable_defaults(); } -int gic_version(void) -{ - assert(gic_common_ops); - return gic_common_ops->gic_version; -} - u32 gic_read_iar(void) { assert(gic_common_ops && gic_common_ops->read_iar);
Remove version from ops as it's not an op, nor necessary. Signed-off-by: Andrew Jones <drjones@redhat.com> --- lib/arm/gic.c | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-)