public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* Re: libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2)
       [not found]       ` <20071108180256.GB3491@havoc.gtf.org>
@ 2007-11-08 18:13         ` Andrew Morton
  2007-11-08 18:19           ` Jeff Garzik
  2007-11-08 18:22           ` Matthew Garrett
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Morton @ 2007-11-08 18:13 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: roppedisano, linux-kernel, linux-ide, rjw, linux-acpi

> On Thu, 8 Nov 2007 13:02:56 -0500 Jeff Garzik <jeff@garzik.org> wrote:
> On Thu, Nov 08, 2007 at 09:49:58AM -0800, Andrew Morton wrote:
> > > On Thu, 08 Nov 2007 17:43:59 +0100 Roberto Oppedisano <roppedisano@infracomspa.it> wrote:
> > > Andrew Morton wrote, On 11/07/2007 09:13 PM:
> > > >> On Wed, 07 Nov 2007 15:15:07 +0100 Roberto Oppedisano <roppedisano@infracomspa.it> wrote:
> > > >> Hello.
> > > >> I noticed that after suspending to ram the DVD-ROM/CDRW
> > > >> drive in no more recognized on my laptop. Looking at dmesg
> > > >> after suspend i found:
> > > >>
> > > >> [    5.313446] ata2.00: _GTF unexpected object type 0x1
> > > >> [    5.313453] ata2.00: ACPI on devcfg failed the second time, disabling 
> > > >> (errno=-22)
> > > >> [    5.313457] ata2.00: revalidation failed (errno=1)
> > > >> [    5.313459] ata2.00: disabled
> > > >>
> > > >>
> > > >> Not sure when the bug was introduced or if it has been always been there
> > > >> (but I can investigate if needed).
> > > >>
> > > >> More details on request.
> > > >>
> > > >> Following dmesg pre and after suspend.
> > > > Yes, it would be useful if you could work ot whether this worked OK in an
> > > > earlier kernel, thanks.
> > > Hello Andrew.
> > > The bug has been recently added.
> > > 
> > > I did a git-bisect, but the result is probably not completely useful,
> > > because many kernel did non build with my config, and I marked them as bad.
> > 
> > Those build errors are bad.  Please report them.  They directly prevented
> > you from finding the commit which caused this regression.
> > 
> > The only way in which we'll stop people doing crap like this is to rub
> > their noses (and the person who committed its nose) in it.
> > 
> > > Anyway here's the output of git-bisect:
> > > 
> > > rao@infra-bb:/usr/src/linux-2.6$ git bisect bad
> > > 99874d50481c093adfe74e796436024d88b6a48c is first bad commit
> > > commit 99874d50481c093adfe74e796436024d88b6a48c
> > > Author: Jens Axboe <jens.axboe@oracle.com>
> > > Date:   Fri Oct 12 12:50:41 2007 +0200
> > > 
> > >     [BLOCK] Only include the compat ioctl code if CONFIG_BLOCK is set
> > > 
> > >     Add an extra CONFIG_BLOCK_COMPAT that we can use in the Makefile
> > > 
> > >     Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
> > > 
> > > :040000 040000 f88b7b0e496edb8fbdd4bc74abd1a742a6a1d6d9
> > > 32ead3bd57b52a664cc8ccb662195041868d7632 M      block
> > > 
> > > ...
> > > 
> > > If needed I can do further investigation, changing the assumption of
> > > efefc6eb38d43b8e5daef482f575d767b002004e to good and see if the bisect
> > > points to a different commit.
> > 
> > Yes, it'll be some other commit.  Sorry.
> > 
> > It would help you (and me) if an ata developer could pay some attention.
> > 
> > Rafael, please track this as an ata regression.
> 
> We unfortunately need to bounce this ACPI people.
> 
> We recently turned on ACPI by default in libata, which INTRODUCES the
> ability of the BIOS to pass random, unknown-quality ATA commands to the
> device.
> 
> I highly expect some breakage related to this...  if ACPI folks
> determine it is not an ACPI bug, then its a firmware bug and we will
> have to avoid that firmware on that platform.
> 
> Set module option "noacpi" to 1, to disable ACPI and see if that fixes
> the problem.
> 
> This will be a common diagnosis and workaround, FWIW...
> 

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


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

* Re: libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2)
  2007-11-08 18:13         ` libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2) Andrew Morton
@ 2007-11-08 18:19           ` Jeff Garzik
  2007-11-08 19:02             ` Andrew Morton
  2007-11-08 18:22           ` Matthew Garrett
  1 sibling, 1 reply; 9+ messages in thread
