Patchwork [02/28] openpic: remove dead code to make a PCI device version

login
register
mail settings
Submitter Anthony Liguori
Date Jan. 24, 2012, 7:32 p.m.
Message ID <1327433600-7403-3-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/137650/
State New
Headers show

Comments

Anthony Liguori - Jan. 24, 2012, 7:32 p.m.
bus is always NULL so the code in this if clause is dead (and therefore
untested).

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/openpic.c      |   30 +++---------------------------
 hw/openpic.h      |    2 +-
 hw/ppc_newworld.c |    2 +-
 3 files changed, 5 insertions(+), 29 deletions(-)
Andreas Färber - Jan. 26, 2012, 11:57 p.m.
Am 24.01.2012 20:32, schrieb Anthony Liguori:
> bus is always NULL so the code in this if clause is dead (and therefore
> untested).
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  hw/openpic.c      |   30 +++---------------------------
>  hw/openpic.h      |    2 +-
>  hw/ppc_newworld.c |    2 +-
>  3 files changed, 5 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/openpic.c b/hw/openpic.c
> index 22fc275..280b7a9 100644
> --- a/hw/openpic.c
> +++ b/hw/openpic.c
> @@ -1181,41 +1181,17 @@ static void openpic_irq_raise(openpic_t *opp, int n_CPU, IRQ_src_t *src)
>      qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
>  }
>  
> -qemu_irq *openpic_init (PCIBus *bus, MemoryRegion **pmem, int nb_cpus,
> +qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
>                          qemu_irq **irqs, qemu_irq irq_out)

Here a reindentation (space) would've been nice.

>  {
>      openpic_t *opp;
> -    uint8_t *pci_conf;
>      int i, m;
>  
>      /* XXX: for now, only one CPU is supported */
>      if (nb_cpus != 1)
>          return NULL;
> -    if (bus) {
> -        opp = (openpic_t *)pci_register_device(bus, "OpenPIC", sizeof(openpic_t),
> -                                               -1, NULL, NULL);
> -        pci_conf = opp->pci_dev.config;
> -        pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_IBM);
> -        pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_IBM_OPENPIC2);
> -        pci_config_set_class(pci_conf, PCI_CLASS_SYSTEM_OTHER); // FIXME?
> -        pci_conf[0x3d] = 0x00; // no interrupt pin

I'm not too fond of losing PCI device infos from dead code, but
registering an unused PCIDeviceInfo doesn't seem right either...

AFAIU this function implements a PCI and a non-PCI version of the
device. The PCI version is unused and is now being ripped out.

> -
> -        memory_region_init_io(&opp->mem, &openpic_ops, opp, "openpic", 0x40000);
> -#if 0 // Don't implement ISU for now
> -        opp_io_memory = cpu_register_io_memory(openpic_src_read,
> -                                               openpic_src_write, NULL
> -                                               DEVICE_NATIVE_ENDIAN);
> -        cpu_register_physical_memory(isu_base, 0x20 * (EXT_IRQ + 2),
> -                                     opp_io_memory);
> -#endif
> -
> -        /* Register I/O spaces */
> -        pci_register_bar(&opp->pci_dev, 0,
> -                         PCI_BASE_ADDRESS_SPACE_MEMORY, &opp->mem);
> -    } else {
> -        opp = g_malloc0(sizeof(openpic_t));
> -        memory_region_init_io(&opp->mem, &openpic_ops, opp, "openpic", 0x40000);
> -    }
> +    opp = g_malloc0(sizeof(openpic_t));

Ugh. But just moved around.
(More non-qdev'ified code lurking in that file.)

> +    memory_region_init_io(&opp->mem, &openpic_ops, opp, "openpic", 0x40000);
>  
>      //    isu_base &= 0xFFFC0000;
>      opp->nb_cpus = nb_cpus;
> diff --git a/hw/openpic.h b/hw/openpic.h
> index 715f084..8556030 100644
> --- a/hw/openpic.h
> +++ b/hw/openpic.h
> @@ -11,7 +11,7 @@ enum {
>      OPENPIC_OUTPUT_NB,
>  };
>  
> -qemu_irq *openpic_init (PCIBus *bus, MemoryRegion **pmem, int nb_cpus,
> +qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
>                          qemu_irq **irqs, qemu_irq irq_out);

Space.

>  qemu_irq *mpic_init (MemoryRegion *address_space, target_phys_addr_t base,
>                       int nb_cpus, qemu_irq **irqs, qemu_irq irq_out);
> diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
> index a746c9c..506187b 100644
> --- a/hw/ppc_newworld.c
> +++ b/hw/ppc_newworld.c
> @@ -311,7 +311,7 @@ static void ppc_core99_init (ram_addr_t ram_size,
>              exit(1);
>          }
>      }
> -    pic = openpic_init(NULL, &pic_mem, smp_cpus, openpic_irqs, NULL);
> +    pic = openpic_init(&pic_mem, smp_cpus, openpic_irqs, NULL);
>      if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
>          /* 970 gets a U3 bus */
>          pci_bus = pci_pmac_u3_init(pic, get_system_memory(), get_system_io());

