Re: [PATCH 5/6] Add the aehd-i8259 device type.

[Haitao Shan]

On Fri, Mar 3, 2023 at 2:09 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 3/3/23 03:26, Haitao Shan wrote:
> > The aehd-i8259 device type represents the AEHD in kernel PICs.
> > The irqchips should be always in kernel when AEHD is used.
> >
> > Signed-off-by: Haitao Shan <hshan@google.com>
> > ---
> >   hw/i386/aehd/i8259.c     | 165 +++++++++++++++++++++++++++++++++++++++
> >   hw/i386/aehd/meson.build |   1 +
> >   hw/i386/pc.c             |   2 +
> >   include/hw/intc/i8259.h  |   1 +
> >   4 files changed, 169 insertions(+)
> >   create mode 100644 hw/i386/aehd/i8259.c
>
>
> > +static void aehd_pic_reset(DeviceState *dev)
> > +{
> > +    PICCommonState *s = PIC_COMMON(dev);
> > +
> > +    s->elcr = 0;
> > +    pic_reset_common(s);
> > +
> > +    aehd_pic_put(s);
> > +}
> > +
> > +static void aehd_pic_set_irq(void *opaque, int irq, int level)
> > +{
> > +    pic_stat_update_irq(irq, level);
> > +    aehd_set_irq(aehd_state, irq, level);
> > +}
> > +
> > +static void aehd_pic_realize(DeviceState *dev, Error **errp)
> > +{
> > +    PICCommonState *s = PIC_COMMON(dev);
> > +    AEHDPICClass *kpc = AEHD_PIC_GET_CLASS(dev);
> > +
> > +    memory_region_init_io(&s->base_io, OBJECT(dev), NULL, NULL, 
> > "aehd-pic", 2);
> > +    memory_region_init_io(&s->elcr_io, OBJECT(dev), NULL, NULL, 
> > "aehd-elcr", 1);
> > +
> > +
> > +    kpc->parent_realize(dev, errp);
> > +}
>
> Again, this should be drastically simplified by adding a some fields &
> handlers to the abstract TYPE_INKERNEL_IOAPIC class:
>
> struct InKernelAPICCommonClass {
>      APICCommonClass parent_class;
>
>      void (*pic_put)(PICCommonState *s)
>      ...
> };
>
> Eventually this could even belong to APICCommonClass, but I haven't
> checked in detail.
I got your idea now. I will address it in the next version. And this also
replies to the APIC and IOAPIC patch too.



-- 
Haitao @Google