diff mbox

[v1,1/3] ahci: Separate the AHCI state structure into the header

Message ID d0e44d6e4957765be2e6aa0a58d04195cc529978.1437699224.git.alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis July 27, 2015, 6:37 p.m. UTC 
Pull the AHCI state structure out into the header. This allows
other containers to access the struct. This is required to add
the device to modern SoC containers.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Sai Pavan Boddu <saipava@xilinx.com>
---
 hw/ide/ahci.c |   13 -------------
 hw/ide/ahci.h |   14 ++++++++++++++
 2 files changed, 14 insertions(+), 13 deletions(-)

Comments

John Snow July 29, 2015, 10:21 p.m. UTC | #1 
On 07/27/2015 02:37 PM, Alistair Francis wrote:
> Pull the AHCI state structure out into the header. This allows
> other containers to access the struct. This is required to add
> the device to modern SoC containers.
> 

Is there a reason this structure needs to be directly inlined into
XlnxZynqMPState, instead of using e.g.
qdev_create/object_property_add_child and letting SysbusAHCIState be the
business of ahci.c?

Asking as a bit of a qdev outsider.

--js

> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> Reviewed-by: Sai Pavan Boddu <saipava@xilinx.com>
> ---
>  hw/ide/ahci.c |   13 -------------
>  hw/ide/ahci.h |   14 ++++++++++++++
>  2 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 48749c1..02d85fa 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -25,7 +25,6 @@
>  #include <hw/pci/msi.h>
>  #include <hw/i386/pc.h>
>  #include <hw/pci/pci.h>
> -#include <hw/sysbus.h>
>  
>  #include "qemu/error-report.h"
>  #include "sysemu/block-backend.h"
> @@ -1625,18 +1624,6 @@ const VMStateDescription vmstate_ahci = {
>      },
>  };
>  
> -#define TYPE_SYSBUS_AHCI "sysbus-ahci"
> -#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
> -
> -typedef struct SysbusAHCIState {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    AHCIState ahci;
> -    uint32_t num_ports;
> -} SysbusAHCIState;
> -
>  static const VMStateDescription vmstate_sysbus_ahci = {
>      .name = "sysbus-ahci",
>      .fields = (VMStateField[]) {
> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
> index 68d5074..5ab8ea4 100644
> --- a/hw/ide/ahci.h
> +++ b/hw/ide/ahci.h
> @@ -24,6 +24,8 @@
>  #ifndef HW_IDE_AHCI_H
>  #define HW_IDE_AHCI_H
>  
> +#include <hw/sysbus.h>
> +
>  #define AHCI_MEM_BAR_SIZE         0x1000
>  #define AHCI_MAX_PORTS            32
>  #define AHCI_MAX_SG               168 /* hardware max is 64K */
> @@ -369,4 +371,16 @@ void ahci_reset(AHCIState *s);
>  
>  void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd);
>  
> +#define TYPE_SYSBUS_AHCI "sysbus-ahci"
> +#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
> +
> +typedef struct SysbusAHCIState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    AHCIState ahci;
> +    uint32_t num_ports;
> +} SysbusAHCIState;
> +
>  #endif /* HW_IDE_AHCI_H */
>
Alistair Francis July 30, 2015, 12:12 a.m. UTC | #2 
On Wed, Jul 29, 2015 at 3:21 PM, John Snow <jsnow@redhat.com> wrote:
>
>
> On 07/27/2015 02:37 PM, Alistair Francis wrote:
>> Pull the AHCI state structure out into the header. This allows
>> other containers to access the struct. This is required to add
>> the device to modern SoC containers.
>>
>
> Is there a reason this structure needs to be directly inlined into
> XlnxZynqMPState, instead of using e.g.
> qdev_create/object_property_add_child and letting SysbusAHCIState be the
> business of ahci.c?

AFAIK there isn't a huge advantage, it ends up achieving basically the
same thing. It just means that we don't need to use
the old qdev_* functions (as much). It also follows with QOMificatiom and
the device creation can be split up over the init and realise functions.

This is also the way I did machine creation for the Netduino 2 and how
it is done for the rest of the ZynqMP device.

Thanks,

Alistair

