Re: [Qemu-devel] [PATCH v4 3/6] ahci: add support for non-PCI based controllers

["Andreas Färber" <afaerber@suse.de>]

Am 09.01.2012 17:59, schrieb Mark Langsdorf:
> From: Rob Herring <rob.herring@calxeda.com>
> 
> Add support for ahci on sysbus.
> 
> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
> Signed-off-by: Mark Langsdorf <mark.langsdorf@calxeda.com>
> ---
> Changes from v3
>         Renamed plat-ahci to sysbus-ahci

The idea was to get rid of "plat" everywhere, not just in the qdev name.
Maybe I'm biased because platt is the German adjective for flat.

Please call it sysbus/SysBus consistently.

> Changes from v1, v2
>         Corrected indentation of PlatAHCIState members
>         Made plat_ahci_info into a single structure, not a list
> 
>  hw/ide/ahci.c |   31 +++++++++++++++++++++++++++++++
>  1 files changed, 31 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 135d0ee..982ee31 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -25,6 +25,7 @@
>  #include <hw/msi.h>
>  #include <hw/pc.h>
>  #include <hw/pci.h>
> +#include <hw/sysbus.h>
>  
>  #include "monitor.h"
>  #include "dma.h"
> @@ -1214,3 +1215,33 @@ void ahci_reset(void *opaque)
>          ahci_reset_port(s, i);
>      }
>  }
> +
> +typedef struct PlatAHCIState {
> +    SysBusDevice busdev;
> +    AHCIState ahci;
> +} PlatAHCIState;
> +
> +static int plat_ahci_init(SysBusDevice *dev)
> +{
> +    PlatAHCIState *s = FROM_SYSBUS(PlatAHCIState, dev);
> +    ahci_init(&s->ahci, &dev->qdev, 1);

Maybe the number of ports (1) should be a qdev property?

> +
> +    sysbus_init_mmio(dev, &s->ahci.mem);
> +    sysbus_init_irq(dev, &s->ahci.irq);
> +
> +    qemu_register_reset(ahci_reset, &s->ahci);
> +    return 0;
> +}
> +
> +static SysBusDeviceInfo plat_ahci_info = {
> +    .qdev.name    = "sysbus-ahci",
> +    .qdev.size    = sizeof(PlatAHCIState),

You should prepare a dummy VMStateDescription to mark it as unmigratable
for now (cf. hw/ide/ich.c). Migration support is the upcoming series I
recently cc'ed you on.

Otherwise looking good.

Andreas

> +    .init         = plat_ahci_init,
> +};
> +
> +static void plat_ahci_register(void)
> +{
> +    sysbus_register_withprop(&plat_ahci_info);
> +}
> +
> +device_init(plat_ahci_register);

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg