Re: [ugly patch] Save .15W-.5W by AHCI powersaving

2008-02-26 Thread Matthew Garrett
On Mon, Feb 25, 2008 at 05:42:58PM -0500, Jeff Garzik wrote:

> BTW we can also save power by allowing the user to choose to disable 
> hotplugging support.  Then we can power down PHYs that are not in use.
> 
> That requires the addition of some policy controls, because it is 
> user-specific whether or not to waste power waiting for a plug-in event.

For AHCI, if you've enabled link power management then you've already 
disabled hotplug. We might as well power down unused phys in that case. 
Note that laptop bays still seem to tend to use platform-specific 
hotplug notification, even when they're sata - we'll get the hotplug 
notify for them even if the phy's powered down, so that case also needs 
to be handled.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: 2.6.24-rc4-git5: Reported regressions from 2.6.23

2007-12-08 Thread Matthew Garrett
On Sat, Dec 08, 2007 at 02:20:01AM -0800, Andrew Morton wrote:
> On Sat, 8 Dec 2007 11:12:57 +0100 Andreas Mohr <[EMAIL PROTECTED]> wrote:
> > ACPI Exception (exoparg2-0442): AE_AML_PACKAGE_LIMIT, Index (0) is 
> > beyond end of object [20070126]
> > ACPI Error (psparse-0537): Method parse/execution failed 
> > [\_SB_.PCI0.IDE0.GTF_] (Node c180b990), AE_AML_PACKAGE_LIMIT
> > ACPI Error (psparse-0537): Method parse/execution failed 
> > [\_SB_.PCI0.IDE0.CHN0.DRV1._GTF] (Node c180b888), AE_AML_PACKAGE_LIMIT
> > ata1.01: _GTF evaluation failed (AE 0x300d)

037f6bb79f753c014bc84bca0de9bf98bb5ab169 ought to have fixed this?

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Don't fail ata device revalidation for bad _GTF methods

2007-11-08 Thread Matthew Garrett
Experience suggests that the _GTF method may be bad. We currently fail 
device revalidation in that case, which seems excessive.

Signed-off-by: Matthew Garrett <[EMAIL PROTECTED]>

---

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 08a52dd..545ea86 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -312,7 +312,7 @@ EXPORT_SYMBOL_GPL(ata_acpi_stm);
  *
  * RETURNS:
  * Number of taskfiles on success, 0 if _GTF doesn't exist or doesn't
- * contain valid data.  -errno on other errors.
+ * contain valid data.
  */
 static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
   void **ptr_to_free)
@@ -339,7 +339,6 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct 
ata_acpi_gtf **gtf,
ata_dev_printk(dev, KERN_WARNING,
   "_GTF evaluation failed (AE 0x%x)\n",
   status);
-   rc = -EIO;
}
goto out_free;
}
@@ -359,7 +358,6 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct 
ata_acpi_gtf **gtf,
ata_dev_printk(dev, KERN_WARNING,
   "_GTF unexpected object type 0x%x\n",
   out_obj->type);
-   rc = -EINVAL;
goto out_free;
}
 
@@ -367,7 +365,6 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct 
ata_acpi_gtf **gtf,
ata_dev_printk(dev, KERN_WARNING,
   "unexpected _GTF length (%d)\n",
   out_obj->buffer.length);
-   rc = -EINVAL;
goto out_free;
}
 
@@ -511,10 +508,7 @@ static int ata_acpi_exec_tfs(struct ata_device *dev)
int gtf_count, i, rc;
 
/* get taskfiles */
-   rc = ata_dev_get_GTF(dev, >f, &ptr_to_free);
-   if (rc < 0)
-   return rc;
-   gtf_count = rc;
+   gtf_count = ata_dev_get_GTF(dev, >f, &ptr_to_free);
 
/* execute them */
    for (i = 0, rc = 0; i < gtf_count; i++) {

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2)

2007-11-08 Thread Matthew Garrett
On Thu, Nov 08, 2007 at 10:13:41AM -0800, Andrew Morton wrote:

> I suspect it wold be best to disable the feature for the 2.6.24 release,
> then reenable it afterwards and keep doing this until the code is
> sufficiently stable.

GTF method execution failure currently looks like it's fatal, when it 
probably shouldn't be. I'll send a patch shortly.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Bugme-new] [Bug 9320] New: PATA scan: ACPI Exception AE_AML_PACKAGE_LIMIT... is beyond end of object

2007-11-07 Thread Matthew Garrett
On Wed, Nov 07, 2007 at 02:07:54PM -0800, Andrew Morton wrote:

> Seems to be another post-2.6.23 regression.  Is it an acpi thing or an ata
> thing, of just something which the new acpi+ata stuff exposed??

I suspect that this is just because acpi is enabled by default on ata 
now. Is it actually causing any problems?

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-10-02 Thread 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 <[EMAIL PROTECTED]>

---

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 
 #include 
 #include 
+#include 
 #include "libata.h"
 
 #include 
@@ -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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-10-02 Thread Matthew Garrett
Fix libata-acpi.c build failure.

Signed-off-by: Matthew Garrett <[EMAIL PROTECTED]>

--- 

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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-10-02 Thread 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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-10-02 Thread 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 <[EMAIL PROTECTED]>

---

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 
 #include 
 #include 
+#include 
 #include "libata.h"
 
 #include 
@@ -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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-10-02 Thread 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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-09-27 Thread 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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-09-24 Thread 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 <[EMAIL PROTECTED]>

---

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 
 #include 
 #include 
+#include 
 #include "libata.h"
 
 #include 
@@ -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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-09-20 Thread 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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-09-20 Thread 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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-09-20 Thread 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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-09-20 Thread 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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-09-15 Thread 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 <[EMAIL PROTECTED]>

