linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* RFC: ACPI/scsi/libata integration and hotswap
@ 2005-12-08  3:02 Matthew Garrett
  2005-12-08  9:15 ` Christoph Hellwig
  2005-12-13 18:14 ` Randy Dunlap
  0 siblings, 2 replies; 64+ messages in thread
From: Matthew Garrett @ 2005-12-08  3:02 UTC (permalink / raw)
  To: randy_d_dunlap, linux-ide, linux-scsi, linux-kernel, acpi-devel

Hi!

The included patch does three things:

1) It adds basic support for binding SCSI and SATA devices to ACPI 
device handles. At the moment this is limited to hosts, and in practice 
it's probably limited to SATA ones (ACPI doesn't spec how SCSI devices 
should appear in the DSDT, so I'm guessing that in general they don't). 
Given a host, you can DEVICE_ACPI_HANDLE(dev) it to get the handle to 
the ACPI device - this should be handy for implementing suspend 
functions, since the methods should be in a standard location underneath 
this.

Support for binding the devices hasn't been implemented yet. I'm not 
entire sure where they should be bound (the target, presumably?), and I 
haven't actually got it to work...

2) It adds support for attaching notification events to the host. These 
can be host-driver specific (they're just part of the host template). 
Whenever the hardware generates an event on that bus, the host will be 
called. I've added one of these to ata_piix (since that's what I have), 
which should really be implemented in the host and only call the generic 
one when appropriate. But still.

3) Adds a generic libata notification handler that currently treats all 
notifications as drive removal/insertion. It then calls Lukasz 
Kosewski's hotplug code to remove/add the drive as appropriate.

Most laptops generate ACPI notifications requesting bus rescans whenever 
a bay drive is inserted or removed. This handles the event 
appropriately. On my Dell d610, removing or plugging the drive results 
in it appearing or disappearing from /proc/scsi/scsi as appropriate.

Patch:

diff -u drivers/scsi/ata_piix.c /tmp/drivers/scsi/ata_piix.c
--- drivers/scsi/ata_piix.c	2005-12-05 14:30:58 +0000
+++ /tmp/drivers/scsi/ata_piix.c	2005-12-08 02:16:59 +0000
@@ -148,6 +148,7 @@
 	.ordered_flush		= 1,
 	.resume			= ata_scsi_device_resume,
 	.suspend		= ata_scsi_device_suspend,
+	.acpi_notify		= ata_acpi_notify,
 };
 
 static const struct ata_port_operations piix_pata_ops = {
diff -u drivers/scsi/libata-core.c /tmp/drivers/scsi/libata-core.c
--- drivers/scsi/libata-core.c	2005-12-08 02:27:02 +0000
+++ /tmp/drivers/scsi/libata-core.c	2005-12-08 02:16:50 +0000
@@ -50,6 +50,7 @@
 #include <linux/workqueue.h>
 #include <linux/jiffies.h>
 #include <linux/scatterlist.h>
+#include <linux/acpi.h>
 #include <scsi/scsi.h>
 #include "scsi_priv.h"
 #include <scsi/scsi_cmnd.h>
@@ -75,9 +76,11 @@
 				unsigned int *xfer_shift_out);
 static void __ata_qc_complete(struct ata_queued_cmd *qc);
 static void ata_pio_error(struct ata_port *ap);
+static int ata_bus_probe(struct ata_port *ap);
 
 static unsigned int ata_unique_id = 1;
 static struct workqueue_struct *ata_wq;
+static struct workqueue_struct *ata_hotplug_wq;
 
 int atapi_enabled = 1;
 module_param(atapi_enabled, int, 0444);
@@ -88,6 +91,17 @@
 MODULE_LICENSE("GPL");
 MODULE_VERSION(DRV_VERSION);
 
+void ata_acpi_notify(acpi_handle device, u32 type, void *data) {
+	struct device *dev = acpi_get_physical_device(device);
+	struct Scsi_Host *shost =  dev_to_shost(dev);
+	struct ata_port *ap = (struct ata_port *) &shost->hostdata[0];
+	
+	if (!ata_bus_probe(ap))
+		ata_hotplug_plug(ap);
+	else
+		ata_hotplug_unplug(ap);
+}
+
 /**
  *	ata_tf_load_pio - send taskfile registers to host controller
  *	@ap: Port to which output is sent
diff -u drivers/scsi/scsi.c /tmp/drivers/scsi/scsi.c
--- drivers/scsi/scsi.c	2005-12-04 16:48:07 +0000
+++ /tmp/drivers/scsi/scsi.c	2005-12-08 02:16:28 +0000
@@ -55,6 +55,7 @@
 #include <linux/interrupt.h>
 #include <linux/notifier.h>
 #include <linux/cpu.h>
+#include <linux/acpi.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -1305,6 +1306,48 @@
 #define unregister_scsi_cpu()
 #endif /* CONFIG_HOTPLUG_CPU */
 
+#ifdef CONFIG_ACPI
+static int scsi_acpi_find_device(struct device *dev, acpi_handle *handle)
+{
+	return -ENODEV;
+}
+
+void ata_acpi_notify(acpi_handle handle, u32 type, void *data);
+
+static int scsi_acpi_find_channel(struct device *dev, acpi_handle *handle)
+{
+	int i;
+	struct Scsi_Host *shost;
+	
+	if (sscanf(dev->bus_id, "host%u", &i) != 1) {
+		return -ENODEV;
+	}
+
+	*handle = acpi_get_child(DEVICE_ACPI_HANDLE(dev->parent), (acpi_integer) i);	
+	if (!*handle)
+		return -ENODEV;
+
+	shost = dev_to_shost(dev);
+	
+	if (shost->hostt->acpi_notify) { 
+		acpi_install_notify_handler(*handle, ACPI_ALL_NOTIFY, &ata_acpi_notify, (void *)i);
+	}
+	
+	return 0;
+}
+
+static struct acpi_bus_type scsi_acpi_bus = {
+	.bus = &scsi_bus_type,
+	.find_bridge = scsi_acpi_find_channel,
+	.find_device = scsi_acpi_find_device,
+};
+
+static __init int scsi_acpi_register(void)
+{
+	return register_acpi_bus_type(&scsi_acpi_bus);
+}
+#endif /* CONFIG_ACPI */
+
 MODULE_DESCRIPTION("SCSI core");
 MODULE_LICENSE("GPL");
 
@@ -1333,6 +1376,11 @@
 	error = scsi_sysfs_register();
 	if (error)
 		goto cleanup_sysctl;
+#ifdef CONFIG_ACPI
+	error = scsi_acpi_register();
+	if (error)
+		goto cleanup_sysfs;
+#endif
 
 	for (i = 0; i < NR_CPUS; i++)
 		INIT_LIST_HEAD(&per_cpu(scsi_done_q, i));
@@ -1343,6 +1391,8 @@
 	printk(KERN_NOTICE "SCSI subsystem initialized\n");
 	return 0;
 
+cleanup_sysfs:
+	scsi_sysfs_unregister();
 cleanup_sysctl:
 	scsi_exit_sysctl();
 cleanup_hosts:
--- include/linux/libata.h	2005-12-05 14:32:50 +0000
+++ /tmp/include/linux/libata.h	2005-12-08 02:35:59 +0000
@@ -453,7 +468,9 @@
 extern int ata_scsi_device_suspend(struct scsi_device *);
 extern int ata_device_resume(struct ata_port *, struct ata_device *);
 extern int ata_device_suspend(struct ata_port *, struct ata_device *);
-
+#ifdef CONFIG_ACPI
+extern void ata_acpi_notify(acpi_handle device, u32 type, void *data);
+#endif
 /*
  * Default driver ops implementations
  */
diff -u include/linux/libata.h /tmp/include/linux/libata.h
--- include/linux/libata.h	2005-12-05 14:32:50 +0000
+++ /tmp/include/linux/libata.h	2005-12-08 02:10:03 +0000
@@ -448,12 +463,16 @@
 extern int ata_scsi_error(struct Scsi_Host *host);
 extern int ata_scsi_release(struct Scsi_Host *host);
 extern unsigned int ata_host_intr(struct ata_port *ap, struct ata_queued_cmd *qc);
+extern void ata_hotplug_unplug(struct ata_port *ap);
+extern void ata_hotplug_plug(struct ata_port *ap);
 extern int ata_ratelimit(void);
 extern int ata_scsi_device_resume(struct scsi_device *);
 extern int ata_scsi_device_suspend(struct scsi_device *);
 extern int ata_device_resume(struct ata_port *, struct ata_device *);
 extern int ata_device_suspend(struct ata_port *, struct ata_device *);
-
+#ifdef CONFIG_ACPI
+extern void ata_acpi_notify(acpi_handle device, u32 type, void *data);
+#endif
 /*
  * Default driver ops implementations
  */
diff -u include/scsi/scsi_host.h /tmp/include/scsi/scsi_host.h
--- include/scsi/scsi_host.h	2005-11-14 14:57:13 +0000
+++ /tmp/include/scsi/scsi_host.h	2005-12-08 02:12:14 +0000
@@ -5,6 +5,7 @@
 #include <linux/list.h>
 #include <linux/types.h>
 #include <linux/workqueue.h>
+#include <linux/acpi.h>
 
 struct block_device;
 struct completion;
@@ -300,6 +301,13 @@
 	 */
 	int (*resume)(struct scsi_device *);
 	int (*suspend)(struct scsi_device *);
