Message ID | 1430818249-12107-1-git-send-email-mrezanin@redhat.com |
---|---|
State | New |
Headers | show |
On Tue, 5 May 2015 11:30:49 +0200 mrezanin@redhat.com wrote: > From: Miroslav Rezanina <mrezanin@redhat.com> > > Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored > out initialization to parallel_hds_isa_init that is not build. > > Make calling parallel_hds_isa_init depending on CONFIG_PARALLEL so it can > be correctly disabled. > > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> > --- > hw/i386/pc.c | 2 ++ > hw/mips/mips_fulong2e.c | 2 ++ > hw/mips/mips_malta.c | 2 ++ > hw/sparc64/sun4u.c | 2 ++ > 4 files changed, 8 insertions(+) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index a8e6be1..560464e 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1465,7 +1465,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, > } > > serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS); > +#ifdef CONFIG_PARALLEL > parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS); > +#endif Not sure, but is this pre-processor macro really defined if CONFIG_PARALLEL has been set in the makefile? I've hit some similar problem in the past and I had to discover that only the CONFIG_* options from config-host.mak get translated into #defines, all the others don't get translated. I might be wrong, but just to be sure, could you please double-check that CONFIG_PARALLEL is #defined if it's enabled in the .mak file? If not: Where does the build break exactly? Does it fail for all three types, i386, mips and sun? Thomas
On Tue, May 05, 2015 at 11:59:50AM +0200, Thomas Huth wrote: > On Tue, 5 May 2015 11:30:49 +0200 > mrezanin@redhat.com wrote: > > > From: Miroslav Rezanina <mrezanin@redhat.com> > > > > Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored > > out initialization to parallel_hds_isa_init that is not build. > > > > Make calling parallel_hds_isa_init depending on CONFIG_PARALLEL so it can > > be correctly disabled. > > > > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> > > --- > > hw/i386/pc.c | 2 ++ > > hw/mips/mips_fulong2e.c | 2 ++ > > hw/mips/mips_malta.c | 2 ++ > > hw/sparc64/sun4u.c | 2 ++ > > 4 files changed, 8 insertions(+) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index a8e6be1..560464e 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1465,7 +1465,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, > > } > > > > serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS); > > +#ifdef CONFIG_PARALLEL > > parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS); > > +#endif > > Not sure, but is this pre-processor macro really defined if > CONFIG_PARALLEL has been set in the makefile? I've hit some similar > problem in the past and I had to discover that only the CONFIG_* > options from config-host.mak get translated into #defines, all the > others don't get translated. I might be wrong, but just to be sure, > could you please double-check that CONFIG_PARALLEL is #defined if it's > enabled in the .mak file? > Yeah, you're right..I switch the behavior so thought of CONFIG_* from config-host.mak to be the one to not translated. And ifdef masked the issue. This patch is not working and has to be redone. > If not: Where does the build break exactly? Does it fail for all three > types, i386, mips and sun? > I can't test mips/sun build but for i386 it fails on linking where parallel_hds_isa_init does not exists. Mirek > Thomas >
On Tue, 5 May 2015 12:18:05 +0200 Miroslav Rezanina <mrezanin@redhat.com> wrote: > On Tue, May 05, 2015 at 11:59:50AM +0200, Thomas Huth wrote: > > On Tue, 5 May 2015 11:30:49 +0200 > > mrezanin@redhat.com wrote: > > > > > From: Miroslav Rezanina <mrezanin@redhat.com> > > > > > > Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored > > > out initialization to parallel_hds_isa_init that is not build. > > > > > > Make calling parallel_hds_isa_init depending on CONFIG_PARALLEL so it can > > > be correctly disabled. > > > > > > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> > > > --- > > > hw/i386/pc.c | 2 ++ > > > hw/mips/mips_fulong2e.c | 2 ++ > > > hw/mips/mips_malta.c | 2 ++ > > > hw/sparc64/sun4u.c | 2 ++ > > > 4 files changed, 8 insertions(+) > > > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > > index a8e6be1..560464e 100644 > > > --- a/hw/i386/pc.c > > > +++ b/hw/i386/pc.c > > > @@ -1465,7 +1465,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, > > > } > > > > > > serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS); > > > +#ifdef CONFIG_PARALLEL > > > parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS); > > > +#endif > > > > Not sure, but is this pre-processor macro really defined if > > CONFIG_PARALLEL has been set in the makefile? I've hit some similar > > problem in the past and I had to discover that only the CONFIG_* > > options from config-host.mak get translated into #defines, all the > > others don't get translated. I might be wrong, but just to be sure, > > could you please double-check that CONFIG_PARALLEL is #defined if it's > > enabled in the .mak file? > > > > Yeah, you're right..I switch the behavior so thought of CONFIG_* from > config-host.mak to be the one to not translated. And ifdef masked the > issue. > > This patch is not working and has to be redone. > > > If not: Where does the build break exactly? Does it fail for all three > > types, i386, mips and sun? > > > > I can't test mips/sun build but for i386 it fails on linking where > parallel_hds_isa_init does not exists. That's strange, there is a CONFIG_PARALLEL=y in both, default-configs/x86_64-softmmu.mak and i386-softmmu.mak so I wonder why the parallel.o object file is not linked in your case? Thomas
On Tue, May 05, 2015 at 12:32:37PM +0200, Thomas Huth wrote: > On Tue, 5 May 2015 12:18:05 +0200 > Miroslav Rezanina <mrezanin@redhat.com> wrote: > > > On Tue, May 05, 2015 at 11:59:50AM +0200, Thomas Huth wrote: > > > On Tue, 5 May 2015 11:30:49 +0200 > > > mrezanin@redhat.com wrote: > > > > > > > From: Miroslav Rezanina <mrezanin@redhat.com> > > > > > > > > Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored > > > > out initialization to parallel_hds_isa_init that is not build. > > > > > > > > Make calling parallel_hds_isa_init depending on CONFIG_PARALLEL so it can > > > > be correctly disabled. > > > > > > > > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> > > > > --- > > > > hw/i386/pc.c | 2 ++ > > > > hw/mips/mips_fulong2e.c | 2 ++ > > > > hw/mips/mips_malta.c | 2 ++ > > > > hw/sparc64/sun4u.c | 2 ++ > > > > 4 files changed, 8 insertions(+) > > > > > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > > > index a8e6be1..560464e 100644 > > > > --- a/hw/i386/pc.c > > > > +++ b/hw/i386/pc.c > > > > @@ -1465,7 +1465,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, > > > > } > > > > > > > > serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS); > > > > +#ifdef CONFIG_PARALLEL > > > > parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS); > > > > +#endif > > > > > > Not sure, but is this pre-processor macro really defined if > > > CONFIG_PARALLEL has been set in the makefile? I've hit some similar > > > problem in the past and I had to discover that only the CONFIG_* > > > options from config-host.mak get translated into #defines, all the > > > others don't get translated. I might be wrong, but just to be sure, > > > could you please double-check that CONFIG_PARALLEL is #defined if it's > > > enabled in the .mak file? > > > > > > > Yeah, you're right..I switch the behavior so thought of CONFIG_* from > > config-host.mak to be the one to not translated. And ifdef masked the > > issue. > > > > This patch is not working and has to be redone. > > > > > If not: Where does the build break exactly? Does it fail for all three > > > types, i386, mips and sun? > > > > > > > I can't test mips/sun build but for i386 it fails on linking where > > parallel_hds_isa_init does not exists. > > That's strange, there is a CONFIG_PARALLEL=y in both, > default-configs/x86_64-softmmu.mak and i386-softmmu.mak so I wonder > why the parallel.o object file is not linked in your case? > > Thomas It is enabled by default but build fails in case you disable the option for these architectures. Mirek
mrezanin@redhat.com writes: > From: Miroslav Rezanina <mrezanin@redhat.com> > > Disabling CONFIG_PARALLEL cause build failure as commit 07dc788 factored > out initialization to parallel_hds_isa_init that is not build. > > Make calling parallel_hds_isa_init depending on CONFIG_PARALLEL so it can > be correctly disabled. > > Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com> Sorry for breaking this. Hope I didn't break more in the same series. Have you tried CONFIG_SERIAL? > --- > hw/i386/pc.c | 2 ++ > hw/mips/mips_fulong2e.c | 2 ++ > hw/mips/mips_malta.c | 2 ++ > hw/sparc64/sun4u.c | 2 ++ > 4 files changed, 8 insertions(+) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index a8e6be1..560464e 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1465,7 +1465,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, > } > > serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS); > +#ifdef CONFIG_PARALLEL > parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS); > +#endif > > a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2); > i8042 = isa_create_simple(isa_bus, "i8042"); We could confine the #ifdef to just one place, by doing patching pc.h like +#ifdef CONFIG_PARALLEL void parallel_hds_isa_init(ISABus *bus, int n); +#else +static inline void parallel_hds_isa_init(ISABus *bus, int n) { } +#endif But I'm fine with your patch as is. Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff --git a/hw/i386/pc.c b/hw/i386/pc.c index a8e6be1..560464e 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1465,7 +1465,9 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, } serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS); +#ifdef CONFIG_PARALLEL parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS); +#endif a20_line = qemu_allocate_irqs(handle_a20_line_change, first_cpu, 2); i8042 = isa_create_simple(isa_bus, "i8042"); diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c index dea941a..bd73fa6 100644 --- a/hw/mips/mips_fulong2e.c +++ b/hw/mips/mips_fulong2e.c @@ -384,7 +384,9 @@ static void mips_fulong2e_init(MachineState *machine) rtc_init(isa_bus, 2000, NULL); serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS); +#ifdef CONFIG_PARALLEL parallel_hds_isa_init(isa_bus, 1); +#endif /* Sound card */ audio_init(pci_bus); diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c index 482250d..f2246fb 100644 --- a/hw/mips/mips_malta.c +++ b/hw/mips/mips_malta.c @@ -1173,7 +1173,9 @@ void mips_malta_init(MachineState *machine) rtc_init(isa_bus, 2000, NULL); serial_hds_isa_init(isa_bus, 2); +#ifdef CONFIG_PARALLEL parallel_hds_isa_init(isa_bus, 1); +#endif for(i = 0; i < MAX_FD; i++) { fd[i] = drive_get(IF_FLOPPY, 0, i); diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c index 6f34e87..6b88558 100644 --- a/hw/sparc64/sun4u.c +++ b/hw/sparc64/sun4u.c @@ -855,7 +855,9 @@ static void sun4uv_init(MemoryRegion *address_space_mem, } serial_hds_isa_init(isa_bus, MAX_SERIAL_PORTS); +#ifdef CONFIG_PARALLEL parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS); +#endif for(i = 0; i < nb_nics; i++) pci_nic_init_nofail(&nd_table[i], pci_bus, "ne2k_pci", NULL);