Re: [Qemu-devel] [PATCH v2 10/29] ide: Include hw/ide/internal a bit less outside hw/ide/

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 8/6/19 11:40 PM, John Snow wrote:
> On 8/6/19 11:14 AM, Markus Armbruster wrote:
>> According to hw/ide/internal's file comment, only files in hw/ide/ are
>> supposed to include it.  Drag reality slightly closer to supposition.
>>
> 
> The downside of having your internal include ripped away and stuck in
> the includes/ folder.

Not sure this is a 'downside', it looks more than proper design issue to
me (often the case with very old subsystems).

>> Three includes outside hw/ide remain: hw/arm/sbsa-ref.c,
>> include/hw/ide/pci.h, and include/hw/misc/macio/macio.h.  Turns out
>> board code needs ide-internal.h to wire up IDE stuff.  More cleanup is
>> needed.  Left for another day.
>>
> 
> Yuh. This is mostly why I haven't touched it.
> 
>> Cc: John Snow <jsnow@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> 
> Thank you.
> 
> Reviewed-by: John Snow <jsnow@redhat.com>

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

>> ---
>>  hw/arm/allwinner-a10.c         | 1 +
>>  hw/arm/cubieboard.c            | 1 +
>>  hw/arm/xlnx-zynqmp.c           | 1 +
>>  hw/ide/ahci_internal.h         | 1 +
>>  hw/ppc/mac.h                   | 1 -
>>  include/hw/arm/allwinner-a10.h | 1 -
>>  include/hw/arm/xlnx-zynqmp.h   | 1 -
>>  include/hw/misc/mos6522.h      | 1 -
>>  8 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
>> index 35e906ca54..3b0d3eccdd 100644
>> --- a/hw/arm/allwinner-a10.c
>> +++ b/hw/arm/allwinner-a10.c
>> @@ -16,6 +16,7 @@
>>   */
>>  
>>  #include "qemu/osdep.h"
>> +#include "exec/address-spaces.h"
>>  #include "qapi/error.h"
>>  #include "qemu/module.h"
>>  #include "cpu.h"
>> diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
>> index f7c8a5985a..38e0ca0f53 100644
>> --- a/hw/arm/cubieboard.c
>> +++ b/hw/arm/cubieboard.c
>> @@ -16,6 +16,7 @@
>>   */
>>  
>>  #include "qemu/osdep.h"
>> +#include "exec/address-spaces.h"
>>  #include "qapi/error.h"
>>  #include "cpu.h"
>>  #include "hw/sysbus.h"
>> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
>> index a60830d37a..0f587e63d3 100644
>> --- a/hw/arm/xlnx-zynqmp.c
>> +++ b/hw/arm/xlnx-zynqmp.c
>> @@ -24,6 +24,7 @@
>>  #include "hw/boards.h"
>>  #include "exec/address-spaces.h"
>>  #include "sysemu/kvm.h"
>> +#include "sysemu/sysemu.h"
>>  #include "kvm_arm.h"
>>  
>>  #define GIC_NUM_SPI_INTR 160
>> diff --git a/hw/ide/ahci_internal.h b/hw/ide/ahci_internal.h
>> index 95ecddcd3c..73424516da 100644
>> --- a/hw/ide/ahci_internal.h
>> +++ b/hw/ide/ahci_internal.h
>> @@ -25,6 +25,7 @@
>>  #define HW_IDE_AHCI_INTERNAL_H
>>  
>>  #include "hw/ide/ahci.h"
>> +#include "hw/ide/internal.h"
>>  #include "hw/sysbus.h"
>>  
>>  #define AHCI_MEM_BAR_SIZE         0x1000
>> diff --git a/hw/ppc/mac.h b/hw/ppc/mac.h
>> index a741300ac9..6af87d1fa0 100644
>> --- a/hw/ppc/mac.h
>> +++ b/hw/ppc/mac.h
>> @@ -30,7 +30,6 @@
>>  #include "exec/memory.h"
>>  #include "hw/boards.h"
>>  #include "hw/sysbus.h"
>> -#include "hw/ide/internal.h"
>>  #include "hw/input/adb.h"
>>  #include "hw/misc/mos6522.h"
>>  #include "hw/pci/pci_host.h"
>> diff --git a/include/hw/arm/allwinner-a10.h b/include/hw/arm/allwinner-a10.h
>> index 7182ce5c4b..101b72a71d 100644
>> --- a/include/hw/arm/allwinner-a10.h
>> +++ b/include/hw/arm/allwinner-a10.h
>> @@ -7,7 +7,6 @@
>>  #include "hw/timer/allwinner-a10-pit.h"
>>  #include "hw/intc/allwinner-a10-pic.h"
>>  #include "hw/net/allwinner_emac.h"
>> -#include "hw/ide/pci.h"
>>  #include "hw/ide/ahci.h"
>>  
>>  #include "sysemu/sysemu.h"
>> diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
>> index 6cb65e7537..d7483c3b42 100644
>> --- a/include/hw/arm/xlnx-zynqmp.h
>> +++ b/include/hw/arm/xlnx-zynqmp.h
>> @@ -22,7 +22,6 @@
>>  #include "hw/intc/arm_gic.h"
>>  #include "hw/net/cadence_gem.h"
>>  #include "hw/char/cadence_uart.h"
>> -#include "hw/ide/pci.h"
>>  #include "hw/ide/ahci.h"
>>  #include "hw/sd/sdhci.h"
>>  #include "hw/ssi/xilinx_spips.h"
>> diff --git a/include/hw/misc/mos6522.h b/include/hw/misc/mos6522.h
>> index 03d9f0c059..493c907537 100644
>> --- a/include/hw/misc/mos6522.h
>> +++ b/include/hw/misc/mos6522.h
>> @@ -29,7 +29,6 @@
>>  
>>  #include "exec/memory.h"
>>  #include "hw/sysbus.h"
>> -#include "hw/ide/internal.h"
>>  #include "hw/input/adb.h"
>>  
>>  /* Bits in ACR */
>>
>