On Wed, 28 May 2008 16:38:57 +0200 Holger Macht <hmacht@xxxxxxx> wrote: > * Differentiate between bay devices in dock stations and others: > > - When an ACPI_NOTIFY_EJECT_REQUEST appears, just signal uevent to > userspace (that is when the optional eject button on a bay device is > pressed/pulled) giving the possibility to unmount file systems and to > clean up. Also, only send uevent in case we get an EJECT_REQUEST > without doing anything else. In other cases, you'll get an add/remove > event because libata attaches/detaches the device. > > - In case of a dock event, which in turn signals an > ACPI_NOTIFY_EJECT_REQUEST, immediately detach the device, because it > may already have been gone > > * In case of an ACPI_NOTIFY_DEVICE/BUS_CHECK, evaluate _STA to check if > the device has been plugged or unplugged. If plugged, hotplug it, if > unplugged, just signal event to userspace > (initial patch by Matthew Garrett <mjg59@xxxxxxxxxxxxx>) > > * Call ACPI _EJ0 for detached devices > > ... > > +static void ata_acpi_eject_device(acpi_handle handle) > +{ > + struct acpi_object_list arg_list; > + union acpi_object arg; > + > + arg_list.count = 1; > + arg_list.pointer = &arg; > + arg.type = ACPI_TYPE_INTEGER; > + arg.integer.value = 1; This might look nicer (and safer) if it used the union acpi_object arg = { .foo = bar, ... }; form. However this can cause gcc to emit poor code. > + if (ACPI_FAILURE(acpi_evaluate_object(handle, "_EJ0", > + &arg_list, NULL))) > + printk(KERN_ERR "Failed to evaluate _EJ0!\n"); > +} It would be better if the printk were to self-identify where it is coming from. If your kernel just blurts "Failed to evaluate _EJ0!" it's a bit of a head-scratcher. "libata-acpi: failed to evaluate _EJ0", perhaps? > +static void ata_acpi_detach_device(struct ata_port *ap, struct ata_device *dev) > +{ > + if (dev) > + dev->flags |= ATA_DFLAG_DETACH; > + else { > + struct ata_link *tlink; > + struct ata_device *tdev; > + > + ata_port_for_each_link(tlink, ap) > + ata_link_for_each_dev(tdev, tlink) > + tdev->flags |= ATA_DFLAG_DETACH; > + } > + > + ata_port_freeze(ap); > + ata_port_schedule_eh(ap); > +} I guess the significance of `dev==0' is known to those who are steeped in ata_acpi_handle_hotplug() knowledge, but it's pretty inscrutable to the occasional visitor. Some comments would help. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html