[PATCH] libata: Integrate ACPI-based PATA/SATA hotplug

[PATCH] libata: Integrate ACPI-based PATA/SATA hotplug

[Matthew Garrett]

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.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

---

I've tested this on the Dell I have here - the removal is a bit chatty 
(and it spends several seconds attempting to revalidate), but it 
destroys the device happily enough and handles reinsertion without 
complaining.

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index c059f78..bc6bea6 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -66,6 +66,17 @@ 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;
+
+	ata_ehi_clear_desc(ehi);
+	ata_ehi_push_desc(ehi, "ACPI event");
+	ata_ehi_hotplugged(ehi);
+	ata_port_freeze(ap);
+}
+
 /**
  * ata_acpi_associate - associate ATA host with ACPI objects
  * @host: target ATA host
@@ -97,6 +108,12 @@ void ata_acpi_associate(struct ata_host *host)
 			ata_acpi_associate_sata_port(ap);
 		else
 			ata_acpi_associate_ide_port(ap);
+
+		if (ap->device->acpi_handle)
+			acpi_install_notify_handler (ap->device->acpi_handle,
+						     ACPI_SYSTEM_NOTIFY,
+						     ata_acpi_notify,
+						     ap);
 	}
 }
 
-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Matthew Garrett]

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.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

---

Testing on an HP with the hotplug device as the slave on a PATA channel 
flagged a bug - the notification needs to be tied to the channel handle 
as well as the device ones. With this version, I can happily hotswap the 
HP device. It ends up sitting for a few seconds while failing to 
revalidate, but then recovers with everything working fine.

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index c059f78..99f3179 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -66,6 +66,17 @@ 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;
+
+	ata_ehi_clear_desc(ehi);
+	ata_ehi_push_desc(ehi, "ACPI event");
+	ata_ehi_hotplugged(ehi);
+	ata_port_freeze(ap);
+}
+
 /**
  * ata_acpi_associate - associate ATA host with ACPI objects
  * @host: target ATA host
@@ -81,7 +92,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 +108,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_MAX_DEVICES; j++) {
+			struct ata_device *dev = &ap->device[j];
+
+			if (dev->acpi_handle)
+				acpi_install_notify_handler (ap->device->acpi_handle,
+							     ACPI_SYSTEM_NOTIFY,
+							     ata_acpi_notify,
+							     ap);
+		}
 	}
 }
  
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Tejun Heo]

Matthew Garrett wrote:
> +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;
> +
> +	ata_ehi_clear_desc(ehi);
> +	ata_ehi_push_desc(ehi, "ACPI event");
> +	ata_ehi_hotplugged(ehi);
> +	ata_port_freeze(ap);

You probably want to wrap the above inside ap->lock.

-- 
tejun

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Jeff Garzik]

Matthew Garrett 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.
> 
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
> 
> ---
> 
> Testing on an HP with the hotplug device as the slave on a PATA channel 
> flagged a bug - the notification needs to be tied to the channel handle 
> as well as the device ones. With this version, I can happily hotswap the 
> HP device. It ends up sitting for a few seconds while failing to 
> revalidate, but then recovers with everything working fine.

the code looks correct.  I have one main reservation.

how can we be sure that this is active only where other hand-programmed 
hotplug code is absent?

ACPI doesn't inherently know which of our controllers are already set up 
to do hotplug, so it seems like there is a possibility of conflict if 
both the firmware and the controller are chirping.

If we can quell my fears in that area, I'm ok with the change.

	Jeff

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Matthew Garrett]

On Thu, Sep 20, 2007 at 06:04:22PM -0400, Jeff Garzik wrote:

> the code looks correct.  I have one main reservation.
> 
> how can we be sure that this is active only where other hand-programmed 
> hotplug code is absent?

Yes, that's difficult. As Tejun pointed out, there's missing locking 
here at the moment - if I add that, am I right in thinking that the 
worst case scenario is that the hotplugging path will be called twice?

One option would be to limit this to PATA-style controllers - I'm not 
aware of any mobile hardware shipping with natively hotplug-capable 
controllers, so that would probably do for the moment. If (when) they 
move to AHCI, with luck the native hotplugging will work and we won't 
have to worry about this so much.

The alternative would be to add a flag to the ap structure indicating 
whether the hotplugging is handled by the firmware or not. If we find a 
reference to a controller or port in the firmware tables, it probably 
indicates that the hardware has opinions about how this should be 
handled. We might be safer leaving it to the firmware in those cases, 
and using that flag to skip the controller-specific hotplug code.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Tejun Heo]

Matthew Garrett wrote:
> On Thu, Sep 20, 2007 at 06:04:22PM -0400, Jeff Garzik wrote:
> 
>> the code looks correct.  I have one main reservation.
>>
>> how can we be sure that this is active only where other hand-programmed 
>> hotplug code is absent?
> 
> Yes, that's difficult. As Tejun pointed out, there's missing locking 
> here at the moment - if I add that, am I right in thinking that the 
> worst case scenario is that the hotplugging path will be called twice?

Yeah, should be and libata EH won't have too much problem handling
duplicate events.

> One option would be to limit this to PATA-style controllers - I'm not 
> aware of any mobile hardware shipping with natively hotplug-capable 
> controllers, so that would probably do for the moment. If (when) they 
> move to AHCI, with luck the native hotplugging will work and we won't 
> have to worry about this so much.
> 
> The alternative would be to add a flag to the ap structure indicating 
> whether the hotplugging is handled by the firmware or not. If we find a 
> reference to a controller or port in the firmware tables, it probably 
> indicates that the hardware has opinions about how this should be 
> handled. We might be safer leaving it to the firmware in those cases, 
> and using that flag to skip the controller-specific hotplug code.

I was thinking the other way around.  I'd rather depend on hardware
provided events than firmware provided ones.  How about flagging drivers
which can do native hoplugging and using ACPI hotplugging only if the
driver can't do it natively?

Thanks.

-- 
tejun

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Matthew Garrett]

On Fri, Sep 21, 2007 at 11:35:05AM +0900, Tejun Heo wrote:
> Matthew Garrett wrote:
> > The alternative would be to add a flag to the ap structure indicating 
> > whether the hotplugging is handled by the firmware or not. If we find a 
> > reference to a controller or port in the firmware tables, it probably 
> > indicates that the hardware has opinions about how this should be 
> > handled. We might be safer leaving it to the firmware in those cases, 
> > and using that flag to skip the controller-specific hotplug code.
> 
> I was thinking the other way around.  I'd rather depend on hardware
> provided events than firmware provided ones.  How about flagging drivers
> which can do native hoplugging and using ACPI hotplugging only if the
> driver can't do it natively?

If the manufacturers have added firmware-level hotswap interrupts, then 
there's all sorts of insane ways they might have wired the bay up. I 
don't really trust them to have managed to do so without breaking native 
hotplug :) It doesn't really matter at the moment, though, since I 
haven't actually seen any examples of hardware using anything that can 
manage native hotplug. If anyone out there does have one, it would be 
nice to get some feedback about what it does.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Tejun Heo]

Matthew Garrett wrote:
> On Fri, Sep 21, 2007 at 11:35:05AM +0900, Tejun Heo wrote:
>> Matthew Garrett wrote:
>>> The alternative would be to add a flag to the ap structure indicating 
>>> whether the hotplugging is handled by the firmware or not. If we find a 
>>> reference to a controller or port in the firmware tables, it probably 
>>> indicates that the hardware has opinions about how this should be 
>>> handled. We might be safer leaving it to the firmware in those cases, 
>>> and using that flag to skip the controller-specific hotplug code.
>> I was thinking the other way around.  I'd rather depend on hardware
>> provided events than firmware provided ones.  How about flagging drivers
>> which can do native hoplugging and using ACPI hotplugging only if the
>> driver can't do it natively?
> 
> If the manufacturers have added firmware-level hotswap interrupts, then 
> there's all sorts of insane ways they might have wired the bay up. I 
> don't really trust them to have managed to do so without breaking native 
> hotplug :) It doesn't really matter at the moment, though, since I 
> haven't actually seen any examples of hardware using anything that can 
> manage native hotplug. If anyone out there does have one, it would be 
> nice to get some feedback about what it does.

Maybe just letting both events in is the best idea.  It's not like two
duplicate events are gonna break anything and I don't think many vendors
are gonna implement separate mechanism when the default SATA phy based
one works.

-- 
tejun

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Matthew Garrett]

On Fri, Sep 21, 2007 at 11:53:33AM +0900, Tejun Heo wrote:

> Maybe just letting both events in is the best idea.  It's not like two
> duplicate events are gonna break anything and I don't think many vendors
> are gonna implement separate mechanism when the default SATA phy based
> one works.

Works for me. Unrelatedly, is it expected that the EH take some time 
attempting to revalidate the port before finally deciding that the drive 
has gone? I seem to lose 15 seconds or so to that, which is more 
irritating on PATA systems where it tends to block the channel.

(Quite why HP put their hotswap optical drives on the same PATA channel 
as the internal drive is somewhat beyond me, but...)
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Tejun Heo]

Matthew Garrett wrote:
> On Fri, Sep 21, 2007 at 11:53:33AM +0900, Tejun Heo wrote:
> 
>> Maybe just letting both events in is the best idea.  It's not like two
>> duplicate events are gonna break anything and I don't think many vendors
>> are gonna implement separate mechanism when the default SATA phy based
>> one works.
> 
> Works for me. Unrelatedly, is it expected that the EH take some time 
> attempting to revalidate the port before finally deciding that the drive 
> has gone? I seem to lose 15 seconds or so to that, which is more 
> irritating on PATA systems where it tends to block the channel.
> 
> (Quite why HP put their hotswap optical drives on the same PATA channel 
> as the internal drive is somewhat beyond me, but...)

Yeah, that's the intended behavior.  SATA PHY link can break from time
to time (have ever seen a SATA storage box going through ECC testing?
PHY goes offline as soon as you begin to hit it with some EM pulses) and
you don't really wanna lose your root partition over power fluctuation.

I'm thinking about improving EH such that it doesn't hold up the bus for
the whole period.  It will be needed for PMP too.  However, for now,
that's the expected behavior.

Thanks.

-- 
tejun

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Matthew Garrett]

On Fri, Sep 21, 2007 at 12:08:07PM +0900, Tejun Heo wrote:

> Yeah, that's the intended behavior.  SATA PHY link can break from time
> to time (have ever seen a SATA storage box going through ECC testing?
> PHY goes offline as soon as you begin to hit it with some EM pulses) and
> you don't really wanna lose your root partition over power fluctuation.

In this case we explicitly know that it's in response to a hotplug event 
(well, either that or the firmware is on impressive crack). Would it be 
possible to communicate that in order to avoid the revalidation?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 2

[Tejun Heo]

Matthew Garrett wrote:
> On Fri, Sep 21, 2007 at 12:08:07PM +0900, Tejun Heo wrote:
> 
>> Yeah, that's the intended behavior.  SATA PHY link can break from time
>> to time (have ever seen a SATA storage box going through ECC testing?
>> PHY goes offline as soon as you begin to hit it with some EM pulses) and
>> you don't really wanna lose your root partition over power fluctuation.
> 
> In this case we explicitly know that it's in response to a hotplug event 
> (well, either that or the firmware is on impressive crack). Would it be 
> possible to communicate that in order to avoid the revalidation?

No, I don't thinks so && I don't really think trusting firmware that
much is a good idea.  Also, it's much better to fix the problem in
general than just fixing it for the firmware case which will be much
less than the rest.

-- 
tejun

[PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 3

[Matthew Garrett]

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.
                                                                                                                                                                                                        
Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>                        

---

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@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 3

[Kristen Carlson Accardi]

On Tue, 25 Sep 2007 00:14:36 +0100
Matthew Garrett <mjg59@srcf.ucam.org> 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@srcf.ucam.org>                        
> 
> ---
> 
> 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@srcf.ucam.org
> 

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 3

[Matthew Garrett]

On Thu, Sep 27, 2007 at 10:26:59AM -0700, Kristen Carlson Accardi wrote:

> 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.

Hm. I'd been working on the assumption that ejecting the doc would 
trigger the bay notifications as well, but I've got no hardware here 
with those capabilities so it's kind of hard to check...

> 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 spec seems to imply that even if the drive hotswap bay and the 
battery bay are physically the same, they're logically distinct. 10.2.1 
specifies that the battery bay should always be considered present, and 
that any insertion notification will be generated from the battery bay 
rather than the drive bay (so Notify (BAT1, 0x81) rather than Notify 
(_SB.MISC.OTHR.BONG.PRIM.SLAV, 0x81)). My code will only grab the 
latter notification, not the former.

I /suspect/ that _STA on the bay device won't return true if there's a 
battery in there, and so we aren't expected to call the _EJ0 method if 
the user wants to remove a battery. But I'm also lacking hardware to 
test this one, so it's possible that I'm utterly wrong :)

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 3

[Henrique de Moraes Holschuh]

On Thu, 27 Sep 2007, Matthew Garrett wrote:
> The spec seems to imply that even if the drive hotswap bay and the 
> battery bay are physically the same, they're logically distinct. 10.2.1 

That holds true for thinkpads up to the T43, at least.  I don't know about
the newer ones.

You get bay ejects/inserts notifications from the device node corresponding
to what was inserted/removed from the multi-purpose bay.

> I /suspect/ that _STA on the bay device won't return true if there's a 
> battery in there, and so we aren't expected to call the _EJ0 method if 

Correct, on thinkpads up to the T43 at least.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 3

[Jeff Garzik]

Matthew Garrett 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.
>                                                                                                                                                                                                         
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>                        
> 
> ---
> 
> 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);

Mostly OK.  Notes:

1) Check dev->sdev for NULL

2) remove the unnecessary ata_device loop.  If you know the ata_device 
pointer, you should not throw it away and then do a search to find it again.

You need two functions, ata_acpi_ap_notify() and ata_acpi_dev_notify(). 
  Pass 'ap' to the former, and 'dev' to the latter.

Both functions should marshal their arguments, then call a common 
function (presumably what 95% of current ata_acpi_notify does).

3) Won't this result in a single hotplug event calling 
ata_ehi_hotplugged() multiple times -- once for the port, and once for 
each device attached to the port?

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 3

[Matthew Garrett]

On Tue, Oct 02, 2007 at 11:04:41AM -0400, Jeff Garzik wrote:

> 1) Check dev->sdev for NULL

Ok.

> 2) remove the unnecessary ata_device loop.  If you know the ata_device 
> pointer, you should not throw it away and then do a search to find it again.
> 
> You need two functions, ata_acpi_ap_notify() and ata_acpi_dev_notify(). 
>  Pass 'ap' to the former, and 'dev' to the latter.
> 
> Both functions should marshal their arguments, then call a common 
> function (presumably what 95% of current ata_acpi_notify does).

Sure.

> 3) Won't this result in a single hotplug event calling 
> ata_ehi_hotplugged() multiple times -- once for the port, and once for 
> each device attached to the port?

No - the platform will either send an event for the port or for an 
individual device. It'll never simultaneously send one for both the port 
and a device. Semantically the one from the port is a "check all 
children" request and the one from the device a "check this individual 
device", but I believe these are both equivalent in the current hotswap 
implementation.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 4

[Matthew Garrett]

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.
                                                                                                                                                                                                        
Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>                        

---

This incorporates Jeff's feedback. sdev is checked for NULL, and 
different notifications are registered for ap-level and dev-level 
handlers. The core code is split out into a helper function called by 
both of these. The other change is the removal of the extraneous newline 
from the end of the notification event, to match the upstream change in 
the bay driver.

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index c059f78..4d8f1ba 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,47 @@ static void ata_acpi_associate_ide_port(struct ata_port *ap)
 	}
 }
 
+static void ata_acpi_handle_hotplug (struct ata_port *ap, struct kobject *kobj,
+				     u32 event)
+{
+	char event_string[12];
+	char *envp[] = { event_string, NULL };
+	struct ata_eh_info *ehi = &ap->eh_info;
+
+	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", event);
+		kobject_uevent_env(kobj, KOBJ_CHANGE, envp);
+	}
+}
+
+static void ata_acpi_dev_notify(acpi_handle handle, u32 event, void *data)
+{
+	struct ata_device *dev = data;
+	struct kobject *kobj = NULL;
+
+	if (dev->sdev)
+		kobj = &dev->sdev->sdev_gendev.kobj;
+
+	ata_acpi_handle_hotplug (dev->ap, kobj, event);
+}
+
+static void ata_acpi_ap_notify(acpi_handle handle, u32 event, void *data)
+{
+	struct ata_port *ap = data;
+
+	ata_acpi_handle_hotplug (ap, &ap->dev->kobj, event);
+}
+
 /**
  * ata_acpi_associate - associate ATA host with ACPI objects
  * @host: target ATA host
@@ -81,7 +123,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 +139,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_ap_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_dev_notify,
+							     dev);
+		}
 	}
 }
 
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 4

[Jeff Garzik]

Matthew Garrett 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.
>                                                                                                                                                                                                         
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>                        
> 
> ---
> 
> This incorporates Jeff's feedback. sdev is checked for NULL, and 
> different notifications are registered for ap-level and dev-level 
> handlers. The core code is split out into a helper function called by 
> both of these. The other change is the removal of the extraneous newline 
> from the end of the notification event, to match the upstream change in 
> the bay driver.

applied

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 4

[Jeff Garzik]

Jeff Garzik wrote:
> Matthew Garrett 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.
>>                                                                                                                                                                                                         
>> Signed-off-by: Matthew Garrett 
>> <mjg59@srcf.ucam.org>                       
>> ---
>>
>> This incorporates Jeff's feedback. sdev is checked for NULL, and 
>> different notifications are registered for ap-level and dev-level 
>> handlers. The core code is split out into a helper function called by 
>> both of these. The other change is the removal of the extraneous 
>> newline from the end of the notification event, to match the upstream 
>> change in the bay driver.
> 
> applied

Come on, dude!  This doesn't even build:

   UPD     include/linux/compile.h
drivers/ata/libata-acpi.c: In function ‘ata_acpi_handle_hotplug’:
drivers/ata/libata-acpi.c:104: error: ‘struct ata_port’ has no member 
named ‘eh_info’
drivers/ata/libata-acpi.c: In function ‘ata_acpi_dev_notify’:
drivers/ata/libata-acpi.c:130: error: ‘struct ata_device’ has no member 
named ‘ap’
drivers/ata/libata-acpi.c: In function ‘ata_acpi_associate’:
drivers/ata/libata-acpi.c:178: error: implicit declaration of function 
‘ata_port_max_devices’
drivers/ata/libata-acpi.c:179: error: ‘struct ata_port’ has no member 
named ‘device’
make[2]: *** [drivers/ata/libata-acpi.o] Error 1


-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 4

[Matthew Garrett]

On Tue, Oct 02, 2007 at 04:38:19PM -0400, Jeff Garzik wrote:

> Come on, dude!  This doesn't even build:

Crap, sorry, I've pulled that from the wrong tree. I'll grab you a 
working one now.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 4

[Matthew Garrett]

Fix libata-acpi.c build failure.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

--- 

Sorry, I'd diffed the original against libata-dev master rather than 
ALL. This tidies up the changes.

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 6896831..5ebbf16 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -101,7 +101,7 @@ static void ata_acpi_handle_hotplug (struct ata_port *ap, struct kobject *kobj,
 {
 	char event_string[12];
 	char *envp[] = { event_string, NULL };
-	struct ata_eh_info *ehi = &ap->eh_info;
+	struct ata_eh_info *ehi = &ap->link.eh_info;
 
 	if (event == 0 || event == 1) {
 	       unsigned long flags;
@@ -127,7 +127,7 @@ static void ata_acpi_dev_notify(acpi_handle handle, u32 event, void *data)
 	if (dev->sdev)
 		kobj = &dev->sdev->sdev_gendev.kobj;
 
-	ata_acpi_handle_hotplug (dev->ap, kobj, event);
+	ata_acpi_handle_hotplug (dev->link->ap, kobj, event);
 }
 
 static void ata_acpi_ap_notify(acpi_handle handle, u32 event, void *data)
@@ -175,8 +175,8 @@ void ata_acpi_associate(struct ata_host *host)
 						     ata_acpi_ap_notify,
 						     ap);
 
-		for (j = 0; j < ata_port_max_devices(ap); j++) {
-			struct ata_device *dev = &ap->device[j];
+		for (j = 0; j < ata_link_max_devices(&ap->link); j++) {
+			struct ata_device *dev = &ap->link.device[j];
 
 			if (dev->acpi_handle)
 				acpi_install_notify_handler (dev->acpi_handle,

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 4

[Jeff Garzik]

Matthew Garrett wrote:
> Fix libata-acpi.c build failure.
> 
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
> 
> --- 
> 
> Sorry, I'd diffed the original against libata-dev master rather than 
> ALL. This tidies up the changes.

I had to yank the patch.  For bisect-ability, we really really want to 
avoid pushing patches upstream that don't build; so, please resend a 
fixed version.

BTW, you generally want to diff against libata-dev.git#upstream. 
'upstream' branch is always what is queued for the next kernel release. 
  'ALL' is a meta-branch that includes #upstream, but also other stuff 
we want in -mm for testing.

	Jeff

[PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 5

[Matthew Garrett]

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.

Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

---

Diffed against #upstream. Fairly sure I've got it right this time...

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index a276c06..5ebbf16 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>
@@ -95,6 +96,47 @@ static void ata_acpi_associate_ide_port(struct ata_port *ap)
 	}
 }
 
+static void ata_acpi_handle_hotplug (struct ata_port *ap, struct kobject *kobj,
+				     u32 event)
+{
+	char event_string[12];
+	char *envp[] = { event_string, NULL };
+	struct ata_eh_info *ehi = &ap->link.eh_info;
+
+	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", event);
+		kobject_uevent_env(kobj, KOBJ_CHANGE, envp);
+	}
+}
+
+static void ata_acpi_dev_notify(acpi_handle handle, u32 event, void *data)
+{
+	struct ata_device *dev = data;
+	struct kobject *kobj = NULL;
+
+	if (dev->sdev)
+		kobj = &dev->sdev->sdev_gendev.kobj;
+
+	ata_acpi_handle_hotplug (dev->link->ap, kobj, event);
+}
+
+static void ata_acpi_ap_notify(acpi_handle handle, u32 event, void *data)
+{
+	struct ata_port *ap = data;
+
+	ata_acpi_handle_hotplug (ap, &ap->dev->kobj, event);
+}
+
 /**
  * ata_acpi_associate - associate ATA host with ACPI objects
  * @host: target ATA host
@@ -110,7 +152,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;
@@ -126,6 +168,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_ap_notify,
+						     ap);
+
+		for (j = 0; j < ata_link_max_devices(&ap->link); j++) {
+			struct ata_device *dev = &ap->link.device[j];
+
+			if (dev->acpi_handle)
+				acpi_install_notify_handler (dev->acpi_handle,
+							     ACPI_SYSTEM_NOTIFY,
+							     ata_acpi_dev_notify,
+							     dev);
+		}
 	}
 }

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH] libata: Integrate ACPI-based PATA/SATA hotplug - version 5

[Jeff Garzik]

Matthew Garrett 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.
> 
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
> 
> ---
> 
> Diffed against #upstream. Fairly sure I've got it right this time...

indeed you did sir :)  applied, thanks