[PATCH #upstream-fixes] libata-acpi: missing _SDD is not an error

[PATCH #upstream-fixes] libata-acpi: missing _SDD is not an error

[Tejun Heo]

Missing _SDD is not an error.  Don't treat it as one.

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Takashi Iwai <tiwai@suse.de>
---
 drivers/ata/libata-acpi.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index b0882cd..1245838 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -807,12 +807,11 @@ static int ata_acpi_exec_tfs(struct ata_device *dev, int *nr_executed)
  * EH context.
  *
  * RETURNS:
- * 0 on success, -errno on failure.
+ * 0 on success, -ENOENT if _SDD doesn't exist, -errno on failure.
  */
 static int ata_acpi_push_id(struct ata_device *dev)
 {
 	struct ata_port *ap = dev->link->ap;
-	int err;
 	acpi_status status;
 	struct acpi_object_list input;
 	union acpi_object in_params[1];
@@ -835,12 +834,16 @@ static int ata_acpi_push_id(struct ata_device *dev)
 	status = acpi_evaluate_object(dev->acpi_handle, "_SDD", &input, NULL);
 	swap_buf_le16(dev->id, ATA_ID_WORDS);
 
-	err = ACPI_FAILURE(status) ? -EIO : 0;
-	if (err < 0)
+	if (status == AE_NOT_FOUND)
+		return -ENOENT;
+
+	if (ACPI_FAILURE(status)) {
 		ata_dev_printk(dev, KERN_WARNING,
 			       "ACPI _SDD failed (AE 0x%x)\n", status);
+		return -EIO;
+	}
 
-	return err;
+	return 0;
 }
 
 /**
@@ -971,7 +974,7 @@ int ata_acpi_on_devcfg(struct ata_device *dev)
 	/* do _SDD if SATA */
 	if (acpi_sata) {
 		rc = ata_acpi_push_id(dev);
-		if (rc)
+		if (rc && rc != -ENOENT)
 			goto acpi_err;
 	}
 

Re: [PATCH #upstream-fixes] libata-acpi: missing _SDD is not an error

[Jeff Garzik]

On 11/18/2009 08:24 AM, Tejun Heo wrote:
> Missing _SDD is not an error.  Don't treat it as one.
>
> Signed-off-by: Tejun Heo<tj@kernel.org>
> Reported-by: Takashi Iwai<tiwai@suse.de>
> ---
>   drivers/ata/libata-acpi.c |   15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)

applied

Re: [PATCH #upstream-fixes] libata-acpi: missing _SDD is not an error

[Jeff Garzik]

On 11/18/2009 08:24 AM, Tejun Heo wrote:
> Missing _SDD is not an error.  Don't treat it as one.
>
> Signed-off-by: Tejun Heo<tj@kernel.org>
> Reported-by: Takashi Iwai<tiwai@suse.de>
> ---
>   drivers/ata/libata-acpi.c |   15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)


hmmmm, do you have a link to the bug report?

How critical is this?  Is it a regression fix?

We are very late into 2.6.32-rc, where we try to minimize the patches 
applied as much as possible.

	Jeff

Re: [PATCH #upstream-fixes] libata-acpi: missing _SDD is not an error

[Takashi Iwai]

At Fri, 20 Nov 2009 03:48:22 -0500,
Jeff Garzik wrote:
> 
> On 11/18/2009 08:24 AM, Tejun Heo wrote:
> > Missing _SDD is not an error.  Don't treat it as one.
> >
> > Signed-off-by: Tejun Heo<tj@kernel.org>
> > Reported-by: Takashi Iwai<tiwai@suse.de>
> > ---
> >   drivers/ata/libata-acpi.c |   15 +++++++++------
> >   1 file changed, 9 insertions(+), 6 deletions(-)
> 
> 
> hmmmm, do you have a link to the bug report?

http://bugzilla.novell.com/show_bug.cgi?id=551942

But the access is restricted because of SLE*11 stuff (and the machine
is too new :)

I can forward dmesg of any other information, but ...

> How critical is this?  Is it a regression fix?

... it's neither critical nor regression fix.
Essentially it suppresses the unneeded warning messages at boot
(and gives a more clear error code).

> We are very late into 2.6.32-rc, where we try to minimize the patches 
> applied as much as possible.

It's fine for 2.6.33, I suppose.


thanks,

Takashi

Re: [PATCH #upstream-fixes] libata-acpi: missing _SDD is not an error

[Jeff Garzik]

On 11/20/2009 03:56 AM, Takashi Iwai wrote:
> At Fri, 20 Nov 2009 03:48:22 -0500,
>> We are very late into 2.6.32-rc, where we try to minimize the patches
>> applied as much as possible.
>
> It's fine for 2.6.33, I suppose.

OK, thanks!

	Jeff

Re: [PATCH #upstream-fixes] libata-acpi: missing _SDD is not an error

[Tejun Heo]

Hello,

11/20/2009 05:48 PM, Jeff Garzik wrote:
> On 11/18/2009 08:24 AM, Tejun Heo wrote:
>> Missing _SDD is not an error.  Don't treat it as one.
>>
>> Signed-off-by: Tejun Heo<tj@kernel.org>
>> Reported-by: Takashi Iwai<tiwai@suse.de>
>> ---
>>   drivers/ata/libata-acpi.c |   15 +++++++++------
>>   1 file changed, 9 insertions(+), 6 deletions(-)
> 
> 
> hmmmm, do you have a link to the bug report?

Unfortunately, it's from novell internal bugzilla entry, but the
following is how it looks like without the patch.

[   15.720046] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[   16.540846] ata1.00: ACPI _SDD failed (AE 0x5)
[   16.553907] ata1.00: ACPI: failed the second time, disabled
[   16.567311] ata1.00: configured for UDMA/100

> How critical is this?  Is it a regression fix?

It's not critical.  libata-acpi code will give up after a couple of
tries and just turn off ACPI and is not a regression.

> We are very late into 2.6.32-rc, where we try to minimize the patches
> applied as much as possible.

This can go into #upstream then.  No biggie.

Thanks.

-- 
tejun