Re: [Qemu-devel] [PATCH v1 1/3] ahci: Separate the AHCI state structure into the header

[Alistair Francis]

On Sat, Aug 15, 2015 at 2:25 PM, Peter Crosthwaite
<address@hidden> wrote:
> On Sat, Aug 15, 2015 at 2:21 PM, Peter Crosthwaite
> <address@hidden> wrote:
>> On Mon, Jul 27, 2015 at 11:37 AM, Alistair Francis
>> <address@hidden> 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 <address@hidden>
>>> Reviewed-by: Sai Pavan Boddu <address@hidden>
>>> ---
>>>  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 <address@hidden>
>>
>
> Sorry old habit, try:
>
> Reviewed-by: Peter Crosthwaite <address@hidden>

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
>>>
>>>
>