On Tue, 25 Sep 2007 00:14:36 +0100
Matthew Garrett <mjg59@xxxxxxxxxxxxx> wrote:
> Modern laptops with hotswap bays still tend to utilise a PATA interface
> on a SATA bridge, generally with the host controller in some legacy
> emulation mode rather than AHCI. This means that the existing hotplug
> code in libata is unable to work. The ACPI specification states that
> these devices can send notifications when hotswapped, which avoids the
> need to obtain notification from the controller. This patch uses the
> existing libata-acpi code and simply registers a notification in order
> to trigger a rescan whenever the firmware signals an event.
>
Hi Matthew,
While I love the idea of integrating the Bay support with libata, and
I think this is a good patch, I have 2 concerns which don't seem to be
addressed here.
1. How does it handle things when you have a bay that is located behind
a dock and the dock ejects? In the acpi bay driver, I use the mechanism
that the dock driver exports to get undock notifications so that the bay
can eject as well.
2. What if someone wants to use their bay to charge their battery? While
I never bothered to implement this in acpi/bay.c, nothing ever prevented
anyone from adding that support to the driver, where now it is prevented
since this driver and another cannot coexist.
The basic problem is that there's no way to have multiple drivers register
a notifier for the same ACPI event on the same object. So, my solution
to this in my acpi drivers was to export ways to share from the driver.
Thanks,
Kristen
> Signed-off-by: Matthew Garrett <mjg59@xxxxxxxxxxxxx>
>
> ---
>
> This makes two changes to the previous patch:
>
> 1) It implements the locking suggested by Tejun
> 2) It sends a uevent on the device kobject. I've implemented this
> because grabbing the notification handler means that the bay driver can
> no longer do it, so it's necessary to generate compatible events. If the
> event type is 3, it indicates that the user has merely requested an
> eject - the drive hasn't gone at this point. Sending the notification
> allows userspace to attempt to unmount the filesystems before sending a
> command to initiate the eject.
>
> I'm not especially happy about the chain used to get the scsi device
> kobject. Is there a cleaner way to do that? Other than that, I've now
> tested this on multiple systems (a 965-based Thinkpad, a 915-era Dell
> and even an HP with no SATA whatsoever) without any obvious breakage.
>
> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
> index c059f78..68bb7fa 100644
> --- a/drivers/ata/libata-acpi.c
> +++ b/drivers/ata/libata-acpi.c
> @@ -14,6 +14,7 @@
> #include <linux/acpi.h>
> #include <linux/libata.h>
> #include <linux/pci.h>
> +#include <scsi/scsi_device.h>
> #include "libata.h"
>
> #include <acpi/acpi_bus.h>
> @@ -66,6 +67,41 @@ static void ata_acpi_associate_ide_port(struct ata_port *ap)
> }
> }
>
> +static void ata_acpi_notify(acpi_handle handle, u32 event, void *data)
> +{
> + struct ata_port *ap = data;
> + struct ata_eh_info *ehi = &ap->eh_info;
> + char event_string[12];
> + char *envp[] = { event_string, NULL };
> + struct kobject *kobj = NULL;
> + int i;
> +
> + if (ap->acpi_handle && handle == ap->acpi_handle)
> + kobj = &ap->dev->kobj;
> + else {
> + for (i = 0; i < ata_port_max_devices(ap); i++) {
> + struct ata_device *dev = &ap->device[i];
> + if (dev->acpi_handle && handle == dev->acpi_handle)
> + kobj = &dev->sdev->sdev_gendev.kobj;
> + }
> + }
> +
> + if (event == 0 || event == 1) {
> + unsigned long flags;
> + spin_lock_irqsave(ap->lock, flags);
> + ata_ehi_clear_desc(ehi);
> + ata_ehi_push_desc(ehi, "ACPI event");
> + ata_ehi_hotplugged(ehi);
> + ata_port_freeze(ap);
> + spin_unlock_irqrestore(ap->lock, flags);
> + }
> +
> + if (kobj) {
> + sprintf(event_string, "BAY_EVENT=%d\n", event);
> + kobject_uevent_env(kobj, KOBJ_CHANGE, envp);
> + }
> +}
> +
> /**
> * ata_acpi_associate - associate ATA host with ACPI objects
> * @host: target ATA host
> @@ -81,7 +117,7 @@ static void ata_acpi_associate_ide_port(struct ata_port *ap)
> */
> void ata_acpi_associate(struct ata_host *host)
> {
> - int i;
> + int i, j;
>
> if (!is_pci_dev(host->dev) || libata_noacpi)
> return;
> @@ -97,6 +133,22 @@ void ata_acpi_associate(struct ata_host *host)
> ata_acpi_associate_sata_port(ap);
> else
> ata_acpi_associate_ide_port(ap);
> +
> + if (ap->acpi_handle)
> + acpi_install_notify_handler (ap->acpi_handle,
> + ACPI_SYSTEM_NOTIFY,
> + ata_acpi_notify,
> + ap);
> +
> + for (j = 0; j < ata_port_max_devices(ap); j++) {
> + struct ata_device *dev = &ap->device[j];
> +
> + if (dev->acpi_handle)
> + acpi_install_notify_handler (dev->acpi_handle,
> + ACPI_SYSTEM_NOTIFY,
> + ata_acpi_notify,
> + ap);
> + }
> }
> }
>
> --
> Matthew Garrett | mjg59@xxxxxxxxxxxxx
>
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
