Re: [Qemu-trivial] [PATCH v2 30/39] hw/display/vga: remove the old i386/pc dependency

[Philippe Mathieu-Daudé]

Hi Thomas,

>> diff --git a/hw/display/vga_int.h b/hw/display/vga_int.h
>> index ad34a1f048..fe23b81442 100644
>> --- a/hw/display/vga_int.h
>> +++ b/hw/display/vga_int.h
>> @@ -25,8 +25,9 @@
>>  #ifndef HW_VGA_INT_H
>>  #define HW_VGA_INT_H
>>  
>> -#include "hw/hw.h"
>> +#include "exec/ioport.h"
>>  #include "exec/memory.h"
>> +#include "ui/console.h"
> 
> So vga_int.h now includes console.h (is it really necessary?) ...

The VGACommonState struct has a GraphicHwOps *hw_ops member.

> 
> [...]
>>  static inline bool isa_ne2000_init(ISABus *bus, int base, int irq, NICInfo 
>> *nd)
>>  {
>> diff --git a/hw/display/vga-isa-mm.c b/hw/display/vga-isa-mm.c
>> index 51ccbccc41..e887b45651 100644
>> --- a/hw/display/vga-isa-mm.c
>> +++ b/hw/display/vga-isa-mm.c
>> @@ -23,11 +23,9 @@
>>   */
>>  #include "qemu/osdep.h"
>>  #include "hw/hw.h"
>> -#include "ui/console.h"
> 
> ... and here you also remove it from the .c file - OK ...
> 
>> -#include "hw/i386/pc.h"
>> +#include "hw/display/vga.h"
>>  #include "vga_int.h"
>>  #include "ui/pixel_ops.h"
>> -#include "qemu/timer.h"
>>  
>>  #define VGA_RAM_SIZE (8192 * 1024)
>>  
>> diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
>> index 1af95562f2..9af43abee8 100644
>> --- a/hw/display/vga-isa.c
>> +++ b/hw/display/vga-isa.c
>> @@ -26,7 +26,7 @@
>>  #include "qemu/osdep.h"
>>  #include "hw/hw.h"
>>  #include "ui/console.h"
> 
> ... but here you keep it? I think you should either remove it here, too,

I missed this one.

> or not add the include in the header (which I'd prefer if the header
> itself does not depend on console.h).

Do you suggest me do declare GraphicHwOps opaque in "qemu/typedefs.h"?

> 
>> -#include "hw/i386/pc.h"
>> +#include "hw/isa/isa.h"
>>  #include "vga_int.h"
>>  #include "ui/pixel_ops.h"
>>  #include "qemu/timer.h"
>> diff --git a/hw/display/vga.c b/hw/display/vga.c
>> index ed24ef7076..0da4942a09 100644
>> --- a/hw/display/vga.c
>> +++ b/hw/display/vga.c
>> @@ -24,9 +24,9 @@
>>  #include "qemu/osdep.h"
>>  #include "qapi/error.h"
>>  #include "hw/hw.h"
>> -#include "vga.h"
>> +#include "hw/display/vga.h"
>>  #include "ui/console.h"
> 
> Here's another one --^

Oh this one too.