Re: [PATCH 2/7] hw/ide: Split qdev.c into ide-bus.c and ide-dev.c

[BALATON Zoltan]

On Mon, 19 Feb 2024, Thomas Huth wrote:
> qdev.c is a mixture between IDE bus specific functions and IDE device
> functions. Let's split it up to make it more obvious which part is
> related to bus handling and which part is related to device handling.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> hw/ide/ide-bus.c             | 111 +++++++++++++++++++++++++++++++++++
> hw/ide/{qdev.c => ide-dev.c} |  87 +--------------------------
> hw/arm/Kconfig               |   2 +
> hw/ide/Kconfig               |  30 ++++++----
> hw/ide/meson.build           |   3 +-
> 5 files changed, 134 insertions(+), 99 deletions(-)
> create mode 100644 hw/ide/ide-bus.c
> rename hw/ide/{qdev.c => ide-dev.c} (78%)
[...]
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 29abe1da29..b372b819a4 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -275,6 +275,8 @@ config SBSA_REF
>     select USB_XHCI_SYSBUS
>     select WDT_SBSA
>     select BOCHS_DISPLAY
> +    select IDE_BUS
> +    select IDE_DEV
>
> config SABRELITE
>     bool
> diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig
> index b93d6743d5..6dfc5a2129 100644
> --- a/hw/ide/Kconfig
> +++ b/hw/ide/Kconfig
> @@ -1,51 +1,58 @@
> config IDE_CORE
>     bool
>
> -config IDE_QDEV
> +config IDE_BUS
>     bool
>     select IDE_CORE

Maybe we can assume if something has an IDE bus it also wants to connect 
IDE devices to it so just select IDE_DEV here and not at every place 
IDE_BUS is selected? Or is there a place that only wants IDE_BUS?

Regards,
BALATON Zoltan

> +config IDE_DEV
> +    bool
> +    depends on IDE_BUS
> +
> config IDE_PCI
>     bool
>     depends on PCI
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>
> config IDE_ISA
>     bool
>     depends on ISA_BUS
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>
> config IDE_PIIX
>     bool
>     select IDE_PCI
> -    select IDE_QDEV
>
> config IDE_CMD646
>     bool
>     select IDE_PCI
> -    select IDE_QDEV
>
> config IDE_MACIO
>     bool
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>
> config IDE_MMIO
>     bool
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>
> config IDE_VIA
>     bool
>     select IDE_PCI
> -    select IDE_QDEV
>
> config MICRODRIVE
>     bool
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>     depends on PCMCIA
>
> config AHCI
>     bool
> -    select IDE_QDEV
> +    select IDE_BUS
> +    select IDE_DEV
>
> config AHCI_ICH9
>     bool
> @@ -56,8 +63,7 @@ config AHCI_ICH9
> config IDE_SII3112
>     bool
>     select IDE_PCI
> -    select IDE_QDEV
>
> config IDE_CF
>     bool
> -    default y if IDE_QDEV
> +    default y if IDE_BUS
> diff --git a/hw/ide/meson.build b/hw/ide/meson.build
> index d2e5b45c9e..d09705cac0 100644
> --- a/hw/ide/meson.build
> +++ b/hw/ide/meson.build
> @@ -1,15 +1,16 @@
> system_ss.add(when: 'CONFIG_AHCI', if_true: files('ahci.c'))
> system_ss.add(when: 'CONFIG_AHCI_ICH9', if_true: files('ich.c'))
> system_ss.add(when: 'CONFIG_ALLWINNER_A10', if_true: files('ahci-allwinner.c'))
> +system_ss.add(when: 'CONFIG_IDE_BUS', if_true: files('ide-bus.c'))
> system_ss.add(when: 'CONFIG_IDE_CF', if_true: files('cf.c'))
> system_ss.add(when: 'CONFIG_IDE_CMD646', if_true: files('cmd646.c'))
> system_ss.add(when: 'CONFIG_IDE_CORE', if_true: files('core.c', 'atapi.c'))
> +system_ss.add(when: 'CONFIG_IDE_DEV', if_true: files('ide-dev.c'))
> system_ss.add(when: 'CONFIG_IDE_ISA', if_true: files('isa.c', 'ioport.c'))
> system_ss.add(when: 'CONFIG_IDE_MACIO', if_true: files('macio.c'))
> system_ss.add(when: 'CONFIG_IDE_MMIO', if_true: files('mmio.c'))
> system_ss.add(when: 'CONFIG_IDE_PCI', if_true: files('pci.c'))
> system_ss.add(when: 'CONFIG_IDE_PIIX', if_true: files('piix.c', 'ioport.c'))
> -system_ss.add(when: 'CONFIG_IDE_QDEV', if_true: files('qdev.c'))
> system_ss.add(when: 'CONFIG_IDE_SII3112', if_true: files('sii3112.c'))
> system_ss.add(when: 'CONFIG_IDE_VIA', if_true: files('via.c'))
> system_ss.add(when: 'CONFIG_MICRODRIVE', if_true: files('microdrive.c'))