---

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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2007-09-14 Thread 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 <[EMAIL PROTECTED]>

---

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 | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] hook ACPI _PSx method to IDE power on/off

2007-08-02 Thread Matthew Garrett
On Thu, Aug 02, 2007 at 02:14:08PM +0800, Shaohua Li wrote:
> ACPI spec defines the sequence of IDE power on/off:

Most distributions seem to be using the libata PATA code now - any plans 
to implement it there as well?
-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

2007-06-12 Thread Matthew Garrett
On Tue, Jun 12, 2007 at 11:46:56AM -0400, Jeff Garzik wrote:
> Matthew Garrett wrote:
> >On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh 
> >wrote:
> >>On Tue, 12 Jun 2007, Matthew Garrett wrote:
> >>>Laptop bays are designed to deal with hotplugging PATA - I don't think 
> >>>this is too much of an issue :)
> >>The new SATA ones use the SATA hardware hotplug ;-)   Just like the pci-e
> >>cards use usb2.0 and pci-e hotplug...
> >
> >Yes, but they'll also send an ACPI interrupt even if the SATA host 
> >controller doesn't - it's part of the spec for bays.
> 
> Regardless, having a laptop does not imply having a docking bay.

Excluding the corner case of an Expresscard SATA controller (where I 
suspect you'd want different policy), I doubt there are any cases where 
you have a laptop with hotplug capabilities without it being implemented 
as an ACPI bay.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

2007-06-12 Thread Matthew Garrett
On Wed, Jun 13, 2007 at 12:45:21AM +0900, Tejun Heo wrote:
> Matthew Garrett wrote:
> > Yes, but they'll also send an ACPI interrupt even if the SATA host 
> > controller doesn't - it's part of the spec for bays.
> 
> Does the spec mandate that the ACPI interrupt shouldn't depend on SATA
> phy status?  I don't think vendors are likely to implement separate
> mechanism when SATA phy status can do the job fine.

I suspect that the spec allows them to do that, but think that it's 
unlikely to actually happen in most cases. Bear in mind that Windows 
doesn't tend to drive the hardware in AHCI mode, and that their 
implementation is likely to be very similar to the implementation they 
used for PATA devices.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

2007-06-12 Thread Matthew Garrett
On Tue, Jun 12, 2007 at 11:17:14AM -0300, Henrique de Moraes Holschuh wrote:
> On Tue, 12 Jun 2007, Matthew Garrett wrote:
> > Laptop bays are designed to deal with hotplugging PATA - I don't think 
> > this is too much of an issue :)
> 
> The new SATA ones use the SATA hardware hotplug ;-)   Just like the pci-e
> cards use usb2.0 and pci-e hotplug...

Yes, but they'll also send an ACPI interrupt even if the SATA host 
controller doesn't - it's part of the spec for bays.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

2007-06-12 Thread Matthew Garrett
On Tue, Jun 12, 2007 at 09:18:19AM -0300, Henrique de Moraes Holschuh wrote:
> On Tue, 12 Jun 2007, Matthew Garrett wrote:
> > 
> > On laptops, I suspect that we'll probably get an ACPI interrupt even if 
> > the AHCI hotplug pathway can't manage.
> 
> As long as we don't crash the drive or AHCI controller because we hotplugged
> it in a way it didn't like (like trying to hotplug a ICH5R would cause).

Laptop bays are designed to deal with hotplugging PATA - I don't think 
this is too much of an issue :)

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch 3/3] Enable Aggressive Link Power management for AHCI controllers.

2007-06-12 Thread Matthew Garrett
On Mon, Jun 11, 2007 at 08:59:46PM -0700, Arjan van de Ven wrote:

> that's a temporary shortcoming; even with these power savings you can 
> do hotplug as long as you're willing to poll for it at a reasonable 
> interval and are willing to wait the time between polls for a hotplug 
> to take effect..

On laptops, I suspect that we'll probably get an ACPI interrupt even if 
the AHCI hotplug pathway can't manage.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] HPA support: Revised patch

2007-04-10 Thread Matthew Garrett
On Mon, Apr 09, 2007 at 10:22:41PM +0100, Alan Cox wrote:

> Please apply Tejun's fix for LBA48 data and try again. Hopefully its just
> that which is causing the problem.

Yes, that works absolutely fine now.
-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] HPA support: Revised patch

2007-04-06 Thread Matthew Garrett
On Thu, Apr 05, 2007 at 02:13:52PM +0100, Alan Cox wrote:
> This one should fix the problems with slave devices and the Macintosh hang

Better, but still not happy with ata_piix - I get the following:

[   10.972000] ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 
16337840
[   10.972000] ata3.01: ATA-8: FUJITSU MHW2120BH, 00810013, max UDMA/100
[   10.972000] ata3.01: 234441648 sectors, multi 16: LBA48 NCQ (depth 0/32)
[   10.98] ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 
-1342616656
[   10.98] ata3.01: Host Protected Area detected:
[   10.98]  current size: 234441648 sectors
[   10.98]  native size: -1342616656 sectors
 ^^^!?!?!?!?!?!?!?

so I'm not especially keen on letting it reprogram stuff. With ahci it 
works fine. Still on a Macbook Pro - fuller logs below.

