Re: [Qemu-devel] [PATCHv3 1/2] Move parallel_hds_isa_init to hw/isa/isa-bus.c

[Miroslav Rezanina]

On Wed, May 13, 2015 at 10:01:12AM +0200, Markus Armbruster wrote:
> address@hidden writes:
> 
> > From: Miroslav Rezanina <address@hidden>
> >
> > Disabling CONFIG_PARALLEL cause removing parallel_hds_isa_init defined in
> > parallel.c. This function is called during initialization of some boards so
> > disabling CONFIG_PARALLEL cause build failure.
> >
> > This patch moves parallel_hds_isa_init to hw/isa/isa-bus.c so it is included
> > in case of disabled CONFIG_PARALLEL. Build is successful but qemu will abort
> > with "Unknown device" error when function is called.
> 
> Can't reproduce this error.  Test case: compile with CONFIG_PARALLEL=y
> taken out, run like
> 
> $ qemu-system-x86_64 -nodefaults -S -display none -parallel stdio
> 
> > Signed-off-by: Miroslav Rezanina <address@hidden>
> > ---
> >  hw/char/parallel.c | 25 -------------------------
> >  hw/isa/isa-bus.c   | 29 +++++++++++++++++++++++++++++
> >  2 files changed, 29 insertions(+), 25 deletions(-)
> >
> > diff --git a/hw/char/parallel.c b/hw/char/parallel.c
> > index 4079554..c2b553f 100644
> > --- a/hw/char/parallel.c
> > +++ b/hw/char/parallel.c
> > @@ -641,28 +641,3 @@ static void parallel_register_types(void)
> >  }
> >  
> >  type_init(parallel_register_types)
> > -
> > -static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
> > -{
> > -    DeviceState *dev;
> > -    ISADevice *isadev;
> > -
> > -    isadev = isa_create(bus, "isa-parallel");
> > -    dev = DEVICE(isadev);
> > -    qdev_prop_set_uint32(dev, "index", index);
> > -    qdev_prop_set_chr(dev, "chardev", chr);
> > -    qdev_init_nofail(dev);
> > -}
> > -
> > -void parallel_hds_isa_init(ISABus *bus, int n)
> > -{
> > -    int i;
> > -
> > -    assert(n <= MAX_PARALLEL_PORTS);
> > -
> > -    for (i = 0; i < n; i++) {
> > -        if (parallel_hds[i]) {
> > -            parallel_init(bus, i, parallel_hds[i]);
> > -        }
> > -    }
> > -}
> > diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> > index 825aa62..94f645c 100644
> > --- a/hw/isa/isa-bus.c
> > +++ b/hw/isa/isa-bus.c
> > @@ -21,6 +21,7 @@
> >  #include "hw/sysbus.h"
> >  #include "sysemu/sysemu.h"
> >  #include "hw/isa/isa.h"
> > +#include "hw/i386/pc.h"
> >  
> >  static ISABus *isabus;
> >  
> > @@ -267,3 +268,31 @@ MemoryRegion *isa_address_space_io(ISADevice *dev)
> >  }
> >  
> >  type_init(isabus_register_types)
> > +
> > +static void parallel_init(ISABus *bus, int index, CharDriverState *chr)
> > +{
> > +    DeviceState *dev;
> > +    ISADevice *isadev;
> > +
> > +    isadev = isa_try_create(bus, "isa-parallel");
> > +    if (!isadev) {
> > +       return;
> > +    }
> 
> Not just code motion!  You change the code to ignore the error.
> 
> If that's what we want, the commit message needs fixing.

Ouch,this is mixed in change from different code version. Intention was to move 
code only. This is the reason to different behavior. I will send fixed
version.

Mirek
> 
> > +    dev = DEVICE(isadev);
> > +    qdev_prop_set_uint32(dev, "index", index);
> > +    qdev_prop_set_chr(dev, "chardev", chr);
> > +    qdev_init_nofail(dev);
> > +}
> > +
> > +void parallel_hds_isa_init(ISABus *bus, int n)
> > +{
> > +    int i;
> > +
> > +    assert(n <= MAX_PARALLEL_PORTS);
> > +
> > +    for (i = 0; i < n; i++) {
> > +        if (parallel_hds[i]) {
> > +            parallel_init(bus, i, parallel_hds[i]);
> > +        }
> > +    }
> > +}