From: Jeff Garzik @ 2007-11-08 18:19 UTC (permalink / raw)
  To: Andrew Morton; +Cc: roppedisano, linux-kernel, linux-ide, rjw, linux-acpi

On Thu, Nov 08, 2007 at 10:13:41AM -0800, Andrew Morton wrote:
> > On Thu, 8 Nov 2007 13:02:56 -0500 Jeff Garzik <jeff@garzik.org> wrote:
> > On Thu, Nov 08, 2007 at 09:49:58AM -0800, Andrew Morton wrote:
> > > > On Thu, 08 Nov 2007 17:43:59 +0100 Roberto Oppedisano <roppedisano@infracomspa.it> wrote:
> > > > Andrew Morton wrote, On 11/07/2007 09:13 PM:
> > > > >> On Wed, 07 Nov 2007 15:15:07 +0100 Roberto Oppedisano <roppedisano@infracomspa.it> wrote:
> > > > >> Hello.
> > > > >> I noticed that after suspending to ram the DVD-ROM/CDRW
> > > > >> drive in no more recognized on my laptop. Looking at dmesg
> > > > >> after suspend i found:
> > > > >>
> > > > >> [    5.313446] ata2.00: _GTF unexpected object type 0x1
> > > > >> [    5.313453] ata2.00: ACPI on devcfg failed the second time, disabling 
> > > > >> (errno=-22)
> > > > >> [    5.313457] ata2.00: revalidation failed (errno=1)
> > > > >> [    5.313459] ata2.00: disabled
> > > > >>
> > > > >>
> > > > >> Not sure when the bug was introduced or if it has been always been there
> > > > >> (but I can investigate if needed).
> > > > >>
> > > > >> More details on request.
> > > > >>
> > > > >> Following dmesg pre and after suspend.
> > > > > Yes, it would be useful if you could work ot whether this worked OK in an
> > > > > earlier kernel, thanks.
> > > > Hello Andrew.
> > > > The bug has been recently added.
> > > > 
> > > > I did a git-bisect, but the result is probably not completely useful,
> > > > because many kernel did non build with my config, and I marked them as bad.
> > > 
> > > Those build errors are bad.  Please report them.  They directly prevented
> > > you from finding the commit which caused this regression.
> > > 
> > > The only way in which we'll stop people doing crap like this is to rub
> > > their noses (and the person who committed its nose) in it.
> > > 
> > > > Anyway here's the output of git-bisect:
> > > > 
> > > > rao@infra-bb:/usr/src/linux-2.6$ git bisect bad
> > > > 99874d50481c093adfe74e796436024d88b6a48c is first bad commit
> > > > commit 99874d50481c093adfe74e796436024d88b6a48c
> > > > Author: Jens Axboe <jens.axboe@oracle.com>
> > > > Date:   Fri Oct 12 12:50:41 2007 +0200
> > > > 
> > > >     [BLOCK] Only include the compat ioctl code if CONFIG_BLOCK is set
> > > > 
> > > >     Add an extra CONFIG_BLOCK_COMPAT that we can use in the Makefile
> > > > 
> > > >     Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
> > > > 
> > > > :040000 040000 f88b7b0e496edb8fbdd4bc74abd1a742a6a1d6d9
> > > > 32ead3bd57b52a664cc8ccb662195041868d7632 M      block
> > > > 
> > > > ...
> > > > 
> > > > If needed I can do further investigation, changing the assumption of
> > > > efefc6eb38d43b8e5daef482f575d767b002004e to good and see if the bisect
> > > > points to a different commit.
> > > 
> > > Yes, it'll be some other commit.  Sorry.
> > > 
> > > It would help you (and me) if an ata developer could pay some attention.
> > > 
> > > Rafael, please track this as an ata regression.
> > 
> > We unfortunately need to bounce this ACPI people.
> > 
> > We recently turned on ACPI by default in libata, which INTRODUCES the
> > ability of the BIOS to pass random, unknown-quality ATA commands to the
> > device.
> > 
> > I highly expect some breakage related to this...  if ACPI folks
> > determine it is not an ACPI bug, then its a firmware bug and we will
> > have to avoid that firmware on that platform.
> > 
> > Set module option "noacpi" to 1, to disable ACPI and see if that fixes
> > the problem.
> > 
> > This will be a common diagnosis and workaround, FWIW...
> > 
> 
> I suspect it wold be best to disable the feature for the 2.6.24 release,
> then reenable it afterwards and keep doing this until the code is
> sufficiently stable.

