Re: [PATCH 2/2] hw/char/parallel-isa: Export struct ISAParallelState

[Bernhard Beschow]

Am 11. Juni 2023 13:15:58 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>On Sun, 11 Jun 2023, Bernhard Beschow wrote:
>> Allows the struct to be embedded directly into device models without 
>> additional
>> allocation.
>> 
>> Suggested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>
>Patches missing SoB, checkpatch should have cought this.

Thanks for catching again. Fixed in v2.

>
>I don't see any of the machines or device models actually embedding 
>ISAParallelState or ParallelState so don't know what this patch is trying to 
>achieve. Please post the whole series with the patches that this is a 
>preparation for so we can se where this leads.

No further plans from my side.

Best regards,
Bernhard

>
>Regards,
>BALATON Zoltan
>
>> ---
>> include/hw/char/parallel-isa.h | 46 ++++++++++++++++++++++++++++++++++
>> include/hw/char/parallel.h     |  2 --
>> hw/char/parallel-isa.c         |  1 +
>> hw/char/parallel.c             | 12 +--------
>> hw/i386/pc_piix.c              |  2 +-
>> hw/i386/pc_q35.c               |  2 +-
>> hw/isa/isa-superio.c           |  1 +
>> hw/sparc64/sun4u.c             |  2 +-
>> 8 files changed, 52 insertions(+), 16 deletions(-)
>> create mode 100644 include/hw/char/parallel-isa.h
>> 
>> diff --git a/include/hw/char/parallel-isa.h b/include/hw/char/parallel-isa.h
>> new file mode 100644
>> index 0000000000..27bdacf1a3
>> --- /dev/null
>> +++ b/include/hw/char/parallel-isa.h
>> @@ -0,0 +1,46 @@
>> +/*
>> + * QEMU ISA Parallel PORT emulation
>> + *
>> + * Copyright (c) 2003-2005 Fabrice Bellard
>> + * Copyright (c) 2007 Marko Kohtala
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a 
>> copy
>> + * of this software and associated documentation files (the "Software"), to 
>> deal
>> + * in the Software without restriction, including without limitation the 
>> rights
>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
>> + * copies of the Software, and to permit persons to whom the Software is
>> + * furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be included 
>> in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>> OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
>> FROM,
>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
>> + * THE SOFTWARE.
>> + */
>> +
>> +#ifndef HW_PARALLEL_ISA_H
>> +#define HW_PARALLEL_ISA_H
>> +
>> +#include "parallel.h"
>> +
>> +#include "hw/isa/isa.h"
>> +#include "qom/object.h"
>> +
>> +#define TYPE_ISA_PARALLEL "isa-parallel"
>> +OBJECT_DECLARE_SIMPLE_TYPE(ISAParallelState, ISA_PARALLEL)
>> +
>> +struct ISAParallelState {
>> +    ISADevice parent_obj;
>> +
>> +    uint32_t index;
>> +    uint32_t iobase;
>> +    uint32_t isairq;
>> +    ParallelState state;
>> +};
>> +
>> +#endif /* HW_PARALLEL_ISA_H */
>> diff --git a/include/hw/char/parallel.h b/include/hw/char/parallel.h
>> index 9f76edca81..7b5a309a03 100644
>> --- a/include/hw/char/parallel.h
>> +++ b/include/hw/char/parallel.h
>> @@ -25,8 +25,6 @@ typedef struct ParallelState {
>>     PortioList portio_list;
>> } ParallelState;
>> 
>> -#define TYPE_ISA_PARALLEL "isa-parallel"
>> -
>> void parallel_hds_isa_init(ISABus *bus, int n);
>> 
>> bool parallel_mm_init(MemoryRegion *address_space,
>> diff --git a/hw/char/parallel-isa.c b/hw/char/parallel-isa.c
>> index 547ae69304..ab0f879998 100644
>> --- a/hw/char/parallel-isa.c
>> +++ b/hw/char/parallel-isa.c
>> @@ -13,6 +13,7 @@
>> #include "sysemu/sysemu.h"
>> #include "hw/isa/isa.h"
>> #include "hw/qdev-properties.h"
>> +#include "hw/char/parallel-isa.h"
>> #include "hw/char/parallel.h"
>> #include "qapi/error.h"
>> 
>> diff --git a/hw/char/parallel.c b/hw/char/parallel.c
>> index e75fc5019d..147c900f0d 100644
>> --- a/hw/char/parallel.c
>> +++ b/hw/char/parallel.c
>> @@ -31,6 +31,7 @@
>> #include "hw/qdev-properties.h"
>> #include "hw/qdev-properties-system.h"
>> #include "migration/vmstate.h"
>> +#include "hw/char/parallel-isa.h"
>> #include "hw/char/parallel.h"
>> #include "sysemu/reset.h"
>> #include "sysemu/sysemu.h"
>> @@ -73,17 +74,6 @@
>> 
>> #define PARA_CTR_SIGNAL 
>> (PARA_CTR_SELECT|PARA_CTR_INIT|PARA_CTR_AUTOLF|PARA_CTR_STROBE)
>> 
>> -OBJECT_DECLARE_SIMPLE_TYPE(ISAParallelState, ISA_PARALLEL)
>> -
>> -struct ISAParallelState {
>> -    ISADevice parent_obj;
>> -
>> -    uint32_t index;
>> -    uint32_t iobase;
>> -    uint32_t isairq;
>> -    ParallelState state;
>> -};
>> -
>> static void parallel_update_irq(ParallelState *s)
>> {
>>     if (s->irq_pending)
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 42af03dbb4..44146e6ff5 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -26,7 +26,7 @@
>> #include CONFIG_DEVICES
>> 
>> #include "qemu/units.h"
>> -#include "hw/char/parallel.h"
>> +#include "hw/char/parallel-isa.h"
>> #include "hw/dma/i8257.h"
>> #include "hw/loader.h"
>> #include "hw/i386/x86.h"
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index 6155427e48..a9a59ed42b 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -30,7 +30,7 @@
>> 
>> #include "qemu/osdep.h"
>> #include "qemu/units.h"
>> -#include "hw/char/parallel.h"
>> +#include "hw/char/parallel-isa.h"
>> #include "hw/loader.h"
>> #include "hw/i2c/smbus_eeprom.h"
>> #include "hw/rtc/mc146818rtc.h"
>> diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
>> index 9292ec3bcf..7dbfc374da 100644
>> --- a/hw/isa/isa-superio.c
>> +++ b/hw/isa/isa-superio.c
>> @@ -21,6 +21,7 @@
>> #include "hw/isa/superio.h"
>> #include "hw/qdev-properties.h"
>> #include "hw/input/i8042.h"
>> +#include "hw/char/parallel-isa.h"
>> #include "hw/char/serial.h"
>> #include "trace.h"
>> 
>> diff --git a/hw/sparc64/sun4u.c b/hw/sparc64/sun4u.c
>> index e2858a0331..29e9b6cc26 100644
>> --- a/hw/sparc64/sun4u.c
>> +++ b/hw/sparc64/sun4u.c
>> @@ -35,7 +35,7 @@
>> #include "hw/qdev-properties.h"
>> #include "hw/pci-host/sabre.h"
>> #include "hw/char/serial.h"
>> -#include "hw/char/parallel.h"
>> +#include "hw/char/parallel-isa.h"
>> #include "hw/rtc/m48t59.h"
>> #include "migration/vmstate.h"
>> #include "hw/input/i8042.h"
>>