| Message ID | 1385620533-17685-2-git-send-email-Bharat.Bhushan@freescale.com |
|---|---|
| State | New |
| Headers | show |
On 28.11.2013, at 07:35, Bharat Bhushan <r65777@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". > > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@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. The rest looks good to me :). Alex
> -----Original Message----- > From: Alexander Graf [mailto:agraf@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 <r65777@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.Bhushan@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 > >
On 19.12.2013, at 16:38, Bharat.Bhushan@freescale.com wrote: > > >> -----Original Message----- >> From: Alexander Graf [mailto:agraf@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 <r65777@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.Bhushan@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 ? Then we should also rename the variable :). Alex
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) { 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) { 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); } static const VMStateDescription vmstate_pci_outbound = { diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index cfdd84b..e437d06 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -610,7 +610,9 @@ void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params) target_long initrd_size = 0; target_ulong cur_base = 0; int i; - unsigned int pci_irq_nrs[4] = {1, 2, 3, 4}; + /* irq num for pin INTA, INTB, INTC and INTD is 1, 2, 3 and + * 4 respectively */ + unsigned int pci_irq_nrs[PCI_NUM_PINS] = {1, 2, 3, 4}; qemu_irq **irqs, *mpic; DeviceState *dev; CPUPPCState *firstenv = NULL; @@ -712,10 +714,10 @@ void ppce500_init(QEMUMachineInitArgs *args, PPCE500Params *params) qdev_prop_set_uint32(dev, "first_slot", params->pci_first_slot); qdev_init_nofail(dev); s = SYS_BUS_DEVICE(dev); - sysbus_connect_irq(s, 0, mpic[pci_irq_nrs[0]]); - sysbus_connect_irq(s, 1, mpic[pci_irq_nrs[1]]); - sysbus_connect_irq(s, 2, mpic[pci_irq_nrs[2]]); - sysbus_connect_irq(s, 3, mpic[pci_irq_nrs[3]]); + for (i = 0; i < PCI_NUM_PINS; i++) { + sysbus_connect_irq(s, i, mpic[pci_irq_nrs[i]]); + } + memory_region_add_subregion(ccsr_addr_space, MPC8544_PCI_REGS_OFFSET, sysbus_mmio_get_region(s, 0));
- Use PCI_NUM_PINS rather than hardcoding - use "pin" wherever possible Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com> --- hw/pci-host/ppce500.c | 14 +++++++------- hw/ppc/e500.c | 12 +++++++----- 2 files changed, 14 insertions(+), 12 deletions(-)