On 05.01.2012, at 15:16, Andreas Färber wrote: > Am 05.01.2012 14:52, schrieb Mark Langsdorf: >> From: Rob Herring <rob.herr...@calxeda.com> >> >> Add support for ahci on sysbus. >> >> Signed-off-by: Rob Herring <rob.herr...@calxeda.com> >> Signed-off-by: Mark Langsdorf <mark.langsd...@calxeda.com> >> --- >> 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..f052e55 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); >> + >> + sysbus_init_mmio(dev, &s->ahci.mem); >> + sysbus_init_irq(dev, &s->ahci.irq);
It's still unclear to me how you connect an irq line on the command line. How do you instantiate this device? >> + >> + qemu_register_reset(ahci_reset, &s->ahci); >> + return 0; >> +} >> + >> +static SysBusDeviceInfo plat_ahci_info = { >> + .qdev.name = "plat-ahci", > > The commit message does not given an indication where "plat" comes from > - is that an ARM device name? "plat" here means "platform device". I'm not sure I like the naming though. Basically it's a sysbus version of AHCI, similar to how virtio-mmio is a sysbus version of virtio. How about "sysbus-ahci"? > >> + .qdev.size = sizeof(PlatAHCIState), >> + .init = plat_ahci_init, >> +}; >> + >> +static void plat_ahci_register(void) >> +{ >> + sysbus_register_withprop(&plat_ahci_info); >> +} >> +device_init(plat_ahci_register); >> + > > Please move the empty line to above the device_init(). > > Does this patch actually make something work? If yes, please state so, > including usage instructions. If not, then I would suggest to hold this > one back and to send it together with any follow-on patches that wire it > up on some machine. You can always just create the device manually with -device and hook it up in the guest device tree or machine description manually. However the question still stands: Have you verified this code works? Alex