qemu-devel
[Top][All Lists]
Advanced

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

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


From: Li Zhang
Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH v9 1/1] Add USB option in machine options
Date: Sun, 2 Sep 2012 21:46:55 +0800

On Sat, Aug 25, 2012 at 3:43 PM, Blue Swirl <address@hidden> wrote:
> On Wed, Aug 22, 2012 at 10:31 AM, Li Zhang <address@hidden> wrote:
>> When -usb option is used, global varible usb_enabled is set.
>> And all the plafrom 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
>>
>> Signed-off-by: Li Zhang <address@hidden>
>>
>> diff --git a/hw/nseries.c b/hw/nseries.c
>> index 4df2670..c67e95a 100644
>> --- a/hw/nseries.c
>> +++ b/hw/nseries.c
>> @@ -1322,7 +1322,7 @@ 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))
>
> Please add braces.
OK, I will do that.

>
> I don't like this usb_enabled(false) way very much but I don't have
> anything better to suggest.

Thanks,
It seems that default value is false on most of platforms, only true
for several platforms.

>>          n8x0_usb_setup(s);
>>
>>      if (kernel_filename) {
>> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
>> index 0c0096f..b662192 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 7a87616..feeb903 100644
>> --- a/hw/ppc_prep.c
>> +++ b/hw/ppc_prep.c
>> @@ -662,7 +662,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 be533ee..2e31797 100644
>> --- a/hw/spapr.c
>> +++ b/hw/spapr.c
>> @@ -766,7 +766,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(QLIST_FIRST(&spapr->phbs)->host_state.bus,
>>                            -1, "pci-ohci");
>>          if (spapr->has_graphics) {
>> 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..be0905e 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,8 @@ 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);
>> +void set_usb_option(bool usb_option);
>> +
>>  #endif
>> diff --git a/vl.c b/vl.c
>> index 7c577fa..4702e33 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,27 @@ 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;
>> +}
>
> It would be more optimal to go through the options only once after
> machine selection in vl.c, setting some flags as a result. Then this
> and below functions would only check against those flags.
>
The following function is called when using -usb or -usbdevice.
It also sets USB option when -usb or -usbdevice is used.
The following function hasn't be called by other platforms.
>> +
>> +void set_usb_option(bool usb_option)
>> +{
>> +    QemuOpts *mach_opts;
>> +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
>> +    if (mach_opts) {
>> +        qemu_opt_set_bool(mach_opts, "usb", usb_option);
>> +    }
>> +}
>> +
>> +
>>  /***********************************************************/
>>  /* QEMU Block devices */
>>
>> @@ -1078,8 +1098,9 @@ static int usb_device_add(const char *devname)
>>      const char *p;
>>      USBDevice *dev = NULL;
>>
>> -    if (!usb_enabled)
>> +    if (!usb_enabled(false)) {
>>          return -1;
>> +    }
>>
>>      /* drivers with .usbdevice_name entry in USBDeviceInfo */
>>      dev = usbdevice_create(devname);
>> @@ -1115,8 +1136,9 @@ static int usb_device_del(const char *devname)
>>      if (strstart(devname, "host:", &p))
>>          return usb_host_device_close(p);
>>
>> -    if (!usb_enabled)
>> +    if (!usb_enabled(false)) {
>>          return -1;
>> +    }
>>
>>      p = strchr(devname, '.');
>>      if (!p)
>> @@ -3062,10 +3084,10 @@ int main(int argc, char **argv, char **envp)
>>                  }
>>                  break;
>>              case QEMU_OPTION_usb:
>> -                usb_enabled = 1;
>> +                set_usb_option(true);
>>                  break;
>>              case QEMU_OPTION_usbdevice:
>> -                usb_enabled = 1;
>> +                set_usb_option(true);
>>                  add_device_config(DEV_USB, optarg);
>>                  break;
>>              case QEMU_OPTION_device:
>> @@ -3623,7 +3645,7 @@ int main(int argc, char **argv, char **envp)
>>      current_machine = machine;
>>
>>      /* init USB devices */
>> -    if (usb_enabled) {
>> +    if (usb_enabled(false)) {
>>          if (foreach_device_config(DEV_USB, usb_parse) < 0)
>>              exit(1);
>>      }
>> --
>> 1.7.7.6
>>
>>



-- 

Best Regards
-Li



reply via email to

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