|
From: Jeff G. <jg...@po...> - 2005-12-09 11:29:25
|
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 |
|
From: Mark L. <li...@rt...> - 2005-12-10 04:01:20
|
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 |
|
From: Matthew G. <mj...@sr...> - 2005-12-09 11:55:31
|
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 | mj...@sr... |
|
From: Bartlomiej Z. <bzo...@gm...> - 2005-12-09 13:22:42
|
On 12/9/05, Matthew Garrett <mj...@sr...> 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=3Don" 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=3Dn) for PATA. Thanks, Bartlomiej |
|
From: Jeff G. <jg...@po...> - 2005-12-08 13:52:41
|
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 |
|
From: Alan C. <al...@lx...> - 2005-12-08 14:08:45
|
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 |
|
From: Jeff G. <jg...@po...> - 2005-12-08 14:15:04
|
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 |
|
From: Alan C. <al...@lx...> - 2005-12-08 14:31:39
|
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 |
|
From: Jeff G. <jg...@po...> - 2005-12-09 11:42:23
|
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 |
|
From: Matthew G. <mj...@sr...> - 2005-12-08 14:13:11
|
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 | mj...@sr... |
|
From: Alan C. <al...@lx...> - 2005-12-08 14:02:38
|
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 |
|
From: Matthew G. <mj...@sr...> - 2005-12-08 14:18:24
|
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 | mj...@sr... |
|
From: Alan C. <al...@lx...> - 2005-12-08 14:34:37
|
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 |
|
From: Matthew G. <mj...@sr...> - 2005-12-08 14:53:17
|
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 | mj...@sr... |
|
From: Alan C. <al...@lx...> - 2005-12-08 14:56:09
|
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 |
|
From: Matthew G. <mj...@sr...> - 2005-12-08 17:19:25
|
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 | mj...@sr... |
|
From: Christoph H. <hc...@in...> - 2005-12-09 11:42:51
|
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. |
|
From: Jeff G. <jg...@po...> - 2005-12-09 11:49:57
|
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 |
|
From: Matthew G. <mj...@sr...> - 2005-12-09 11:52:58
|
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 | mj...@sr... |
|
From: Matthew G. <mj...@sr...> - 2005-12-09 11:50:33
|
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 | mj...@sr... |
|
From: Christoph H. <hc...@in...> - 2005-12-09 11:55:27
|
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. |
|
From: Jeff G. <jg...@po...> - 2005-12-09 11:58:55
|
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 |
|
From: Matthew G. <mj...@sr...> - 2005-12-09 12:11:52
|
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 | mj...@sr... |
|
From: Jeff G. <jg...@po...> - 2005-12-09 12:16:54
|
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 |
|
From: Matthew G. <mj...@sr...> - 2005-12-09 12:25:16
|
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 | mj...@sr... |