Re-read my message :)

The code is stable.  Behavior _by definition_ will vary by BIOS.

This feature (a) enables suspend/resume, but (b) now sends random
unvalidated shite to the device that we hope will work.

Look at all the messages where turning on ACPI in libata _fixed_
suspend/resume (because its obviously required for many, including
laptops).

So it's not an easy "turn it off" answer, you break shitloads of
suspend/resume that way, that we just fixed.

The message "_GTF unexpected object type" indicates a broken BIOS, so
IMO we should proceed in that direction, blacklisting that platform.

	Jeff



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

* Re: libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2)
  2007-11-08 18:13         ` libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2) Andrew Morton
  2007-11-08 18:19           ` Jeff Garzik
@ 2007-11-08 18:22           ` Matthew Garrett
  2007-11-08 18:37             ` [PATCH] Don't fail ata device revalidation for bad _GTF methods Matthew Garrett
  1 sibling, 1 reply; 9+ messages in thread
From: Matthew Garrett @ 2007-11-08 18:22 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jeff Garzik, roppedisano, linux-kernel, linux-ide, rjw,
	linux-acpi

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

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

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

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

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

* [PATCH] Don't fail ata device revalidation for bad _GTF methods
  2007-11-08 18:22           ` Matthew Garrett
@ 2007-11-08 18:37             ` Matthew Garrett
  2007-11-09 12:29               ` Roberto Oppedisano
  2007-11-10  5:32               ` Jeff Garzik
  0 siblings, 2 replies; 9+ messages in thread
From: Matthew Garrett @ 2007-11-08 18:37 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jeff Garzik, roppedisano, linux-kernel, linux-ide, rjw,
	linux-acpi

Experience suggests that the _GTF method may be bad. We currently fail 
device revalidation in that case, which seems excessive.

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

---

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

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

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

