For the downstream distribution of QEMU, we want to compile without
CONFIG_PARALLEL. Commit 9157eee1b1c076ff3 already moved the function
parallel_hds_isa_init() (which is still required for linking) into a file
that is included anyway, but commit bb3d5ea858e7f888563a moved it
to a separate file which is only compiled again if CONFIG_PARALLEL is
set. To be able to link QEMU again without CONFIG_PARALLEL, let's
move this file unconditionally to common-obj-y again. And while we're
at it, also rename it to parallel-helper.c (since parallel.c is also
about ISA already), add a proper comment in there with the rationale
for the separate file, and a check via object_class_by_name() to see
whether the device class is available in the binary or not.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/char/Makefile.objs                         |  2 +-
 hw/char/{parallel-isa.c => parallel-helper.c} | 10 +++++++++-
 hw/char/parallel.c                            |  1 -
 hw/i386/Kconfig                               |  2 --
 include/hw/char/parallel.h                    |  2 ++
 5 files changed, 12 insertions(+), 5 deletions(-)
 rename hw/char/{parallel-isa.c => parallel-helper.c} (70%)

diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
index c4947d7..5476803 100644
--- a/hw/char/Makefile.objs
+++ b/hw/char/Makefile.objs
@@ -2,7 +2,7 @@ common-obj-$(CONFIG_IPACK) += ipoctal232.o
 common-obj-$(CONFIG_ESCC) += escc.o
 common-obj-$(CONFIG_NRF51_SOC) += nrf51_uart.o
 common-obj-$(CONFIG_PARALLEL) += parallel.o
-common-obj-$(CONFIG_PARALLEL) += parallel-isa.o
+common-obj-y += parallel-helper.o
 common-obj-$(CONFIG_PL011) += pl011.o
 common-obj-$(CONFIG_SERIAL) += serial.o
 common-obj-$(CONFIG_SERIAL_ISA) += serial-isa.o
diff --git a/hw/char/parallel-isa.c b/hw/char/parallel-helper.c
similarity index 70%
rename from hw/char/parallel-isa.c
rename to hw/char/parallel-helper.c
index 639e179..c3e868b 100644
--- a/hw/char/parallel-isa.c
+++ b/hw/char/parallel-helper.c
@@ -1,11 +1,15 @@
 /*
  * QEMU Parallel PORT (ISA bus helpers)
  *
+ * These functions reside in a separate file since they also might be
+ * required for linking when compiling QEMU without CONFIG_PARALLEL.
+ *
  * Copyright (c) 2003 Fabrice Bellard
  *
  * SPDX-License-Identifier: MIT
  */
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 #include "hw/isa/isa.h"
 #include "hw/char/parallel.h"
@@ -15,7 +19,7 @@ static void parallel_init(ISABus *bus, int index, Chardev *chr)
     DeviceState *dev;
     ISADevice *isadev;
 
-    isadev = isa_create(bus, "isa-parallel");
+    isadev = isa_create(bus, TYPE_ISA_PARALLEL);
     dev = DEVICE(isadev);
     qdev_prop_set_uint32(dev, "index", index);
     qdev_prop_set_chr(dev, "chardev", chr);
@@ -28,6 +32,10 @@ void parallel_hds_isa_init(ISABus *bus, int n)
 
     assert(n <= MAX_PARALLEL_PORTS);
 
