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 | [email protected]
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.
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 | [email protected]
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.
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 | [email protected]
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.
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
Christoph Hellwig writes:
> 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.
The patch doesn't look like too much of a hack, pretty clean
implementation in support of a hot-swap of an adapter using ACPI hooks.
It is not specific to x86 platforms. I'd prefer that it looked more like
a suspend/resume interface rather than a raw ACPI event.
There needs to be a power management interface to SCSI and to the LLD's
in support of hot-swap of PCI attached devices. Otherwise you are asking
for the driver to be blindsided on any platform that supports this
necessary service feature.
Sincerely -- Mark Salyzyn
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
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
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 | [email protected]
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
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 | [email protected]
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
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
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 | [email protected]
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 | [email protected]
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)
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
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 | [email protected]
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.
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
Quoting Jeff Garzik <[email protected]>:
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
------------------------------------------
This message was penned by the hand of Dom
Dominic Ijichi wrote:
> Quoting Jeff Garzik <[email protected]>:
>
>
>>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
On Thu, 08 Dec 2005 15:43:44 -0500
Jeff Garzik <[email protected]> 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
Quoting Jeff Garzik <[email protected]>:
> Dominic Ijichi wrote:
> > Quoting Jeff Garzik <[email protected]>:
> >
> >
> >>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
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
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).
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
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.
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
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
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
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.
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.
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
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.
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
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
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 | [email protected]
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 | [email protected]
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 | [email protected]
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.
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
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.
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
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 | [email protected]
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
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 | [email protected]
On 12/9/05, Matthew Garrett <[email protected]> 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
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
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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
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 | [email protected]
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
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
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 | [email protected]
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 | [email protected]
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
On 12/9/05, Jeff Garzik <[email protected]> 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
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
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
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 <[email protected]> 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
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 | [email protected]
On Tue, 13 Dec 2005 18:26:51 +0000
Matthew Garrett <[email protected]> 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
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.