* Re: libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2)
  2007-11-08 18:19           ` Jeff Garzik
@ 2007-11-08 19:02             ` Andrew Morton
  2007-11-08 19:49               ` Mark Lord
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Morton @ 2007-11-08 19:02 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: roppedisano, linux-kernel, linux-ide, rjw, linux-acpi

> On Thu, 8 Nov 2007 13:19:05 -0500 Jeff Garzik <jeff@garzik.org> wrote:
> On Thu, Nov 08, 2007 at 10:13:41AM -0800, Andrew Morton wrote:
> > > On Thu, 8 Nov 2007 13:02:56 -0500 Jeff Garzik <jeff@garzik.org> wrote:
> > > On Thu, Nov 08, 2007 at 09:49:58AM -0800, Andrew Morton wrote:
> > > > > On Thu, 08 Nov 2007 17:43:59 +0100 Roberto Oppedisano <roppedisano@infracomspa.it> wrote:
> > > > > Andrew Morton wrote, On 11/07/2007 09:13 PM:
> > > > > >> On Wed, 07 Nov 2007 15:15:07 +0100 Roberto Oppedisano <roppedisano@infracomspa.it> wrote:
> > > > > >> Hello.
> > > > > >> I noticed that after suspending to ram the DVD-ROM/CDRW
> > > > > >> drive in no more recognized on my laptop. Looking at dmesg
> > > > > >> after suspend i found:
> > > > > >>
> > > > > >> [    5.313446] ata2.00: _GTF unexpected object type 0x1
> > > > > >> [    5.313453] ata2.00: ACPI on devcfg failed the second time, disabling 
> > > > > >> (errno=-22)
> > > > > >> [    5.313457] ata2.00: revalidation failed (errno=1)
> > > > > >> [    5.313459] ata2.00: disabled
> > > > > >>
> > > > > >>
> > > > > >> Not sure when the bug was introduced or if it has been always been there
> > > > > >> (but I can investigate if needed).
> > > > > >>
> > > > > >> More details on request.
> > > > > >>
> > > > > >> Following dmesg pre and after suspend.
> > > > > > Yes, it would be useful if you could work ot whether this worked OK in an
> > > > > > earlier kernel, thanks.
> > > > > Hello Andrew.
> > > > > The bug has been recently added.
> > > > > 
> > > > > I did a git-bisect, but the result is probably not completely useful,
> > > > > because many kernel did non build with my config, and I marked them as bad.
> > > > 
> > > > Those build errors are bad.  Please report them.  They directly prevented
> > > > you from finding the commit which caused this regression.
> > > > 
> > > > The only way in which we'll stop people doing crap like this is to rub
> > > > their noses (and the person who committed its nose) in it.
> > > > 
> > > > > Anyway here's the output of git-bisect:
> > > > > 
> > > > > rao@infra-bb:/usr/src/linux-2.6$ git bisect bad
> > > > > 99874d50481c093adfe74e796436024d88b6a48c is first bad commit
> > > > > commit 99874d50481c093adfe74e796436024d88b6a48c
> > > > > Author: Jens Axboe <jens.axboe@oracle.com>
> > > > > Date:   Fri Oct 12 12:50:41 2007 +0200
> > > > > 
> > > > >     [BLOCK] Only include the compat ioctl code if CONFIG_BLOCK is set
> > > > > 
> > > > >     Add an extra CONFIG_BLOCK_COMPAT that we can use in the Makefile
> > > > > 
> > > > >     Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
> > > > > 
> > > > > :040000 040000 f88b7b0e496edb8fbdd4bc74abd1a742a6a1d6d9
> > > > > 32ead3bd57b52a664cc8ccb662195041868d7632 M      block
> > > > > 
> > > > > ...
> > > > > 
> > > > > If needed I can do further investigation, changing the assumption of
> > > > > efefc6eb38d43b8e5daef482f575d767b002004e to good and see if the bisect
> > > > > points to a different commit.
> > > > 
> > > > Yes, it'll be some other commit.  Sorry.
> > > > 
> > > > It would help you (and me) if an ata developer could pay some attention.
> > > > 
> > > > Rafael, please track this as an ata regression.
> > > 
> > > We unfortunately need to bounce this ACPI people.
> > > 
> > > We recently turned on ACPI by default in libata, which INTRODUCES the
> > > ability of the BIOS to pass random, unknown-quality ATA commands to the
> > > device.
> > > 
> > > I highly expect some breakage related to this...  if ACPI folks
> > > determine it is not an ACPI bug, then its a firmware bug and we will
> > > have to avoid that firmware on that platform.
> > > 
> > > Set module option "noacpi" to 1, to disable ACPI and see if that fixes
> > > the problem.
> > > 
> > > This will be a common diagnosis and workaround, FWIW...
> > > 
> > 
> > I suspect it wold be best to disable the feature for the 2.6.24 release,
> > then reenable it afterwards and keep doing this until the code is
> > sufficiently stable.
> 
> Re-read my message :)
> 
> The code is stable.  Behavior _by definition_ will vary by BIOS.
> 
> This feature (a) enables suspend/resume, but (b) now sends random
> unvalidated shite to the device that we hope will work.
> 
> Look at all the messages where turning on ACPI in libata _fixed_
> suspend/resume (because its obviously required for many, including
> laptops).

We fixed a somewhat-known number of machines and broke an unknown number. 
Linus will come after you with a pointy stick if he finds out.

Fixing previously-broken machines is nice, but breaking previously-working
ones gets people a lot more upset.

> So it's not an easy "turn it off" answer, you break shitloads of
> suspend/resume that way, that we just fixed.
> 
> The message "_GTF unexpected object type" indicates a broken BIOS, so
> IMO we should proceed in that direction, blacklisting that platform.
> 

Suggest that the feature be disabled until we have most of these
blacklistings in place.

We have a tremendous number of regressions in 2.6.24 (and they're just the
ones which we know of) and the regression reports for 2.6.23 are still
coming in.  At some stage we'll need to get seriosu about this.

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

* Re: libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2)
  2007-11-08 19:02             ` Andrew Morton
@ 2007-11-08 19:49               ` Mark Lord
  2007-11-08 19:58                 ` Andrew Morton
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Lord @ 2007-11-08 19:49 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jeff Garzik, roppedisano, linux-kernel, linux-ide, rjw,
	linux-acpi

