ide_get_geometry() and ide_get_bios_chs_trans() are only
used by the TYPE_PC_MACHINE.
"hw/ide.h" is a mixed bag of lost IDE declarations. In order
to remove this (almost) pointless header soon, move these
declarations to "hw/ide/internal.h".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c              | 3 ++-
 include/hw/ide.h          | 4 ----
 include/hw/ide/internal.h | 4 ++++
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 6e592bd969..79297a6ecd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -34,7 +34,8 @@
 #include "hw/i386/vmport.h"
 #include "sysemu/cpus.h"
 #include "hw/block/fdc.h"
-#include "hw/ide.h"
+#include "hw/ide/internal.h"
+#include "hw/ide/isa.h"
 #include "hw/pci/pci.h"
 #include "hw/pci/pci_bus.h"
 #include "hw/pci-bridge/pci_expander_bridge.h"
diff --git a/include/hw/ide.h b/include/hw/ide.h
index 24a7aa2925..db963bdb77 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -3,10 +3,6 @@
 
 #include "exec/memory.h"
 
-int ide_get_geometry(BusState *bus, int unit,
-                     int16_t *cyls, int8_t *heads, int8_t *secs);
-int ide_get_bios_chs_trans(BusState *bus, int unit);
-
 /* ide/core.c */
 void ide_drive_get(DriveInfo **hd, int max_bus);
 
diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h
index c2b794150f..d9f1f77dd5 100644
--- a/include/hw/ide/internal.h
+++ b/include/hw/ide/internal.h
@@ -647,6 +647,10 @@ void ide_bus_init(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
                   int bus_id, int max_units);
 IDEDevice *ide_bus_create_drive(IDEBus *bus, int unit, DriveInfo *drive);
 
+int ide_get_geometry(BusState *bus, int unit,
+                     int16_t *cyls, int8_t *heads, int8_t *secs);
+int ide_get_bios_chs_trans(BusState *bus, int unit);
+
 int ide_handle_rw_error(IDEState *s, int error, int op);
 
 #endif /* HW_IDE_INTERNAL_H */
-- 
2.38.1

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> ide_get_geometry() and ide_get_bios_chs_trans() are only
> used by the TYPE_PC_MACHINE.
> "hw/ide.h" is a mixed bag of lost IDE declarations. In order
> to remove this (almost) pointless header soon, move these
> declarations to "hw/ide/internal.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/i386/pc.c              | 3 ++-
>  include/hw/ide.h          | 4 ----
>  include/hw/ide/internal.h | 4 ++++
>  3 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 6e592bd969..79297a6ecd 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -34,7 +34,8 @@
>  #include "hw/i386/vmport.h"
>  #include "sysemu/cpus.h"
>  #include "hw/block/fdc.h"
> -#include "hw/ide.h"
> +#include "hw/ide/internal.h"
> +#include "hw/ide/isa.h"

I do kind of wonder why hw/ide/internal.h isn't in the appropriate
subdir (e.g. hw/ide and included as #include "internal.h") rather than
the "public" include directory. However QEMU isn't super consistent with
that:

  ➜  find . -iname "internal.h"
  ./accel/tcg/internal.h
  ./target/ppc/internal.h
  ./target/mips/internal.h
  ./target/hexagon/internal.h
  ./include/hw/ide/internal.h
  🕙11:15:58 alex@zen:qemu.git  on  review/qom-housekeeping-v2 [$?] took 7s 
  ➜  find . -iname "internals.h"
  ./tests/fp/berkeley-softfloat-3/source/include/internals.h
  ./target/arm/internals.h
  ./target/riscv/internals.h
  ./target/loongarch/internals.h
  ./gdbstub/internals.h

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

On 27/2/23 12:17, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> ide_get_geometry() and ide_get_bios_chs_trans() are only
>> used by the TYPE_PC_MACHINE.
>> "hw/ide.h" is a mixed bag of lost IDE declarations. In order
>> to remove this (almost) pointless header soon, move these
>> declarations to "hw/ide/internal.h".
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/i386/pc.c              | 3 ++-
>>   include/hw/ide.h          | 4 ----
>>   include/hw/ide/internal.h | 4 ++++
>>   3 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 6e592bd969..79297a6ecd 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -34,7 +34,8 @@
>>   #include "hw/i386/vmport.h"
>>   #include "sysemu/cpus.h"
>>   #include "hw/block/fdc.h"
>> -#include "hw/ide.h"
>> +#include "hw/ide/internal.h"
>> +#include "hw/ide/isa.h"
> 
> I do kind of wonder why hw/ide/internal.h isn't in the appropriate
> subdir (e.g. hw/ide and included as #include "internal.h") rather than
> the "public" include directory.

It should be internal but is included by PCI and MacIO:

include/hw/ide/pci.h:4:#include "hw/ide/internal.h"
include/hw/misc/macio/macio.h:31:#include "hw/ide/internal.h"

Long term I'd like to split it in internal vs public API.

> However QEMU isn't super consistent with
> that:
> 
>    ➜  find . -iname "internal.h"
>    ./accel/tcg/internal.h
>    ./target/ppc/internal.h
>    ./target/mips/internal.h
>    ./target/hexagon/internal.h
>    ./include/hw/ide/internal.h
>    🕙11:15:58 alex@zen:qemu.git  on  review/qom-housekeeping-v2 [$?] took 7s
>    ➜  find . -iname "internals.h"
>    ./tests/fp/berkeley-softfloat-3/source/include/internals.h
>    ./target/arm/internals.h
>    ./target/riscv/internals.h
>    ./target/loongarch/internals.h
>    ./gdbstub/internals.h
> 
> Anyway:
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Thanks!