* [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
@ 2015-05-05 9:30 mrezanin
2015-05-05 9:59 ` Thomas Huth
2015-05-05 14:37 ` Markus Armbruster
0 siblings, 2 replies; 6+ messages in thread
From: mrezanin @ 2015-05-05 9:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Miroslav Rezanina
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
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);
--
2.1.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
2015-05-05 9:30 [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL mrezanin
@ 2015-05-05 9:59 ` Thomas Huth
2015-05-05 10:18 ` Miroslav Rezanina
2015-05-05 14:37 ` Markus Armbruster
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2015-05-05 9:59 UTC (permalink / raw)
To: mrezanin; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
2015-05-05 9:59 ` Thomas Huth
@ 2015-05-05 10:18 ` Miroslav Rezanina
2015-05-05 10:32 ` Thomas Huth
0 siblings, 1 reply; 6+ messages in thread
From: Miroslav Rezanina @ 2015-05-05 10:18 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
2015-05-05 10:18 ` Miroslav Rezanina
@ 2015-05-05 10:32 ` Thomas Huth
2015-05-05 10:42 ` Miroslav Rezanina
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2015-05-05 10:32 UTC (permalink / raw)
To: Miroslav Rezanina; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
2015-05-05 10:32 ` Thomas Huth
@ 2015-05-05 10:42 ` Miroslav Rezanina
0 siblings, 0 replies; 6+ messages in thread
From: Miroslav Rezanina @ 2015-05-05 10:42 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL
2015-05-05 9:30 [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL mrezanin
2015-05-05 9:59 ` Thomas Huth
@ 2015-05-05 14:37 ` Markus Armbruster
1 sibling, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2015-05-05 14:37 UTC (permalink / raw)
To: mrezanin; +Cc: qemu-devel
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>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-05-06 7:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-05 9:30 [Qemu-devel] [PATCH] parallel: Allow to disable CONFIG_PARALLEL mrezanin
2015-05-05 9:59 ` Thomas Huth
2015-05-05 10:18 ` Miroslav Rezanina
2015-05-05 10:32 ` Thomas Huth
2015-05-05 10:42 ` Miroslav Rezanina
2015-05-05 14:37 ` Markus Armbruster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).