| Message ID | 1465318599-14137-1-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 06/07/2016 07:56 PM, Eduardo Habkost wrote: > This wrapper for machine_usb(current_machine) is not necessary, > replace all usages of usb_enabled() with machine_usb(). > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > hw/arm/nseries.c | 2 +- > hw/arm/pxa2xx.c | 7 +++++-- > hw/arm/realview.c | 2 +- > hw/arm/versatilepb.c | 2 +- > hw/i386/pc_piix.c | 2 +- > hw/i386/pc_q35.c | 2 +- > hw/ppc/mac_oldworld.c | 2 +- > hw/ppc/prep.c | 2 +- > include/sysemu/sysemu.h | 1 - > vl.c | 11 +++-------- > 10 files changed, 15 insertions(+), 18 deletions(-) > > diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c > index d4eb141..fea911e 100644 > --- a/hw/arm/nseries.c > +++ b/hw/arm/nseries.c > @@ -1351,7 +1351,7 @@ static void n8x0_init(MachineState *machine, > n8x0_dss_setup(s); > n8x0_cbus_setup(s); > n8x0_uart_setup(s); > - if (usb_enabled()) { > + if (machine_usb(machine)) { > n8x0_usb_setup(s); > } > > diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c > index e41a7c9..6d8606d 100644 > --- a/hw/arm/pxa2xx.c > +++ b/hw/arm/pxa2xx.c > @@ -13,6 +13,7 @@ > #include "cpu.h" > #include "hw/sysbus.h" > #include "hw/arm/pxa.h" > +#include "hw/boards.h" > #include "sysemu/sysemu.h" > #include "hw/char/serial.h" > #include "hw/i2c/i2c.h" Hi Eduardo, > @@ -2062,6 +2063,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space, > int i; > DriveInfo *dinfo; > s = g_new0(PXA2xxState, 1); > + MachineState *machine = MACHINE(qdev_get_machine()); > All the callers of pxa270_init have a reference to the machine, you can pass it as parameter IMO. > if (revision && strncmp(revision, "pxa27", 5)) { > fprintf(stderr, "Machine requires a PXA27x processor.\n"); > @@ -2165,7 +2167,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space, > s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi"); > } > > - if (usb_enabled()) { > + if (machine_usb(machine)) { > sysbus_create_simple("sysbus-ohci", 0x4c000000, > qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1)); > } > @@ -2201,6 +2203,7 @@ PXA2xxState *pxa255_init(MemoryRegion *address_space, unsigned int sdram_size) > PXA2xxState *s; > int i; > DriveInfo *dinfo; > + MachineState *machine = MACHINE(qdev_get_machine()); > Same here. Other than that it looks good to me. Thanks, Marcel > s = g_new0(PXA2xxState, 1); > > @@ -2298,7 +2301,7 @@ PXA2xxState *pxa255_init(MemoryRegion *address_space, unsigned int sdram_size) > s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi"); > } > > - if (usb_enabled()) { > + if (machine_usb(machine)) { > sysbus_create_simple("sysbus-ohci", 0x4c000000, > qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1)); > } > diff --git a/hw/arm/realview.c b/hw/arm/realview.c > index 7d0aa6f..8eafcca 100644 > --- a/hw/arm/realview.c > +++ b/hw/arm/realview.c > @@ -254,7 +254,7 @@ static void realview_init(MachineState *machine, > sysbus_connect_irq(busdev, 2, pic[50]); > sysbus_connect_irq(busdev, 3, pic[51]); > pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci"); > - if (usb_enabled()) { > + if (machine_usb(machine)) { > pci_create_simple(pci_bus, -1, "pci-ohci"); > } > n = drive_get_max_bus(IF_SCSI); > diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c > index 20dd356..8ae5392 100644 > --- a/hw/arm/versatilepb.c > +++ b/hw/arm/versatilepb.c > @@ -276,7 +276,7 @@ static void versatile_init(MachineState *machine, int board_id) > pci_nic_init_nofail(nd, pci_bus, "rtl8139", NULL); > } > } > - if (usb_enabled()) { > + if (machine_usb(machine)) { > pci_create_simple(pci_bus, -1, "pci-ohci"); > } > n = drive_get_max_bus(IF_SCSI); > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index a6a6604..d5eb063 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -245,7 +245,7 @@ static void pc_init1(MachineState *machine, > > pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); > > - if (pcmc->pci_enabled && usb_enabled()) { > + if (pcmc->pci_enabled && machine_usb(machine)) { > pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); > } > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 04aae89..31a6a59 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -234,7 +234,7 @@ static void pc_q35_init(MachineState *machine) > ide_drive_get(hd, ICH_AHCI(ahci)->ahci.ports); > ahci_ide_create_devs(ahci, hd); > > - if (usb_enabled()) { > + if (machine_usb(machine)) { > /* Should we create 6 UHCI according to ich9 spec? */ > ehci_create_ich9_with_companions(host_bus, 0x1d); > } > diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c > index a9bb1c2..4479487 100644 > --- a/hw/ppc/mac_oldworld.c > +++ b/hw/ppc/mac_oldworld.c > @@ -309,7 +309,7 @@ static void ppc_heathrow_init(MachineState *machine) > dev = qdev_create(adb_bus, TYPE_ADB_MOUSE); > qdev_init_nofail(dev); > > - if (usb_enabled()) { > + if (machine_usb(machine)) { > pci_create_simple(pci_bus, -1, "pci-ohci"); > } > > diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c > index 07ffe72..054af1e 100644 > --- a/hw/ppc/prep.c > +++ b/hw/ppc/prep.c > @@ -649,7 +649,7 @@ static void ppc_prep_init(MachineState *machine) > memory_region_add_subregion(sysmem, 0xFEFF0000, xcsr); > #endif > > - if (usb_enabled()) { > + if (machine_usb(machine)) { > pci_create_simple(pci_bus, -1, "pci-ohci"); > } > > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 9428141..7313673 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -233,7 +233,6 @@ void qemu_boot_set(const char *boot_order, Error **errp); > QemuOpts *qemu_get_machine_opts(void); > > bool defaults_enabled(void); > -bool usb_enabled(void); > > extern QemuOptsList qemu_legacy_drive_opts; > extern QemuOptsList qemu_common_drive_opts; > diff --git a/vl.c b/vl.c > index fafc7df..ee2dc36 100644 > --- a/vl.c > +++ b/vl.c > @@ -1071,11 +1071,6 @@ bool defaults_enabled(void) > return has_defaults; > } > > -bool usb_enabled(void) > -{ > - return machine_usb(current_machine); > -} > - > #ifndef _WIN32 > static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp) > { > @@ -1392,7 +1387,7 @@ static int usb_device_add(const char *devname) > const char *p; > #endif > > - if (!usb_enabled()) { > + if (!machine_usb(current_machine)) { > return -1; > } > > @@ -1424,7 +1419,7 @@ static int usb_device_del(const char *devname) > return -1; > } > > - if (!usb_enabled()) { > + if (!machine_usb(current_machine)) { > return -1; > } > > @@ -4500,7 +4495,7 @@ int main(int argc, char **argv, char **envp) > } > > /* init USB devices */ > - if (usb_enabled()) { > + if (machine_usb(current_machine)) { > if (foreach_device_config(DEV_USB, usb_parse) < 0) > exit(1); > } >
----- Original Message ----- > From: "Marcel Apfelbaum" <marcel@redhat.com> > To: "Eduardo Habkost" <ehabkost@redhat.com>, qemu-devel@nongnu.org > Cc: "Paolo Bonzini" <pbonzini@redhat.com>, "Andrzej Zaborowski" <balrogg@gmail.com>, "Peter Maydell" > <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, "Alexander Graf" <agraf@suse.de> > Sent: Wednesday, June 8, 2016 1:25:41 PM > Subject: Re: [PATCH] vl: Eliminate usb_enabled() > > On 06/07/2016 07:56 PM, Eduardo Habkost wrote: > > This wrapper for machine_usb(current_machine) is not necessary, > > replace all usages of usb_enabled() with machine_usb(). > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > hw/arm/nseries.c | 2 +- > > hw/arm/pxa2xx.c | 7 +++++-- > > hw/arm/realview.c | 2 +- > > hw/arm/versatilepb.c | 2 +- > > hw/i386/pc_piix.c | 2 +- > > hw/i386/pc_q35.c | 2 +- > > hw/ppc/mac_oldworld.c | 2 +- > > hw/ppc/prep.c | 2 +- > > include/sysemu/sysemu.h | 1 - > > vl.c | 11 +++-------- > > 10 files changed, 15 insertions(+), 18 deletions(-) > > > > diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c > > index d4eb141..fea911e 100644 > > --- a/hw/arm/nseries.c > > +++ b/hw/arm/nseries.c > > @@ -1351,7 +1351,7 @@ static void n8x0_init(MachineState *machine, > > n8x0_dss_setup(s); > > n8x0_cbus_setup(s); > > n8x0_uart_setup(s); > > - if (usb_enabled()) { > > + if (machine_usb(machine)) { > > n8x0_usb_setup(s); > > } > > > > diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c > > index e41a7c9..6d8606d 100644 > > --- a/hw/arm/pxa2xx.c > > +++ b/hw/arm/pxa2xx.c > > @@ -13,6 +13,7 @@ > > #include "cpu.h" > > #include "hw/sysbus.h" > > #include "hw/arm/pxa.h" > > +#include "hw/boards.h" > > #include "sysemu/sysemu.h" > > #include "hw/char/serial.h" > > #include "hw/i2c/i2c.h" > > Hi Eduardo, > > > @@ -2062,6 +2063,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space, > > int i; > > DriveInfo *dinfo; > > s = g_new0(PXA2xxState, 1); > > + MachineState *machine = MACHINE(qdev_get_machine()); > > > > All the callers of pxa270_init have a reference to the machine, > you can pass it as parameter IMO. Another possibility is to pass a "bool usb_enabled" argument. Paolo
On 8 June 2016 at 12:48, Paolo Bonzini <pbonzini@redhat.com> wrote: > ----- Original Message ----- >> From: "Marcel Apfelbaum" <marcel@redhat.com> >> To: "Eduardo Habkost" <ehabkost@redhat.com>, qemu-devel@nongnu.org >> Cc: "Paolo Bonzini" <pbonzini@redhat.com>, "Andrzej Zaborowski" <balrogg@gmail.com>, "Peter Maydell" >> <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, "Alexander Graf" <agraf@suse.de> >> Sent: Wednesday, June 8, 2016 1:25:41 PM >> Subject: Re: [PATCH] vl: Eliminate usb_enabled() >> >> On 06/07/2016 07:56 PM, Eduardo Habkost wrote: >> > @@ -2062,6 +2063,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space, >> > int i; >> > DriveInfo *dinfo; >> > s = g_new0(PXA2xxState, 1); >> > + MachineState *machine = MACHINE(qdev_get_machine()); >> > >> >> All the callers of pxa270_init have a reference to the machine, >> you can pass it as parameter IMO. > > Another possibility is to pass a "bool usb_enabled" argument. Or we could just create the USB controller always -- this is a part of the SoC, not a removable pluggable device, so it doesn't make much sense for the user to be able to create a system where it's not present. thanks -- PMM
On Wed, Jun 08, 2016 at 02:34:16PM +0100, Peter Maydell wrote: > On 8 June 2016 at 12:48, Paolo Bonzini <pbonzini@redhat.com> wrote: > > ----- Original Message ----- > >> From: "Marcel Apfelbaum" <marcel@redhat.com> > >> To: "Eduardo Habkost" <ehabkost@redhat.com>, qemu-devel@nongnu.org > >> Cc: "Paolo Bonzini" <pbonzini@redhat.com>, "Andrzej Zaborowski" <balrogg@gmail.com>, "Peter Maydell" > >> <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, "Alexander Graf" <agraf@suse.de> > >> Sent: Wednesday, June 8, 2016 1:25:41 PM > >> Subject: Re: [PATCH] vl: Eliminate usb_enabled() > >> > >> On 06/07/2016 07:56 PM, Eduardo Habkost wrote: > >> > @@ -2062,6 +2063,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space, > >> > int i; > >> > DriveInfo *dinfo; > >> > s = g_new0(PXA2xxState, 1); > >> > + MachineState *machine = MACHINE(qdev_get_machine()); > >> > > >> > >> All the callers of pxa270_init have a reference to the machine, > >> you can pass it as parameter IMO. > > > > Another possibility is to pass a "bool usb_enabled" argument. > > Or we could just create the USB controller always -- this > is a part of the SoC, not a removable pluggable device, > so it doesn't make much sense for the user to be able > to create a system where it's not present. If you think this is better, I will do it and send v2. It will make things simpler. BTW, I am working on a series to refactor all the *_enabled/no_* code. I will take this into account and add something that will warn the user if they try to enable something that is never present in the machine, or disable something that is always present.
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c index d4eb141..fea911e 100644 --- a/hw/arm/nseries.c +++ b/hw/arm/nseries.c @@ -1351,7 +1351,7 @@ static void n8x0_init(MachineState *machine, n8x0_dss_setup(s); n8x0_cbus_setup(s); n8x0_uart_setup(s); - if (usb_enabled()) { + if (machine_usb(machine)) { n8x0_usb_setup(s); } diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c index e41a7c9..6d8606d 100644 --- a/hw/arm/pxa2xx.c +++ b/hw/arm/pxa2xx.c @@ -13,6 +13,7 @@ #include "cpu.h" #include "hw/sysbus.h" #include "hw/arm/pxa.h" +#include "hw/boards.h" #include "sysemu/sysemu.h" #include "hw/char/serial.h" #include "hw/i2c/i2c.h" @@ -2062,6 +2063,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space, int i; DriveInfo *dinfo; s = g_new0(PXA2xxState, 1); + MachineState *machine = MACHINE(qdev_get_machine()); if (revision && strncmp(revision, "pxa27", 5)) { fprintf(stderr, "Machine requires a PXA27x processor.\n"); @@ -2165,7 +2167,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space, s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi"); } - if (usb_enabled()) { + if (machine_usb(machine)) { sysbus_create_simple("sysbus-ohci", 0x4c000000, qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1)); } @@ -2201,6 +2203,7 @@ PXA2xxState *pxa255_init(MemoryRegion *address_space, unsigned int sdram_size) PXA2xxState *s; int i; DriveInfo *dinfo; + MachineState *machine = MACHINE(qdev_get_machine()); s = g_new0(PXA2xxState, 1); @@ -2298,7 +2301,7 @@ PXA2xxState *pxa255_init(MemoryRegion *address_space, unsigned int sdram_size) s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi"); } - if (usb_enabled()) { + if (machine_usb(machine)) { sysbus_create_simple("sysbus-ohci", 0x4c000000, qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1)); } diff --git a/hw/arm/realview.c b/hw/arm/realview.c index 7d0aa6f..8eafcca 100644 --- a/hw/arm/realview.c +++ b/hw/arm/realview.c @@ -254,7 +254,7 @@ static void realview_init(MachineState *machine, sysbus_connect_irq(busdev, 2, pic[50]); sysbus_connect_irq(busdev, 3, pic[51]); pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci"); - if (usb_enabled()) { + if (machine_usb(machine)) { pci_create_simple(pci_bus, -1, "pci-ohci"); } n = drive_get_max_bus(IF_SCSI); diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c index 20dd356..8ae5392 100644 --- a/hw/arm/versatilepb.c +++ b/hw/arm/versatilepb.c @@ -276,7 +276,7 @@ static void versatile_init(MachineState *machine, int board_id) pci_nic_init_nofail(nd, pci_bus, "rtl8139", NULL); } } - if (usb_enabled()) { + if (machine_usb(machine)) { pci_create_simple(pci_bus, -1, "pci-ohci"); } n = drive_get_max_bus(IF_SCSI); diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index a6a6604..d5eb063 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -245,7 +245,7 @@ static void pc_init1(MachineState *machine, pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); - if (pcmc->pci_enabled && usb_enabled()) { + if (pcmc->pci_enabled && machine_usb(machine)) { pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 04aae89..31a6a59 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -234,7 +234,7 @@ static void pc_q35_init(MachineState *machine) ide_drive_get(hd, ICH_AHCI(ahci)->ahci.ports); ahci_ide_create_devs(ahci, hd); - if (usb_enabled()) { + if (machine_usb(machine)) { /* Should we create 6 UHCI according to ich9 spec? */ ehci_create_ich9_with_companions(host_bus, 0x1d); } diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c index a9bb1c2..4479487 100644 --- a/hw/ppc/mac_oldworld.c +++ b/hw/ppc/mac_oldworld.c @@ -309,7 +309,7 @@ static void ppc_heathrow_init(MachineState *machine) dev = qdev_create(adb_bus, TYPE_ADB_MOUSE); qdev_init_nofail(dev); - if (usb_enabled()) { + if (machine_usb(machine)) { pci_create_simple(pci_bus, -1, "pci-ohci"); } diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c index 07ffe72..054af1e 100644 --- a/hw/ppc/prep.c +++ b/hw/ppc/prep.c @@ -649,7 +649,7 @@ static void ppc_prep_init(MachineState *machine) memory_region_add_subregion(sysmem, 0xFEFF0000, xcsr); #endif - if (usb_enabled()) { + if (machine_usb(machine)) { pci_create_simple(pci_bus, -1, "pci-ohci"); } diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index 9428141..7313673 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -233,7 +233,6 @@ void qemu_boot_set(const char *boot_order, Error **errp); QemuOpts *qemu_get_machine_opts(void); bool defaults_enabled(void); -bool usb_enabled(void); extern QemuOptsList qemu_legacy_drive_opts; extern QemuOptsList qemu_common_drive_opts; diff --git a/vl.c b/vl.c index fafc7df..ee2dc36 100644 --- a/vl.c +++ b/vl.c @@ -1071,11 +1071,6 @@ bool defaults_enabled(void) return has_defaults; } -bool usb_enabled(void) -{ - return machine_usb(current_machine); -} - #ifndef _WIN32 static int parse_add_fd(void *opaque, QemuOpts *opts, Error **errp) { @@ -1392,7 +1387,7 @@ static int usb_device_add(const char *devname) const char *p; #endif - if (!usb_enabled()) { + if (!machine_usb(current_machine)) { return -1; } @@ -1424,7 +1419,7 @@ static int usb_device_del(const char *devname) return -1; } - if (!usb_enabled()) { + if (!machine_usb(current_machine)) { return -1; } @@ -4500,7 +4495,7 @@ int main(int argc, char **argv, char **envp) } /* init USB devices */ - if (usb_enabled()) { + if (machine_usb(current_machine)) { if (foreach_device_config(DEV_USB, usb_parse) < 0) exit(1); }
This wrapper for machine_usb(current_machine) is not necessary, replace all usages of usb_enabled() with machine_usb(). Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- hw/arm/nseries.c | 2 +- hw/arm/pxa2xx.c | 7 +++++-- hw/arm/realview.c | 2 +- hw/arm/versatilepb.c | 2 +- hw/i386/pc_piix.c | 2 +- hw/i386/pc_q35.c | 2 +- hw/ppc/mac_oldworld.c | 2 +- hw/ppc/prep.c | 2 +- include/sysemu/sysemu.h | 1 - vl.c | 11 +++-------- 10 files changed, 15 insertions(+), 18 deletions(-)