[PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h

Thomas Huth posted 7 patches 8 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240220085505.30255-1-thuth@redhat.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, John Snow <jsnow@redhat.com>, BALATON Zoltan <balaton@eik.bme.hu>
MAINTAINERS                  |   1 -
include/hw/ide.h             |   9 --
include/hw/ide/ide-bus.h     |  42 +++++++
include/hw/ide/ide-dev.h     | 184 ++++++++++++++++++++++++++++++
include/hw/ide/ide-dma.h     |  37 ++++++
include/hw/ide/internal.h    | 211 +----------------------------------
include/hw/ide/pci.h         |   2 +-
hw/i386/pc.c                 |   2 +-
hw/ide/cf.c                  |  58 ++++++++++
hw/ide/cmd646.c              |   1 +
hw/ide/ide-bus.c             | 111 ++++++++++++++++++
hw/ide/{qdev.c => ide-dev.c} | 137 +----------------------
hw/ide/pci.c                 |   1 +
hw/ide/piix.c                |   1 +
hw/ide/sii3112.c             |   1 +
hw/ide/via.c                 |   1 +
hw/arm/Kconfig               |   2 +
hw/ide/Kconfig               |  32 ++++--
hw/ide/meson.build           |   4 +-
19 files changed, 470 insertions(+), 367 deletions(-)
delete mode 100644 include/hw/ide.h
create mode 100644 include/hw/ide/ide-bus.h
create mode 100644 include/hw/ide/ide-dev.h
create mode 100644 include/hw/ide/ide-dma.h
create mode 100644 hw/ide/cf.c
create mode 100644 hw/ide/ide-bus.c
rename hw/ide/{qdev.c => ide-dev.c} (67%)
[PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h
Posted by Thomas Huth 8 months, 3 weeks ago
While trying to make it possible to compile-out the CompactFlash IDE device
in downstream distributions (first patch), we noticed that there are more
things in the IDE code that could use a proper clean up:

First, hw/ide/qdev.c is quite a mix between IDE BUS specific functions
and (disk) device specific functions. Thus the second patch splits qdev.c
into two new separate files to make it more obvious which part belongs
to which kind of devices.

The remaining patches unentangle include/hw/ide/internal.h, which is meant
as a header that should only be used internally to the IDE subsystem, but
which is currently exposed to the world since include/hw/ide/pci.h includes
this header, too. Thus we move the definitions that are also required for
non-IDE code to other new header files, so we can finally change pci.h to
stop including internal.h. After these changes, internal.h is only included
by files in hw/ide/ as it should be.

v2:
- Change the order of the DMA patch and move typedef struct IDEDMAOps
  and typedef struct IDEDMA IDEDMA into ide-dma.h, too
- Make sure that the headers are self-contained (i.e. #include the
  right other headers)
- Remove some more remnants of include/hw/ide.h

Thomas Huth (7):
  hw/ide: Add the possibility to disable the CompactFlash device in the
    build
  hw/ide: Split qdev.c into ide-bus.c and ide-dev.c
  hw/ide: Move IDE DMA related definitions to a separate header
    ide-dma.h
  hw/ide: Move IDE device related definitions to ide-dev.h
  hw/ide: Move IDE bus related definitions to a new header ide-bus.h
  hw/ide: Remove the include/hw/ide.h legacy file
  hw/ide: Stop exposing internal.h to non-IDE files

 MAINTAINERS                  |   1 -
 include/hw/ide.h             |   9 --
 include/hw/ide/ide-bus.h     |  42 +++++++
 include/hw/ide/ide-dev.h     | 184 ++++++++++++++++++++++++++++++
 include/hw/ide/ide-dma.h     |  37 ++++++
 include/hw/ide/internal.h    | 211 +----------------------------------
 include/hw/ide/pci.h         |   2 +-
 hw/i386/pc.c                 |   2 +-
 hw/ide/cf.c                  |  58 ++++++++++
 hw/ide/cmd646.c              |   1 +
 hw/ide/ide-bus.c             | 111 ++++++++++++++++++
 hw/ide/{qdev.c => ide-dev.c} | 137 +----------------------
 hw/ide/pci.c                 |   1 +
 hw/ide/piix.c                |   1 +
 hw/ide/sii3112.c             |   1 +
 hw/ide/via.c                 |   1 +
 hw/arm/Kconfig               |   2 +
 hw/ide/Kconfig               |  32 ++++--
 hw/ide/meson.build           |   4 +-
 19 files changed, 470 insertions(+), 367 deletions(-)
 delete mode 100644 include/hw/ide.h
 create mode 100644 include/hw/ide/ide-bus.h
 create mode 100644 include/hw/ide/ide-dev.h
 create mode 100644 include/hw/ide/ide-dma.h
 create mode 100644 hw/ide/cf.c
 create mode 100644 hw/ide/ide-bus.c
 rename hw/ide/{qdev.c => ide-dev.c} (67%)

-- 
2.43.2
Re: [PATCH v2 0/7] hw/ide: Clean up hw/ide/qdev.c and include/hw/ide/internal.h
Posted by Mark Cave-Ayland 8 months, 3 weeks ago
On 20/02/2024 08:54, Thomas Huth wrote:

> While trying to make it possible to compile-out the CompactFlash IDE device
> in downstream distributions (first patch), we noticed that there are more
> things in the IDE code that could use a proper clean up:
> 
> First, hw/ide/qdev.c is quite a mix between IDE BUS specific functions
> and (disk) device specific functions. Thus the second patch splits qdev.c
> into two new separate files to make it more obvious which part belongs
> to which kind of devices.
> 
> The remaining patches unentangle include/hw/ide/internal.h, which is meant
> as a header that should only be used internally to the IDE subsystem, but
> which is currently exposed to the world since include/hw/ide/pci.h includes
> this header, too. Thus we move the definitions that are also required for
> non-IDE code to other new header files, so we can finally change pci.h to
> stop including internal.h. After these changes, internal.h is only included
> by files in hw/ide/ as it should be.
> 
> v2:
> - Change the order of the DMA patch and move typedef struct IDEDMAOps
>    and typedef struct IDEDMA IDEDMA into ide-dma.h, too
> - Make sure that the headers are self-contained (i.e. #include the
>    right other headers)
> - Remove some more remnants of include/hw/ide.h
> 
> Thomas Huth (7):
>    hw/ide: Add the possibility to disable the CompactFlash device in the
>      build
>    hw/ide: Split qdev.c into ide-bus.c and ide-dev.c
>    hw/ide: Move IDE DMA related definitions to a separate header
>      ide-dma.h
>    hw/ide: Move IDE device related definitions to ide-dev.h
>    hw/ide: Move IDE bus related definitions to a new header ide-bus.h
>    hw/ide: Remove the include/hw/ide.h legacy file
>    hw/ide: Stop exposing internal.h to non-IDE files
> 
>   MAINTAINERS                  |   1 -
>   include/hw/ide.h             |   9 --
>   include/hw/ide/ide-bus.h     |  42 +++++++
>   include/hw/ide/ide-dev.h     | 184 ++++++++++++++++++++++++++++++
>   include/hw/ide/ide-dma.h     |  37 ++++++
>   include/hw/ide/internal.h    | 211 +----------------------------------
>   include/hw/ide/pci.h         |   2 +-
>   hw/i386/pc.c                 |   2 +-
>   hw/ide/cf.c                  |  58 ++++++++++
>   hw/ide/cmd646.c              |   1 +
>   hw/ide/ide-bus.c             | 111 ++++++++++++++++++
>   hw/ide/{qdev.c => ide-dev.c} | 137 +----------------------
>   hw/ide/pci.c                 |   1 +
>   hw/ide/piix.c                |   1 +
>   hw/ide/sii3112.c             |   1 +
>   hw/ide/via.c                 |   1 +
>   hw/arm/Kconfig               |   2 +
>   hw/ide/Kconfig               |  32 ++++--
>   hw/ide/meson.build           |   4 +-
>   19 files changed, 470 insertions(+), 367 deletions(-)
>   delete mode 100644 include/hw/ide.h
>   create mode 100644 include/hw/ide/ide-bus.h
>   create mode 100644 include/hw/ide/ide-dev.h
>   create mode 100644 include/hw/ide/ide-dma.h
>   create mode 100644 hw/ide/cf.c
>   create mode 100644 hw/ide/ide-bus.c
>   rename hw/ide/{qdev.c => ide-dev.c} (67%)

I've had a quick skim of this series, and it looks like a good tidy-up to me so:

Acked-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.