Re: [Qemu-ppc] [PATCH 1/2] ppc-e500: some pci related cleanup

[Alexander Graf]

On 19.12.2013, at 16:38, address@hidden wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:address@hidden
>> 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 <address@hidden> 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 <address@hidden>
>>> ---
>>> 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