+    if (!object_class_by_name(TYPE_ISA_PARALLEL)) {
+        return;
+    }
+
     for (i = 0; i < n; i++) {
         if (parallel_hds[i]) {
             parallel_init(bus, i, parallel_hds[i]);
diff --git a/hw/char/parallel.c b/hw/char/parallel.c
index a80da47..a19e7d9 100644
--- a/hw/char/parallel.c
+++ b/hw/char/parallel.c
@@ -85,7 +85,6 @@ typedef struct ParallelState {
     PortioList portio_list;
 } ParallelState;
 
-#define TYPE_ISA_PARALLEL "isa-parallel"
 #define ISA_PARALLEL(obj) \
     OBJECT_CHECK(ISAParallelState, (obj), TYPE_ISA_PARALLEL)
 
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 78fd703..4d8247a 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -25,8 +25,6 @@ config PC
     select I82374
     select I8257
     select MC146818RTC
-    # Needed by the board code:
-    select PARALLEL
     # For ACPI builder:
     select SERIAL_ISA
     select ACPI_VMGENID
diff --git a/include/hw/char/parallel.h b/include/hw/char/parallel.h
index d6dd62f..e0a7fe1 100644
--- a/include/hw/char/parallel.h
+++ b/include/hw/char/parallel.h
@@ -5,6 +5,8 @@
 #include "hw/isa/isa.h"
 #include "chardev/char.h"
 
+#define TYPE_ISA_PARALLEL "isa-parallel"
+
 void parallel_hds_isa_init(ISABus *bus, int n);
 
 bool parallel_mm_init(MemoryRegion *address_space,
-- 
1.8.3.1

On 09/03/2019 09.12, Thomas Huth wrote:
> For the downstream distribution of QEMU, we want to compile without
> CONFIG_PARALLEL. Commit 9157eee1b1c076ff3 already moved the function
> parallel_hds_isa_init() (which is still required for linking) into a file
> that is included anyway, but commit bb3d5ea858e7f888563a moved it
> to a separate file which is only compiled again if CONFIG_PARALLEL is
> set. To be able to link QEMU again without CONFIG_PARALLEL, let's
> move this file unconditionally to common-obj-y again. And while we're
> at it, also rename it to parallel-helper.c (since parallel.c is also
> about ISA already), add a proper comment in there with the rationale
> for the separate file, and a check via object_class_by_name() to see
> whether the device class is available in the binary or not.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/char/Makefile.objs                         |  2 +-
>  hw/char/{parallel-isa.c => parallel-helper.c} | 10 +++++++++-
>  hw/char/parallel.c                            |  1 -
>  hw/i386/Kconfig                               |  2 --
>  include/hw/char/parallel.h                    |  2 ++
>  5 files changed, 12 insertions(+), 5 deletions(-)
>  rename hw/char/{parallel-isa.c => parallel-helper.c} (70%)
> 
> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> index c4947d7..5476803 100644
> --- a/hw/char/Makefile.objs
> +++ b/hw/char/Makefile.objs
> @@ -2,7 +2,7 @@ common-obj-$(CONFIG_IPACK) += ipoctal232.o
>  common-obj-$(CONFIG_ESCC) += escc.o
>  common-obj-$(CONFIG_NRF51_SOC) += nrf51_uart.o
>  common-obj-$(CONFIG_PARALLEL) += parallel.o
> -common-obj-$(CONFIG_PARALLEL) += parallel-isa.o
> +common-obj-y += parallel-helper.o

Self-NACK.

This has to be "common-obj-$(CONFIG_ISA_BUS) += parallel-helper.o" since
it uses the function isa_create(). ... thus, maybe I should also not
rename the file here... I'll ponder about that in a v2...

 Thomas

Patchew URL: https://patchew.org/QEMU/1552119170-15474-1-git-send-email-thuth@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  GEN     x86_64-softmmu/qemu-system-x86_64.exe
../hw/char/parallel-helper.o: In function `parallel_init':
/tmp/qemu-test/src/hw/char/parallel-helper.c:22: undefined reference to `isa_create'
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:212: qemu-system-aarch64w.exe] Error 1
make: *** [Makefile:449: subdir-aarch64-softmmu] Error 2
make: *** Waiting for unfinished jobs....


The full log is available at
http://patchew.org/logs/1552119170-15474-1-git-send-email-thuth@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

On Sat, 9 Mar 2019 at 08:21, Thomas Huth <thuth@redhat.com> wrote:
>
> For the downstream distribution of QEMU, we want to compile without
> CONFIG_PARALLEL. Commit 9157eee1b1c076ff3 already moved the function
> parallel_hds_isa_init() (which is still required for linking) into a file
> that is included anyway, but commit bb3d5ea858e7f888563a moved it
> to a separate file which is only compiled again if CONFIG_PARALLEL is
> set. To be able to link QEMU again without CONFIG_PARALLEL, let's
> move this file unconditionally to common-obj-y again. And while we're
> at it, also rename it to parallel-helper.c (since parallel.c is also
> about ISA already)

parallel.c also provides an MMIO interface to the
parallel port via parallel_mm_init(), used by the MIPS
Jazz board. (In an ideal world this would be a proper
device, but it is ancient code.)

thanks
-- PMM