qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-ppc] [PATCH v10 1/1] Add USB option in machine op


From: Li Zhang
Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v10 1/1] Add USB option in machine options
Date: Wed, 19 Sep 2012 11:08:57 +0800

On Wed, Sep 19, 2012 at 1:59 AM, Alexander Graf <address@hidden> wrote:
> 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 <address@hidden>
>>> ---
>>>   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.

Got it. Thanks. -:)

>
> Acked-by: Alexander Graf <address@hidden>
>
> Gerd, could you please check if you like this too? :)
>
>
> Alex
>



-- 

Best Regards
-Li



reply via email to

[Prev in Thread] Current Thread [Next in Thread]