With ata_piix:
[9.984000] ata1: PATA max UDMA/133 cmd 0x000101f0 ctl 0x000103f6 bmdma 
0x000140b0 irq 14
[9.984000] ata2: PATA max UDMA/133 cmd 0x00010170 ctl 0x00010376 bmdma 
0x000140b8 irq 15
[9.984000] scsi0 : ata_piix
[   10.304000] ata1.00: ATAPI, max UDMA/66
[   10.468000] ata1.00: configured for UDMA/66
[   10.468000] scsi1 : ata_piix
[   10.632000] ATA: abnormal status 0x7F on port 0x00010177
[   10.636000] scsi 0:0:0:0: CD-ROMMATSHITA DVD-R   UJ-857D  KCV9 
PQ: 0 ANSI: 5
[   10.636000] ata_piix :00:1f.2: MAP [ P0 P2 XX XX ]
[   10.636000] ata_piix :00:1f.2: invalid MAP value 0
[   10.792000] ata3: SATA max UDMA/133 cmd 0x000140c8 ctl 0x000140e6 bmdma 
0x000140a0 irq 20
[   10.792000] ata4: SATA max UDMA/133 cmd 0x000140c0 ctl 0x000140e2 bmdma 
0x000140a8 irq 20
[   10.792000] scsi2 : ata_piix
[   10.948000] ATA: abnormal status 0x7F on port 0x000140cf
[   10.964000] ATA: abnormal status 0x7F on port 0x000140cf
[   10.972000] ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 
16337840
[   10.972000] ata3.01: ATA-8: FUJITSU MHW2120BH, 00810013, max UDMA/100
[   10.972000] ata3.01: 234441648 sectors, multi 16: LBA48 NCQ (depth 0/32)
[   10.98] ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 
-1342616656
[   10.98] ata3.01: Host Protected Area detected:
[   10.98]  current size: 234441648 sectors
[   10.98]  native size: -1342616656 sectors
[   10.98] ata3.01: configured for UDMA/100
[   10.98] scsi3 : ata_piix
[   11.136000] ATA: abnormal status 0x7F on port 0x000140c7
[   11.14] scsi 2:0:1:0: Direct-Access ATA  FUJITSU MHW2120B 0081 
PQ: 0 ANSI: 5

With ahci:
[   10.052000] libata version 2.20 loaded.
[   10.052000] ahci :00:1f.2: version 2.1
[   10.052000] ACPI: PCI Interrupt :00:1f.2[B] -> GSI 19 (level, low) -> 
IRQ 20
[   11.056000] PCI: Setting latency timer of device :00:1f.2 to 64
[   11.056000] ahci :00:1f.2: AHCI 0001.0100 32 slots 4 ports 1.5 Gbps 0x4 
impl IDE mode
[   11.056000] ahci :00:1f.2: flags: 64bit ncq ilck pm led clo pio slum 
part 
[   11.056000] ata1: SATA max UDMA/133 cmd 0xf88a8100 ctl 0x bmdma 
0x irq 20
[   11.056000] ata2: SATA max UDMA/133 cmd 0xf88a8180 ctl 0x bmdma 
0x irq 20
[   11.056000] ata3: SATA max UDMA/133 cmd 0xf88a8200 ctl 0x bmdma 
0x irq 20
[   11.056000] ata4: SATA max UDMA/133 cmd 0xf88a8280 ctl 0x bmdma 
0x irq 20
[   11.056000] scsi0 : ahci
[   11.30] ieee1394: Host added: ID:BUS[0-00:1023]  GUID[0017f2fffe812952]
[   11.368000] ata1: SATA link down (SStatus 0 SControl 0)
[   11.368000] scsi1 : ahci
[   11.68] ata2: SATA link down (SStatus 0 SControl 0)
[   11.68] scsi2 : ahci
[   12.164000] ata3: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
[   12.164000] ata3.00: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 
234441648
[   12.164000] ata3.00: ATA-8: FUJITSU MHW2120BH, 00810013, max UDMA/100
[   12.164000] ata3.00: 234441648 sectors, multi 16: LBA48 NCQ (depth 31/32)
[   12.164000] ata3.00: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 
234441648
[   12.164000] ata3.00: configured for UDMA/100
[   12.164000] scsi3 : ahci
[   12.476000] ata4: SATA link down (SStatus 0 SControl 0)
[   12.476000] scsi 2:0:0:0: Direct-Access ATA  FUJITSU MHW2120B 0081 
PQ: 0 ANSI: 5
[   12.476000] ata_piix :00:1f.1: version 2.10ac1
[   12.476000] ACPI: PCI Interrupt :00:1f.1[A] -> GSI 18 (level, low) -> 
IRQ 18
[   12.476000] PCI: Setting latency timer of device :00:1f.1 to 64
[   12.476000] ata5: PATA max UDMA/133 cmd 0x000101f0 ctl 0x000103f6 bmdma 
0x000140b0 irq 14
[   12.476000] ata6: PATA max UDMA/133 cmd 0x00010170 ctl 0x00010376 bmdma 
0x000140b8 irq 15
[   12.476000] scsi4 : ata_piix
[   12.48] SCSI device sda: 234441648 512-byte hdwr sectors (120034 MB)

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


Re: libata-acpi: summary, problems, questions and proposal

2007-03-28 Thread Matthew Garrett
On Thu, Mar 29, 2007 at 10:42:03AM +0900, Tejun Heo wrote:

> Yeap, I think it's in the right direction but we need to go further.
> 
> * I'm not sure whether the complex walk libata-acpi is doing is justifiable.

Yeah, that may well be less than ideal.

> * You'll end up doing _STM/_GTM on ahci controller on some BIOSen - 1. 
> it can be dangerous 2. you might get partial or incorrect mapping - 
> think about ICH8-split-to-two-PCI-fn-in-piix-mode case.

So far I haven't seen any DSDTs that include _GTM and _STM methods for 
controllers that come up flagged as ahci, though I'm perfectly willing 
to believe that they're out there...

> I think PM functions in libata-eh is better place and you also need to 
> do _GTF after _GTM during resume.

Well, "need" - I haven't actually found hardware that seems upset about 
the missing _GTF call :) But you're right, it ought to be done. Can you 
point me at the right bits of the libata-eh code? I was trying to work 
through it earlier, but keeping track of the conditional paths is a bit 
tricky.