>
> Asking as a bit of a qdev outsider.
>
> --js
>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> Reviewed-by: Sai Pavan Boddu <saipava@xilinx.com>
>> ---
>>  hw/ide/ahci.c |   13 -------------
>>  hw/ide/ahci.h |   14 ++++++++++++++
>>  2 files changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index 48749c1..02d85fa 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -25,7 +25,6 @@
>>  #include <hw/pci/msi.h>
>>  #include <hw/i386/pc.h>
>>  #include <hw/pci/pci.h>
>> -#include <hw/sysbus.h>
>>
>>  #include "qemu/error-report.h"
>>  #include "sysemu/block-backend.h"
>> @@ -1625,18 +1624,6 @@ const VMStateDescription vmstate_ahci = {
>>      },
>>  };
>>
>> -#define TYPE_SYSBUS_AHCI "sysbus-ahci"
>> -#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
>> -
>> -typedef struct SysbusAHCIState {
>> -    /*< private >*/
>> -    SysBusDevice parent_obj;
>> -    /*< public >*/
>> -
>> -    AHCIState ahci;
>> -    uint32_t num_ports;
>> -} SysbusAHCIState;
>> -
>>  static const VMStateDescription vmstate_sysbus_ahci = {
>>      .name = "sysbus-ahci",
>>      .fields = (VMStateField[]) {
>> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
>> index 68d5074..5ab8ea4 100644
>> --- a/hw/ide/ahci.h
>> +++ b/hw/ide/ahci.h
>> @@ -24,6 +24,8 @@
>>  #ifndef HW_IDE_AHCI_H
>>  #define HW_IDE_AHCI_H
>>
>> +#include <hw/sysbus.h>
>> +
>>  #define AHCI_MEM_BAR_SIZE         0x1000
>>  #define AHCI_MAX_PORTS            32
>>  #define AHCI_MAX_SG               168 /* hardware max is 64K */
>> @@ -369,4 +371,16 @@ void ahci_reset(AHCIState *s);
>>
>>  void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd);
>>
>> +#define TYPE_SYSBUS_AHCI "sysbus-ahci"
>> +#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
>> +
>> +typedef struct SysbusAHCIState {
>> +    /*< private >*/
>> +    SysBusDevice parent_obj;
>> +    /*< public >*/
>> +
>> +    AHCIState ahci;
>> +    uint32_t num_ports;
>> +} SysbusAHCIState;
>> +
>>  #endif /* HW_IDE_AHCI_H */
>>
>
Peter Crosthwaite Aug. 15, 2015, 9:18 p.m. UTC | #3 
On Wed, Jul 29, 2015 at 5:12 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Wed, Jul 29, 2015 at 3:21 PM, John Snow <jsnow@redhat.com> wrote:
>>
>>
>> On 07/27/2015 02:37 PM, Alistair Francis wrote:
>>> Pull the AHCI state structure out into the header. This allows
>>> other containers to access the struct. This is required to add
>>> the device to modern SoC containers.
>>>
>>
>> Is there a reason this structure needs to be directly inlined into
>> XlnxZynqMPState, instead of using e.g.
>> qdev_create/object_property_add_child and letting SysbusAHCIState be the
>> business of ahci.c?
>

This has come up a few times, and the conclusion was to do the
inlining by preference. I vaguely remember PMM started an effort to
build infrastructure to hide the internals from use by outsiders, but
the struct def is desirable for the inlining even if the internals are
private a different module. It has been the policy on ARM SoC for
quite some time. We have made a quite a few of these conversions for
an example of a big one, see:

[PATCH v4 00/24] arm: ARM11MPCore+A9MPCore+A15MPCore QOM'ification

> AFAIK there isn't a huge advantage, it ends up achieving basically the
> same thing. It just means that we don't need to use
> the old qdev_* functions (as much). It also follows with QOMificatiom and
> the device creation can be split up over the init and realise functions.
>

You can use object_new in place of object_initialize to achieve the
split without inlining as well.

Regards,
Peter

> This is also the way I did machine creation for the Netduino 2 and how
> it is done for the rest of the ZynqMP device.
>
> Thanks,
>
> Alistair
>
>>
>> Asking as a bit of a qdev outsider.
>>
>> --js
>>
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> Reviewed-by: Sai Pavan Boddu <saipava@xilinx.com>
>>> ---
>>>  hw/ide/ahci.c |   13 -------------
>>>  hw/ide/ahci.h |   14 ++++++++++++++
>>>  2 files changed, 14 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>>> index 48749c1..02d85fa 100644
>>> --- a/hw/ide/ahci.c
>>> +++ b/hw/ide/ahci.c
>>> @@ -25,7 +25,6 @@
>>>  #include <hw/pci/msi.h>
>>>  #include <hw/i386/pc.h>
>>>  #include <hw/pci/pci.h>
>>> -#include <hw/sysbus.h>
>>>
>>>  #include "qemu/error-report.h"
>>>  #include "sysemu/block-backend.h"
>>> @@ -1625,18 +1624,6 @@ const VMStateDescription vmstate_ahci = {
>>>      },
>>>  };
>>>
>>> -#define TYPE_SYSBUS_AHCI "sysbus-ahci"
>>> -#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
>>> -
>>> -typedef struct SysbusAHCIState {
>>> -    /*< private >*/
>>> -    SysBusDevice parent_obj;
>>> -    /*< public >*/
>>> -
>>> -    AHCIState ahci;
>>> -    uint32_t num_ports;
>>> -} SysbusAHCIState;
>>> -
>>>  static const VMStateDescription vmstate_sysbus_ahci = {
>>>      .name = "sysbus-ahci",
>>>      .fields = (VMStateField[]) {
>>> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
>>> index 68d5074..5ab8ea4 100644
>>> --- a/hw/ide/ahci.h
>>> +++ b/hw/ide/ahci.h
>>> @@ -24,6 +24,8 @@
>>>  #ifndef HW_IDE_AHCI_H
>>>  #define HW_IDE_AHCI_H
>>>
>>> +#include <hw/sysbus.h>
>>> +
>>>  #define AHCI_MEM_BAR_SIZE         0x1000
>>>  #define AHCI_MAX_PORTS            32
>>>  #define AHCI_MAX_SG               168 /* hardware max is 64K */
>>> @@ -369,4 +371,16 @@ void ahci_reset(AHCIState *s);
>>>
>>>  void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd);
>>>
>>> +#define TYPE_SYSBUS_AHCI "sysbus-ahci"
>>> +#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
>>> +
>>> +typedef struct SysbusAHCIState {
>>> +    /*< private >*/
>>> +    SysBusDevice parent_obj;
>>> +    /*< public >*/
>>> +
>>> +    AHCIState ahci;
>>> +    uint32_t num_ports;
>>> +} SysbusAHCIState;
>>> +
>>>  #endif /* HW_IDE_AHCI_H */
>>>
>>
>
Peter Crosthwaite Aug. 15, 2015, 9:21 p.m. UTC | #4 
On Mon, Jul 27, 2015 at 11:37 AM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> Pull the AHCI state structure out into the header. This allows
> other containers to access the struct. This is required to add
> the device to modern SoC containers.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> Reviewed-by: Sai Pavan Boddu <saipava@xilinx.com>
> ---
>  hw/ide/ahci.c |   13 -------------
>  hw/ide/ahci.h |   14 ++++++++++++++
>  2 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 48749c1..02d85fa 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -25,7 +25,6 @@
>  #include <hw/pci/msi.h>
>  #include <hw/i386/pc.h>
>  #include <hw/pci/pci.h>
> -#include <hw/sysbus.h>
>
>  #include "qemu/error-report.h"
>  #include "sysemu/block-backend.h"
> @@ -1625,18 +1624,6 @@ const VMStateDescription vmstate_ahci = {
>      },
>  };
>
> -#define TYPE_SYSBUS_AHCI "sysbus-ahci"
> -#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
> -
> -typedef struct SysbusAHCIState {
> -    /*< private >*/
> -    SysBusDevice parent_obj;
> -    /*< public >*/
> -
> -    AHCIState ahci;
> -    uint32_t num_ports;
> -} SysbusAHCIState;
> -
>  static const VMStateDescription vmstate_sysbus_ahci = {
>      .name = "sysbus-ahci",
>      .fields = (VMStateField[]) {
> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
> index 68d5074..5ab8ea4 100644
> --- a/hw/ide/ahci.h
> +++ b/hw/ide/ahci.h
> @@ -24,6 +24,8 @@
>  #ifndef HW_IDE_AHCI_H
>  #define HW_IDE_AHCI_H
>
> +#include <hw/sysbus.h>
> +

Odd that this is the only header. Out-of scope, but should this header
be including the same for PCI? It uses PCIDevice * defs, so I am
guessing it is relying on clients to pre-include the deps.

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Regards,
Peter

>  #define AHCI_MEM_BAR_SIZE         0x1000
>  #define AHCI_MAX_PORTS            32
>  #define AHCI_MAX_SG               168 /* hardware max is 64K */
> @@ -369,4 +371,16 @@ void ahci_reset(AHCIState *s);
>
>  void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd);
>
> +#define TYPE_SYSBUS_AHCI "sysbus-ahci"
> +#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
> +
> +typedef struct SysbusAHCIState {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    AHCIState ahci;
> +    uint32_t num_ports;
> +} SysbusAHCIState;
> +
>  #endif /* HW_IDE_AHCI_H */
> --
> 1.7.1
>
>
Peter Crosthwaite Aug. 15, 2015, 9:25 p.m. UTC | #5 
On Sat, Aug 15, 2015 at 2:21 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Mon, Jul 27, 2015 at 11:37 AM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> Pull the AHCI state structure out into the header. This allows
>> other containers to access the struct. This is required to add
>> the device to modern SoC containers.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> Reviewed-by: Sai Pavan Boddu <saipava@xilinx.com>
>> ---
>>  hw/ide/ahci.c |   13 -------------
>>  hw/ide/ahci.h |   14 ++++++++++++++
>>  2 files changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index 48749c1..02d85fa 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -25,7 +25,6 @@
>>  #include <hw/pci/msi.h>
>>  #include <hw/i386/pc.h>
>>  #include <hw/pci/pci.h>
>> -#include <hw/sysbus.h>
>>
>>  #include "qemu/error-report.h"
>>  #include "sysemu/block-backend.h"
>> @@ -1625,18 +1624,6 @@ const VMStateDescription vmstate_ahci = {
>>      },
>>  };
>>
>> -#define TYPE_SYSBUS_AHCI "sysbus-ahci"
>> -#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
>> -
>> -typedef struct SysbusAHCIState {
>> -    /*< private >*/
>> -    SysBusDevice parent_obj;
>> -    /*< public >*/
>> -
>> -    AHCIState ahci;
>> -    uint32_t num_ports;
>> -} SysbusAHCIState;
>> -
>>  static const VMStateDescription vmstate_sysbus_ahci = {
>>      .name = "sysbus-ahci",
>>      .fields = (VMStateField[]) {
>> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
>> index 68d5074..5ab8ea4 100644
>> --- a/hw/ide/ahci.h
>> +++ b/hw/ide/ahci.h
>> @@ -24,6 +24,8 @@
>>  #ifndef HW_IDE_AHCI_H
>>  #define HW_IDE_AHCI_H
>>
>> +#include <hw/sysbus.h>
>> +
>
> Odd that this is the only header. Out-of scope, but should this header
> be including the same for PCI? It uses PCIDevice * defs, so I am
> guessing it is relying on clients to pre-include the deps.
>
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>

Sorry old habit, try:

Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

> Regards,
> Peter
>
>>  #define AHCI_MEM_BAR_SIZE         0x1000
>>  #define AHCI_MAX_PORTS            32
>>  #define AHCI_MAX_SG               168 /* hardware max is 64K */
>> @@ -369,4 +371,16 @@ void ahci_reset(AHCIState *s);
>>
>>  void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd);
>>
>> +#define TYPE_SYSBUS_AHCI "sysbus-ahci"
>> +#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
>> +
>> +typedef struct SysbusAHCIState {
>> +    /*< private >*/
>> +    SysBusDevice parent_obj;
>> +    /*< public >*/
>> +
>> +    AHCIState ahci;
>> +    uint32_t num_ports;
>> +} SysbusAHCIState;
>> +
>>  #endif /* HW_IDE_AHCI_H */
>> --
>> 1.7.1
>>
>>
Peter Maydell Aug. 15, 2015, 9:57 p.m. UTC | #6 
On 15 August 2015 at 22:18, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> This has come up a few times, and the conclusion was to do the
> inlining by preference. I vaguely remember PMM started an effort to
> build infrastructure to hide the internals from use by outsiders, but
> the struct def is desirable for the inlining even if the internals are
> private a different module.

The idea was just to mark up the fields which are supposed
to be private to the implementation, so everybody can use
the same header, but outsiders get a warning/error if they
try to touch things they shouldn't.

https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg01846.html

I should probably get back to that at some point -- Paolo
had an interesting suggestion involving some cpp macros
which I never got round to investigating.

thanks
-- PMM
Alistair Francis Aug. 17, 2015, 10:31 p.m. UTC | #7 
On Sat, Aug 15, 2015 at 2:25 PM, Peter Crosthwaite
<crosthwaitepeter@gmail.com> wrote:
> On Sat, Aug 15, 2015 at 2:21 PM, Peter Crosthwaite
> <crosthwaitepeter@gmail.com> wrote:
>> On Mon, Jul 27, 2015 at 11:37 AM, Alistair Francis
>> <alistair.francis@xilinx.com> wrote:
>>> Pull the AHCI state structure out into the header. This allows
>>> other containers to access the struct. This is required to add
>>> the device to modern SoC containers.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> Reviewed-by: Sai Pavan Boddu <saipava@xilinx.com>
>>> ---
>>>  hw/ide/ahci.c |   13 -------------
>>>  hw/ide/ahci.h |   14 ++++++++++++++
>>>  2 files changed, 14 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>>> index 48749c1..02d85fa 100644
>>> --- a/hw/ide/ahci.c
>>> +++ b/hw/ide/ahci.c
>>> @@ -25,7 +25,6 @@
>>>  #include <hw/pci/msi.h>
>>>  #include <hw/i386/pc.h>
>>>  #include <hw/pci/pci.h>
>>> -#include <hw/sysbus.h>
>>>
>>>  #include "qemu/error-report.h"
>>>  #include "sysemu/block-backend.h"
>>> @@ -1625,18 +1624,6 @@ const VMStateDescription vmstate_ahci = {
>>>      },
>>>  };
>>>
>>> -#define TYPE_SYSBUS_AHCI "sysbus-ahci"
>>> -#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
>>> -
>>> -typedef struct SysbusAHCIState {
>>> -    /*< private >*/
>>> -    SysBusDevice parent_obj;
>>> -    /*< public >*/
>>> -
>>> -    AHCIState ahci;
>>> -    uint32_t num_ports;
>>> -} SysbusAHCIState;
>>> -
>>>  static const VMStateDescription vmstate_sysbus_ahci = {
>>>      .name = "sysbus-ahci",
>>>      .fields = (VMStateField[]) {
>>> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
>>> index 68d5074..5ab8ea4 100644
>>> --- a/hw/ide/ahci.h
>>> +++ b/hw/ide/ahci.h
>>> @@ -24,6 +24,8 @@
>>>  #ifndef HW_IDE_AHCI_H
>>>  #define HW_IDE_AHCI_H
>>>
>>> +#include <hw/sysbus.h>
>>> +
>>
>> Odd that this is the only header. Out-of scope, but should this header
>> be including the same for PCI? It uses PCIDevice * defs, so I am
>> guessing it is relying on clients to pre-include the deps.
>>
>> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
>>
>
> Sorry old habit, try:
>
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

Thanks Peter,

Alistair

>
>> Regards,
>> Peter
>>
>>>  #define AHCI_MEM_BAR_SIZE         0x1000
>>>  #define AHCI_MAX_PORTS            32
>>>  #define AHCI_MAX_SG               168 /* hardware max is 64K */
>>> @@ -369,4 +371,16 @@ void ahci_reset(AHCIState *s);
>>>
>>>  void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd);
>>>
>>> +#define TYPE_SYSBUS_AHCI "sysbus-ahci"
>>> +#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
>>> +
>>> +typedef struct SysbusAHCIState {
>>> +    /*< private >*/
>>> +    SysBusDevice parent_obj;
>>> +    /*< public >*/
>>> +
>>> +    AHCIState ahci;
>>> +    uint32_t num_ports;
>>> +} SysbusAHCIState;
>>> +
>>>  #endif /* HW_IDE_AHCI_H */
>>> --
>>> 1.7.1
>>>
>>>
>
diff mbox

Patch

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 48749c1..02d85fa 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -25,7 +25,6 @@ 
 #include <hw/pci/msi.h>
 #include <hw/i386/pc.h>
 #include <hw/pci/pci.h>
-#include <hw/sysbus.h>
 
 #include "qemu/error-report.h"
 #include "sysemu/block-backend.h"
@@ -1625,18 +1624,6 @@  const VMStateDescription vmstate_ahci = {
     },
 };
 
