Re: [PATCH 3/7] hw/ide: Move IDE device related definitions to ide-dev.h

[Philippe Mathieu-Daudé]

On 19/2/24 20:17, Thomas Huth wrote:
> On 19/02/2024 12.32, Philippe Mathieu-Daudé wrote:
> > On 19/2/24 11:49, Thomas Huth wrote:
> > > Let's start to unentangle internal.h by moving public IDE device
> > > related definitions to ide-dev.h.
> > >
> > > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > > ---
> > >   include/hw/ide/ide-dev.h  | 145 +++++++++++++++++++++++++++++++++++++-
> > >   include/hw/ide/internal.h | 145 +-------------------------------------
> > >   hw/ide/ide-dev.c          |   1 +
> > >   3 files changed, 146 insertions(+), 145 deletions(-)
> > >
> > > diff --git a/include/hw/ide/ide-dev.h b/include/hw/ide/ide-dev.h
> > > index 7e9663cda9..de88784a25 100644
> > > --- a/include/hw/ide/ide-dev.h
> > > +++ b/include/hw/ide/ide-dev.h
> > > @@ -20,9 +20,152 @@
> > >   #ifndef IDE_DEV_H
> > >   #define IDE_DEV_H
> > > +#include "sysemu/dma.h"
> >
> > Not required.
>
> It's required for QEMUSGList that is used in struct IDEState.

Oh, OK.

> > >   #include "hw/qdev-properties.h"
> > >   #include "hw/block/block.h"
> > > -#include "hw/ide/internal.h"
> > > +
> > > +typedef struct IDEDevice IDEDevice;
> > > +typedef struct IDEState IDEState;
> >
> > > +typedef struct IDEDMA IDEDMA;
> > > +typedef struct IDEDMAOps IDEDMAOps;
> > > +typedef struct IDEBus IDEBus;
> >
> > Looking at next patches, better forward-declare IDEBus and
> > IDEDMA in "qemu/typedefs.h".
>
> I really dislike using qemu/typedefs.h for things that are not really 
> part of the core framework, since it's a 
> touch-it-once-and-everything-gets-recompiled header. So IMHO the 
> typedefs here are the lesser evil.

OK then.

> > IDEDMAOps and "sysemu/dma.h" belong to "hw/ide/ide-dma.h.
>
> Ok, I can move the typedef for IDEDMAOps to ide-dma.h instead.
>
>   Thomas