> >I think this is just a matter of making sure that the sata and pata 
> >handle matching code matches reality now :)
> 
> Currently 1/2 of libata-acpi code is dealing with the above.  I'm trying 
> to figure out why it needs to be that complex.

I wrote a patch at one point that simplified this to an extent 
(http://lkml.org/lkml/2005/12/7/425), but it got somewhat bogged down in 
discussion about where the right place to do the binding was. Personally 
I'd prefer to handle this in a similar way to PCI - that is, register 
the ata bus with ACPI and then find handles as and when new ata devices 
are added to that bus. This has the advantage that it's easy to tie ACPI 
events to specific ata devices, which could then be integrated with the 
bay and dock drivers.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: HPA patches

2007-03-28 Thread Matthew Garrett
On Wed, Mar 28, 2007 at 10:54:31PM +0100, Alan Cox wrote:

> I wonder if the firmware is dying when we ask the disk to go zero sized
> rather than erroring politely. I'm not sure hth HPA sectors can come back
> as zero but we can be fairly sure 0 means "no HPA" in this case I guess ?

No, it seems to be looking at 0 because ata_read_native_max_address_ext 
returns 0 in the error case - the error that ata_exec_internal generates 
seems to be AC_ERR_HSM. Since 0 isn't > the size reported, we'll never 
try to resize it anyway, judging by ata_hpa_resize - that is, it seems 
to be the ata_read_native_max_address_ext call that breaks it.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: HPA patches

2007-03-28 Thread Matthew Garrett
On Wed, Mar 28, 2007 at 10:57:54AM +0100, Alan Cox wrote:

> Ok thanks. This is interesting as the only thing new it is doing is
> asking for the HPA size. Does I think explain the problem however: Can
> you move the HPA setting call to after the mode has been set and see if
> that makes the problem vanish ?

Hm. I tried adding it in the eh code after ata_set_mode() in 
ata_eh_recover(), which alters the problem slightly - hpa_sectors is 
still 0, so the taskfile call is still failing, but now the system just 
stops at around the time that anything attempts to access sda with no 
errors other than

sd: 2:0:1:0: timing out command, waited 180s
sd: 2:0:1:0: SCSI error: return code = 0x0028
end_request: I/O error, dev sda, sector 0
Buffer I/O error on device sda, logical block 0

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: libata-acpi: summary, problems, questions and proposal

2007-03-28 Thread Matthew Garrett
On Wed, Mar 28, 2007 at 04:30:02PM +0900, Tejun Heo wrote:

Hi Tejun,

Firstly, could I ask you to take a look at the patch in 
http://permalink.gmane.org/gmane.linux.acpi.devel/22066/ ? It deals with 
some of these issues.

> ACPI support implementation in libata-dev supports both IDE and SATA
> ACPI object layouts and subset of ATA ACPI methods - _SDD and _GTF.
> It incorrectly uses ap->cbl (the port's cable type) to choose between
> the two ACPI layouts.  Association between the host and its ACPI
> object is performed every time ACPI methods are invoked but the
> association between an ATA device and its ACPI object is cached in
> ata_device object.

These issues are both fixed in my patch, I believe.

> 2-2. Missing proper _GTM/_STM support.  As stated above, although -mm
> contains _GTM/_STM support, it does not hook it to regular
> exception handling path and thus _GTF cannot be used in a lot of
> cases.

I've added _GTM and _STM support over suspend/resume. Right now they're 
in the host power management code - I'm not sure whether they should be 
here or the SCSI glue layer?

> 2-3. Misplaced _GTF hook.  _GTF currently is called prior to every
> device configuration.  This is unnecessary and incorrect.  The
> ACPI spec specifies that _GTM/_STM and _GTF should be executed
> during suspend/resume cycles not on every reset or
> reconfiguration.  This, for example, causes the following
> problem.

That should be quite easily fixable with the above patch.

> 4-1. Depending on how questions in section 3 are answered, fix and
> clean up ATA host/device <-> ACPI object association.  Whether
> IDE or SATA native style hierarchy is used should be determined
> by driver flag not cable type.  e.g. ahci and sata_sil24 should
> use SATA native style hierarchy while ata_piix should use IDE
> hierarchy whether the port is SATA or PATA.

I think this is just a matter of making sure that the sata and pata 
handle matching code matches reality now :)

> 4-2. Only associate once during initialization.  There is no reason to
> try to associate hosts and devices with ACPI objects at each try.
> Do it once during host initialization and use it if available or
> forget about ACPI if not available.

Fixed.

> 4-3. Integrate _GTM/_STM support and invoke methods only when the spec
> specifies to.  For IDE object, do _GTM during suspend and _STM
> followed by _GTF during resume.  There is no reason to call them
> anytime else.  For SATA native object, do _SDD followed by _GTF
> after every hardreset.

Patrially fixed.

> 4-4. Implement helpers for cable detection using _GTM/_STM and use it
> in sata_nv if CK804.  This is to substitute independent pata_acpi
> driver.  Low level driver should know when _GTM/_STM should be
> used for cable detection and/or device programming and doing it
> this way reduces user confusion (sata_nv also supports ck804 but
> you probably need to load pata_acpi if ACPI is available) and
> allows better integration with the rest of the low level driver
> (e.g. ADMA mode + _GTM/_STM cable detection).

Not done.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: ata_piix can't drive Mac hardware properly

2007-03-27 Thread Matthew Garrett
On Wed, Mar 28, 2007 at 02:16:08AM +0100, Matthew Garrett wrote:

> It's ata_read_native_max_address_ext failing, and it's fine if I use 
> ahci rather than ata_piix, so I'll just chalk this up to Apple's 
> firmware being broken (again) and putting the hardware into some sort of 
> "I can't believe it's not piix" mode and see what I can do about 
> ensuring we use ahci instead.

Slightly oddly, with ata_piix it seems to be failing with AC_ERR_HSM. 
Any idea why that might be?

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


ata_piix can't drive Mac hardware properly

2007-03-27 Thread Matthew Garrett
On Wed, Mar 28, 2007 at 01:16:10AM +0100, Matthew Garrett wrote:

> comment seems to be wrong (or, alternatively, it's the 
> ata_read_native_max_address_ext call that's failing and returning 
> garbage? I'll look into that)

It's ata_read_native_max_address_ext failing, and it's fine if I use 
ahci rather than ata_piix, so I'll just chalk this up to Apple's 
firmware being broken (again) and putting the hardware into some sort of 
"I can't believe it's not piix" mode and see what I can do about 
ensuring we use ahci instead.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: HPA patches

2007-03-27 Thread Matthew Garrett
On Wed, Mar 28, 2007 at 01:08:52AM +0100, Matthew Garrett wrote:
> ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0
^
Does this just indicate the lack of an hpa? If so, the

/* if no hpa, both should be equal */

comment seems to be wrong (or, alternatively, it's the 
ata_read_native_max_address_ext call that's failing and returning 
garbage? I'll look into that)

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: HPA patches

2007-03-27 Thread Matthew Garrett
On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote:
> For reference this is what I am currently using with 2.6.21-rc4-mm1 and
> it is working for all my test cases so far: Its basically Kyle's patch
> with a libata switch to turn it on/off and some minor fixups from
> the original patch as posted

Fails for me on a Macbook Pro with:

ata1: PATA max UDMA/133 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x000140b0 irq 14
ata2: PATA max UDMA/133 cmd 0x00010170 ctl 0x00010376 bmdma 0x000140b8 irq 15
scsi0: ata_piix
ata1.00: ATAPI, max UDMA/66
ata1.00: configured for UDMA/66
scsi1 : ata_piix
ATA: abornaml status 0x7f on port 0x00010177
scsi 0:0:0:0: CD-ROM MATSHITA DVD-R UJ-857D KCV9 PQ : 0 ANSI: 5
ata_piix :00:1f.2: MAP [ P0 p" XX XX ]
ata_piix :00:1f.2: invalid MAP value 0
ata3: SATA max UDMA/133 cmd 0x000140c8 ctl 0x000140e6 bmdma 0x000140a0 irq 20
ata4: SATA max UDMA/133 cmd 0x000140c0 ctl 0x000140e2 bmdma 0x000140a8 irq 20
scsi2 : ata_piix
ATA: abnormal status 0x7F on port 0x000140cf
ATA: abnormal status 0x7F on port 0x000140cf
ATA: abnormal status 0x7F on port 0x000140cf
ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0
ata3.01: ATA-8: FUJITSU MHW2120BH, 00810013, max UDMA/100
ata3.01: 234441648 sectors, multi 16: LBA48 NCQ (depth 0/32)
ata3.01: failed to set xfermode (err_mask=0x40)
ata3: failed to recover some devices, retrying in 5 secs
ATA: abnormal status 0x7F on port 0x000140cf
ATA: abnormal status 0x7F on port 0x000140cf
ATA: abnormal status 0x7F on port 0x000140cf
ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0
ata3.01: failed to set xfermode (err_mask=0x40)
ata3.01: limiting speed to UDMA/100:PIO3
ata3: failed to recover some devices, retrying in 5 secs
ATA: abnormal status 0x7F on port 0x000140cf
ATA: abnormal status 0x7F on port 0x000140cf
ATA: abnormal status 0x7F on port 0x000140cf
ata3.01: qc timeout (cmd 0x27)
ata3.01: ata_hpa_resize 1: sectors = 234441648, hpa_sectors = 0
ata3.01: failed to set xfermode (err_mask=0x40)
ata3.01: disabled
scsi3: ata_piix
ATA: abnormal status 0x7F on port 0x000140c7

and I end up with no root filesystem. Reverting the patch leaves things 
working. This is the ubuntu tree - I can try libata-dev if you think 
there's likely to be any relevant difference.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add _GTM and _STM support to libata

2007-03-26 Thread Matthew Garrett
>ports[i];
+   ata_acpi_stm(ap, ap->gtm);
+   }
+
ata_host_request_pm(host, PMSG_ON, ATA_EH_SOFTRESET,
ATA_EHI_NO_AUTOPSY | ATA_EHI_QUIET, 0);
host->dev->power.power_state = PMSG_ON;
@@ -5917,6 +5926,7 @@ int ata_device_add(const struct ata_probe_ent *ent)
struct ata_port *ap = host->ports[i];
 
ata_scsi_scan_host(ap);
+   ata_acpi_setup(ap);
}
 
VPRINTK("EXIT, returning %u\n", ent->n_ports);
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index c426714..eb41263 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -97,21 +97,6 @@ extern void ata_port_init(struct ata_port *ap, struct 
ata_host *host,
 extern struct ata_probe_ent *ata_probe_ent_alloc(struct device *dev,
 const struct ata_port_info 
*port);
 
-/* libata-acpi.c */
-#ifdef CONFIG_SATA_ACPI
-extern int ata_acpi_exec_tfs(struct ata_port *ap);
-extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
-#else
-static inline int ata_acpi_exec_tfs(struct ata_port *ap)
-{
-   return 0;
-}
-static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
-{
-   return 0;
-}
-#endif
-
 /* libata-scsi.c */
 extern struct scsi_transport_template ata_scsi_transport_template;
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e3f32f3..4774c08 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -590,6 +590,10 @@ struct ata_port {
void*private_data;
 
u8  sector_buf[ATA_SECT_SIZE]; /* owned by EH */
+   struct acpi_gtm *gtm;
+#ifdef CONFIG_SATA_ACPI
+   struct ata_acpi_port_link *acpi_port_link;
+#endif
 };
 
 struct ata_port_operations {

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add _GTM and _STM support to libata

2007-03-26 Thread Matthew Garrett
ata_acpi_setup(ap);
}
 
VPRINTK("EXIT, returning %u\n", ent->n_ports);
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index c426714..eb41263 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -97,21 +97,6 @@ extern void ata_port_init(struct ata_port *ap, struct 
ata_host *host,
 extern struct ata_probe_ent *ata_probe_ent_alloc(struct device *dev,
 const struct ata_port_info 
*port);
 
-/* libata-acpi.c */
-#ifdef CONFIG_SATA_ACPI
-extern int ata_acpi_exec_tfs(struct ata_port *ap);
-extern int ata_acpi_push_id(struct ata_port *ap, unsigned int ix);
-#else
-static inline int ata_acpi_exec_tfs(struct ata_port *ap)
-{
-   return 0;
-}
-static inline int ata_acpi_push_id(struct ata_port *ap, unsigned int ix)
-{
-   return 0;
-}
-#endif
-
 /* libata-scsi.c */
 extern struct scsi_transport_template ata_scsi_transport_template;
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index e3f32f3..4774c08 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -590,6 +590,10 @@ struct ata_port {
void*private_data;
 
u8  sector_buf[ATA_SECT_SIZE]; /* owned by EH */
+   struct acpi_gtm *gtm;
+#ifdef CONFIG_SATA_ACPI
+   struct ata_acpi_port_link *acpi_port_link;
+#endif
 };
 
 struct ata_port_operations {

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add _GTM and _STM support to libata

2007-03-26 Thread Matthew Garrett
On Mon, Mar 26, 2007 at 11:58:07AM +0100, Alan Cox wrote:
> On Mon, 26 Mar 2007 04:22:23 +0100
> Matthew Garrett <[EMAIL PROTECTED]> wrote:
> 
> > I've ported the drivers/ide code for handling _GTM and _STM to libata. 
> > I'm not utterly convinced that I'm doing the calls in the right place - 
> 
> Please see the various patches I've posted or the -mm tree. I did that
> some time ago and the pata_acpi driver makes full use of them already.

Ok, missed that. How about something like the following? I've changed 
some of the interfaces slightly, so the pata_acpi driver would need to 
be altered to match. This gets rid of the CABLE_SATA checking in 
libata-acpi.c, stores the handle in the ata_port structure so we can 
delete a load of code that keeps regenerating it, and ensures that the 
acpi code is setup up at device creation time. Finally, it hooks the 
calls into the core libata code.

Any comments?

Signed-off-by: Matthew Garrett <[EMAIL PROTECTED]>

--- 
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 0bd4789..99e534c 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -291,8 +291,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,
unsigned long *obj_loc)
 {
acpi_status status;
-   acpi_handle dev_handle = NULL;
-   acpi_handle chan_handle, drive_handle;
+   acpi_handle drive_handle;
acpi_integerpcidevfn = 0;
u32 dev_adr;
struct acpi_buffer  output;
@@ -321,47 +320,12 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,
goto out;
}
 
-   /* Don't continue if device has no _ADR method.
-* _GTF is intended for known motherboard devices. */
-   if (!(ap->cbl == ATA_CBL_SATA)) {
-   err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
-   if (err < 0) {
-   if (ata_msg_probe(ap))
-   ata_dev_printk(atadev, KERN_DEBUG,
-   "%s: pata_get_dev_handle failed (%d)\n",
-   __FUNCTION__, err);
-   goto out;
-   }
-   } else {
-   err = sata_get_dev_handle(dev, &dev_handle, &pcidevfn);
-   if (err < 0) {
-   if (ata_msg_probe(ap))
-   ata_dev_printk(atadev, KERN_DEBUG,
-   "%s: sata_get_dev_handle failed (%d\n",
-   __FUNCTION__, err);
-   goto out;
-   }
-   }
-
/* Get this drive's _ADR info. if not already known. */
if (!atadev->obj_handle) {
-   if (!(ap->cbl == ATA_CBL_SATA)) {
-   /* get child objects of dev_handle == channel objects,
-* + _their_ children == drive objects */
-   /* channel is ap->port_no */
-   chan_handle = acpi_get_child(dev_handle,
-   ap->port_no);
-   if (ata_msg_probe(ap))
-   ata_dev_printk(atadev, KERN_DEBUG,
-   "%s: chan adr=%d: chan_handle=0x%p\n",
-   __FUNCTION__, ap->port_no,
-   chan_handle);
-   if (!chan_handle) {
-   err = -ENODEV;
-   goto out;
-   }
+   if (ap->acpi_port_link->is_pata) {
/* TBD: could also check ACPI object VALID bits */
-   drive_handle = acpi_get_child(chan_handle, ix);
+   drive_handle = 
+   acpi_get_child(ap->acpi_port_link->handle, ix);
if (!drive_handle) {
err = -ENODEV;
goto out;
@@ -370,7 +334,7 @@ static int do_drive_get_GTF(struct ata_port *ap, int ix,
atadev->obj_handle = drive_handle;
} else {/* for SATA mode */
dev_adr = SATA_ADR_RSVD;
-   err = get_sata_adr(dev, dev_handle, pcidevfn, 0,
+   err = get_sata_adr(dev, ap->acpi_port_link->handle, 
pcidevfn, 0,
ap, atadev, &dev_adr);
}
if (err < 0 || dev_adr == SATA_ADR_RSVD ||
@@ -531,7 +495,7 @@ static int do_drive_set_taskfiles(struct ata_port *ap,
ata_dev_printk(atadev,

Re: [PATCH] Add _GTM and _STM support to libata

2007-03-25 Thread Matthew Garrett
> +printk(KERN_ERR
> +"%s: unexpected _GTM length (0x%x)[should be 0x%zx] 
> or \
> +"
> +"addr (0x%p)\n",
> +__FUNCTION__, out_obj->buffer.length,
> +sizeof(struct GTM_buffer), out_obj->buffer.pointer);

Yeah, erm.

> + * _STM requires Identify Drive data, which has to passed as an argument.
> + * Unfortunately hd_driveid is a mangled version which we can't readily
> + * use; hence we'll get the information afresh.

And clearly this doesn't actually match reality either. I'll send a 
tidied up patch when I've actually slept.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Add _GTM and _STM support to libata

2007-03-25 Thread Matthew Garrett
I've ported the drivers/ide code for handling _GTM and _STM to libata. 
I'm not utterly convinced that I'm doing the calls in the right place - 
should they be attached to the scsi code rather than the ata host code? 
Anyway, this fixes suspend/resume on nc6220, which is what I was aiming 
for...

Signed-off-by: Matthew Garrett <[EMAIL PROTECTED]>

--- 

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index c428a56..0e542ea 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -34,6 +34,19 @@ struct taskfile_array {
u8  tfa[REGS_PER_GTF];  /* regs. 0x1f1 - 0x1f7 */
 };
 
+struct GTM_buffer {
+u32 PIO_speed0;
+u32 DMA_speed0;
+u32 PIO_speed1;
+u32 DMA_speed1;
+u32 GTM_flags;
+};
+
+struct ata_acpi_port_link {
+acpi_handle  obj_handle;
+struct GTM_buffergtm;
+};
+
 /*
  * Helper - belongs in the PCI layer somewhere eventually
  */
@@ -706,4 +719,125 @@ out:
return 0;
 }
 
+/**
+ * ata_acpi_get_timings - get the channel (controller) timings
+ * @hwif: target IDE interface (channel)
+ *
+ * This function executes the _GTM ACPI method for the target channel.
+ *
+ */
+void ata_acpi_get_timings(struct ata_port *ap)
+{
+acpi_status status;
+   int err;
+acpi_handle dev_handle = NULL;
+acpi_integerpcidevfn = 0;
+   struct device   *dev = ap->host->dev;
+struct acpi_buffer  output;
+union acpi_object   *out_obj;
+
+if (noacpi)
+   return;
+
+   if (ap->cbl == ATA_CBL_SATA)
+   return;
 
+   err = pata_get_dev_handle(dev, &dev_handle, &pcidevfn);
+
+   if (err < 0)
+   return;
+
+   ap->port_handle = acpi_get_child(dev_handle, ap->port_no);
+
+/* Setting up output buffer for _GTM */
+output.length = ACPI_ALLOCATE_BUFFER;
+output.pointer = NULL;  /* ACPI-CA sets this; save/free it later */
+
+/* _GTM has no input parameters */
+status = acpi_evaluate_object(ap->port_handle, "_GTM",
+  NULL, &output);
+
+if (ACPI_FAILURE(status))
+ return;
+
+if (!output.length || !output.pointer) {
+kfree(output.pointer);
+return;
+}
+
+out_obj = output.pointer;
+if (out_obj->type != ACPI_TYPE_BUFFER) {
+kfree(output.pointer);
+return;
+}
+
+if (!out_obj->buffer.length || !out_obj->buffer.pointer ||
+out_obj->buffer.length != sizeof(struct GTM_buffer)) {
+kfree(output.pointer);
+printk(KERN_ERR
+"%s: unexpected _GTM length (0x%x)[should be 0x%zx] or 
\
+"
+  "addr (0x%p)\n",
+  __FUNCTION__, out_obj->buffer.length,
+  sizeof(struct GTM_buffer), out_obj->buffer.pointer);
+return;
+}
+
+   if (!ap->acpidata) {
+   ap->acpidata = kzalloc(sizeof(struct ata_acpi_port_link), 
GFP_KERNEL);
+   if (!ap->acpidata)
+   return;
+   }
+
+memcpy(&ap->acpidata->gtm, out_obj->buffer.pointer,
+   sizeof(struct GTM_buffer));
+
+kfree(output.pointer);
+}
+EXPORT_SYMBOL_GPL(ata_acpi_get_timings);
+
+/**
+ * ata_acpi_push_timings - set the channel (controller) timings
+ * @hwif: target IDE interface (channel)
+ *
+ * This function executes the _STM ACPI method for the target channel.
+ *
+ * _STM requires Identify Drive data, which has to passed as an argument.
+ * Unfortunately hd_driveid is a mangled version which we can't readily
+ * use; hence we'll get the information afresh.
+ */
+void ata_acpi_push_timings(struct ata_port *ap)
+{
+acpi_status status;
+struct acpi_object_list input;
+union acpi_object   in_params[3];
+
+if (noacpi)
+return;
+
+if (!ap->acpidata)
+return;
+
+/* Give the GTM buffer + drive Identify data to the channel via the
+ * _STM method: */
+/* setup input parameters buffer for _STM */
+input.count = 3;
+input.pointer = in_params;
+in_params[0].type = ACPI_TYPE_BUFFER;
+in_params[0].buffer.length = sizeof(struct GTM_buffer);
+in_params[0].buffer.pointer = (u8 *)&ap->acpidata->gtm;
+in_params[1].type = ACPI_TYPE_BUFFER;
+in_params[1].buffer.length = sizeof(ap->device[0].id[0]) * 
ATA_ID_WORDS;
+in_params[1].buffer.pointer = (u8 *)&ap->device[0].id;
+in_par

Re: HPA patches

2007-03-23 Thread Matthew Garrett
On Fri, Mar 23, 2007 at 07:13:21PM +, Alan Cox wrote:
> +static int ata_ignore_hpa = 0;
> +module_param_named(ignore_hpa, ata_ignore_hpa, int, 0644);
> +MODULE_PARM_DESC(ignore_hpa, "Ignore HPA (0=off 1=on)");

I'm not sure I like the language here. "Ignore HPA" appears to mean 
"Explicitly disable the HPA", which I guess is one interpretation of 
"ignore" - however, naively I'd expect "Ignore HPA" to mean "Don't touch 
the HPA" with the result that it would remain inaccessible to userspace.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFT] libata hpa support

2007-03-21 Thread Matthew Garrett
On Wed, Mar 21, 2007 at 06:51:59PM +, Alan Cox wrote:
> > https://launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/91940 
> > gives the traces we have.
> 
> Wrong bug number ? - seems to be nothing there that looks HPA related at
> all

That's with Kyle's HPA code - reverting it results in that failure 
vanishing.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFT] libata hpa support

2007-03-21 Thread Matthew Garrett
On Wed, Mar 21, 2007 at 06:34:55PM +, Alan Cox wrote:

> I'm testing a variant of it at the moment with no problems on x86. My
> first thought would be to check for endianness problems and the like but
> I don't see any.

Macbooks are x86.

> It may also fail to allow any resizing if the Macbook issues a security
> freeze from the firmware before the OS boots, if so we won't be able to
> resize the HPA or set/change passwords on the disk. If they are doing
> that then its a fairly sensible security choice.

https://launchpad.net/ubuntu/+source/linux-source-2.6.20/+bug/91940 
gives the traces we have.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: CONFIG_IBM_BAY

2007-03-19 Thread Matthew Garrett
On Mon, Mar 19, 2007 at 02:48:22PM +0100, Holger Macht wrote:

>   1. libata integration into the bay driver

It actually makes sense to tie all PATA/SATA devices to their ACPI 
handles where appropriate - there's already code to do that in 
libata-acpi.c, but it would be nicer if it was integrated in the same 
way that PCI devices are.

>   2. The dock station driver has to inform the bay driver that an undock
>  event took place, right?

Yeah.

> But you still have to deal with mounted filesystems, no matter if it a
> cardbus or a cdrom. Wouldn't we need something like 'save removal'
> triggered from userspace like you maybe know from 'the other' operating
> system?

Yes, there's a need for a mechanism to deal with all of this safely, but 
the same is true of any storage device that can be hotplugged (USB, 
firewire, anything in a hotplug bay...)

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: CONFIG_IBM_BAY

2007-03-19 Thread Matthew Garrett
On Sun, Mar 18, 2007 at 09:27:51AM +0100, Holger Macht wrote:

> I don't prefer any solution, whether doing it inside the kernel, or doing
> it in userspace. What would be good would be to know what's the 'right'
> way to go, or at least what both kernel people and userspace people can
> agree on so that we can find a solution across distributions, whatever.
> I'm currently just looking how to integrate the generic dock and bay
> driver into the openSUSE distribution, and this seems to be quite hard,
> especially because of the above mentioned already working solution ;-)

If the kernel knows that a bay device has just been added or removed, it 
makes sense for the device removal to take place in the kernel rather 
than bouncing it out to userspace and then back into the kernel. Pulling 
out a cardbus card doesn't require us to run a userspace helper to 
detach the hardware.

-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: SATA status report updated

2005-08-12 Thread Matthew Garrett
Jeff Garzik <[EMAIL PROTECTED]> wrote:
> 
> Things in SATA-land have been moving along recently, so I updated the 
> software status report:
> 
>   http://linux.yyz.us/sata/software-status.html

I couldn't see any reference to system-wide power management (ie,
suspend/resume of machines with SATA interfaces) - is any work going on
in that area at the moment?

Thanks,
-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


libata suspend/resume

2005-07-28 Thread Matthew Garrett
Hi,

An increasing number of laptop vendors seem to be including sata 
controllers now, and so we're looking at shipping the libata 
suspend/resume patch. The discussion about suspend in the scsi layer 
doesn't seem to have ended very conclusively, and I haven't been able to 
find any sign of that code ending up anywhere. What's the current 
status with this?

Thanks,
-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Host protected area on suspend/resume

2005-07-12 Thread Matthew Garrett
On Tue, Jul 12, 2005 at 11:49:35PM +0900, Tejun Heo wrote:

>  This has come up several times now.  One thing I'm curious about is 
> why we are disabling HPA on boot without consent from the user.  AFAIK, 
> HPA is mostly used to implement hidden recovery/suspend storage areas 
> and disabling automatically on boot increases the likeliness of 
> destroying them.  What do we gain by disabling HPA on boot?  Are there 
> some dumb machines which unnecessarily sets HPA and reduces the capacity 
> of drives excessively?  Even in such cases, wouldn't it be better to do 
> idedisk_check_hpa() only when kernel parameter explicitly says so?

I'm not sure why we're doing it, but reverting this behaviour is likely 
to make some systems unbootable (install with kernel which disables HPA, 
format entire drive, put filesystem on it).
-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Host protected area on suspend/resume

2005-07-12 Thread Matthew Garrett
On boot, Linux will attempt to disable the host protected area on a 
disk. After a suspend/resume cycle, the BIOS may reenable it (seen on a 
Thinkpad T40 and R40). As a result, the kernel is now unable to access 
the HPA.

Is there any issue with just adding a call to idedisk_check_hpa() in the 
IDE resume code?
-- 
Matthew Garrett | [EMAIL PROTECTED]
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html