diff mbox

parallel: Allow to disable CONFIG_PARALLEL

Message ID 1430818249-12107-1-git-send-email-mrezanin@redhat.com
State New
Headers show

Commit Message

Miroslav Rezanina May 5, 2015, 9:30 a.m. UTC 
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(+)

Comments

Thomas Huth May 5, 2015, 9:59 a.m. UTC | #1 
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
Miroslav Rezanina May 5, 2015, 10:18 a.m. UTC | #2 
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
>
Thomas Huth May 5, 2015, 10:32 a.m. UTC | #3 
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
Miroslav Rezanina May 5, 2015, 10:42 a.m. UTC | #4 
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
Markus Armbruster May 5, 2015, 2:37 p.m. UTC | #5 
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 mbox

Patch

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);