Message ID | 20190704180350.2086-1-julio.montes@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | hw/i386: Fix linker error when ISAPC is disabled | expand |
Patchew URL: https://patchew.org/QEMU/20190704180350.2086-1-julio.montes@intel.com/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [Qemu-devel] [PATCH] hw/i386: Fix linker error when ISAPC is disabled Type: series Message-id: 20190704180350.2086-1-julio.montes@intel.com === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Switched to a new branch 'test' 66d554b hw/i386: Fix linker error when ISAPC is disabled === OUTPUT BEGIN === ERROR: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 65 lines checked Commit 66d554ba6645 (hw/i386: Fix linker error when ISAPC is disabled) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20190704180350.2086-1-julio.montes@intel.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On 04/07/19 20:03, Julio Montes wrote: > How about a new header file with all devices? (see below patch) Yes, looks good. Paolo > --- > Makefile.target | 5 +++++ > hw/i386/pc_piix.c | 11 ++++++++--- > include/qemu/osdep.h | 1 + > scripts/create_config | 2 ++ > 4 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/Makefile.target b/Makefile.target > index a6919e0caf..65eda0994d 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -45,6 +45,9 @@ include $(SRC_PATH)/tests/tcg/Makefile.include > config-target.h: config-target.h-timestamp > config-target.h-timestamp: config-target.mak > > +config-devices.h: config-devices.h-timestamp > +config-devices.h-timestamp: config-devices.mak > + > ifdef CONFIG_TRACE_SYSTEMTAP > stap: $(QEMU_PROG).stp-installed $(QEMU_PROG).stp $(QEMU_PROG)-simpletrace.stp $(QEMU_PROG)-log.stp > > @@ -170,6 +173,8 @@ generated-files-y += hmp-commands.h hmp-commands-info.h > > endif # CONFIG_SOFTMMU > > +generated-files-y += config-devices.h > + > dummy := $(call unnest-vars,,obj-y) > all-obj-y := $(obj-y) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index c07c4a5b38..b93f9327be 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -61,9 +61,11 @@ > > #define MAX_IDE_BUS 2 > > +#ifdef CONFIG_IDE_ISA > static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 }; > static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 }; > static const int ide_irq[MAX_IDE_BUS] = { 14, 15 }; > +#endif > > /* PC hardware initialisation */ > static void pc_init1(MachineState *machine, > @@ -254,7 +256,10 @@ static void pc_init1(MachineState *machine, > } > idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0"); > idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1"); > - } else { > + pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); > + } > +#ifdef CONFIG_IDE_ISA > +else { > for(i = 0; i < MAX_IDE_BUS; i++) { > ISADevice *dev; > char busname[] = "ide.0"; > @@ -268,9 +273,9 @@ static void pc_init1(MachineState *machine, > busname[4] = '0' + i; > idebus[i] = qdev_get_child_bus(DEVICE(dev), busname); > } > + pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); > } > - > - pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); > +#endif > > if (pcmc->pci_enabled && machine_usb(machine)) { > pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index af2b91f0b8..83b49a1e63 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -30,6 +30,7 @@ > #include "config-host.h" > #ifdef NEED_CPU_H > #include "config-target.h" > +#include "config-devices.h" > #else > #include "exec/poison.h" > #endif > diff --git a/scripts/create_config b/scripts/create_config > index d727e5e36e..00e86c82b0 100755 > --- a/scripts/create_config > +++ b/scripts/create_config > @@ -58,6 +58,8 @@ case $line in > name=${line%=*} > echo "#define $name 1" > ;; > + CONFIG_*=n) # configuration > + ;; > CONFIG_*=*) # configuration > name=${line%=*} > value=${line#*=} > -- > 2.17.2 >
On 04/07/19 20:03, Julio Montes wrote: > How about a new header file with all devices? (see below patch) Yes, looks good. Paolo > --- > Makefile.target | 5 +++++ > hw/i386/pc_piix.c | 11 ++++++++--- > include/qemu/osdep.h | 1 + > scripts/create_config | 2 ++ > 4 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/Makefile.target b/Makefile.target > index a6919e0caf..65eda0994d 100644 > --- a/Makefile.target > +++ b/Makefile.target > @@ -45,6 +45,9 @@ include $(SRC_PATH)/tests/tcg/Makefile.include > config-target.h: config-target.h-timestamp > config-target.h-timestamp: config-target.mak > > +config-devices.h: config-devices.h-timestamp > +config-devices.h-timestamp: config-devices.mak > + > ifdef CONFIG_TRACE_SYSTEMTAP > stap: $(QEMU_PROG).stp-installed $(QEMU_PROG).stp $(QEMU_PROG)-simpletrace.stp $(QEMU_PROG)-log.stp > > @@ -170,6 +173,8 @@ generated-files-y += hmp-commands.h hmp-commands-info.h > > endif # CONFIG_SOFTMMU > > +generated-files-y += config-devices.h > + > dummy := $(call unnest-vars,,obj-y) > all-obj-y := $(obj-y) > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > index c07c4a5b38..b93f9327be 100644 > --- a/hw/i386/pc_piix.c > +++ b/hw/i386/pc_piix.c > @@ -61,9 +61,11 @@ > > #define MAX_IDE_BUS 2 > > +#ifdef CONFIG_IDE_ISA > static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 }; > static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 }; > static const int ide_irq[MAX_IDE_BUS] = { 14, 15 }; > +#endif > > /* PC hardware initialisation */ > static void pc_init1(MachineState *machine, > @@ -254,7 +256,10 @@ static void pc_init1(MachineState *machine, > } > idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0"); > idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1"); > - } else { > + pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); > + } > +#ifdef CONFIG_IDE_ISA > +else { > for(i = 0; i < MAX_IDE_BUS; i++) { > ISADevice *dev; > char busname[] = "ide.0"; > @@ -268,9 +273,9 @@ static void pc_init1(MachineState *machine, > busname[4] = '0' + i; > idebus[i] = qdev_get_child_bus(DEVICE(dev), busname); > } > + pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); > } > - > - pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); > +#endif > > if (pcmc->pci_enabled && machine_usb(machine)) { > pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); > diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h > index af2b91f0b8..83b49a1e63 100644 > --- a/include/qemu/osdep.h > +++ b/include/qemu/osdep.h > @@ -30,6 +30,7 @@ > #include "config-host.h" > #ifdef NEED_CPU_H > #include "config-target.h" > +#include "config-devices.h" > #else > #include "exec/poison.h" > #endif > diff --git a/scripts/create_config b/scripts/create_config > index d727e5e36e..00e86c82b0 100755 > --- a/scripts/create_config > +++ b/scripts/create_config > @@ -58,6 +58,8 @@ case $line in > name=${line%=*} > echo "#define $name 1" > ;; > + CONFIG_*=n) # configuration > + ;; > CONFIG_*=*) # configuration > name=${line%=*} > value=${line#*=} > -- > 2.17.2 >
Patchew URL: https://patchew.org/QEMU/20190704180350.2086-1-julio.montes@intel.com/ Hi, This series failed build test on s390x host. Please find the details below. === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch set -e echo echo "=== ENV ===" env echo echo "=== PACKAGES ===" rpm -qa echo echo "=== UNAME ===" uname -a CC=$HOME/bin/cc INSTALL=$PWD/install BUILD=$PWD/build mkdir -p $BUILD $INSTALL SRC=$PWD cd $BUILD $SRC/configure --cc=$CC --prefix=$INSTALL make -j4 # XXX: we need reliable clean up # make check -j4 V=1 make install === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20190704180350.2086-1-julio.montes@intel.com/testing.s390x/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
On 7/5/19 7:20 PM, no-reply@patchew.org wrote: > Patchew URL: https://patchew.org/QEMU/20190704180350.2086-1-julio.montes@intel.com/ > > Hi, > > This series failed build test on s390x host. Please find the details below. > > The full log is available at > http://patchew.org/logs/20190704180350.2086-1-julio.montes@intel.com/testing.s390x/?type=message. > --- make[1]: *** No rule to make target 'config-devices.mak', needed by 'config-devices.h-timestamp'. Stop. make: *** [Makefile:472: aarch64_be-linux-user/all] Error 2 make: *** Waiting for unfinished jobs.... I guess linux-user (and bsd-user) doesn't need that header...
On Fri, 2019-07-05 at 19:25 +0200, Philippe Mathieu-Daudé wrote: > On 7/5/19 7:20 PM, no-reply@patchew.org wrote: > > Patchew URL: > > https://patchew.org/QEMU/20190704180350.2086-1-julio.montes@intel.com/ > > > > Hi, > > > > This series failed build test on s390x host. Please find the > > details below. > > > > The full log is available at > > http://patchew.org/logs/20190704180350.2086-1-julio.montes@intel.com/testing.s390x/?type=message. > > --- > > make[1]: *** No rule to make target 'config-devices.mak', needed by > 'config-devices.h-timestamp'. Stop. > make: *** [Makefile:472: aarch64_be-linux-user/all] Error 2 > make: *** Waiting for unfinished jobs.... > > I guess linux-user (and bsd-user) doesn't need that header... Good catch! sending v3 right away, thanks
diff --git a/Makefile.target b/Makefile.target index a6919e0caf..65eda0994d 100644 --- a/Makefile.target +++ b/Makefile.target @@ -45,6 +45,9 @@ include $(SRC_PATH)/tests/tcg/Makefile.include config-target.h: config-target.h-timestamp config-target.h-timestamp: config-target.mak +config-devices.h: config-devices.h-timestamp +config-devices.h-timestamp: config-devices.mak + ifdef CONFIG_TRACE_SYSTEMTAP stap: $(QEMU_PROG).stp-installed $(QEMU_PROG).stp $(QEMU_PROG)-simpletrace.stp $(QEMU_PROG)-log.stp @@ -170,6 +173,8 @@ generated-files-y += hmp-commands.h hmp-commands-info.h endif # CONFIG_SOFTMMU +generated-files-y += config-devices.h + dummy := $(call unnest-vars,,obj-y) all-obj-y := $(obj-y) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index c07c4a5b38..b93f9327be 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -61,9 +61,11 @@ #define MAX_IDE_BUS 2 +#ifdef CONFIG_IDE_ISA static const int ide_iobase[MAX_IDE_BUS] = { 0x1f0, 0x170 }; static const int ide_iobase2[MAX_IDE_BUS] = { 0x3f6, 0x376 }; static const int ide_irq[MAX_IDE_BUS] = { 14, 15 }; +#endif /* PC hardware initialisation */ static void pc_init1(MachineState *machine, @@ -254,7 +256,10 @@ static void pc_init1(MachineState *machine, } idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0"); idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1"); - } else { + pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); + } +#ifdef CONFIG_IDE_ISA +else { for(i = 0; i < MAX_IDE_BUS; i++) { ISADevice *dev; char busname[] = "ide.0"; @@ -268,9 +273,9 @@ static void pc_init1(MachineState *machine, busname[4] = '0' + i; idebus[i] = qdev_get_child_bus(DEVICE(dev), busname); } + pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); } - - pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state); +#endif if (pcmc->pci_enabled && machine_usb(machine)) { pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci"); diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index af2b91f0b8..83b49a1e63 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -30,6 +30,7 @@ #include "config-host.h" #ifdef NEED_CPU_H #include "config-target.h" +#include "config-devices.h" #else #include "exec/poison.h" #endif diff --git a/scripts/create_config b/scripts/create_config index d727e5e36e..00e86c82b0 100755 --- a/scripts/create_config +++ b/scripts/create_config @@ -58,6 +58,8 @@ case $line in name=${line%=*} echo "#define $name 1" ;; + CONFIG_*=n) # configuration + ;; CONFIG_*=*) # configuration name=${line%=*} value=${line#*=}