Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
On Tue, May 05, 2015 at 12:32:37PM +0200, Thomas Huth wrote: On Tue, 5 May 2015 12:18:05 +0200 Miroslav Rezanina mreza...@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 mreza...@redhat.com wrote: From: Miroslav Rezanina mreza...@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 mreza...@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
Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
On Tue, May 05, 2015 at 11:59:50AM +0200, Thomas Huth wrote: On Tue, 5 May 2015 11:30:49 +0200 mreza...@redhat.com wrote: From: Miroslav Rezanina mreza...@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 mreza...@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
Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
mreza...@redhat.com writes:
From: Miroslav Rezanina mreza...@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 mreza...@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 arm...@redhat.com
Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
On Tue, 5 May 2015 11:30:49 +0200 mreza...@redhat.com wrote: From: Miroslav Rezanina mreza...@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 mreza...@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
Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
On Tue, 5 May 2015 12:18:05 +0200 Miroslav Rezanina mreza...@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 mreza...@redhat.com wrote: From: Miroslav Rezanina mreza...@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 mreza...@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