Andrew Morton wrote:
>> On Thu, 8 Nov 2007 13:19:05 -0500 Jeff Garzik <jeff@garzik.org> wrote:
>> On Thu, Nov 08, 2007 at 10:13:41AM -0800, Andrew Morton wrote:
>>>> On Thu, 8 Nov 2007 13:02:56 -0500 Jeff Garzik <jeff@garzik.org> wrote:
>>>> On Thu, Nov 08, 2007 at 09:49:58AM -0800, Andrew Morton wrote:
..
>>> I suspect it wold be best to disable the feature for the 2.6.24 release,
>>> then reenable it afterwards and keep doing this until the code is
>>> sufficiently stable.
>> Re-read my message :)
>>
>> The code is stable.  Behavior _by definition_ will vary by BIOS.
>>
>> This feature (a) enables suspend/resume, but (b) now sends random
>> unvalidated shite to the device that we hope will work.
>>
>> Look at all the messages where turning on ACPI in libata _fixed_
>> suspend/resume (because its obviously required for many, including
>> laptops).
> 
> We fixed a somewhat-known number of machines and broke an unknown number. 
> Linus will come after you with a pointy stick if he finds out.
> 
> Fixing previously-broken machines is nice, but breaking previously-working
> ones gets people a lot more upset.
> 
>> So it's not an easy "turn it off" answer, you break shitloads of
>> suspend/resume that way, that we just fixed.
>>
>> The message "_GTF unexpected object type" indicates a broken BIOS, so
>> IMO we should proceed in that direction, blacklisting that platform.
>>
> 
> Suggest that the feature be disabled until we have most of these
> blacklistings in place.
..

The problem is, this code has already sat out the last release,
and nobody noticed problems exactly because it was not enabled before.

If Jeff disables it again, then it will sit out another cycle without
anybody exercising it.  At some point, we need to turn it on, and collect
information about where there are problems (and fix them).

Tricky, that.

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

* Re: libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2)
  2007-11-08 19:49               ` Mark Lord
@ 2007-11-08 19:58                 ` Andrew Morton
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Morton @ 2007-11-08 19:58 UTC (permalink / raw)
  To: Mark Lord; +Cc: jeff, roppedisano, linux-kernel, linux-ide, rjw, linux-acpi

> On Thu, 08 Nov 2007 14:49:33 -0500 Mark Lord <liml@rtr.ca> wrote:
> Andrew Morton wrote:
> >> On Thu, 8 Nov 2007 13:19:05 -0500 Jeff Garzik <jeff@garzik.org> wrote:
> >> On Thu, Nov 08, 2007 at 10:13:41AM -0800, Andrew Morton wrote:
> >>>> On Thu, 8 Nov 2007 13:02:56 -0500 Jeff Garzik <jeff@garzik.org> wrote:
> >>>> On Thu, Nov 08, 2007 at 09:49:58AM -0800, Andrew Morton wrote:
> ..
> >>> I suspect it wold be best to disable the feature for the 2.6.24 release,
> >>> then reenable it afterwards and keep doing this until the code is
> >>> sufficiently stable.
> >> Re-read my message :)
> >>
> >> The code is stable.  Behavior _by definition_ will vary by BIOS.
> >>
> >> This feature (a) enables suspend/resume, but (b) now sends random
> >> unvalidated shite to the device that we hope will work.
> >>
> >> Look at all the messages where turning on ACPI in libata _fixed_
> >> suspend/resume (because its obviously required for many, including
> >> laptops).
> > 
> > We fixed a somewhat-known number of machines and broke an unknown number. 
> > Linus will come after you with a pointy stick if he finds out.
> > 
> > Fixing previously-broken machines is nice, but breaking previously-working
> > ones gets people a lot more upset.
> > 
> >> So it's not an easy "turn it off" answer, you break shitloads of
> >> suspend/resume that way, that we just fixed.
> >>
> >> The message "_GTF unexpected object type" indicates a broken BIOS, so
> >> IMO we should proceed in that direction, blacklisting that platform.
> >>
> > 
> > Suggest that the feature be disabled until we have most of these
> > blacklistings in place.
> ..
> 
> The problem is, this code has already sat out the last release,
> and nobody noticed problems exactly because it was not enabled before.
> 
> If Jeff disables it again, then it will sit out another cycle without
> anybody exercising it.  At some point, we need to turn it on, and collect
> information about where there are problems (and fix them).
> 

We get a decent amount of testing during the -rc's.  I think it's OK to turn
a feature on during -rc and off for release while it gets settled in.

Hopefully Matthew's fix will address this particular problem.

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

