From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45020) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SnB55-0005pK-9H for qemu-devel@nongnu.org; Fri, 06 Jul 2012 12:08:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SnB50-0007v3-2Y for qemu-devel@nongnu.org; Fri, 06 Jul 2012 12:08:30 -0400 Message-ID: <4FF70D75.7050104@suse.de> Date: Fri, 06 Jul 2012 18:08:21 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <8035D2DA-91B8-4F15-890A-EAC2113847BF@suse.de> <4FF70B30.2090902@suse.de> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-ppc][PATCH v5 1/4] Add usb option in machine options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: benh@au1.ibm.com, shangw@linux.vnet.ibm.com, qemu-devel@nongnu.org, Li Zhang , qemu-ppc@nongnu.org, zhlcindy@gmail.com Am 06.07.2012 18:03, schrieb Alexander Graf: >=20 > On 06.07.2012, at 17:58, Andreas F=E4rber wrote: >=20 >> Am 06.07.2012 15:43, schrieb Alexander Graf: >>> >>> On 02.07.2012, at 07:25, zhlcindy@gmail.com wrote: >>> >>>> From: Li Zhang >>>> >>>> pSeries machine needs to enable USB to add a USB >>>> keyboard or USB mouse. -usb option won't be used in >>>> the future, and machine options are a better way to >>>> enable USB. >>>> >>>> So this patch is to add USB option to machine options >>>> (-machine type=3Dpseries,usb=3Don/off) to enable/disable >>>> USB controller. And machines will get the machine option >>>> and create a USB controller if USB is on. >>>> >>>> By the way, USB is on by default on pSeries machine. >>>> So USB controller should be turned off explicitly through >>>> the command line for "-nodefault" case as the following: >>>> -machine type=3Dpseries,usb=3Doff. >>>> >>>> Signed-off-by: Li Zhang >>>> Reviewed-by: Andreas F=E4rber >>>> --- >>>> hw/spapr.c | 11 +++++++++++ >>>> qemu-config.c | 4 ++++ >>>> 2 files changed, 15 insertions(+) >>>> >>>> diff --git a/hw/spapr.c b/hw/spapr.c >>>> index 81c9343..973de1b 100644 >>>> --- a/hw/spapr.c >>>> +++ b/hw/spapr.c >> [...] >>>> @@ -710,6 +712,15 @@ static void ppc_spapr_init(ram_addr_t ram_size, >>>> spapr_vscsi_create(spapr->vio_bus); >>>> } >>>> >>>> + machine_opts =3D qemu_opts_find(qemu_find_opts("machine"), 0); >>>> + if (machine_opts) { >>>> + add_usb =3D qemu_opt_get_bool(machine_opts, "usb", true); >>>> + } >>>> + >>>> + if (add_usb) { >>>> + pci_create_simple(QLIST_FIRST(&spapr->phbs)->host_state.bus= , >>>> + -1, "pci-ohci"); >>>> + } >>> >>> Didn't I ask you to extract this out to generic code? I don't want to= have a "usb" machine opt that is only available for -M pseries. >> >> Isn't the mapping from "usb=3Don" to device-level actions >> machine-specific? We have ohci, uhci, ehci, xhci to choose from. And t= he >> bus to place it on is machine-specific, too. >> >> So did you rather mean adding usb=3D awareness to more machines? If we >> generalize usb=3Don to usb=3Dnone/ohci/... plus some usbbus=3D we get = -device. >=20 > I was thinking of replacing usb_enabled with >=20 > static inline int usb_enabled(bool default) { > machine_opts =3D qemu_opts_find(qemu_find_opts("machine"), 0); > if (machine_opts) { > return qemu_opt_get_bool(machine_opts, "usb", default); > } > return default; > } >=20 > Whereas -usb in vl.c would set machine.usb =3D on. Then all users of us= b_enabled would automatically be converted to the new syntax and we could= even use the same logic in spapr.c. Oh sure, if we return bool and keep QemuOpts* locally it's perfect. :) I understood you wanted to generalize the if (add_usb) {} that you replied under. ;) Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg