Re: [PATCH v2 5/5] isa/piix4: Resolve redundant i8259[] attribute

[BALATON Zoltan]

On Sat, 12 Feb 2022, Bernhard Beschow wrote:
> Am 12. Februar 2022 14:19:51 MEZ schrieb BALATON Zoltan <balaton@eik.bme.hu>:
> > On Sat, 12 Feb 2022, Bernhard Beschow wrote:
> > > This is a follow-up on patch "malta: Move PCI interrupt handling from
> > > gt64xxx to piix4" where i8259[] was moved from MaltaState to PIIX4State
> > > to make the code movement more obvious. However, i8259[] seems redundant
> > > to *isa, so remove it.
> >
> > Should this be squashed in patch 1 or at least come after it? Or are there
> > some other changes needed before this patch?
>
> I didn't want to change the patch order since they've been reviewed already. 
> But yeah, you're right: It makes sense to get this out of the way early in the 
> patch series. I will do a v3.

I had another comment in the bottom of this message, not sure you've 
found that too, I forgot to put a warning that there are more comments 
below here. Changing the patch order or patches according to review is OK 
and adding a new patch between already reviewed ones is OK too then you 
can keep Reviewed-by on patches that did not change.

Regards,
BALATON Zoltan

> Regards,
> Bernhard
>
> > Regards,
> > BALATON Zoltan
> >
> > > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> > > ---
> > > hw/isa/piix4.c | 7 +------
> > > 1 file changed, 1 insertion(+), 6 deletions(-)
> > >
> > > diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> > > index a9322851db..692fa8d1f9 100644
> > > --- a/hw/isa/piix4.c
> > > +++ b/hw/isa/piix4.c
> > > @@ -43,7 +43,6 @@ struct PIIX4State {
> > >     PCIDevice dev;
> > >     qemu_irq cpu_intr;
> > >     qemu_irq *isa;
> > > -    qemu_irq i8259[ISA_NUM_IRQS];
> > >
> > >     int32_t pci_irq_levels[PIIX_NUM_PIRQS];
> > >
> > > @@ -73,7 +72,7 @@ static void piix4_set_irq(void *opaque, int irq_num, int 
> > > level)
> > >                 pic_level |= s->pci_irq_levels[i];
> > >             }
> > >         }
> > > -        qemu_set_irq(s->i8259[pic_irq], pic_level);
> > > +        qemu_set_irq(s->isa[pic_irq], pic_level);
> > >     }
> > > }
> > >
> > > @@ -323,9 +322,5 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus 
> > > **isa_bus, I2CBus **smbus)
> > >
> > >     pci_bus_irqs(pci_bus, piix4_set_irq, pci_slot_get_pirq, s, PIIX_NUM_PIRQS);
> > >
> > > -    for (int i = 0; i < ISA_NUM_IRQS; i++) {
> > > -        s->i8259[i] = qdev_get_gpio_in_named(dev, "isa", i);
> > > -    }
> > > -
> > >     return dev;
> > > }