Patchwork [03/25] pci: introduce constant PCI_NUM_PINS for the number of interrupt pins, 4.

login
register
mail settings
Submitter Isaku Yamahata
Date Oct. 2, 2009, 8:15 p.m.
Message ID <1254514577-11896-4-git-send-email-yamahata@valinux.co.jp>
Download mbox | patch
Permalink /patch/34883/
State Superseded
Headers show

Comments

Isaku Yamahata - Oct. 2, 2009, 8:15 p.m.
introduce constant PCI_NUM_PINS for the number of interrupt pins, 4.
and use it.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
---
 hw/pci.c |    4 ++--
 hw/pci.h |    4 +++-
 2 files changed, 5 insertions(+), 3 deletions(-)
Michael S. Tsirkin - Oct. 4, 2009, 10:04 a.m.
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
> 
>
Isaku Yamahata - Oct. 5, 2009, 9:32 a.m.
On Sun, Oct 04, 2009 at 12:04:53PM +0200, Michael S. Tsirkin wrote:
> 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 ...

It is a version of save/load format.
Perhaps Juan Quintela could answer more detailed.

Patch

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),
         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 */
+
 /* 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;