* Re: [PATCH] Don't fail ata device revalidation for bad _GTF methods
  2007-11-08 18:37             ` [PATCH] Don't fail ata device revalidation for bad _GTF methods Matthew Garrett
@ 2007-11-09 12:29               ` Roberto Oppedisano
  2007-11-10  5:32               ` Jeff Garzik
  1 sibling, 0 replies; 9+ messages in thread
From: Roberto Oppedisano @ 2007-11-09 12:29 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Andrew Morton, Jeff Garzik, linux-kernel, linux-ide, rjw,
	linux-acpi

I can confirm that this patch solves the problem with s2ram on my laptop.

R

Matthew Garrett wrote:
> Experience suggests that the _GTF method may be bad. We currently fail 
> device revalidation in that case, which seems excessive.
>
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>
>
> ---
>
> diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
> index 08a52dd..545ea86 100644
> --- a/drivers/ata/libata-acpi.c
> +++ b/drivers/ata/libata-acpi.c
> @@ -312,7 +312,7 @@ EXPORT_SYMBOL_GPL(ata_acpi_stm);
>   *
>   * RETURNS:
>   * Number of taskfiles on success, 0 if _GTF doesn't exist or doesn't
> - * contain valid data.  -errno on other errors.
> + * contain valid data.
>   */
>  static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
>  			   void **ptr_to_free)
> @@ -339,7 +339,6 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
>  			ata_dev_printk(dev, KERN_WARNING,
>  				       "_GTF evaluation failed (AE 0x%x)\n",
>  				       status);
> -			rc = -EIO;
>  		}
>  		goto out_free;
>  	}
> @@ -359,7 +358,6 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
>  		ata_dev_printk(dev, KERN_WARNING,
>  			       "_GTF unexpected object type 0x%x\n",
>  			       out_obj->type);
> -		rc = -EINVAL;
>  		goto out_free;
>  	}
>  
> @@ -367,7 +365,6 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
>  		ata_dev_printk(dev, KERN_WARNING,
>  			       "unexpected _GTF length (%d)\n",
>  			       out_obj->buffer.length);
> -		rc = -EINVAL;
>  		goto out_free;
>  	}
>  
> @@ -511,10 +508,7 @@ static int ata_acpi_exec_tfs(struct ata_device *dev)
>  	int gtf_count, i, rc;
>  
>  	/* get taskfiles */
> -	rc = ata_dev_get_GTF(dev, &gtf, &ptr_to_free);
> -	if (rc < 0)
> -		return rc;
> -	gtf_count = rc;
> +	gtf_count = ata_dev_get_GTF(dev, &gtf, &ptr_to_free);
>  
>  	/* execute them */
>  	for (i = 0, rc = 0; i < gtf_count; i++) {
>   

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

* Re: [PATCH] Don't fail ata device revalidation for bad _GTF methods
  2007-11-08 18:37             ` [PATCH] Don't fail ata device revalidation for bad _GTF methods Matthew Garrett
  2007-11-09 12:29               ` Roberto Oppedisano
@ 2007-11-10  5:32               ` Jeff Garzik
  1 sibling, 0 replies; 9+ messages in thread
From: Jeff Garzik @ 2007-11-10  5:32 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Andrew Morton, roppedisano, linux-kernel, linux-ide, rjw,
	linux-acpi

Matthew Garrett wrote:
> Experience suggests that the _GTF method may be bad. We currently fail 
> device revalidation in that case, which seems excessive.
> 
> Signed-off-by: Matthew Garrett <mjg59@srcf.ucam.org>

applied



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

end of thread, other threads:[~2007-11-10  5:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <4731C86B.1040704@infracomspa.it>
     [not found] ` <20071107121355.5eaf4496.akpm@linux-foundation.org>
     [not found]   ` <47333CCF.1050403@infracomspa.it>
     [not found]     ` <20071108094958.5c9b1b22.akpm@linux-foundation.org>
     [not found]       ` <20071108180256.GB3491@havoc.gtf.org>
2007-11-08 18:13         ` libata: cdrw/dvdrom disabed after s2ram (2.6.24-rc2) Andrew Morton
2007-11-08 18:19           ` Jeff Garzik
2007-11-08 19:02             ` Andrew Morton
2007-11-08 19:49               ` Mark Lord
2007-11-08 19:58                 ` Andrew Morton
2007-11-08 18:22           ` Matthew Garrett
2007-11-08 18:37             ` [PATCH] Don't fail ata device revalidation for bad _GTF methods Matthew Garrett
2007-11-09 12:29               ` Roberto Oppedisano
2007-11-10  5:32               ` Jeff Garzik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox