On Tue, 20 May 2008 15:58:30 +0200
Holger Macht <hmacht@xxxxxxx> wrote:
> On Tue 20. May - 14:22:17, Matthew Garrett wrote:
> > On Tue, May 20, 2008 at 03:18:32PM +0200, Holger Macht wrote:
> >
> > > + if (kobj && !is_dock_event) {
> > > + sprintf(event_string, "BAY_EVENT=%d", event);
> > > + kobject_uevent_env(kobj, KOBJ_CHANGE, envp);
> >
> > I think we want to do the _EJ0 checking before this, otherwise we'll
> > generate two uevents for the same removal on some hardware. Otherwise,
> > looks good.
>
> So once again:
>
> Handle bay devices in dock stations
>
> * 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>)
>
> Signed-off-by: Holger Macht <hmacht@xxxxxxx>
> ---
>
> --- linux-2.6.25/drivers/ata/libata-acpi.c.orig 2008-05-20 13:25:50.000000000 +0200
> +++ linux-2.6.25/drivers/ata/libata-acpi.c 2008-05-20 15:56:38.000000000 +0200
> @@ -118,8 +118,25 @@ static void ata_acpi_associate_ide_port(
> ap->pflags |= ATA_PFLAG_INIT_GTM_VALID;
> }
>
> +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;
Looks odd. I guess this:
--- a/drivers/ata/libata-acpi.c~ata-acpi-hotplug-handle-bay-devices-in-dock-stations-cleanup
+++ a/drivers/ata/libata-acpi.c
@@ -128,7 +128,7 @@ static void ata_acpi_detach_device(struc
ata_port_for_each_link(tlink, ap)
ata_link_for_each_dev(tdev, tlink)
- tdev->flags |= ATA_DFLAG_DETACH;
+ tdev->flags |= ATA_DFLAG_DETACH;
}
ata_port_freeze(ap);
_
was intended.
> + }
> +
> + ata_port_freeze(ap);
> + ata_port_schedule_eh(ap);
> +}
> +
> static void ata_acpi_handle_hotplug(struct ata_port *ap, struct ata_device
> - *dev, u32 event)
> + *dev, u32 event, int is_dock_event)
> {
> char event_string[12];
> char *envp[] = { event_string, NULL };
> @@ -135,83 +152,92 @@ static void ata_acpi_handle_hotplug(stru
> ap = dev->link->ap;
> ehi = &ap->link.eh_info;
>
> - spin_lock_irqsave(ap->lock, flags);
> -
> - if (dev)
> + if (dev) {
> + if (dev->sdev)
> + kobj = &dev->sdev->sdev_gendev.kobj;
> handle = dev->acpi_handle;
> - else
> + } else {
> + kobj = &ap->dev->kobj;
> handle = ap->acpi_handle;
> + }
>
> status = acpi_get_handle(handle, "_EJ0", &tmphandle);
> if (ACPI_FAILURE(status)) {
> - /* This device is not ejectable */
> - spin_unlock_irqrestore(ap->lock, flags);
> + /* This device does not support hotplug */
> return;
> }
>
> - status = acpi_evaluate_integer(handle, "_STA", NULL, &sta);
> - if (ACPI_FAILURE(status)) {
> - printk ("Unable to determine bay status\n");
> - spin_unlock_irqrestore(ap->lock, flags);
> - return;
> + if (kobj && !is_dock_event) {
> + sprintf(event_string, "BAY_EVENT=%d", event);
> + kobject_uevent_env(kobj, KOBJ_CHANGE, envp);
> }
>
> + spin_lock_irqsave(ap->lock, flags);
Wow, lots of stuff happening under spinlock here, but it all looks OK to me.
> switch (event) {
> case ACPI_NOTIFY_BUS_CHECK:
> case ACPI_NOTIFY_DEVICE_CHECK:
> ata_ehi_push_desc(ehi, "ACPI event");
> - if (!sta) {
> - /* Device has been unplugged */
> - 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;
> - }
> - }
The old code was less odd ;)
--
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