Anyway, to keep moving forward,

Reviewed-by: Andreas Färber <afaerber@suse.de>

Andreas

Patch

diff --git a/hw/openpic.c b/hw/openpic.c
index 22fc275..280b7a9 100644
--- a/hw/openpic.c
+++ b/hw/openpic.c
@@ -1181,41 +1181,17 @@  static void openpic_irq_raise(openpic_t *opp, int n_CPU, IRQ_src_t *src)
     qemu_irq_raise(opp->dst[n_CPU].irqs[OPENPIC_OUTPUT_INT]);
 }
 
-qemu_irq *openpic_init (PCIBus *bus, MemoryRegion **pmem, int nb_cpus,
+qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
                         qemu_irq **irqs, qemu_irq irq_out)
 {
     openpic_t *opp;
-    uint8_t *pci_conf;
     int i, m;
 
     /* XXX: for now, only one CPU is supported */
     if (nb_cpus != 1)
         return NULL;
-    if (bus) {
-        opp = (openpic_t *)pci_register_device(bus, "OpenPIC", sizeof(openpic_t),
-                                               -1, NULL, NULL);
-        pci_conf = opp->pci_dev.config;
-        pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_IBM);
-        pci_config_set_device_id(pci_conf, PCI_DEVICE_ID_IBM_OPENPIC2);
-        pci_config_set_class(pci_conf, PCI_CLASS_SYSTEM_OTHER); // FIXME?
-        pci_conf[0x3d] = 0x00; // no interrupt pin
-
-        memory_region_init_io(&opp->mem, &openpic_ops, opp, "openpic", 0x40000);
-#if 0 // Don't implement ISU for now
-        opp_io_memory = cpu_register_io_memory(openpic_src_read,
-                                               openpic_src_write, NULL
-                                               DEVICE_NATIVE_ENDIAN);
-        cpu_register_physical_memory(isu_base, 0x20 * (EXT_IRQ + 2),
-                                     opp_io_memory);
-#endif
-
-        /* Register I/O spaces */
-        pci_register_bar(&opp->pci_dev, 0,
-                         PCI_BASE_ADDRESS_SPACE_MEMORY, &opp->mem);
-    } else {
-        opp = g_malloc0(sizeof(openpic_t));
-        memory_region_init_io(&opp->mem, &openpic_ops, opp, "openpic", 0x40000);
-    }
+    opp = g_malloc0(sizeof(openpic_t));
+    memory_region_init_io(&opp->mem, &openpic_ops, opp, "openpic", 0x40000);
 
     //    isu_base &= 0xFFFC0000;
     opp->nb_cpus = nb_cpus;
diff --git a/hw/openpic.h b/hw/openpic.h
index 715f084..8556030 100644
--- a/hw/openpic.h
+++ b/hw/openpic.h
@@ -11,7 +11,7 @@  enum {
     OPENPIC_OUTPUT_NB,
 };
 
-qemu_irq *openpic_init (PCIBus *bus, MemoryRegion **pmem, int nb_cpus,
+qemu_irq *openpic_init (MemoryRegion **pmem, int nb_cpus,
                         qemu_irq **irqs, qemu_irq irq_out);
 qemu_irq *mpic_init (MemoryRegion *address_space, target_phys_addr_t base,
                      int nb_cpus, qemu_irq **irqs, qemu_irq irq_out);
diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c
index a746c9c..506187b 100644
--- a/hw/ppc_newworld.c
+++ b/hw/ppc_newworld.c
@@ -311,7 +311,7 @@  static void ppc_core99_init (ram_addr_t ram_size,
             exit(1);
         }
     }
-    pic = openpic_init(NULL, &pic_mem, smp_cpus, openpic_irqs, NULL);
+    pic = openpic_init(&pic_mem, smp_cpus, openpic_irqs, NULL);
     if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
         /* 970 gets a U3 bus */
         pci_bus = pci_pmac_u3_init(pic, get_system_memory(), get_system_io());