From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TE25t-0005HA-13 for qemu-devel@nongnu.org; Tue, 18 Sep 2012 14:00:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TE25Y-00007e-RM for qemu-devel@nongnu.org; Tue, 18 Sep 2012 14:00:18 -0400 Message-ID: <5058B69B.9060108@suse.de> Date: Tue, 18 Sep 2012 19:59:55 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1346649928-2998-1-git-send-email-zhlcindy@linux.vnet.ibm.com> <5058B129.3090409@suse.de> In-Reply-To: <5058B129.3090409@suse.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v10 1/1] Add USB option in machine options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Li Zhang Cc: qemu-devel@nongnu.org, Li Zhang , qemu-ppc@nongnu.org, Gerd Hoffmann , anthony@codemonkey.ws, david@gibson.dropbear.id.au On 09/18/2012 07:36 PM, Alexander Graf wrote: > On 09/03/2012 07:25 AM, Li Zhang wrote: >> When -usb option is used, global varible usb_enabled is set. >> And all the plaform will create one USB controller according >> to this variable. In fact, global varibles make code hard >> to read. >> >> So this patch is to remove global variable usb_enabled and >> add USB option in machine options. All the plaforms will get >> USB option value from machine options. >> >> USB option of machine options will be set either by: >> * -usb >> * -machine type=pseries,usb=on >> >> Both these ways can work now. They both set USB option in >> machine options. In the future, the first way will be removed. >> >> Signed-off-by: Li Zhang >> --- >> v7 -> v8 : >> * Declare usb_enabled() and set_usb_option() in sysemu.h >> * Separate USB enablement on sPAPR platform. >> >> v8 -> v9: >> * Fix usb_enable() default value on sPAPR and MAC99 >> >> v9 -> v10: >> * remove set_usb_option() >> * Fix one place of coding style. >> >> hw/nseries.c | 3 ++- >> hw/pc_piix.c | 2 +- >> hw/ppc_newworld.c | 19 +++++++------------ >> hw/ppc_oldworld.c | 2 +- >> hw/ppc_prep.c | 2 +- >> hw/pxa2xx.c | 4 ++-- >> hw/realview.c | 2 +- >> hw/spapr.c | 2 +- >> hw/versatilepb.c | 2 +- >> qemu-config.c | 4 ++++ >> sysemu.h | 4 +++- >> vl.c | 30 ++++++++++++++++++++++++------ >> 12 files changed, 48 insertions(+), 28 deletions(-) >> >> diff --git a/hw/nseries.c b/hw/nseries.c >> index 4df2670..a96c9b1 100644 >> --- a/hw/nseries.c >> +++ b/hw/nseries.c >> @@ -1322,8 +1322,9 @@ static void n8x0_init(ram_addr_t ram_size, >> const char *boot_device, >> n8x0_dss_setup(s); >> n8x0_cbus_setup(s); >> n8x0_uart_setup(s); >> - if (usb_enabled) >> + if (usb_enabled(false)) { >> n8x0_usb_setup(s); >> + } >> if (kernel_filename) { >> /* Or at the linux loader. */ >> diff --git a/hw/pc_piix.c b/hw/pc_piix.c >> index 88ff041..b603fab 100644 >> --- a/hw/pc_piix.c >> +++ b/hw/pc_piix.c >> @@ -267,7 +267,7 @@ static void pc_init1(MemoryRegion *system_memory, >> pc_cmos_init(below_4g_mem_size, above_4g_mem_size, boot_device, >> floppy, idebus[0], idebus[1], rtc_state); >> - if (pci_enabled && usb_enabled) { >> + if (pci_enabled && usb_enabled(false)) { >> pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); >> } >> diff --git a/hw/ppc_newworld.c b/hw/ppc_newworld.c >> index e95cfe8..1d4f494 100644 >> --- a/hw/ppc_newworld.c >> +++ b/hw/ppc_newworld.c >> @@ -348,10 +348,6 @@ static void ppc_core99_init (ram_addr_t ram_size, >> ide_mem[1] = pmac_ide_init(hd, pic[0x0d], dbdma, 0x16, pic[0x02]); >> ide_mem[2] = pmac_ide_init(&hd[MAX_IDE_DEVS], pic[0x0e], dbdma, >> 0x1a, pic[0x02]); >> - /* cuda also initialize ADB */ >> - if (machine_arch == ARCH_MAC99_U3) { >> - usb_enabled = 1; >> - } >> cuda_init(&cuda_mem, pic[0x19]); >> adb_kbd_init(&adb_bus); >> @@ -360,15 +356,14 @@ static void ppc_core99_init (ram_addr_t ram_size, >> macio_init(pci_bus, PCI_DEVICE_ID_APPLE_UNI_N_KEYL, 0, pic_mem, >> dbdma_mem, cuda_mem, NULL, 3, ide_mem, escc_bar); >> - if (usb_enabled) { >> + if (usb_enabled(machine_arch == ARCH_MAC99_U3)) { >> pci_create_simple(pci_bus, -1, "pci-ohci"); >> - } >> - >> - /* U3 needs to use USB for input because Linux doesn't support >> via-cuda >> - on PPC64 */ >> - if (machine_arch == ARCH_MAC99_U3) { >> - usbdevice_create("keyboard"); >> - usbdevice_create("mouse"); >> + /* U3 needs to use USB for input because Linux doesn't >> support via-cuda >> + on PPC64 */ >> + if (machine_arch == ARCH_MAC99_U3) { >> + usbdevice_create("keyboard"); >> + usbdevice_create("mouse"); >> + } >> } >> if (graphic_depth != 15 && graphic_depth != 32 && >> graphic_depth != 8) >> diff --git a/hw/ppc_oldworld.c b/hw/ppc_oldworld.c >> index 1dcd8a6..1468a32 100644 >> --- a/hw/ppc_oldworld.c >> +++ b/hw/ppc_oldworld.c >> @@ -286,7 +286,7 @@ static void ppc_heathrow_init (ram_addr_t ram_size, >> macio_init(pci_bus, PCI_DEVICE_ID_APPLE_343S1201, 1, pic_mem, >> dbdma_mem, cuda_mem, nvr, 2, ide_mem, escc_bar); >> - if (usb_enabled) { >> + if (usb_enabled(false)) { >> pci_create_simple(pci_bus, -1, "pci-ohci"); >> } >> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c >> index 592b7b2..a0888cd 100644 >> --- a/hw/ppc_prep.c >> +++ b/hw/ppc_prep.c >> @@ -660,7 +660,7 @@ static void ppc_prep_init (ram_addr_t ram_size, >> memory_region_add_subregion(sysmem, 0xFEFF0000, xcsr); >> #endif >> - if (usb_enabled) { >> + if (usb_enabled(false)) { >> pci_create_simple(pci_bus, -1, "pci-ohci"); >> } >> diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c >> index d5f1420..4787279 100644 >> --- a/hw/pxa2xx.c >> +++ b/hw/pxa2xx.c >> @@ -2108,7 +2108,7 @@ PXA2xxState *pxa270_init(MemoryRegion >> *address_space, >> s->ssp[i] = (SSIBus *)qdev_get_child_bus(dev, "ssi"); >> } >> - if (usb_enabled) { >> + if (usb_enabled(false)) { >> sysbus_create_simple("sysbus-ohci", 0x4c000000, >> qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1)); >> } >> @@ -2239,7 +2239,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 (usb_enabled(false)) { >> sysbus_create_simple("sysbus-ohci", 0x4c000000, >> qdev_get_gpio_in(s->pic, PXA2XX_PIC_USBH1)); >> } >> diff --git a/hw/realview.c b/hw/realview.c >> index 19db4d0..a8d6f97 100644 >> --- a/hw/realview.c >> +++ b/hw/realview.c >> @@ -227,7 +227,7 @@ static void realview_init(ram_addr_t ram_size, >> 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 (usb_enabled(false)) { >> pci_create_simple(pci_bus, -1, "pci-ohci"); >> } >> n = drive_get_max_bus(IF_SCSI); >> diff --git a/hw/spapr.c b/hw/spapr.c >> index c34b767..7fefd4b 100644 >> --- a/hw/spapr.c >> +++ b/hw/spapr.c >> @@ -768,7 +768,7 @@ static void ppc_spapr_init(ram_addr_t ram_size, >> spapr->has_graphics = true; >> } >> - if (usb_enabled) { >> + if (usb_enabled(spapr->has_graphics)) { >> pci_create_simple(phb->bus, -1, "pci-ohci"); >> if (spapr->has_graphics) { >> usbdevice_create("keyboard"); >> diff --git a/hw/versatilepb.c b/hw/versatilepb.c >> index 7a92034..df32c8b 100644 >> --- a/hw/versatilepb.c >> +++ b/hw/versatilepb.c >> @@ -247,7 +247,7 @@ static void versatile_init(ram_addr_t ram_size, >> pci_nic_init_nofail(nd, "rtl8139", NULL); >> } >> } >> - if (usb_enabled) { >> + if (usb_enabled(false)) { >> pci_create_simple(pci_bus, -1, "pci-ohci"); >> } >> n = drive_get_max_bus(IF_SCSI); >> diff --git a/qemu-config.c b/qemu-config.c >> index c05ffbc..d1a86cf 100644 >> --- a/qemu-config.c >> +++ b/qemu-config.c >> @@ -612,6 +612,10 @@ static QemuOptsList qemu_machine_opts = { >> .name = "dump-guest-core", >> .type = QEMU_OPT_BOOL, >> .help = "Include guest memory in a core dump", >> + },{ >> + .name = "usb", >> + .type = QEMU_OPT_BOOL, >> + .help = "Set on/off to enable/disable usb", >> }, >> { /* End of list */ } >> }, >> diff --git a/sysemu.h b/sysemu.h >> index 65552ac..e87a262 100644 >> --- a/sysemu.h >> +++ b/sysemu.h >> @@ -119,7 +119,6 @@ extern const char *keyboard_layout; >> extern int win2k_install_hack; >> extern int alt_grab; >> extern int ctrl_grab; >> -extern int usb_enabled; >> extern int smp_cpus; >> extern int max_cpus; >> extern int cursor_hide; >> @@ -189,4 +188,7 @@ void register_devices(void); >> void add_boot_device_path(int32_t bootindex, DeviceState *dev, >> const char *suffix); >> char *get_boot_devices_list(uint32_t *size); >> + >> +bool usb_enabled(bool default_usb); >> + >> #endif >> diff --git a/vl.c b/vl.c >> index 7c577fa..71c58a9 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -203,7 +203,6 @@ CharDriverState *serial_hds[MAX_SERIAL_PORTS]; >> CharDriverState *parallel_hds[MAX_PARALLEL_PORTS]; >> CharDriverState *virtcon_hds[MAX_VIRTIO_CONSOLES]; >> int win2k_install_hack = 0; >> -int usb_enabled = 0; >> int singlestep = 0; >> int smp_cpus = 1; >> int max_cpus = 0; >> @@ -790,6 +789,17 @@ static int parse_sandbox(QemuOpts *opts, void >> *opaque) >> return 0; >> } >> +/*********QEMU USB setting******/ >> +bool usb_enabled(bool default_usb) >> +{ >> + QemuOpts *mach_opts; >> + mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0); >> + if (mach_opts) { >> + return qemu_opt_get_bool(mach_opts, "usb", default_usb); >> + } >> + return default_usb; >> +} >> + >> /***********************************************************/ >> /* QEMU Block devices */ >> @@ -1078,8 +1088,9 @@ static int usb_device_add(const char *devname) >> const char *p; >> USBDevice *dev = NULL; >> - if (!usb_enabled) >> + if (!usb_enabled(false)) { > > Shouldn't this rather be a BUG()? When spawning a device through the > command line, we always enable usb. But then again this is out of scope for this patch. Acked-by: Alexander Graf Gerd, could you please check if you like this too? :) Alex