+	
+#ifdef CONFIG_ACPI
+	/*
+	 * ACPI notification handler
+	 */
+	void (*acpi_notify)(acpi_handle device, u32 type, void *data); 
+#endif
 
 	/*
 	 * Name of proc directory
-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08  3:02 RFC: ACPI/scsi/libata integration and hotswap Matthew Garrett
@ 2005-12-08  9:15 ` Christoph Hellwig
  2005-12-08 13:26   ` Matthew Garrett
  2005-12-13 18:14 ` Randy Dunlap
  1 sibling, 1 reply; 64+ messages in thread
From: Christoph Hellwig @ 2005-12-08  9:15 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: randy_d_dunlap, linux-ide, linux-scsi, linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 03:02:42AM +0000, Matthew Garrett wrote:
> Hi!
> 
> The included patch does three things:
> 
> 1) It adds basic support for binding SCSI and SATA devices to ACPI 
> device handles. At the moment this is limited to hosts, and in practice 
> it's probably limited to SATA ones (ACPI doesn't spec how SCSI devices 
> should appear in the DSDT, so I'm guessing that in general they don't). 
> Given a host, you can DEVICE_ACPI_HANDLE(dev) it to get the handle to 
> the ACPI device - this should be handy for implementing suspend 
> functions, since the methods should be in a standard location underneath 
> this.


NACK.  ACPI-specific hacks do not have any business at all in the scsi layer.


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08  9:15 ` Christoph Hellwig
@ 2005-12-08 13:26   ` Matthew Garrett
  2005-12-08 13:33     ` Christoph Hellwig
  0 siblings, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-08 13:26 UTC (permalink / raw)
  To: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 09:15:42AM +0000, Christoph Hellwig wrote:

> NACK.  ACPI-specific hacks do not have any business at all in the scsi layer.

Ok. What's the right layer to do this? The current ACPI/anything else 
glue depends on specific knowledge about the bus concerned, and needs 
callbacks registered before devices are added to that bus. Doing it in 
the sata layer would have the potential for unhappiness on mixed 
sata/scsi machines.

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

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 13:26   ` Matthew Garrett
@ 2005-12-08 13:33     ` Christoph Hellwig
  2005-12-08 13:39       ` Matthew Garrett
  0 siblings, 1 reply; 64+ messages in thread
From: Christoph Hellwig @ 2005-12-08 13:33 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 01:26:57PM +0000, Matthew Garrett wrote:
> On Thu, Dec 08, 2005 at 09:15:42AM +0000, Christoph Hellwig wrote:
> 
> > NACK.  ACPI-specific hacks do not have any business at all in the scsi layer.
> 
> Ok. What's the right layer to do this? The current ACPI/anything else 
> glue depends on specific knowledge about the bus concerned, and needs 
> callbacks registered before devices are added to that bus. Doing it in 
> the sata layer would have the potential for unhappiness on mixed 
> sata/scsi machines.

Don't do it at all.  We don't need to fuck up every layer and driver for
intels braindamage.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 13:33     ` Christoph Hellwig
@ 2005-12-08 13:39       ` Matthew Garrett
  2005-12-08 13:44         ` Christoph Hellwig
                           ` (2 more replies)
  0 siblings, 3 replies; 64+ messages in thread
From: Matthew Garrett @ 2005-12-08 13:39 UTC (permalink / raw)
  To: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 01:33:08PM +0000, Christoph Hellwig wrote:

> Don't do it at all.  We don't need to fuck up every layer and driver for
> intels braindamage.

Doing SATA suspend/resume properly on x86 depends on knowing the ACPI 
object that corresponds to a host or target. It's also the only way to 
support hotswap on this hardware[1], since there's no way for userspace 
to know which device a notification refers to.

[1] ie, most laptops sold nowadays
-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 13:39       ` Matthew Garrett
@ 2005-12-08 13:44         ` Christoph Hellwig
  2005-12-08 17:18           ` Erik Slagter
  2005-12-08 13:52         ` Jeff Garzik
  2005-12-08 14:01         ` Alan Cox
  2 siblings, 1 reply; 64+ messages in thread
From: Christoph Hellwig @ 2005-12-08 13:44 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 01:39:45PM +0000, Matthew Garrett wrote:
> On Thu, Dec 08, 2005 at 01:33:08PM +0000, Christoph Hellwig wrote:
> 
> > Don't do it at all.  We don't need to fuck up every layer and driver for
> > intels braindamage.
> 
> Doing SATA suspend/resume properly on x86 depends on knowing the ACPI 
> object that corresponds to a host or target. It's also the only way to 
> support hotswap on this hardware[1], since there's no way for userspace 
> to know which device a notification refers to.

Well, bad luck for people buying such broken hardware.  Maybe you can trick
Jeff into adding junk like that to libata, but it surely doesn't have any
business in the scsi layer.

Why oh why do our chipset friends at intel have to fuck up everything they
can?  I wish they'd learn a lesson or two from their cpu collegues.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 13:39       ` Matthew Garrett
  2005-12-08 13:44         ` Christoph Hellwig
@ 2005-12-08 13:52         ` Jeff Garzik
  2005-12-08 14:07           ` [ACPI] " Alan Cox
  2005-12-08 14:12           ` Matthew Garrett
  2005-12-08 14:01         ` Alan Cox
  2 siblings, 2 replies; 64+ messages in thread
From: Jeff Garzik @ 2005-12-08 13:52 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 01:39:45PM +0000, Matthew Garrett wrote:
> On Thu, Dec 08, 2005 at 01:33:08PM +0000, Christoph Hellwig wrote:
> 
> > Don't do it at all.  We don't need to fuck up every layer and driver for
> > intels braindamage.
> 
> Doing SATA suspend/resume properly on x86 depends on knowing the ACPI 
> object that corresponds to a host or target.

Not true.


> It's also the only way to 
> support hotswap on this hardware[1],

Not true.

	Jeff




^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 13:39       ` Matthew Garrett
  2005-12-08 13:44         ` Christoph Hellwig
  2005-12-08 13:52         ` Jeff Garzik
@ 2005-12-08 14:01         ` Alan Cox
  2005-12-08 14:18           ` Matthew Garrett
  2 siblings, 1 reply; 64+ messages in thread
From: Alan Cox @ 2005-12-08 14:01 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Iau, 2005-12-08 at 13:39 +0000, Matthew Garrett wrote:
> On Thu, Dec 08, 2005 at 01:33:08PM +0000, Christoph Hellwig wrote:
> 
> > Don't do it at all.  We don't need to fuck up every layer and driver for
> > intels braindamage.
> 
> Doing SATA suspend/resume properly on x86 depends on knowing the ACPI 
> object that corresponds to a host or target. It's also the only way to 
> support hotswap on this hardware[1], since there's no way for userspace 
> to know which device a notification refers to.
> 
> [1] ie, most laptops sold nowadays

Actually "most PC systems"

Nevertheless Christoph has a point even if its hidden behind a George
Bush approach to diplomacy. The scsi core directly shouldn't need to
know about ACPI or other arch specific PM systems. 

Something like  "pci_to_acpi(struct pcidev *)" belongs in arch specific
code even if we do add a generic "void * pm_device" type pointer to
struct pci_dev or struct device for such a purpose.

Alan


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [ACPI] Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 13:52         ` Jeff Garzik
@ 2005-12-08 14:07           ` Alan Cox
  2005-12-08 14:14             ` Jeff Garzik
  2005-12-08 14:12           ` Matthew Garrett
  1 sibling, 1 reply; 64+ messages in thread
From: Alan Cox @ 2005-12-08 14:07 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Matthew Garrett, Christoph Hellwig, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Iau, 2005-12-08 at 08:52 -0500, Jeff Garzik wrote:
> On Thu, Dec 08, 2005 at 01:39:45PM +0000, Matthew Garrett wrote:
> > On Thu, Dec 08, 2005 at 01:33:08PM +0000, Christoph Hellwig wrote:
> > 
> > > Don't do it at all.  We don't need to fuck up every layer and driver for
> > > intels braindamage.
> > 
> > Doing SATA suspend/resume properly on x86 depends on knowing the ACPI 
> > object that corresponds to a host or target.
> 
> Not true.


Actually he is right. You have to know the ACPI object in order to run
the _GTM/_STM etc functions. If you don't run those your suspend/resume
may not work, may corrupt and so on. The only safe alternative is to
disable acpi which, while it would have been a good idea before the spec
ever got out, is a bit late now.

If you don't run the resume methods your disk subsystem status after a
resume is simply undefined and unsafe.

Alan


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 13:52         ` Jeff Garzik
  2005-12-08 14:07           ` [ACPI] " Alan Cox
@ 2005-12-08 14:12           ` Matthew Garrett
  1 sibling, 0 replies; 64+ messages in thread
From: Matthew Garrett @ 2005-12-08 14:12 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 08:52:25AM -0500, Jeff Garzik wrote:
> On Thu, Dec 08, 2005 at 01:39:45PM +0000, Matthew Garrett wrote:
> > Doing SATA suspend/resume properly on x86 depends on knowing the ACPI 
> > object that corresponds to a host or target.
> 
> Not true.

Well, where "properly" means "conforming to the ACPI spec". If _GTF is 
there, it's meant to be called. The _GTF buffer contents can potentially 
vary depending on BIOS settings, so there's no way for Linux to know 
what the correct commands to send are. And since _GTF responses can also 
depend on the information passed to _SDD, it's necessary to support that 
as well.

> > It's also the only way to 
> > support hotswap on this hardware[1],
> 
> Not true.

I was under the impression that ICH5 and ICH6 in non-AHCI mode didn't 
generate any sort of hotswap interrupt. This gets around that.

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

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [ACPI] Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 14:07           ` [ACPI] " Alan Cox
@ 2005-12-08 14:14             ` Jeff Garzik
  2005-12-08 14:30               ` Alan Cox
  0 siblings, 1 reply; 64+ messages in thread
From: Jeff Garzik @ 2005-12-08 14:14 UTC (permalink / raw)
  To: Alan Cox
  Cc: Matthew Garrett, Christoph Hellwig, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

Alan Cox wrote:
> On Iau, 2005-12-08 at 08:52 -0500, Jeff Garzik wrote:
> 
>>On Thu, Dec 08, 2005 at 01:39:45PM +0000, Matthew Garrett wrote:
>>
>>>On Thu, Dec 08, 2005 at 01:33:08PM +0000, Christoph Hellwig wrote:
>>>
>>>
>>>>Don't do it at all.  We don't need to fuck up every layer and driver for
>>>>intels braindamage.
>>>
>>>Doing SATA suspend/resume properly on x86 depends on knowing the ACPI 
>>>object that corresponds to a host or target.
>>
>>Not true.
> 
> 
> 
> Actually he is right. You have to know the ACPI object in order to run
> the _GTM/_STM etc functions. If you don't run those your suspend/resume

These are only for PATA.  We don't care about _GTM/_STM on SATA.

Further, SATA completely resets and re-initializes the device as if from 
a hardware reset (except on ata_piix, which doesn't support COMRESET, 
and PATA).  This makes _GTF uninteresting, as well.


> may not work, may corrupt and so on. The only safe alternative is to
> disable acpi which, while it would have been a good idea before the spec
> ever got out, is a bit late now.

suspend/resume works just fine with Jens' out-of-tree patch.


> If you don't run the resume methods your disk subsystem status after a
> resume is simply undefined and unsafe.

I initialize the hardware to a defined state.

	Jeff



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 14:01         ` Alan Cox
@ 2005-12-08 14:18           ` Matthew Garrett
  2005-12-08 14:33             ` Alan Cox
  0 siblings, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-08 14:18 UTC (permalink / raw)
  To: Alan Cox
  Cc: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 02:01:38PM +0000, Alan Cox wrote:

> Something like  "pci_to_acpi(struct pcidev *)" belongs in arch specific
> code even if we do add a generic "void * pm_device" type pointer to
> struct pci_dev or struct device for such a purpose.

pci_to_acpi is already implemented in the PCI layer (see 
drivers/pci/pci-acpi.c), with struct device.firmware_data being where 
the acpi_handle ends up. I guess there's no problem in moving my code 
out to scsi-acpi.c and adding an arch_initcall for it. Would that be 
more acceptable? The only problem then is working out a clean way of 
setting up the notification structure.

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

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [ACPI] Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 14:14             ` Jeff Garzik
@ 2005-12-08 14:30               ` Alan Cox
  2005-12-08 14:43                 ` Matthew Garrett
  2005-12-09 11:42                 ` Jeff Garzik
  0 siblings, 2 replies; 64+ messages in thread
From: Alan Cox @ 2005-12-08 14:30 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Matthew Garrett, Christoph Hellwig, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Iau, 2005-12-08 at 09:14 -0500, Jeff Garzik wrote:
> These are only for PATA.  We don't care about _GTM/_STM on SATA.

Even your piix driver supports PATA. Put the foaming (justified ;))
hatred for ACPI aside for a moment and take a look at the real world as
it unfortunately is right now.

> Further, SATA completely resets and re-initializes the device as if from 
> a hardware reset (except on ata_piix, which doesn't support COMRESET, 
> and PATA).  This makes _GTF uninteresting, as well.

You don't know what the sequences the resume method is concerned about
actually are.

> suspend/resume works just fine with Jens' out-of-tree patch.

Only on some systems.

> > If you don't run the resume methods your disk subsystem status after a
> > resume is simply undefined and unsafe.
> 
> I initialize the hardware to a defined state.

Sure, but sometimes the *wrong* defined state. The BIOS ACPI methods
include things like unlocking drive passwords on restore with some
systems. You don't handle that at all.

Having said that I still think ACPI awareness doesn't belong in libata
or scsi because we'd then have awareness of every pm scheme in the wrong
layer and a dozen pm systems all with scsi hooks. Gak...

SCSI/libata can go easily from ata channel to pci device to device. The
rest of the logic belongs outside of scsi/libata.

Alan


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 14:18           ` Matthew Garrett
@ 2005-12-08 14:33             ` Alan Cox
  2005-12-08 14:52               ` Matthew Garrett
  0 siblings, 1 reply; 64+ messages in thread
From: Alan Cox @ 2005-12-08 14:33 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Iau, 2005-12-08 at 14:18 +0000, Matthew Garrett wrote:
> drivers/pci/pci-acpi.c), with struct device.firmware_data being where 
> the acpi_handle ends up. I guess there's no problem in moving my code 
> out to scsi-acpi.c and adding an arch_initcall for it. Would that be 
> more acceptable? The only problem then is working out a clean way of 
> setting up the notification structure.

I would say your code belongs in the ACPI subtree. At most the core code
wants to have the generic supporting functions for 'do a taskfile' and
if need be to call an arch/platform resume function that any pm system
can sensibly use.

SCSI should not know detail about ACPI, APM or anything of that nature.

Alan


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [ACPI] Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 14:30               ` Alan Cox
@ 2005-12-08 14:43                 ` Matthew Garrett
  2005-12-08 14:53                   ` Alan Cox
  2005-12-09 11:42                 ` Jeff Garzik
  1 sibling, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-08 14:43 UTC (permalink / raw)
  To: Alan Cox
  Cc: Jeff Garzik, Christoph Hellwig, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 02:30:57PM +0000, Alan Cox wrote:

> SCSI/libata can go easily from ata channel to pci device to device. The
> rest of the logic belongs outside of scsi/libata.

ACPI methods belong to SATA/PATA targets, not PCI devices. The 
notification you get is something of the form

\SB.PCI.IDE0.SEC.MASTER

on sensible devices, and

\SB.C043.C438.C222.C223

on anything from HP[1]. Somehow, you have to get from there to a 
specific SCSI host and target. By far the easiest way of doing that is 
to register them at device add time, which needs a small amount of 
cooperation from the SCSI or libata layers. And to register the 
notifications in the first place, you need to know the ACPI handles.

[1] Thanks, HP
-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 14:33             ` Alan Cox
@ 2005-12-08 14:52               ` Matthew Garrett
  2005-12-08 14:55                 ` Alan Cox
  2005-12-08 17:19                 ` Matthew Garrett
  0 siblings, 2 replies; 64+ messages in thread
From: Matthew Garrett @ 2005-12-08 14:52 UTC (permalink / raw)
  To: Alan Cox
  Cc: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 02:33:53PM +0000, Alan Cox wrote:

> I would say your code belongs in the ACPI subtree. At most the core code
> wants to have the generic supporting functions for 'do a taskfile' and
> if need be to call an arch/platform resume function that any pm system
> can sensibly use.

How about the hotplug notification events?

> SCSI should not know detail about ACPI, APM or anything of that nature.

Hrm. I guess this can be implemented pretty much just by cutting and 
pasting the code into drivers/acpi rather than drivers/scsi. Would that 
be considered an improvement?
-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [ACPI] Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 14:43                 ` Matthew Garrett
@ 2005-12-08 14:53                   ` Alan Cox
  0 siblings, 0 replies; 64+ messages in thread
From: Alan Cox @ 2005-12-08 14:53 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Jeff Garzik, Christoph Hellwig, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Iau, 2005-12-08 at 14:43 +0000, Matthew Garrett wrote:
> ACPI methods belong to SATA/PATA targets, not PCI devices. The 
> notification you get is something of the form
> 
> \SB.PCI.IDE0.SEC.MASTER


That is fine. Given struct ata_port * you can get

channel = ap->hard_port_no
pci device ptr = to_pci(ap->host_set->dev)

And from the struct ata_device *

slave = (adev->devno == 1)



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 14:52               ` Matthew Garrett
@ 2005-12-08 14:55                 ` Alan Cox
  2005-12-08 17:19                 ` Matthew Garrett
  1 sibling, 0 replies; 64+ messages in thread
From: Alan Cox @ 2005-12-08 14:55 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Iau, 2005-12-08 at 14:52 +0000, Matthew Garrett wrote:
> On Thu, Dec 08, 2005 at 02:33:53PM +0000, Alan Cox wrote:
> 
> > I would say your code belongs in the ACPI subtree. At most the core code
> > wants to have the generic supporting functions for 'do a taskfile' and
> > if need be to call an arch/platform resume function that any pm system
> > can sensibly use.
> 
> How about the hotplug notification events?

Again you want this to be generic. Its not nice to throw the scsi layer
an 'ACPI hotplug'. Instead it wants to receive a generic notification
that could also be generated by other events (eg ISAPnP or platform
specific drivers or from an IRQ handler etc). There is going to be more
than ACPI here and things like PDAs that spot hotplug via an io port
register need to work just as sanely.

So you'd want

	ACPI hotplug event
	Parse into generic form
	Callback in terms of device, channel, unit, event type not ACPI


> > SCSI should not know detail about ACPI, APM or anything of that nature.
> 
> Hrm. I guess this can be implemented pretty much just by cutting and 
> pasting the code into drivers/acpi rather than drivers/scsi. Would that 
> be considered an improvement?

Yep


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 13:44         ` Christoph Hellwig
@ 2005-12-08 17:18           ` Erik Slagter
  2005-12-08 20:43             ` Jeff Garzik
  0 siblings, 1 reply; 64+ messages in thread
From: Erik Slagter @ 2005-12-08 17:18 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Matthew Garrett, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]

On Thu, 2005-12-08 at 13:44 +0000, Christoph Hellwig wrote:
> On Thu, Dec 08, 2005 at 01:39:45PM +0000, Matthew Garrett wrote:
> > On Thu, Dec 08, 2005 at 01:33:08PM +0000, Christoph Hellwig wrote:
> > 
> > > Don't do it at all.  We don't need to fuck up every layer and driver for
> > > intels braindamage.
> > 
> > Doing SATA suspend/resume properly on x86 depends on knowing the ACPI 
> > object that corresponds to a host or target. It's also the only way to 
> > support hotswap on this hardware[1], since there's no way for userspace 
> > to know which device a notification refers to.
> 
> Well, bad luck for people buying such broken hardware.  Maybe you can trick
> Jeff into adding junk like that to libata, but it surely doesn't have any
> business in the scsi layer.

'guess You're not interested in having suspend/resume actually work on
laptops (or other PC's). That's your prerogative but imho it's a bit
narrow-minded to withhold this functionality from other people who
actually would like to have this working, just because you happen to not
like ACPI.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2771 bytes --]

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 14:52               ` Matthew Garrett
  2005-12-08 14:55                 ` Alan Cox
@ 2005-12-08 17:19                 ` Matthew Garrett
  2005-12-09 11:42                   ` Christoph Hellwig
  1 sibling, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-08 17:19 UTC (permalink / raw)
  To: Alan Cox
  Cc: Christoph Hellwig, randy_d_dunlap, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 02:52:57PM +0000, Matthew Garrett wrote:

> Hrm. I guess this can be implemented pretty much just by cutting and 
> pasting the code into drivers/acpi rather than drivers/scsi. Would that 
> be considered an improvement?

This turns out to be quite difficult, and I can't see a clean way of 
doing it without touching scsi or rewriting chunks of the ACPI glue 
code.

The basic flow of code required here is:

1) scsi layer registers a new device
2) platform_notify is called, which is (in this case) 
acpi_platform_notify
3) acpi_platform_notify checks whether it knows dev->bus. If so, it 
calls appropriate callbacks.

Without touching scsi, there doesn't seem to be any way for (3) to work 
if scsi is a module. Would a simple

#ifdef CONFIG_ACPI
acpi_scsi_init(&scsi_bus_type)
#endif

in the scsi code be acceptable? If not, then we have some difficulty. 
The acpi glue code has to be statically linked in, so it can't rely on 
anything that directly references the scsi code.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 17:18           ` Erik Slagter
@ 2005-12-08 20:43             ` Jeff Garzik
  2005-12-08 21:03               ` Dominic Ijichi
                                 ` (2 more replies)
  0 siblings, 3 replies; 64+ messages in thread
From: Jeff Garzik @ 2005-12-08 20:43 UTC (permalink / raw)
  To: Erik Slagter
  Cc: Christoph Hellwig, Matthew Garrett, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

Erik Slagter wrote:
> 'guess You're not interested in having suspend/resume actually work on
> laptops (or other PC's). That's your prerogative but imho it's a bit
> narrow-minded to withhold this functionality from other people who
> actually would like to have this working, just because you happen to not
> like ACPI.

It works just fine on laptops, with Jens' suspend/resume patch.

	Jeff



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 20:43             ` Jeff Garzik
@ 2005-12-08 21:03               ` Dominic Ijichi
  2005-12-08 21:09                 ` Jeff Garzik
  2005-12-08 21:31               ` Randy Dunlap
  2005-12-09  3:28               ` Mark Lord
  2 siblings, 1 reply; 64+ messages in thread
From: Dominic Ijichi @ 2005-12-08 21:03 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Erik Slagter, Christoph Hellwig, Matthew Garrett, randy_d_dunlap,
	linux-ide, linux-scsi, linux-kernel, acpi-devel

Quoting Jeff Garzik <jgarzik@pobox.com>:

> Erik Slagter wrote:
> > 'guess You're not interested in having suspend/resume actually work on
> > laptops (or other PC's). That's your prerogative but imho it's a bit
> > narrow-minded to withhold this functionality from other people who
> > actually would like to have this working, just because you happen to not
> > like ACPI.
> 
> It works just fine on laptops, with Jens' suspend/resume patch.

not on my fujitsu sonoma/ih6 based laptop it doesn't.  in my travels trying to
fix this problem it appears there are many others it doesnt work for either. 
suspend/resume is incredibly important for day-to-day practical use of a laptop,
particularly using linux. the sole reason i still have a windows partition is
because suspend doesnt work in linux and i'm sick of firing everything up again
3 times a day.

thank you very much to all on this list who are pursuing a solution sensibly and
not making unhelpful blanket statements against the most widely used laptop
chipset maker - *particularly* when they are actively contributing to
development on this list.  we (laptop users) dont care about religious
standpoints, we just want it to work.

dom


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


------------------------------------------
This message was penned by the hand of Dom

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 21:03               ` Dominic Ijichi
@ 2005-12-08 21:09                 ` Jeff Garzik
  2005-12-08 21:34                   ` Dominic Ijichi
  0 siblings, 1 reply; 64+ messages in thread
From: Jeff Garzik @ 2005-12-08 21:09 UTC (permalink / raw)
  To: Dominic Ijichi
  Cc: Erik Slagter, Christoph Hellwig, Matthew Garrett, randy_d_dunlap,
	linux-ide, linux-scsi, linux-kernel, acpi-devel

Dominic Ijichi wrote:
> Quoting Jeff Garzik <jgarzik@pobox.com>:
> 
> 
>>Erik Slagter wrote:
>>
>>>'guess You're not interested in having suspend/resume actually work on
>>>laptops (or other PC's). That's your prerogative but imho it's a bit
>>>narrow-minded to withhold this functionality from other people who
>>>actually would like to have this working, just because you happen to not
>>>like ACPI.
>>
>>It works just fine on laptops, with Jens' suspend/resume patch.
> 
> 
> not on my fujitsu sonoma/ih6 based laptop it doesn't.  in my travels trying to
> fix this problem it appears there are many others it doesnt work for either. 
> suspend/resume is incredibly important for day-to-day practical use of a laptop,
> particularly using linux. the sole reason i still have a windows partition is
> because suspend doesnt work in linux and i'm sick of firing everything up again
> 3 times a day.
> 
> thank you very much to all on this list who are pursuing a solution sensibly and
> not making unhelpful blanket statements against the most widely used laptop
> chipset maker - *particularly* when they are actively contributing to
> development on this list.  we (laptop users) dont care about religious
> standpoints, we just want it to work.

I've personally tested it on fuji ich5 and ich6 laptops.  What model do 
you have?  What kernel version did you test?  When did you apply the 
suspend/resume patch?

	Jeff




^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 20:43             ` Jeff Garzik
  2005-12-08 21:03               ` Dominic Ijichi
@ 2005-12-08 21:31               ` Randy Dunlap
  2005-12-09  9:45                 ` Erik Slagter
  2005-12-09  3:28               ` Mark Lord
  2 siblings, 1 reply; 64+ messages in thread
From: Randy Dunlap @ 2005-12-08 21:31 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: erik, hch, mjg59, linux-ide, linux-scsi, linux-kernel, acpi-devel

On Thu, 08 Dec 2005 15:43:44 -0500
Jeff Garzik <jgarzik@pobox.com> wrote:

> Erik Slagter wrote:
> > 'guess You're not interested in having suspend/resume actually work on
> > laptops (or other PC's). That's your prerogative but imho it's a bit
> > narrow-minded to withhold this functionality from other people who
> > actually would like to have this working, just because you happen to not
> > like ACPI.
> 
> It works just fine on laptops, with Jens' suspend/resume patch.

I have seen a few other people report that SATA suspend/resume
works when using Jens's patch.  However, this is done without
the benefit of what the additional ACPI methods provide thru
_GTF and writing those taskfiles, such as:
- enabling write cache
- enabling device power management
- freezing the security password

so even when it "works," those people may be missing some
performance benefits or power savings or security.

In any case, I'm glad to see some discussion of this.

---
~Randy

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 21:09                 ` Jeff Garzik
@ 2005-12-08 21:34                   ` Dominic Ijichi
  0 siblings, 0 replies; 64+ messages in thread
From: Dominic Ijichi @ 2005-12-08 21:34 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Erik Slagter, Christoph Hellwig, Matthew Garrett, randy_d_dunlap,
	linux-ide, linux-scsi, linux-kernel, acpi-devel

Quoting Jeff Garzik <jgarzik@pobox.com>:

> Dominic Ijichi wrote:
> > Quoting Jeff Garzik <jgarzik@pobox.com>:
> >
> >
> >>Erik Slagter wrote:
> >>
> >>>'guess You're not interested in having suspend/resume actually work on
> >>>laptops (or other PC's). That's your prerogative but imho it's a bit
> >>>narrow-minded to withhold this functionality from other people who
> >>>actually would like to have this working, just because you happen to not
> >>>like ACPI.
> >>
> >>It works just fine on laptops, with Jens' suspend/resume patch.
> >
> >
> > not on my fujitsu sonoma/ih6 based laptop it doesn't.  in my travels trying
> to
> > fix this problem it appears there are many others it doesnt work for
> either.
> > suspend/resume is incredibly important for day-to-day practical use of a
> laptop,
> > particularly using linux. the sole reason i still have a windows partition
> is
> > because suspend doesnt work in linux and i'm sick of firing everything up
> again
> > 3 times a day.
> >
> > thank you very much to all on this list who are pursuing a solution
> sensibly and
> > not making unhelpful blanket statements against the most widely used laptop
> > chipset maker - *particularly* when they are actively contributing to
> > development on this list.  we (laptop users) dont care about religious
> > standpoints, we just want it to work.
> 
> I've personally tested it on fuji ich5 and ich6 laptops.  What model do
> you have?  What kernel version did you test?  When did you apply the
> suspend/resume patch?

N3510, 60gb sata model.  2.6.14,2.6.15-rc[1..5], with and without mm patches,
and various suspend patches sent to me by people on the linux-ide list.  in
particular, Jens Axboe's libata_suspend.patch and Randy Dunlap's patches here:
http://www.xenotime.net/linux/SATA/2.6.15-rc/, plus patches from your site:
http://www.kernel.org/pub/linux/kernel/people/jgarzik/patchkits/2.6/.  every
permutation and combination tried!  all with same result - laptop resumes but
hangs on first disk access.

cheers
dom


------------------------------------------
This message was penned by the hand of Dom

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 20:43             ` Jeff Garzik
  2005-12-08 21:03               ` Dominic Ijichi
  2005-12-08 21:31               ` Randy Dunlap
@ 2005-12-09  3:28               ` Mark Lord
  2005-12-09 11:29                 ` Jeff Garzik
  2 siblings, 1 reply; 64+ messages in thread
From: Mark Lord @ 2005-12-09  3:28 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Erik Slagter, Christoph Hellwig, Matthew Garrett, randy_d_dunlap,
	linux-ide, linux-scsi, linux-kernel, acpi-devel

Jeff Garzik wrote:
> Erik Slagter wrote:
> 
>> 'guess You're not interested in having suspend/resume actually work on
>> laptops (or other PC's). That's your prerogative but imho it's a bit
>> narrow-minded to withhold this functionality from other people who
>> actually would like to have this working, just because you happen to not
>> like ACPI.
> 
> 
> It works just fine on laptops, with Jens' suspend/resume patch.
> 
>     Jeff

No.  I use it on my two modern laptops with great success,
but only with *certain* hard disks.  When I replace the ultra modern
100GB drive in my machine with a slightly older 30GB drive,
suspend/resume no longer work.  No other changes.

Other users have reported similar experiences to me.

We really REALLY need libata to get fixed for this stuff.

Cheers

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 21:31               ` Randy Dunlap
@ 2005-12-09  9:45                 ` Erik Slagter
  2005-12-09 10:39                   ` Jens Axboe
  0 siblings, 1 reply; 64+ messages in thread
From: Erik Slagter @ 2005-12-09  9:45 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Jeff Garzik, hch, mjg59, linux-ide, linux-scsi, linux-kernel, acpi-devel

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Thu, 2005-12-08 at 13:31 -0800, Randy Dunlap wrote:

> > It works just fine on laptops, with Jens' suspend/resume patch.
> 
> I have seen a few other people report that SATA suspend/resume
> works when using Jens's patch.  However, this is done without
> the benefit of what the additional ACPI methods provide thru
> _GTF and writing those taskfiles, such as:
> - enabling write cache
> - enabling device power management
> - freezing the security password
> 
> so even when it "works," those people may be missing some
> performance benefits or power savings or security.
> 
> In any case, I'm glad to see some discussion of this.

IMHO available infrastructure (and hardware abstraction!) should be used
instead of being stubborn and pretend we know everything about any
hardware.

Of course this all to a certain degree (== as long as no DRM is
involved).

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2771 bytes --]

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09  9:45                 ` Erik Slagter
@ 2005-12-09 10:39                   ` Jens Axboe
  2005-12-09 10:45                     ` Erik Slagter
  0 siblings, 1 reply; 64+ messages in thread
From: Jens Axboe @ 2005-12-09 10:39 UTC (permalink / raw)
  To: Erik Slagter
  Cc: Randy Dunlap, Jeff Garzik, hch, mjg59, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Fri, Dec 09 2005, Erik Slagter wrote:
> On Thu, 2005-12-08 at 13:31 -0800, Randy Dunlap wrote:
> 
> > > It works just fine on laptops, with Jens' suspend/resume patch.
> > 
> > I have seen a few other people report that SATA suspend/resume
> > works when using Jens's patch.  However, this is done without
> > the benefit of what the additional ACPI methods provide thru
> > _GTF and writing those taskfiles, such as:
> > - enabling write cache
> > - enabling device power management
> > - freezing the security password
> > 
> > so even when it "works," those people may be missing some
> > performance benefits or power savings or security.
> > 
> > In any case, I'm glad to see some discussion of this.
> 
> IMHO available infrastructure (and hardware abstraction!) should be used
> instead of being stubborn and pretend we know everything about any
> hardware.

It's not about being stubborn, it's about maintaining and working on a
clean design. The developers have to do that, not the users. So forgive
people for being a little cautious about shuffling all sorts of ACPI
into the scsi core and/or drivers. We always need to think long term
here.

Users don't care about the maintainability and cleanliness of the code,
they really just want it to work. Which is perfectly understandable.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 10:39                   ` Jens Axboe
@ 2005-12-09 10:45                     ` Erik Slagter
  2005-12-09 11:27                       ` Jeff Garzik
  2005-12-09 11:30                       ` Jens Axboe
  0 siblings, 2 replies; 64+ messages in thread
From: Erik Slagter @ 2005-12-09 10:45 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Randy Dunlap, Jeff Garzik, hch, mjg59, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]

On Fri, 2005-12-09 at 11:39 +0100, Jens Axboe wrote:

> > IMHO available infrastructure (and hardware abstraction!) should be used
> > instead of being stubborn and pretend we know everything about any
> > hardware.
> 
> It's not about being stubborn, it's about maintaining and working on a
> clean design. The developers have to do that, not the users. So forgive
> people for being a little cautious about shuffling all sorts of ACPI
> into the scsi core and/or drivers. We always need to think long term
> here.
> 
> Users don't care about the maintainability and cleanliness of the code,
> they really just want it to work. Which is perfectly understandable.

I perfectly understand that, what I do object against, is rejecting a
concept (like this) totally because the developers(?) do not like the
mechanism that's used (although ACPI is used everywhere else in the
kernel). At least there might be some discussion where this sort of code
belongs to make the design clean and easily maintainable, instead of
instantly completely rejecting the concept, because OP simply doesn't
like acpi.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2771 bytes --]

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 10:45                     ` Erik Slagter
@ 2005-12-09 11:27                       ` Jeff Garzik
  2005-12-09 11:35                         ` Erik Slagter
  2005-12-10  2:19                         ` Douglas Gilbert
  2005-12-09 11:30                       ` Jens Axboe
  1 sibling, 2 replies; 64+ messages in thread
From: Jeff Garzik @ 2005-12-09 11:27 UTC (permalink / raw)
  To: Erik Slagter
  Cc: Jens Axboe, Randy Dunlap, hch, mjg59, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

Erik Slagter wrote:
> On Fri, 2005-12-09 at 11:39 +0100, Jens Axboe wrote:
> 
> 
>>>IMHO available infrastructure (and hardware abstraction!) should be used
>>>instead of being stubborn and pretend we know everything about any
>>>hardware.
>>
>>It's not about being stubborn, it's about maintaining and working on a
>>clean design. The developers have to do that, not the users. So forgive
>>people for being a little cautious about shuffling all sorts of ACPI
>>into the scsi core and/or drivers. We always need to think long term
>>here.
>>
>>Users don't care about the maintainability and cleanliness of the code,
>>they really just want it to work. Which is perfectly understandable.
> 
> 
> I perfectly understand that, what I do object against, is rejecting a
> concept (like this) totally because the developers(?) do not like the
> mechanism that's used (although ACPI is used everywhere else in the
> kernel). At least there might be some discussion where this sort of code
> belongs to make the design clean and easily maintainable, instead of
> instantly completely rejecting the concept, because OP simply doesn't
> like acpi.

Framing the discussion in terms of "like" and "dislike" proves you 
understand nothing about the issues -- and lacking that understanding, 
you feel its best to insult people.

libata suspend/resume needs to work on platforms without ACPI, such as 
ppc64.  libata reset needs to work even when BIOS is not present at all. 
  And by definition, anything that is done in ACPI can be done in the 
driver.

The only thing libata cannot know is BIOS defaults.  Anything else 
libata doesn't know is a driver bug.  We already know there are a ton of 
settings that should be saved/restored, which is not done now.  Until 
that code is added to libata, talk of ACPI is premature.  Further, even 
the premature patch was obviously unacceptable, because you don't patch 
ACPI code directly into libata and SCSI.  That's the wrong approach.

	Jeff



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09  3:28               ` Mark Lord
@ 2005-12-09 11:29                 ` Jeff Garzik
  2005-12-10  4:01                   ` Mark Lord
  0 siblings, 1 reply; 64+ messages in thread
From: Jeff Garzik @ 2005-12-09 11:29 UTC (permalink / raw)
  To: Mark Lord
  Cc: Erik Slagter, Christoph Hellwig, Matthew Garrett, randy_d_dunlap,
	linux-ide, linux-scsi, linux-kernel, acpi-devel

Mark Lord wrote:
> Jeff Garzik wrote:
> 
>> Erik Slagter wrote:
>>
>>> 'guess You're not interested in having suspend/resume actually work on
>>> laptops (or other PC's). That's your prerogative but imho it's a bit
>>> narrow-minded to withhold this functionality from other people who
>>> actually would like to have this working, just because you happen to not
>>> like ACPI.
>>
>>
>>
>> It works just fine on laptops, with Jens' suspend/resume patch.
>>
>>     Jeff
> 
> 
> No.  I use it on my two modern laptops with great success,
> but only with *certain* hard disks.  When I replace the ultra modern
> 100GB drive in my machine with a slightly older 30GB drive,
> suspend/resume no longer work.  No other changes.
> 
> Other users have reported similar experiences to me.
> 
> We really REALLY need libata to get fixed for this stuff.

Patches welcome :)  There is a bunch of stuff that needs to be done for 
suspend/resume.  Saving/restoring settings, additional resets and 
probes, etc.

	Jeff



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 10:45                     ` Erik Slagter
  2005-12-09 11:27                       ` Jeff Garzik
@ 2005-12-09 11:30                       ` Jens Axboe
  1 sibling, 0 replies; 64+ messages in thread
From: Jens Axboe @ 2005-12-09 11:30 UTC (permalink / raw)
  To: Erik Slagter
  Cc: Randy Dunlap, Jeff Garzik, hch, mjg59, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Fri, Dec 09 2005, Erik Slagter wrote:
> On Fri, 2005-12-09 at 11:39 +0100, Jens Axboe wrote:
> 
> > > IMHO available infrastructure (and hardware abstraction!) should be used
> > > instead of being stubborn and pretend we know everything about any
> > > hardware.
> > 
> > It's not about being stubborn, it's about maintaining and working on a
> > clean design. The developers have to do that, not the users. So forgive
> > people for being a little cautious about shuffling all sorts of ACPI
> > into the scsi core and/or drivers. We always need to think long term
> > here.
> > 
> > Users don't care about the maintainability and cleanliness of the code,
> > they really just want it to work. Which is perfectly understandable.
> 
> I perfectly understand that, what I do object against, is rejecting a
> concept (like this) totally because the developers(?) do not like the
> mechanism that's used (although ACPI is used everywhere else in the
> kernel). At least there might be some discussion where this sort of code
> belongs to make the design clean and easily maintainable, instead of
> instantly completely rejecting the concept, because OP simply doesn't
> like acpi.

Not to put words in anyones mouth, but the rejection is mainly based on
the concept of stuffing acpi directly into the SCSI core where it
clearly doesn't belong. I don't think anyone is against utilizing ACPI
(if useful/required) for suspend+resume as a concept, even if lots of
people have reservations on ACPI in generel.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:27                       ` Jeff Garzik
@ 2005-12-09 11:35                         ` Erik Slagter
  2005-12-09 11:40                           ` Christoph Hellwig
  2005-12-09 11:46                           ` Jens Axboe
  2005-12-10  2:19                         ` Douglas Gilbert
  1 sibling, 2 replies; 64+ messages in thread
From: Erik Slagter @ 2005-12-09 11:35 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Jens Axboe, Randy Dunlap, hch, mjg59, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

[-- Attachment #1: Type: text/plain, Size: 1799 bytes --]

On Fri, 2005-12-09 at 06:27 -0500, Jeff Garzik wrote:

> > I perfectly understand that, what I do object against, is rejecting a
> > concept (like this) totally because the developers(?) do not like the
> > mechanism that's used (although ACPI is used everywhere else in the
> > kernel). At least there might be some discussion where this sort of code
> > belongs to make the design clean and easily maintainable, instead of
> > instantly completely rejecting the concept, because OP simply doesn't
> > like acpi.
> 
> Framing the discussion in terms of "like" and "dislike" proves you 
> understand nothing about the issues -- and lacking that understanding, 
> you feel its best to insult people.
> 
> libata suspend/resume needs to work on platforms without ACPI, such as 
> ppc64.  libata reset needs to work even when BIOS is not present at all. 
>   And by definition, anything that is done in ACPI can be done in the 
> driver.
> 
> The only thing libata cannot know is BIOS defaults.  Anything else 
> libata doesn't know is a driver bug.  We already know there are a ton of 
> settings that should be saved/restored, which is not done now.  Until 
> that code is added to libata, talk of ACPI is premature.  Further, even 
> the premature patch was obviously unacceptable, because you don't patch 
> ACPI code directly into libata and SCSI.  That's the wrong approach.

I case this (still) isn't clear, I am addressing the attitude of "It's
ACPI so it's not going to be used, period".

About the exact technical implementation, I do not have an opinion,
because I don't have the knowledge.

BTW I try to not actually insult people (as others here seems to like to
do). If someone really feels offended, my apologies. I cannot hide some
irritation though.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2771 bytes --]

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:35                         ` Erik Slagter
@ 2005-12-09 11:40                           ` Christoph Hellwig
  2005-12-09 11:46                           ` Jens Axboe
  1 sibling, 0 replies; 64+ messages in thread
From: Christoph Hellwig @ 2005-12-09 11:40 UTC (permalink / raw)
  To: Erik Slagter
  Cc: Jeff Garzik, Jens Axboe, Randy Dunlap, hch, mjg59, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Fri, Dec 09, 2005 at 12:35:27PM +0100, Erik Slagter wrote:
> I case this (still) isn't clear, I am addressing the attitude of "It's
> ACPI so it's not going to be used, period".

We're not gonna patch support for any braindead firmware interface into
the scsi midlayer (and trust me, there's a shitload more of them than just
acpi).  Now please sod off.


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [ACPI] Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 14:30               ` Alan Cox
  2005-12-08 14:43                 ` Matthew Garrett
@ 2005-12-09 11:42                 ` Jeff Garzik
  1 sibling, 0 replies; 64+ messages in thread
From: Jeff Garzik @ 2005-12-09 11:42 UTC (permalink / raw)
  To: Alan Cox
  Cc: Matthew Garrett, Christoph Hellwig, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

Alan Cox wrote:
> On Iau, 2005-12-08 at 09:14 -0500, Jeff Garzik wrote:
> 
>>These are only for PATA.  We don't care about _GTM/_STM on SATA.
> 
> 
> Even your piix driver supports PATA. Put the foaming (justified ;))
> hatred for ACPI aside for a moment and take a look at the real world as
> it unfortunately is right now.

First, I clearly said "except on ata_piix ... or PATA"

Second, don't put words in my mouth.  I don't hate ACPI, and libata's 
direction for hotswap and suspend/resume has zero to do with "foaming 
hatred."

Right now, the top priority is getting SATA suspend/resume correct, and 
_hopefully_ doing it in a way that's friendly to PATA.  And as I said, 
we don't care about _GTM/_STM on SATA.

Further, all current ACPI proposed code is completely half-assed.  It's 
"hope and pray", because libata configures the device and does resets -- 
which is bound to CONFLICT WITH ACPI.

Even further, I want to support both ACPI cases (x86[-64]) and non-ACPI 
cases (other arches).  Some platforms want ACPI for passwords or other 
settings.  Some platforms don't have ACPI at all.  Locking libata into 
ACPI _only_ for suspend/resume is completely unacceptable.

I'm not a hope-n-pray kind of guy.  I want to get it right.  People are 
more than welcome to use unapplied patches floating around the 'net 
until we get there.

	Jeff



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08 17:19                 ` Matthew Garrett
@ 2005-12-09 11:42                   ` Christoph Hellwig
  2005-12-09 11:49                     ` Jeff Garzik
  2005-12-09 11:50                     ` Matthew Garrett
  0 siblings, 2 replies; 64+ messages in thread
From: Christoph Hellwig @ 2005-12-09 11:42 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Alan Cox, Christoph Hellwig, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Thu, Dec 08, 2005 at 05:19:01PM +0000, Matthew Garrett wrote:
> This turns out to be quite difficult, and I can't see a clean way of 
> doing it without touching scsi or rewriting chunks of the ACPI glue 
> code.
> 
> The basic flow of code required here is:
> 
> 1) scsi layer registers a new device
> 2) platform_notify is called, which is (in this case) 
> acpi_platform_notify
> 3) acpi_platform_notify checks whether it knows dev->bus. If so, it 
> calls appropriate callbacks.
> 
> Without touching scsi, there doesn't seem to be any way for (3) to work 
> if scsi is a module. Would a simple
> 
> #ifdef CONFIG_ACPI
> acpi_scsi_init(&scsi_bus_type)
> #endif
> 
> in the scsi code be acceptable? If not, then we have some difficulty. 
> The acpi glue code has to be statically linked in, so it can't rely on 
> anything that directly references the scsi code.

As a concept it's _much_ better.  Although it should be platform_scsi_init
and every architecture would provide an, in most cases noop, implementation.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:35                         ` Erik Slagter
  2005-12-09 11:40                           ` Christoph Hellwig
@ 2005-12-09 11:46                           ` Jens Axboe
  2005-12-09 11:55                             ` Matthew Garrett
  2005-12-09 12:01                             ` Erik Slagter
  1 sibling, 2 replies; 64+ messages in thread
From: Jens Axboe @ 2005-12-09 11:46 UTC (permalink / raw)
  To: Erik Slagter
  Cc: Jeff Garzik, Randy Dunlap, hch, mjg59, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Fri, Dec 09 2005, Erik Slagter wrote:
> I case this (still) isn't clear, I am addressing the attitude of "It's
> ACPI so it's not going to be used, period".

The problem seems to be that you are misunderstanding the 'attitude',
which was mainly based on the initial patch sent out which stuffs acpi
directly in everywhere. That seems to be a good trigger for curt/direct
replies.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:42                   ` Christoph Hellwig
@ 2005-12-09 11:49                     ` Jeff Garzik
  2005-12-09 11:52                       ` Matthew Garrett
  2005-12-09 11:50                     ` Matthew Garrett
  1 sibling, 1 reply; 64+ messages in thread
From: Jeff Garzik @ 2005-12-09 11:49 UTC (permalink / raw)
  To: Christoph Hellwig, Matthew Garrett, Alan Cox, randy_d_dunlap,
	linux-ide, linux-scsi, linux-kernel, acpi-devel

On Fri, Dec 09, 2005 at 11:42:46AM +0000, Christoph Hellwig wrote:
> On Thu, Dec 08, 2005 at 05:19:01PM +0000, Matthew Garrett wrote:
> > This turns out to be quite difficult, and I can't see a clean way of 
> > doing it without touching scsi or rewriting chunks of the ACPI glue 
> > code.
> > 
> > The basic flow of code required here is:
> > 
> > 1) scsi layer registers a new device
> > 2) platform_notify is called, which is (in this case) 
> > acpi_platform_notify
> > 3) acpi_platform_notify checks whether it knows dev->bus. If so, it 
> > calls appropriate callbacks.
> > 
> > Without touching scsi, there doesn't seem to be any way for (3) to work 
> > if scsi is a module. Would a simple
> > 
> > #ifdef CONFIG_ACPI
> > acpi_scsi_init(&scsi_bus_type)
> > #endif
> > 
> > in the scsi code be acceptable? If not, then we have some difficulty. 
> > The acpi glue code has to be statically linked in, so it can't rely on 
> > anything that directly references the scsi code.
> 
> As a concept it's _much_ better.  Although it should be platform_scsi_init
> and every architecture would provide an, in most cases noop, implementation.

If this is just for libata, it's still at the wrong level.

libata will eventually make the SCSI simulator optional, which means
any acpi_scsi_init() or whatnot won't work for libata.

	Jeff




^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:42                   ` Christoph Hellwig
  2005-12-09 11:49                     ` Jeff Garzik
@ 2005-12-09 11:50                     ` Matthew Garrett
  2005-12-09 11:55                       ` Christoph Hellwig
  1 sibling, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-09 11:50 UTC (permalink / raw)
  To: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Fri, Dec 09, 2005 at 11:42:46AM +0000, Christoph Hellwig wrote:

> As a concept it's _much_ better.  Although it should be platform_scsi_init
> and every architecture would provide an, in most cases noop, implementation.

How about

if (platform_scsi_init)
	platform_scsi_init(&scsi_bus_type);

? This is similar to how the platform_notify callback code is handled. 
Making it per-arch isn't quite ideal, since x86 can be ACPI or APM and 
kernels need support for both. On the other hand, I can't think of any 
way that APM could do anything useful with the information, so per-arch 
may be reasonable.

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

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:49                     ` Jeff Garzik
@ 2005-12-09 11:52                       ` Matthew Garrett
  2005-12-09 11:58                         ` Jeff Garzik
  0 siblings, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-09 11:52 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Fri, Dec 09, 2005 at 06:49:44AM -0500, Jeff Garzik wrote:

> If this is just for libata, it's still at the wrong level.
> 
> libata will eventually make the SCSI simulator optional, which means
> any acpi_scsi_init() or whatnot won't work for libata.

It depends on notification whenever a device is added to the scsi bus 
class, so it needs access to scsi_bus_type. While that could be put in 
the libata layer, it seems cleaner to leave it in scsi and then add 
another callback for libata when it moves to its own bus class.

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

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:46                           ` Jens Axboe
@ 2005-12-09 11:55                             ` Matthew Garrett
  2005-12-09 13:22                               ` Bartlomiej Zolnierkiewicz
  2005-12-09 12:01                             ` Erik Slagter
  1 sibling, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-09 11:55 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Erik Slagter, Jeff Garzik, Randy Dunlap, hch, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Fri, Dec 09, 2005 at 12:46:42PM +0100, Jens Axboe wrote:
> On Fri, Dec 09 2005, Erik Slagter wrote:
> > I case this (still) isn't clear, I am addressing the attitude of "It's
> > ACPI so it's not going to be used, period".
> 
> The problem seems to be that you are misunderstanding the 'attitude',
> which was mainly based on the initial patch sent out which stuffs acpi
> directly in everywhere. That seems to be a good trigger for curt/direct
> replies.

I was just following the example set by the ide acpi suspend/resume 
patch, which people didn't seem to object to nearly as much. I guess 
IDE's such a hack anyway that people aren't as worried :) I'll try 
produce a patch that just inserts platform-independent code into scsi 
around the start of next week.

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

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:50                     ` Matthew Garrett
@ 2005-12-09 11:55                       ` Christoph Hellwig
  0 siblings, 0 replies; 64+ messages in thread
From: Christoph Hellwig @ 2005-12-09 11:55 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Alan Cox, randy_d_dunlap, linux-ide, linux-scsi, linux-kernel,
	acpi-devel

On Fri, Dec 09, 2005 at 11:50:09AM +0000, Matthew Garrett wrote:
> On Fri, Dec 09, 2005 at 11:42:46AM +0000, Christoph Hellwig wrote:
> 
> > As a concept it's _much_ better.  Although it should be platform_scsi_init
> > and every architecture would provide an, in most cases noop, implementation.
> 
> How about
> 
> if (platform_scsi_init)
> 	platform_scsi_init(&scsi_bus_type);
> 
> ? This is similar to how the platform_notify callback code is handled. 
> Making it per-arch isn't quite ideal, since x86 can be ACPI or APM and 
> kernels need support for both. On the other hand, I can't think of any 
> way that APM could do anything useful with the information, so per-arch 
> may be reasonable.

I think a per-arch hook is better, if an architecture needs different
backend implementations it can dispatch internally.  And the above won't
work unless platform_scsi_init is a function pointer which would be quite
ugly.

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:52                       ` Matthew Garrett
@ 2005-12-09 11:58                         ` Jeff Garzik
  2005-12-09 12:11                           ` Matthew Garrett
  0 siblings, 1 reply; 64+ messages in thread
From: Jeff Garzik @ 2005-12-09 11:58 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

Matthew Garrett wrote:
> On Fri, Dec 09, 2005 at 06:49:44AM -0500, Jeff Garzik wrote:
> 
> 
>>If this is just for libata, it's still at the wrong level.
>>
>>libata will eventually make the SCSI simulator optional, which means
>>any acpi_scsi_init() or whatnot won't work for libata.
> 
> 
> It depends on notification whenever a device is added to the scsi bus 
> class, so it needs access to scsi_bus_type. While that could be put in 
> the libata layer, it seems cleaner to leave it in scsi and then add 
> another callback for libata when it moves to its own bus class.

If this is for hotswap, as I noted, libata doesn't need this at all.

If the hardware supports it, then libata will support it directly. 
There is no ACPI-specific magic, because ACPI does nothing but talk to 
the same hardware libata is talking to.

	Jeff




^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:46                           ` Jens Axboe
  2005-12-09 11:55                             ` Matthew Garrett
@ 2005-12-09 12:01                             ` Erik Slagter
  2005-12-09 12:07                               ` Jens Axboe
  1 sibling, 1 reply; 64+ messages in thread
From: Erik Slagter @ 2005-12-09 12:01 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Jeff Garzik, Randy Dunlap, hch, mjg59, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

[-- Attachment #1: Type: text/plain, Size: 983 bytes --]

On Fri, 2005-12-09 at 12:46 +0100, Jens Axboe wrote:
> On Fri, Dec 09 2005, Erik Slagter wrote:
> > I case this (still) isn't clear, I am addressing the attitude of "It's
> > ACPI so it's not going to be used, period".
> 
> The problem seems to be that you are misunderstanding the 'attitude',
> which was mainly based on the initial patch sent out which stuffs acpi
> directly in everywhere. That seems to be a good trigger for curt/direct
> replies.

This is the post I object to:
[http://marc.theaimsgroup.com/?l=linux-scsi&m=113404894931377&w=2]

>> Ok. What's the right layer to do this? The current ACPI/anything
>> else glue depends on specific knowledge about the bus concerned, and
>> needs callbacks registered before devices are added to that bus.
>> Doing it in the sata layer would have the potential for unhappiness
>> on mixed sata/scsi machines.

> Don't do it at all.  We don't need to fuck up every layer and driver for
> intels braindamage.


[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 2771 bytes --]

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 12:01                             ` Erik Slagter
@ 2005-12-09 12:07                               ` Jens Axboe
  0 siblings, 0 replies; 64+ messages in thread
From: Jens Axboe @ 2005-12-09 12:07 UTC (permalink / raw)
  To: Erik Slagter
  Cc: Jeff Garzik, Randy Dunlap, hch, mjg59, linux-ide, linux-scsi,
	linux-kernel, acpi-devel

On Fri, Dec 09 2005, Erik Slagter wrote:
> On Fri, 2005-12-09 at 12:46 +0100, Jens Axboe wrote:
> > On Fri, Dec 09 2005, Erik Slagter wrote:
> > > I case this (still) isn't clear, I am addressing the attitude of "It's
> > > ACPI so it's not going to be used, period".
> > 
> > The problem seems to be that you are misunderstanding the 'attitude',
> > which was mainly based on the initial patch sent out which stuffs acpi
> > directly in everywhere. That seems to be a good trigger for curt/direct
> > replies.
> 
> This is the post I object to:
> [http://marc.theaimsgroup.com/?l=linux-scsi&m=113404894931377&w=2]

I would imagine, and that is what I'm explaining to you. Can we please
just let this die? Thanks.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:58                         ` Jeff Garzik
@ 2005-12-09 12:11                           ` Matthew Garrett
  2005-12-09 12:16                             ` Jeff Garzik
  0 siblings, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-09 12:11 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Fri, Dec 09, 2005 at 06:58:41AM -0500, Jeff Garzik wrote:

> If this is for hotswap, as I noted, libata doesn't need this at all.
> 
> If the hardware supports it, then libata will support it directly. 
> There is no ACPI-specific magic, because ACPI does nothing but talk to 
> the same hardware libata is talking to.

If libata knows how to talk to the random hardware attached to a Dell 
laptop hotswap bay, I'll be amazed. Ejecting the drive generates a 
system management interrupt, which then causes the ACPI code to check a 
register in a block of machine-specific registers and generate an ACPI 
notification. As far as I can tell, the controller has no say in the 
matter at all - the Intel specs seem to suggest that ICH6 doesn't 
generate a hotswap interrupt unless you're using AHCI (which this 
hardware doesn't).

So, as far as I can tell, there /is/ ACPI-specific magic on 
current-generation hardware. If we're lucky, they'll move to AHCI in 
future and implement things properly there - but I wouldn't count on it.

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

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 12:11                           ` Matthew Garrett
@ 2005-12-09 12:16                             ` Jeff Garzik
  2005-12-09 12:24                               ` Matthew Garrett
  0 siblings, 1 reply; 64+ messages in thread
From: Jeff Garzik @ 2005-12-09 12:16 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

Matthew Garrett wrote:
> On Fri, Dec 09, 2005 at 06:58:41AM -0500, Jeff Garzik wrote:
> 
> 
>>If this is for hotswap, as I noted, libata doesn't need this at all.
>>
>>If the hardware supports it, then libata will support it directly. 
>>There is no ACPI-specific magic, because ACPI does nothing but talk to 
>>the same hardware libata is talking to.
> 
> 
> If libata knows how to talk to the random hardware attached to a Dell 
> laptop hotswap bay, I'll be amazed. Ejecting the drive generates a 
> system management interrupt, which then causes the ACPI code to check a 
> register in a block of machine-specific registers and generate an ACPI 
> notification. As far as I can tell, the controller has no say in the 
> matter at all - the Intel specs seem to suggest that ICH6 doesn't 
> generate a hotswap interrupt unless you're using AHCI (which this 
> hardware doesn't).

libata will immediately notice the ejection without ACPI's help.

	Jeff



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 12:16                             ` Jeff Garzik
@ 2005-12-09 12:24                               ` Matthew Garrett
  2005-12-10  0:40                                 ` Jeff Garzik
  0 siblings, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-09 12:24 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Fri, Dec 09, 2005 at 07:16:43AM -0500, Jeff Garzik wrote:

> libata will immediately notice the ejection without ACPI's help.

http://linux.yyz.us/sata/sata-status.html claims that ICH6 doesn't 
support hotswap. The Intel docs seem to say the same thing. Pulling the 
drive out generates an ACPI interrupt but not a PCI one. I'm really 
happy to be wrong here, it's just that everything I've been able to find 
so far suggests that ACPI is the only way to get a notification that the 
drive has gone missing :)

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

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:55                             ` Matthew Garrett
@ 2005-12-09 13:22                               ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 64+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2005-12-09 13:22 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Jens Axboe, Erik Slagter, Jeff Garzik, Randy Dunlap, hch,
	linux-ide, linux-scsi, linux-kernel, acpi-devel

On 12/9/05, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Fri, Dec 09, 2005 at 12:46:42PM +0100, Jens Axboe wrote:
> > On Fri, Dec 09 2005, Erik Slagter wrote:
> > > I case this (still) isn't clear, I am addressing the attitude of "It's
> > > ACPI so it's not going to be used, period".
> >
> > The problem seems to be that you are misunderstanding the 'attitude',
> > which was mainly based on the initial patch sent out which stuffs acpi
> > directly in everywhere. That seems to be a good trigger for curt/direct
> > replies.
>
> I was just following the example set by the ide acpi suspend/resume
> patch, which people didn't seem to object to nearly as much. I guess
> IDE's such a hack anyway that people aren't as worried :) I'll try
> produce a patch that just inserts platform-independent code into scsi
> around the start of next week.

Sigh, it seems this is what you get for being nice... ;)

Don't get it wrong IDE people (at least me) are also worried but
they know that there are cases in which ACPI support will help
(even if the main method should be talking to hardware directly)
NOW not in X years when we will have proper, architecture
independent suspend/resume handling (we can live with
"ide_acpi=on" kernel parameter before it happens happily).

I also pointed out that I would like to have generic code which
can be shared between libata and IDE drivers (after all both
can provide you with struct pci_dev * and port/device number).

It would be even better if this code will stay in drivers/acpi/ata.c
(?) and libata/IDE will just use the same functions (which will turn
to NOP if CONFIG_ACPI=n) for PATA.

Thanks,
Bartlomiej

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 12:24                               ` Matthew Garrett
@ 2005-12-10  0:40                                 ` Jeff Garzik
  2005-12-10  2:34                                   ` Matthew Garrett
  0 siblings, 1 reply; 64+ messages in thread
From: Jeff Garzik @ 2005-12-10  0:40 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

Matthew Garrett wrote:
> On Fri, Dec 09, 2005 at 07:16:43AM -0500, Jeff Garzik wrote:
> 
> 
>>libata will immediately notice the ejection without ACPI's help.
> 
> 
> http://linux.yyz.us/sata/sata-status.html claims that ICH6 doesn't 
> support hotswap. The Intel docs seem to say the same thing. Pulling the 
> drive out generates an ACPI interrupt but not a PCI one. I'm really 
> happy to be wrong here, it's just that everything I've been able to find 
> so far suggests that ACPI is the only way to get a notification that the 
> drive has gone missing :)

ICH6 and ICH7 support it just fine, through the normal SATA PHY 
registers.  ICH5 only support it if you are clever :)

Further, although one can detect hot-unplug on ICH5, hotplug is probably 
not detectable without polling or SMI.

	Jeff



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:27                       ` Jeff Garzik
  2005-12-09 11:35                         ` Erik Slagter
@ 2005-12-10  2:19                         ` Douglas Gilbert
  2005-12-14 20:52                           ` [ACPI] " Matthew Wilcox
  1 sibling, 1 reply; 64+ messages in thread
From: Douglas Gilbert @ 2005-12-10  2:19 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Erik Slagter, Jens Axboe, Randy Dunlap, hch, mjg59, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

Jeff Garzik wrote:
> Erik Slagter wrote:
> 
>> On Fri, 2005-12-09 at 11:39 +0100, Jens Axboe wrote:
>>
>>
>>>> IMHO available infrastructure (and hardware abstraction!) should be
>>>> used
>>>> instead of being stubborn and pretend we know everything about any
>>>> hardware.
>>>
>>>
>>> It's not about being stubborn, it's about maintaining and working on a
>>> clean design. The developers have to do that, not the users. So forgive
>>> people for being a little cautious about shuffling all sorts of ACPI
>>> into the scsi core and/or drivers. We always need to think long term
>>> here.
>>>
>>> Users don't care about the maintainability and cleanliness of the code,
>>> they really just want it to work. Which is perfectly understandable.
>>
>>
>>
>> I perfectly understand that, what I do object against, is rejecting a
>> concept (like this) totally because the developers(?) do not like the
>> mechanism that's used (although ACPI is used everywhere else in the
>> kernel). At least there might be some discussion where this sort of code
>> belongs to make the design clean and easily maintainable, instead of
>> instantly completely rejecting the concept, because OP simply doesn't
>> like acpi.
> 
> 
> Framing the discussion in terms of "like" and "dislike" proves you
> understand nothing about the issues -- and lacking that understanding,
> you feel its best to insult people.

Jeff,
Point of order.

My email record of this thread indicates that it was
Christoph Hellwig:
http://marc.theaimsgroup.com/?l=linux-scsi&m=113404894931377&w=2
who initially lowered the tone. Just to prove that was not
an aberration he followed that up with this email:
http://marc.theaimsgroup.com/?l=linux-scsi&m=113404957727924&w=2
and there are several others. Given Christoph's posts I
find those of Matthew Garrett and Erik Slagter pretty well
considered and I fail to see what warranted the "you feel its
best to insult people" line above.

I used to think that SCSI was the most maligned part of the
linux kernel, but that no longer seems to be the case.
Can ACPI be really that bad ...

Doug Gilbert

> libata suspend/resume needs to work on platforms without ACPI, such as
> ppc64.  libata reset needs to work even when BIOS is not present at all.
>  And by definition, anything that is done in ACPI can be done in the
> driver.
> 
> The only thing libata cannot know is BIOS defaults.  Anything else
> libata doesn't know is a driver bug.  We already know there are a ton of
> settings that should be saved/restored, which is not done now.  Until
> that code is added to libata, talk of ACPI is premature.  Further, even
> the premature patch was obviously unacceptable, because you don't patch
> ACPI code directly into libata and SCSI.  That's the wrong approach.
> 
>     Jeff
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-10  0:40                                 ` Jeff Garzik
@ 2005-12-10  2:34                                   ` Matthew Garrett
  2005-12-10  2:39                                     ` Jeff Garzik
  2005-12-10  2:41                                     ` Jeff Garzik
  0 siblings, 2 replies; 64+ messages in thread
From: Matthew Garrett @ 2005-12-10  2:34 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Fri, Dec 09, 2005 at 07:40:08PM -0500, Jeff Garzik wrote:

> ICH6 and ICH7 support it just fine, through the normal SATA PHY 
> registers.  ICH5 only support it if you are clever :)

ICH6 supports it even in non-AHCI mode? You may want to update the 
website, then :)

> Further, although one can detect hot-unplug on ICH5, hotplug is probably 
> not detectable without polling or SMI.

ACPI allows us to detect hotplug on ICH5, which sounds like a good 
argument for its inclusion.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-10  2:34                                   ` Matthew Garrett
@ 2005-12-10  2:39                                     ` Jeff Garzik
  2005-12-10  2:47                                       ` Matthew Garrett
  2005-12-10  2:41                                     ` Jeff Garzik
  1 sibling, 1 reply; 64+ messages in thread
From: Jeff Garzik @ 2005-12-10  2:39 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

Matthew Garrett wrote:
> On Fri, Dec 09, 2005 at 07:40:08PM -0500, Jeff Garzik wrote:
> 
> 
>>ICH6 and ICH7 support it just fine, through the normal SATA PHY 
>>registers.  ICH5 only support it if you are clever :)
> 
> 
> ICH6 supports it even in non-AHCI mode? You may want to update the 
> website, then :)

Yes, its a bit outdated.  I just found out that ICH6/7 supports access 
to SATA PHY registers even in non-AHCI mode.


>>Further, although one can detect hot-unplug on ICH5, hotplug is probably 
>>not detectable without polling or SMI.
> 
> 
> ACPI allows us to detect hotplug on ICH5, which sounds like a good 
> argument for its inclusion.

One special case (ICH5 hotplug, but not ICH5 hot unplug), when all other 
cases are handled in the normal way?

That's not a good argument at all.

	Jeff



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-10  2:34                                   ` Matthew Garrett
  2005-12-10  2:39                                     ` Jeff Garzik
@ 2005-12-10  2:41                                     ` Jeff Garzik
  2005-12-10  2:50                                       ` Matthew Garrett
  2005-12-12  0:38                                       ` Alan Cox
  1 sibling, 2 replies; 64+ messages in thread
From: Jeff Garzik @ 2005-12-10  2:41 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

Matthew Garrett wrote:
> ACPI allows us to detect hotplug on ICH5, which sounds like a good 
> argument for its inclusion.

Do you even know if this special case is applicable outside of Dell ICH5 
boxes?  And I thought your previous messages were referring to ICH6?

This is sounding more and more like a special-case-of-a-special-case.

	Jeff



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-10  2:39                                     ` Jeff Garzik
@ 2005-12-10  2:47                                       ` Matthew Garrett
  0 siblings, 0 replies; 64+ messages in thread
From: Matthew Garrett @ 2005-12-10  2:47 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Fri, Dec 09, 2005 at 09:39:21PM -0500, Jeff Garzik wrote:
> Matthew Garrett wrote:
> >On Fri, Dec 09, 2005 at 07:40:08PM -0500, Jeff Garzik wrote:
> >
> >
> >>ICH6 and ICH7 support it just fine, through the normal SATA PHY 
> >>registers.  ICH5 only support it if you are clever :)
> >
> >
> >ICH6 supports it even in non-AHCI mode? You may want to update the 
> >website, then :)
> 
> Yes, its a bit outdated.  I just found out that ICH6/7 supports access 
> to SATA PHY registers even in non-AHCI mode.

Oh, cool. That makes life a /lot/ easier - most laptops I've seen using 
SATA are ICH6, so excellent!

> >>Further, although one can detect hot-unplug on ICH5, hotplug is probably 
> >>not detectable without polling or SMI.
> >
> >
> >ACPI allows us to detect hotplug on ICH5, which sounds like a good 
> >argument for its inclusion.
> 
> One special case (ICH5 hotplug, but not ICH5 hot unplug), when all other 
> cases are handled in the normal way?
> 
> That's not a good argument at all.

Well, we could always just add it to ata_piix and leave it out of the 
acpi and scsi layers. I've no great atachment to ACPI - I just want this 
stuff to work :) Basically, ACPI gives us the possibility of making 
hotplug/unplug work on hosts that don't otherwise support it under a 
limited set of conditions. I think that's worthwhile, especially if it 
can be done in a way that doesn't introduce hugely ugly stuff to the 
rest of the kernel.

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

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-10  2:41                                     ` Jeff Garzik
@ 2005-12-10  2:50                                       ` Matthew Garrett
  2005-12-10  2:57                                         ` Jeff Garzik
  2005-12-12  0:38                                       ` Alan Cox
  1 sibling, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-10  2:50 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Fri, Dec 09, 2005 at 09:41:52PM -0500, Jeff Garzik wrote:

> Do you even know if this special case is applicable outside of Dell ICH5 
> boxes?  And I thought your previous messages were referring to ICH6?

Every laptop I have access to, whether it's SATA or PATA, fires an ACPI 
notification when a drive is removed from a bay. The ICH5 case is 
probably somewhat special-cased, but when we move over to driving PATA 
with libata it's going to be a lot more useful. If ICH6 can be managed 
without resorting to ACPI, it's less necessary in the short term, but I 
think PATA support is going to require it in the end anyway (especially 
since we probably want to call the _GTM and _STM methods on PATA 
devices)

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

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-10  2:50                                       ` Matthew Garrett
@ 2005-12-10  2:57                                         ` Jeff Garzik
  2005-12-10  3:47                                           ` [ACPI] " Andrew Grover
  0 siblings, 1 reply; 64+ messages in thread
From: Jeff Garzik @ 2005-12-10  2:57 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Christoph Hellwig, Alan Cox, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

Matthew Garrett wrote:
> On Fri, Dec 09, 2005 at 09:41:52PM -0500, Jeff Garzik wrote:
> 
> 
>>Do you even know if this special case is applicable outside of Dell ICH5 
>>boxes?  And I thought your previous messages were referring to ICH6?
> 
> 
> Every laptop I have access to, whether it's SATA or PATA, fires an ACPI 
> notification when a drive is removed from a bay. The ICH5 case is 
> probably somewhat special-cased, but when we move over to driving PATA 
> with libata it's going to be a lot more useful. If ICH6 can be managed 
> without resorting to ACPI, it's less necessary in the short term, but I 
> think PATA support is going to require it in the end anyway (especially 
> since we probably want to call the _GTM and _STM methods on PATA 
> devices)

Yes, I do agree with this WRT PATA.  Randy Dunlap's ACPI stuff is 
particularly interesting for this, though I haven't had time to review 
it in depth.

I'm a bit more reluctant WRT SATA.

	Jeff



^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [ACPI] Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-10  2:57                                         ` Jeff Garzik
@ 2005-12-10  3:47                                           ` Andrew Grover
  0 siblings, 0 replies; 64+ messages in thread
From: Andrew Grover @ 2005-12-10  3:47 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Matthew Garrett, Christoph Hellwig, Alan Cox, randy_d_dunlap,
	linux-ide, linux-scsi, linux-kernel, acpi-devel

On 12/9/05, Jeff Garzik <jgarzik@pobox.com> wrote:

> Yes, I do agree with this WRT PATA.  Randy Dunlap's ACPI stuff is
> particularly interesting for this, though I haven't had time to review
> it in depth.
>
> I'm a bit more reluctant WRT SATA.

(side note: Shaohua's patch added ACPI support to PATA. Randy's was
the SATA ACPI support.)

ACPI 3.0 specifically mentions SATA and the control methods that it
expects the OS to make use of: _SDD and _GTF. This is needed for
things like HD password unlocking. So, someone needs to be handling
this whenever the SATA drive is reinitialized, such as on resume. So
there's gotta be some SATA ACPI code, somewhere. (And if there is,
then handling the ICH5 ACPI hotplug interrupt seems like maybe
something it should handle, too.)

I'm sure it's possible to properly abstract things so that
arch-neutral code can remain ACPI-unaware -- I just wanted to make it
clear that even if you don't support ICH5 hotplug there are still ACPI
requirements for SATA.

Regards -- Andy

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-09 11:29                 ` Jeff Garzik
@ 2005-12-10  4:01                   ` Mark Lord
  0 siblings, 0 replies; 64+ messages in thread
From: Mark Lord @ 2005-12-10  4:01 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Erik Slagter, Christoph Hellwig, Matthew Garrett, randy_d_dunlap,
	linux-ide, linux-scsi, linux-kernel, acpi-devel

Jeff Garzik wrote:
>
> Patches welcome :)

That's certainly *not* the impression that one is left with
after reading the many responses to the patches that started
this thread.

Cheers

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-10  2:41                                     ` Jeff Garzik
  2005-12-10  2:50                                       ` Matthew Garrett
@ 2005-12-12  0:38                                       ` Alan Cox
  1 sibling, 0 replies; 64+ messages in thread
From: Alan Cox @ 2005-12-12  0:38 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Matthew Garrett, Christoph Hellwig, randy_d_dunlap, linux-ide,
	linux-scsi, linux-kernel, acpi-devel

On Gwe, 2005-12-09 at 21:41 -0500, Jeff Garzik wrote:
> Do you even know if this special case is applicable outside of Dell ICH5 
> boxes?  And I thought your previous messages were referring to ICH6?

Some older thinkpads seem to support this. Also some laptops generate
pnpbios changes. The different methods alone argue for a generic
interface
h

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-08  3:02 RFC: ACPI/scsi/libata integration and hotswap Matthew Garrett
  2005-12-08  9:15 ` Christoph Hellwig
@ 2005-12-13 18:14 ` Randy Dunlap
  2005-12-13 18:26   ` Matthew Garrett
  1 sibling, 1 reply; 64+ messages in thread
From: Randy Dunlap @ 2005-12-13 18:14 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-ide, linux-scsi, linux-kernel, acpi-devel

Hi Matthew,

I have a few comments and a question on this patch, please.
(Yes, I know it won't be merged.)
Most of these are general patch process comments.
However, the last comment is the important one.

1.  I had problems applying it.  What tree is it against?
    Say so in the description.

2.  use diff -p (in SubmittingPatches)

3.  use diffstat

4.  Why 2 diffs against include/linux/libata.h ?
    I was hoping that diffstat would show this, but it just merges
    the 2 libata.h patches together.  'lsdiff' does show this,
    however.

5.  #includes in alpha order as much as possible.

6.  Patch had some trailing whitespace (usually tabs).
    Some tools detect this and warn about it.

7.  Most important:  What good does the ACPI interface do/add?
    What I mean is that acpi_get_child() in scsi_acpi_find_channel()
    always returns a handle value of 0, so it doesn't get us
    any closer to determining the ACPI address (_ADR) of the SATA
    devices.  The acpi_get_devices() technique in my patch (basically
    walking the ACPI namespace, looking at all "devices") is the
    only way that I know of doing this, but I would certainly
    like to find a better way.


On Thu, 8 Dec 2005 03:02:42 +0000
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> Hi!
> 
> The included patch does three things:
> 
> 1) It adds basic support for binding SCSI and SATA devices to ACPI 
> device handles. At the moment this is limited to hosts, and in practice 
> it's probably limited to SATA ones (ACPI doesn't spec how SCSI devices 
> should appear in the DSDT, so I'm guessing that in general they don't). 
> Given a host, you can DEVICE_ACPI_HANDLE(dev) it to get the handle to 
> the ACPI device - this should be handy for implementing suspend 
> functions, since the methods should be in a standard location underneath 
> this.

[snip]

Thanks,
---
~Randy

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-13 18:14 ` Randy Dunlap
@ 2005-12-13 18:26   ` Matthew Garrett
  2005-12-13 19:07     ` Randy Dunlap
  0 siblings, 1 reply; 64+ messages in thread
From: Matthew Garrett @ 2005-12-13 18:26 UTC (permalink / raw)
  To: Randy Dunlap; +Cc: linux-ide, linux-scsi, linux-kernel, acpi-devel

On Tue, Dec 13, 2005 at 10:14:17AM -0800, Randy Dunlap wrote:

> 1.  I had problems applying it.  What tree is it against?
>     Say so in the description.

It was against 2.6.15-git at the time, but I accidently left a hunk of 
stuff from the hotplug patch in there which probably confused things. 
I'll try to rediff it by the end of the week (and do other tidying)

> 7.  Most important:  What good does the ACPI interface do/add?
>     What I mean is that acpi_get_child() in scsi_acpi_find_channel()
>     always returns a handle value of 0, so it doesn't get us
>     any closer to determining the ACPI address (_ADR) of the SATA
>     devices.  The acpi_get_devices() technique in my patch (basically
>     walking the ACPI namespace, looking at all "devices") is the
>     only way that I know of doing this, but I would certainly
>     like to find a better way.

When the PCI bus is registered, acpi walks it and finds the appropriate 
acpi handle for each PCI device. This is shoved in the 
firmware_data field of the device structure. Later on, we register the 
scsi bus. As each item on the bus is added, the acpi callback gets 
called. If it's not an endpoint, scsi_acpi_find_channel gets called. 
We're worried about the host case. The host number will correspond to 
the appropriate _ADR underneath the PCI device that the host is on, so 
we simply get the handle of the PCI device and then ask for the child 
with the appropriate _ADR. That gives us the handle for the device, and 
returning that sticks it back in the child's firmware_data field.

At least, that's how it works here. If acpi_get_child always returns 0 
for you, then it sounds like something's going horribly wrong. Do you 
have a copy of the DSDT?
-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-13 18:26   ` Matthew Garrett
@ 2005-12-13 19:07     ` Randy Dunlap
  0 siblings, 0 replies; 64+ messages in thread
From: Randy Dunlap @ 2005-12-13 19:07 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: linux-ide, linux-scsi, linux-kernel, acpi-devel

On Tue, 13 Dec 2005 18:26:51 +0000
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Tue, Dec 13, 2005 at 10:14:17AM -0800, Randy Dunlap wrote:
> 
> > 7.  Most important:  What good does the ACPI interface do/add?
> >     What I mean is that acpi_get_child() in scsi_acpi_find_channel()
> >     always returns a handle value of 0, so it doesn't get us
> >     any closer to determining the ACPI address (_ADR) of the SATA
> >     devices.  The acpi_get_devices() technique in my patch (basically
> >     walking the ACPI namespace, looking at all "devices") is the
> >     only way that I know of doing this, but I would certainly
> >     like to find a better way.
> 
> When the PCI bus is registered, acpi walks it and finds the appropriate 
> acpi handle for each PCI device. This is shoved in the 
> firmware_data field of the device structure. Later on, we register the 
> scsi bus. As each item on the bus is added, the acpi callback gets 
> called. If it's not an endpoint, scsi_acpi_find_channel gets called. 
> We're worried about the host case. The host number will correspond to 
> the appropriate _ADR underneath the PCI device that the host is on, so 
> we simply get the handle of the PCI device and then ask for the child 
> with the appropriate _ADR. That gives us the handle for the device, and 
> returning that sticks it back in the child's firmware_data field.
> 
> At least, that's how it works here. If acpi_get_child always returns 0 
> for you, then it sounds like something's going horribly wrong. Do you 
> have a copy of the DSDT?

Thanks for the explanation.
The 136 KB DSDT is at:
  http://www.xenotime.net/linux/SATA/acpitbl.out .

---
~Randy

^ permalink raw reply	[flat|nested] 64+ messages in thread

* Re: [ACPI] Re: RFC: ACPI/scsi/libata integration and hotswap
  2005-12-10  2:19                         ` Douglas Gilbert
@ 2005-12-14 20:52                           ` Matthew Wilcox
  0 siblings, 0 replies; 64+ messages in thread
From: Matthew Wilcox @ 2005-12-14 20:52 UTC (permalink / raw)
  To: Douglas Gilbert
  Cc: Jeff Garzik, Erik Slagter, Jens Axboe, Randy Dunlap, hch, mjg59,
	linux-ide, linux-scsi, linux-kernel, acpi-devel

On Sat, Dec 10, 2005 at 12:19:01PM +1000, Douglas Gilbert wrote:
> I used to think that SCSI was the most maligned part of the
> linux kernel, but that no longer seems to be the case.
> Can ACPI be really that bad ...

Yes.

^ permalink raw reply	[flat|nested] 64+ messages in thread

end of thread, other threads:[~2005-12-14 20:52 UTC | newest]

Thread overview: 64+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-12-08  3:02 RFC: ACPI/scsi/libata integration and hotswap Matthew Garrett
2005-12-08  9:15 ` Christoph Hellwig
2005-12-08 13:26   ` Matthew Garrett
2005-12-08 13:33     ` Christoph Hellwig
2005-12-08 13:39       ` Matthew Garrett
2005-12-08 13:44         ` Christoph Hellwig
2005-12-08 17:18           ` Erik Slagter
2005-12-08 20:43             ` Jeff Garzik
2005-12-08 21:03               ` Dominic Ijichi
2005-12-08 21:09                 ` Jeff Garzik
2005-12-08 21:34                   ` Dominic Ijichi
2005-12-08 21:31               ` Randy Dunlap
2005-12-09  9:45                 ` Erik Slagter
2005-12-09 10:39                   ` Jens Axboe
2005-12-09 10:45                     ` Erik Slagter
2005-12-09 11:27                       ` Jeff Garzik
2005-12-09 11:35                         ` Erik Slagter
2005-12-09 11:40                           ` Christoph Hellwig
2005-12-09 11:46                           ` Jens Axboe
2005-12-09 11:55                             ` Matthew Garrett
2005-12-09 13:22                               ` Bartlomiej Zolnierkiewicz
2005-12-09 12:01                             ` Erik Slagter
2005-12-09 12:07                               ` Jens Axboe
2005-12-10  2:19                         ` Douglas Gilbert
2005-12-14 20:52                           ` [ACPI] " Matthew Wilcox
2005-12-09 11:30                       ` Jens Axboe
2005-12-09  3:28               ` Mark Lord
2005-12-09 11:29                 ` Jeff Garzik
2005-12-10  4:01                   ` Mark Lord
2005-12-08 13:52         ` Jeff Garzik
2005-12-08 14:07           ` [ACPI] " Alan Cox
2005-12-08 14:14             ` Jeff Garzik
2005-12-08 14:30               ` Alan Cox
2005-12-08 14:43                 ` Matthew Garrett
2005-12-08 14:53                   ` Alan Cox
2005-12-09 11:42                 ` Jeff Garzik
2005-12-08 14:12           ` Matthew Garrett
2005-12-08 14:01         ` Alan Cox
2005-12-08 14:18           ` Matthew Garrett
2005-12-08 14:33             ` Alan Cox
2005-12-08 14:52               ` Matthew Garrett
2005-12-08 14:55                 ` Alan Cox
2005-12-08 17:19                 ` Matthew Garrett
2005-12-09 11:42                   ` Christoph Hellwig
2005-12-09 11:49                     ` Jeff Garzik
2005-12-09 11:52                       ` Matthew Garrett
2005-12-09 11:58                         ` Jeff Garzik
2005-12-09 12:11                           ` Matthew Garrett
2005-12-09 12:16                             ` Jeff Garzik
2005-12-09 12:24                               ` Matthew Garrett
2005-12-10  0:40                                 ` Jeff Garzik
2005-12-10  2:34                                   ` Matthew Garrett
2005-12-10  2:39                                     ` Jeff Garzik
2005-12-10  2:47                                       ` Matthew Garrett
2005-12-10  2:41                                     ` Jeff Garzik
2005-12-10  2:50                                       ` Matthew Garrett
2005-12-10  2:57                                         ` Jeff Garzik
2005-12-10  3:47                                           ` [ACPI] " Andrew Grover
2005-12-12  0:38                                       ` Alan Cox
2005-12-09 11:50                     ` Matthew Garrett
2005-12-09 11:55                       ` Christoph Hellwig
2005-12-13 18:14 ` Randy Dunlap
2005-12-13 18:26   ` Matthew Garrett
2005-12-13 19:07     ` Randy Dunlap

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).