On Wed, Jun 27, 2012 at 9:22 PM, Peter Crosthwaite <peter.crosthwa...@petalogix.com> wrote: > On Wed, Jun 27, 2012 at 11:13 PM, Li Zhang <zhlci...@gmail.com> wrote: >> On Wed, Jun 27, 2012 at 8:00 PM, Andreas Färber <afaer...@suse.de> wrote: >>> Am 18.06.2012 11:22, schrieb Li Zhang: >>>> For pseries machine, it needs to enable usb to add >>>> keyboard or usb mouse. -usb option won't be used in > > Grammar: The pseries machine needs to enable USB to add a keyboard or > USB mouse. The -usb option ... > >>>> the future, and machine options is a better way to > > "are" a better way > >>>> enable usb. >>>> >>>> So this patch is to add usb option to machine options > > So this patch adds a USB option ...
Peter, thank you for correcting my English faults. I will modify it and be more careful next time. :) > >>>> (-machine type=psereis,usb=on/off)to enable/disable >>> >>> "pseries" >>> >>> Space after ) please, Western fonts are probably more narrow. ;) >>> >> OK, I will correct it. >>>> usb controller. >>>> >>>> For specific machines, they will get the machine option >>>> and then create usb controller according to usb option. >>>> >>>> In this patch, usb is on by default on pseries. >>>> So, for -nodefault,usb should be set off in the command >>> >>> Space before "usb" please. >>> >>>> line as the following: >>>> -machine type=pseries,usb=off. >>>> >>>> Signed-off-by: Li Zhang <zhlci...@linux.vnet.ibm.com> >>>> --- >>>> hw/spapr.c | 10 ++++++++++ >>>> qemu-config.c | 4 ++++ >>>> 2 files changed, 14 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/hw/spapr.c b/hw/spapr.c >>>> index d0bddbc..8d158d7 100644 >>>> --- a/hw/spapr.c >>>> +++ b/hw/spapr.c >>>> @@ -529,6 +529,8 @@ static void ppc_spapr_init(ram_addr_t ram_size, >>>> long load_limit, rtas_limit, fw_size; >>>> long pteg_shift = 17; >>>> char *filename; >>>> + QemuOpts * machine_opts; >>> >>> QemuOpts *machine_opts; >>> >>> checkpatch.pl sometimes emits bogus warnings about this. >>> >> OK. I will use it to do that. :) >> >>>> + bool usb_on = false; >>> >>> Didn't you want this to be true for sPAPR in absence of -machine? >>> >> It is set in the following: >> >> if (machine_opts) >> usb_on = qemu_opt_get_bool(machine_opts, "usb", true); >> >> It means that when using "-machine" option, usb_on is set as true if >> usb option is not specified. >> >>> Maybe use a more functional naming? It's either added or not, so add_usb >>> perhaps or provide_usb? Purely stylistic of course. >>> >>>> >>>> spapr = g_malloc0(sizeof(*spapr)); >>>> QLIST_INIT(&spapr->phbs); >>>> @@ -661,6 +663,14 @@ static void ppc_spapr_init(ram_addr_t ram_size, >>>> spapr_vscsi_create(spapr->vio_bus); >>>> } >>>> >>>> + machine_opts = qemu_opts_find(qemu_find_opts("machine"), 0); >>>> + if (machine_opts) >>>> + usb_on = qemu_opt_get_bool(machine_opts, "usb", true); >>> >>> Still missing braces for if. >>> >>> Other than that looking good to me now. Where's 2/2? >> 2/2's title is as the following: >> [Qemu-devel][PATCH 2/2] spapr: Add support for -vga option. :) >> >>> >>> Please post the fixed version as a top-level [PATCH v4] along with a >>> small change log after --- or in a cover letter. >>> >> OK. I will do that. :) >> >>> Andreas >>> >>>> + >>>> + if (usb_on) { >>>> + pci_create_simple(QLIST_FIRST(&spapr->phbs)->host_state.bus, >>>> + -1, "pci-ohci"); >>>> + } >>>> if (rma_size < (MIN_RMA_SLOF << 20)) { >>>> fprintf(stderr, "qemu: pSeries SLOF firmware requires >= " >>>> "%ldM guest RMA (Real Mode Area memory)\n", MIN_RMA_SLOF); >>>> diff --git a/qemu-config.c b/qemu-config.c >>>> index bb3bff4..cdab765 100644 >>>> --- a/qemu-config.c >>>> +++ b/qemu-config.c >>>> @@ -583,6 +583,10 @@ static QemuOptsList qemu_machine_opts = { >>>> .name = "dtb", >>>> .type = QEMU_OPT_STRING, >>>> .help = "Linux kernel device tree file", >>>> + },{ >>>> + .name = "usb", >>>> + .type = QEMU_OPT_BOOL, >>>> + .help = "Set on/off to enable/disable usb", >>>> }, >>>> { /* End of list */ } >>>> }, >>> >>> >>> -- >>> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany >>> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >> >> >> >> -- >> >> Best Regards >> -Li >> -- Best Regards -Li