Re: [PATCH v1 35/43] hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)

[Mark Cave-Ayland]

On 18/04/2022 04:48, Richard Henderson wrote:

> On 4/15/22 02:40, Xiaojuan Yang wrote:
> > +        memory_region_init(&s->mmio[cpu], OBJECT(s),
> > +                           "loongarch_extioi", EXTIOI_SIZE);
> > +
> > +        memory_region_init_io(&s->mmio_nodetype[cpu], OBJECT(s),
> > +                              &extioi_nodetype_ops, s,
> > +                              EXTIOI_LINKNAME(.nodetype),
> > +                              IPMAP_OFFSET - APIC_BASE);
> > +        memory_region_add_subregion(&s->mmio[cpu], 0, &s->mmio_nodetype[cpu]);
> > +
> > +        memory_region_init_io(&s->mmio_ipmap_enable[cpu], OBJECT(s),
> > +                              &extioi_ipmap_enable_ops, s,
> > +                              EXTIOI_LINKNAME(.ipmap_enable),
> > +                              BOUNCE_OFFSET - IPMAP_OFFSET);
> > +        memory_region_add_subregion(&s->mmio[cpu], IPMAP_OFFSET - APIC_BASE,
> > +                                    &s->mmio_ipmap_enable[cpu]);
> > +
> > +        memory_region_init_io(&s->mmio_bounce_coreisr[cpu], OBJECT(s),
> > +                              &extioi_bounce_coreisr_ops, s,
> > +                              EXTIOI_LINKNAME(.bounce_coreisr),
> > +                              COREMAP_OFFSET - BOUNCE_OFFSET);
> > +        memory_region_add_subregion(&s->mmio[cpu], BOUNCE_OFFSET - APIC_BASE,
> > +                                    &s->mmio_bounce_coreisr[cpu]);
> > +
> > +        memory_region_init_io(&s->mmio_coremap[cpu], OBJECT(s),
> > +                              &extioi_coremap_ops, s,
> > +                              EXTIOI_LINKNAME(.coremap),
> > +                              EXTIOI_COREMAP_END);
> > +        memory_region_add_subregion(&s->mmio[cpu], COREMAP_OFFSET - APIC_BASE,
> > +                                    &s->mmio_coremap[cpu]);
>
> Why are these separate memory regions, instead of one region?  They're certainly 
> described in a single table in section 11.2 of the 3A5000 manual...

The reason it was done like this is because there were different access sizes in the 
relevant _ops definitions. Certainly when I looked at the patches previously I wasn't 
able to easily see how these could be consolidated without digging deeper into the 
documentation.


ATB,

Mark.