Re: [PATCH 05/16] fdc: Open-code fdctrl_init_isa()

[Philippe Mathieu-Daudé]

On 6/5/20 4:56 PM, Markus Armbruster wrote:
> Helper function fdctrl_init_isa() is less than helpful: one of three
> places creating "isa-fdc" devices use it.  Open-code it there, and
> drop the function.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  include/hw/block/fdc.h |  1 -
>  hw/block/fdc.c         | 14 --------------
>  hw/i386/pc.c           |  8 ++++++--
>  3 files changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h
> index 8855d3476c..d232d3fa1e 100644
> --- a/include/hw/block/fdc.h
> +++ b/include/hw/block/fdc.h
> @@ -10,7 +10,6 @@
>  #define TYPE_ISA_FDC "isa-fdc"
>  
>  void isa_fdc_init_drives(ISADevice *fdc, DriveInfo **fds);
> -ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds);
>  void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
>                          hwaddr mmio_base, DriveInfo **fds);
>  void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base,
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index 8c2d0edd48..35e734b6fb 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2562,20 +2562,6 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, 
> DeviceState *fdc_dev,
>      }
>  }
>  
> -ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds)
> -{
> -    ISADevice *isadev;
> -
> -    isadev = isa_try_new(TYPE_ISA_FDC);
> -    if (!isadev) {
> -        return NULL;
> -    }
> -    isa_realize_and_unref(isadev, bus, &error_fatal);
> -
> -    isa_fdc_init_drives(isadev, fds);
> -    return isadev;
> -}
> -
>  void fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
>                          hwaddr mmio_base, DriveInfo **fds)
>  {
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 0cffb67c2f..25c9577c15 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1141,7 +1141,7 @@ static void pc_superio_init(ISABus *isa_bus, bool 
> create_fdctrl, bool no_vmport)
>      int i;
>      DriveInfo *fd[MAX_FD];
>      qemu_irq *a20_line;
> -    ISADevice *i8042, *port92, *vmmouse;
> +    ISADevice *fdc, *i8042, *port92, *vmmouse;
>  
>      serial_hds_isa_init(isa_bus, 0, MAX_ISA_SERIAL_PORTS);
>      parallel_hds_isa_init(isa_bus, MAX_PARALLEL_PORTS);
> @@ -1151,7 +1151,11 @@ static void pc_superio_init(ISABus *isa_bus, bool 
> create_fdctrl, bool no_vmport)
>          create_fdctrl |= !!fd[i];
>      }
>      if (create_fdctrl) {
> -        fdctrl_init_isa(isa_bus, fd);
> +        fdc = isa_new(TYPE_ISA_FDC);
> +        if (fdc) {
> +            isa_realize_and_unref(fdc, isa_bus, &error_fatal);
> +            isa_fdc_init_drives(fdc, fd);
> +        }
>      }
>  
>      i8042 = isa_create_simple(isa_bus, "i8042");
>