> -----Original Message-----
> From: Alexander Graf [mailto:ag...@suse.de]
> Sent: Thursday, December 19, 2013 3:18 AM
> To: Bhushan Bharat-R65777
> Cc: Wood Scott-B07421; QEMU Developers; qemu-ppc; Bhushan Bharat-R65777
> Subject: Re: [PATCH 1/2] ppc-e500: some pci related cleanup
>
>
> On 28.11.2013, at 07:35, Bharat Bhushan <r65...@freescale.com> wrote:
>
> > - Use PCI_NUM_PINS rather than hardcoding
> > - use "pin" wherever possible
>
> I assume you mean the PCI A/B/C/D pin with "pin".
Yes
>
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com>
> > ---
> > hw/pci-host/ppce500.c | 14 +++++++-------
> > hw/ppc/e500.c | 12 +++++++-----
> > 2 files changed, 14 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c index
> > f00793d..49bfcc6 100644
> > --- a/hw/pci-host/ppce500.c
> > +++ b/hw/pci-host/ppce500.c
> > @@ -87,7 +87,7 @@ struct PPCE500PCIState {
> > struct pci_outbound pob[PPCE500_PCI_NR_POBS];
> > struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
> > uint32_t gasket_time;
> > - qemu_irq irq[4];
> > + qemu_irq irq[PCI_NUM_PINS];
> > uint32_t first_slot;
> > /* mmio maps */
> > MemoryRegion container;
> > @@ -252,26 +252,26 @@ static const MemoryRegionOps e500_pci_reg_ops = {
> > .endianness = DEVICE_BIG_ENDIAN,
> > };
> >
> > -static int mpc85xx_pci_map_irq(PCIDevice *pci_dev, int irq_num)
> > +static int mpc85xx_pci_map_irq(PCIDevice *pci_dev, int pin)
>
> This function converts pin -> irq, so that's fine.
>
> > {
> > int devno = pci_dev->devfn >> 3;
> > int ret;
> >
> > - ret = ppce500_pci_map_irq_slot(devno, irq_num);
> > + ret = ppce500_pci_map_irq_slot(devno, pin);
> >
> > pci_debug("%s: devfn %x irq %d -> %d devno:%x\n", __func__,
> > - pci_dev->devfn, irq_num, ret, devno);
> > + pci_dev->devfn, pin, ret, devno);
> >
> > return ret;
> > }
> >
> > -static void mpc85xx_pci_set_irq(void *opaque, int irq_num, int level)
> > +static void mpc85xx_pci_set_irq(void *opaque, int pin, int level)
>
> While this one ...
>
> > {
> > qemu_irq *pic = opaque;
> >
> > - pci_debug("%s: PCI irq %d, level:%d\n", __func__, irq_num, level);
> > + pci_debug("%s: PCI irq %d, level:%d\n", __func__, pin , level);
> >
> > - qemu_set_irq(pic[irq_num], level);
> > + qemu_set_irq(pic[pin], level);
>
> ... sets an actual irq number on the PIC, so this is not a pin.
pic[] is array of intA/B/C/B pin, no ?
Thanks
-Bharat
>
>
> The rest looks good to me :).
>
>
> Alex
>
>
