Hi,
+static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
+ int irq_type)
+static void ahci_check_irq(AHCIState *s)
MSI support would be nice to have.
+#ifndef min
+#define min(a, b) ((a)< (b) ? (a) : (b))
+#endif
osdep.h has MIN/MAX macros.
+static void ahci_init(AHCIState *s, DeviceState *qdev)
+{
+ ide_bus_new(&ad->port, qdev);
+ ide_init2(&ad->port, 0);
Good.
+ if (dinfo) {
+ ide_create_drive(&ad->port, 0, dinfo);
+ }
That doesn't belong into the qdev init function.
Look how ide/isa.c handles it: The qdev init function (isa_ide_initfn)
doesn't create ide-drives at all. And there is a convinience function
(isa_ide_init) which first creates the controller, then attaches the
drives. The later is called from pc_init() with the drives specified
via -drive if=ide (or -hda).
Does AHCI support drive hotplug btw?
+static void ahci_reset(void *opaque)
+{
+ pci_config_set_vendor_id(d->card.config, PCI_VENDOR_ID_INTEL);
+ pci_config_set_device_id(d->card.config, PCI_DEVICE_ID_INTEL_ICH7M_AHCI);
+ pci_config_set_class(d->card.config, PCI_CLASS_STORAGE_SATA);
+ pci_config_set_prog_interface(d->card.config, AHCI_PROGMODE_MAJOR_REV_1);
+ d->card.config[PCI_HEADER_TYPE] = PCI_HEADER_TYPE_NORMAL;
+ d->card.config[PCI_INTERRUPT_PIN] = 1; /* interrupt pin 0 */
Why is that in the reset function instead of init? It should never ever
change, should it?
+static PCIDeviceInfo ahci_info = {
+ .qdev.name = "ahci",
+ .qdev.size = sizeof(AHCIPciState),
+ .init = pci_ahci_init,
+ .exit = pci_ahci_uninit,
+ .qdev.props = (Property[]) {
+ DEFINE_PROP_END_OF_LIST(),
+ }
If there are no properties you can zap the last tree lines altogether ;)
cheers,
Gerd