-#define TYPE_SYSBUS_AHCI "sysbus-ahci"
-#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
-
-typedef struct SysbusAHCIState {
-    /*< private >*/
-    SysBusDevice parent_obj;
-    /*< public >*/
-
-    AHCIState ahci;
-    uint32_t num_ports;
-} SysbusAHCIState;
-
 static const VMStateDescription vmstate_sysbus_ahci = {
     .name = "sysbus-ahci",
     .fields = (VMStateField[]) {
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 68d5074..5ab8ea4 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -24,6 +24,8 @@ 
 #ifndef HW_IDE_AHCI_H
 #define HW_IDE_AHCI_H
 
+#include <hw/sysbus.h>
+
 #define AHCI_MEM_BAR_SIZE         0x1000
 #define AHCI_MAX_PORTS            32
 #define AHCI_MAX_SG               168 /* hardware max is 64K */
@@ -369,4 +371,16 @@  void ahci_reset(AHCIState *s);
 
 void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **hd);
 
+#define TYPE_SYSBUS_AHCI "sysbus-ahci"
+#define SYSBUS_AHCI(obj) OBJECT_CHECK(SysbusAHCIState, (obj), TYPE_SYSBUS_AHCI)
+
+typedef struct SysbusAHCIState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    AHCIState ahci;
+    uint32_t num_ports;
+} SysbusAHCIState;
+
 #endif /* HW_IDE_AHCI_H */