Re: [Qemu-devel] [PATCH 5/5] parallel: convert isa to qdev

[Markus Armbruster <armbru@redhat.com>]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/mips_malta.c |    2 +-
>  hw/parallel.c   |   80 ++++++++++++++++++++++++++++++++++++++++++------------
>  hw/pc.c         |    3 +-
>  hw/pc.h         |    2 +-
>  hw/sun4u.c      |    3 +-
>  5 files changed, 66 insertions(+), 24 deletions(-)
>
[...]
> diff --git a/hw/parallel.c b/hw/parallel.c
> index faaaa0d..30c53a2 100644
> --- a/hw/parallel.c
> +++ b/hw/parallel.c
> @@ -78,6 +78,13 @@ struct ParallelState {
>      int it_shift;
>  };
>  
> +typedef struct ISAParallelState {
> +    ISADevice dev;
> +    uint32_t iobase;
> +    uint32_t isairq;
> +    ParallelState state;
> +} ISAParallelState;
> +
>  static void parallel_update_irq(ParallelState *s)
>  {
>      if (s->irq_pending)
> @@ -438,38 +445,56 @@ static void parallel_reset(void *opaque)
>      s->last_read_offset = ~0U;
>  }
>  
> -/* If fd is zero, it means that the parallel device uses the console */
> -ParallelState *parallel_init(int base, qemu_irq irq, CharDriverState *chr)
> +static int parallel_isa_initfn(ISADevice *dev)
>  {
> -    ParallelState *s;
> +    ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
> +    ParallelState *s = &isa->state;
>      uint8_t dummy;
>  
> -    s = qemu_mallocz(sizeof(ParallelState));
> -    s->irq = irq;
> -    s->chr = chr;
> +    if (!s->chr) {
> +        fprintf(stderr, "Can't create parallel device, empty char device\n");
> +        exit(1);
> +    }
> +
> +    isa_init_irq(dev, &s->irq, isa->isairq);
>      parallel_reset(s);
>      qemu_register_reset(parallel_reset, s);
>  
> -    if (qemu_chr_ioctl(chr, CHR_IOCTL_PP_READ_STATUS, &dummy) == 0) {
> +    if (qemu_chr_ioctl(s->chr, CHR_IOCTL_PP_READ_STATUS, &dummy) == 0) {
>          s->hw_driver = 1;
>          s->status = dummy;
>      }
>  
>      if (s->hw_driver) {
> -        register_ioport_write(base, 8, 1, parallel_ioport_write_hw, s);
> -        register_ioport_read(base, 8, 1, parallel_ioport_read_hw, s);
> -        register_ioport_write(base+4, 1, 2, parallel_ioport_eppdata_write_hw2, s);
> -        register_ioport_read(base+4, 1, 2, parallel_ioport_eppdata_read_hw2, s);
> -        register_ioport_write(base+4, 1, 4, parallel_ioport_eppdata_write_hw4, s);
> -        register_ioport_read(base+4, 1, 4, parallel_ioport_eppdata_read_hw4, s);
> -        register_ioport_write(base+0x400, 8, 1, parallel_ioport_ecp_write, s);
> -        register_ioport_read(base+0x400, 8, 1, parallel_ioport_ecp_read, s);
> +        register_ioport_write(isa->iobase, 8, 1, parallel_ioport_write_hw, s);
> +        register_ioport_read(isa->iobase, 8, 1, parallel_ioport_read_hw, s);
> +        register_ioport_write(isa->iobase+4, 1, 2, parallel_ioport_eppdata_write_hw2, s);
> +        register_ioport_read(isa->iobase+4, 1, 2, parallel_ioport_eppdata_read_hw2, s);
> +        register_ioport_write(isa->iobase+4, 1, 4, parallel_ioport_eppdata_write_hw4, s);
> +        register_ioport_read(isa->iobase+4, 1, 4, parallel_ioport_eppdata_read_hw4, s);
> +        register_ioport_write(isa->iobase+0x400, 8, 1, parallel_ioport_ecp_write, s);
> +        register_ioport_read(isa->iobase+0x400, 8, 1, parallel_ioport_ecp_read, s);
>      }
>      else {
> -        register_ioport_write(base, 8, 1, parallel_ioport_write_sw, s);
> -        register_ioport_read(base, 8, 1, parallel_ioport_read_sw, s);
> +        register_ioport_write(isa->iobase, 8, 1, parallel_ioport_write_sw, s);
> +        register_ioport_read(isa->iobase, 8, 1, parallel_ioport_read_sw, s);

Suggest to use "int base = isa->iobase" here, to reduce line length.
Also yields a tidier diff.

>      }
> -    return s;
> +    return 0;
> +}
> +
[...]