Re: [Qemu-devel] [Qemu-ppc] [PATCH v7 1/3] Add USB option in machine options

[David Gibson]

On Tue, Aug 14, 2012 at 11:01:19PM +0800, Li Zhang wrote:
> On Tue, Aug 14, 2012 at 6:46 PM, Alexander Graf <address@hidden> wrote:
> > On 08/07/2012 04:41 AM, Li Zhang 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>
> >> ---
> >>   hw/nseries.c      |    9 +++++++++
> >>   hw/pc_piix.c      |    6 ++++++
> >>   hw/ppc_newworld.c |   10 +++++++++-
> >>   hw/ppc_oldworld.c |    8 ++++++++
> >>   hw/ppc_prep.c     |    7 +++++++
> >>   hw/pxa2xx.c       |   15 +++++++++++++++
> >>   hw/realview.c     |    8 ++++++++
> >>   hw/spapr.c        |   12 ++++++++++++
> >>   hw/versatilepb.c  |    8 ++++++++
> >>   qemu-config.c     |    4 ++++
> >>   sysemu.h          |    1 -
> >>   vl.c              |   29 +++++++++++++++++++++++------
> >>   12 files changed, 109 insertions(+), 8 deletions(-)
> >>
> >
> > [...]
> >
> >
> >> diff --git a/hw/spapr.c b/hw/spapr.c
> >> index 81c9343..4dc5e59 100644
> >> --- a/hw/spapr.c
> >> +++ b/hw/spapr.c
> >> @@ -575,6 +575,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 *mach_opts;
> >> +    bool usb_enabled = true;
> >>         spapr = g_malloc0(sizeof(*spapr));
> >>       QLIST_INIT(&spapr->phbs);
> >> @@ -710,6 +712,16 @@ static void ppc_spapr_init(ram_addr_t ram_size,
> >>           spapr_vscsi_create(spapr->vio_bus);
> >>       }
> >>   +    mach_opts = qemu_opts_find(qemu_find_opts("machine"), 0);
> >> +    if (mach_opts) {
> >> +        usb_enabled = qemu_opt_get_bool(mach_opts, "usb", true);
> >> +    }
> >> +
> >> +    if (usb_enabled) {
> >> +        pci_create_simple(QLIST_FIRST(&spapr->phbs)->host_state.bus,
> >> +                          -1, "pci-ohci");
> >> +    }
> >> +
> >
> >
> > This needs to go into a separate patch. This patch is about moving the
> > global usb_enabled variable towards a machine opt. It shouldn't modify any
> > code outside of that scope, least of all add usb_enabled support for a new
> > platform!
> >
> I see. I will redo this patch. Send out it later. :)
> Thanks for your comments.

When you do that, please also change the default to make spapr _not_
have usb.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson