[Qemu-devel] Re: [PATCH 03/25] pci: introduce constant PCI_NUM_PINS for the number of interrupt pins, 4.

["Michael S. Tsirkin" <mst@redhat.com>]

On Sat, Oct 03, 2009 at 05:15:55AM +0900, Isaku Yamahata wrote:
> introduce constant PCI_NUM_PINS for the number of interrupt pins, 4.
> and use it.
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/pci.c |    4 ++--
>  hw/pci.h |    4 +++-
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index d281ee2..40035e6 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -177,7 +177,7 @@ const VMStateDescription vmstate_pci_device = {
>          VMSTATE_INT32_LE(version_id, PCIDevice),
>          VMSTATE_SINGLE(config, PCIDevice, 0, vmstate_info_pci_config,
>                         typeof_field(PCIDevice,config)),
> -        VMSTATE_INT32_ARRAY_V(irq_state, PCIDevice, 4, 2),
> +        VMSTATE_INT32_ARRAY_V(irq_state, PCIDevice, PCI_NUM_PINS, 2),

By the way, what is 2 standing for, here?
The macro definition isn't really informative:
	#define VMSTATE_INT32_ARRAY_V(_f, _s, _n, _v)                         \
	    VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_int32, int32_t)

/me wishes for less multi-parameter macros: { .blabla = 2 } would be
clearer ...

>          VMSTATE_END_OF_LIST()
>      }
>  };
> @@ -337,7 +337,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>      pci_dev->config_read = config_read;
>      pci_dev->config_write = config_write;
>      bus->devices[devfn] = pci_dev;
> -    pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, 4);
> +    pci_dev->irq = qemu_allocate_irqs(pci_set_irq, pci_dev, PCI_NUM_PINS);
>      pci_dev->version_id = 2; /* Current pci device vmstate version */
>      return pci_dev;
>  }
> diff --git a/hw/pci.h b/hw/pci.h
> index caba5c8..aa3090e 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -156,6 +156,8 @@ typedef struct PCIIORegion {
>  /* Size of the standard PCI config space */
>  #define PCI_CONFIG_SPACE_SIZE 0x100
>  
> +#define PCI_NUM_PINS            4       /* A-D */
> +

notpick: single space between PCI_NUM_PINS and 4 - does not appear
aligned to anything

>  /* Bits in cap_present field. */
>  enum {
>      QEMU_PCI_CAP_MSIX = 0x1,
> @@ -191,7 +193,7 @@ struct PCIDevice {
>      qemu_irq *irq;
>  
>      /* Current IRQ levels.  Used internally by the generic PCI code.  */
> -    int irq_state[4];
> +    int irq_state[PCI_NUM_PINS];
>  
>      /* Capability bits */
>      uint32_t cap_present;
> -- 
> 1.6.0.2
> 
>