[PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check

[PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check

[Tejun Heo]

The 80c wire bit is bit 13, not 14.  Bit 14 is always 1 if word93 is
implemented.  This increases the chance of incorrect wire detection
especially because host side cable detection is often unreliable and
we sometimes soley depend on drive side cable detection.  Fix it.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Please consider for -stable.  This bug has been copied from ide and
it's just amazing how long this has gone unnoticed.  Will post
separate patch for ide.

 include/linux/ata.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: work/include/linux/ata.h
===================================================================
--- work.orig/include/linux/ata.h
+++ work/include/linux/ata.h
@@ -347,7 +347,7 @@ static inline int ata_drive_40wire(const
 {
 	if (ata_id_major_version(dev_id) >= 5 && ata_id_is_sata(dev_id))
 		return 0;	/* SATA */
-	if (dev_id[93] & 0x4000)
+	if (dev_id[93] & (1 << 13))
 		return 0;	/* 80 wire */
 	return 1;
 }

Re: [PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check

[Alan]

On Mon, 5 Feb 2007 16:45:35 +0900
Tejun Heo <htejun@gmail.com> wrote:

> The 80c wire bit is bit 13, not 14.  Bit 14 is always 1 if word93 is
> implemented.  This increases the chance of incorrect wire detection
> especially because host side cable detection is often unreliable and
> we sometimes soley depend on drive side cable detection.  Fix it.

NAK to the fix but not the report

We must check bits 15 and 14 to know if bit 13 is valid. The old code is
wrong (follows the old IDE bits) but your change is probably no better as
you remove the needed validity check.

	if ((dev_id[93] & 0xC000) == 0x4000) && (dev_id[93] & 0x2000))
		return 0;

Alan

[PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check, take 2

[Tejun Heo]

The 80c wire bit is bit 13, not 14.  Bit 14 is always 1 if word93 is
implemented.  This increases the chance of incorrect wire detection
especially because host side cable detection is often unreliable and
we sometimes soley depend on drive side cable detection.  Fix the test
and add word93 validity check.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Thanks for pointing out, fixed accordingly.

diff --git a/include/linux/ata.h b/include/linux/ata.h
index 1df9416..ea2d597 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -347,7 +347,7 @@ static inline int ata_drive_40wire(const u16 *dev_id)
 {
 	if (ata_id_major_version(dev_id) >= 5 && ata_id_is_sata(dev_id))
 		return 0;	/* SATA */
-	if (dev_id[93] & 0x4000)
+	if ((dev_id[93] & 0xC000) == 0x4000 && (dev_id[93] & 0x2000))
 		return 0;	/* 80 wire */
 	return 1;
 }

Re: [PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check, take 2

[Alan]

On Mon, 5 Feb 2007 21:41:06 +0900
Tejun Heo <htejun@gmail.com> wrote:

> The 80c wire bit is bit 13, not 14.  Bit 14 is always 1 if word93 is
> implemented.  This increases the chance of incorrect wire detection
> especially because host side cable detection is often unreliable and
> we sometimes soley depend on drive side cable detection.  Fix the test
> and add word93 validity check.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
> Thanks for pointing out, fixed accordingly.

Acked-by: Alan Cox <alan@redhat.com>

> -	if (dev_id[93] & 0x4000)
> +	if ((dev_id[93] & 0xC000) == 0x4000 && (dev_id[93] & 0x2000))
>  		return 0;	/* 80 wire */
>  	return 1;

Can we do 

	if ((dev_id[93] & 0xE000) == 0x6000)

[PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check, take 3

[Tejun Heo]

The 80c wire bit is bit 13, not 14.  Bit 14 is always 1 if word93 is
implemented.  This increases the chance of incorrect wire detection
especially because host side cable detection is often unreliable and
we sometimes soley depend on drive side cable detection.  Fix the test
and add word93 validity check.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Sure, updated as suggested.

diff --git a/include/linux/ata.h b/include/linux/ata.h
index 1df9416..939be94 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -347,7 +347,7 @@ static inline int ata_drive_40wire(const u16 *dev_id)
 {
 	if (ata_id_major_version(dev_id) >= 5 && ata_id_is_sata(dev_id))
 		return 0;	/* SATA */
-	if (dev_id[93] & 0x4000)
+	if ((dev_id[93] & 0xE000) == 0x6000)
 		return 0;	/* 80 wire */
 	return 1;
 }

Re: [PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check, take 3

[Alan]

On Mon, 5 Feb 2007 23:21:19 +0900
Tejun Heo <htejun@gmail.com> wrote:

> The 80c wire bit is bit 13, not 14.  Bit 14 is always 1 if word93 is
> implemented.  This increases the chance of incorrect wire detection
> especially because host side cable detection is often unreliable and
> we sometimes soley depend on drive side cable detection.  Fix the test
> and add word93 validity check.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
> Sure, updated as suggested.

Acked-by: Alan Cox <alan@redhat.com>

Re: [PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check, take 3

[Jeff Garzik]

Tejun Heo wrote:
> The 80c wire bit is bit 13, not 14.  Bit 14 is always 1 if word93 is
> implemented.  This increases the chance of incorrect wire detection
> especially because host side cable detection is often unreliable and
> we sometimes soley depend on drive side cable detection.  Fix the test
> and add word93 validity check.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> ---
> Sure, updated as suggested.
> 
> diff --git a/include/linux/ata.h b/include/linux/ata.h
> index 1df9416..939be94 100644
> --- a/include/linux/ata.h
> +++ b/include/linux/ata.h
> @@ -347,7 +347,7 @@ static inline int ata_drive_40wire(const u16 *dev_id)
>  {
>  	if (ata_id_major_version(dev_id) >= 5 && ata_id_is_sata(dev_id))
>  		return 0;	/* SATA */
> -	if (dev_id[93] & 0x4000)
> +	if ((dev_id[93] & 0xE000) == 0x6000)
>  		return 0;	/* 80 wire */

applied

Re: [PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check, take 3

[Sergei Shtylyov]

Tejun Heo wrote:

> The 80c wire bit is bit 13, not 14.  Bit 14 is always 1 if word93 is
> implemented.  This increases the chance of incorrect wire detection
> especially because host side cable detection is often unreliable and
> we sometimes soley depend on drive side cable detection.  Fix the test
> and add word93 validity check.

> Signed-off-by: Tejun Heo <htejun@gmail.com>

> diff --git a/include/linux/ata.h b/include/linux/ata.h
> index 1df9416..939be94 100644
> --- a/include/linux/ata.h
> +++ b/include/linux/ata.h
> @@ -347,7 +347,7 @@ static inline int ata_drive_40wire(const u16 *dev_id)
>  {
>  	if (ata_id_major_version(dev_id) >= 5 && ata_id_is_sata(dev_id))
>  		return 0;	/* SATA */
> -	if (dev_id[93] & 0x4000)
> +	if ((dev_id[93] & 0xE000) == 0x6000)
>  		return 0;	/* 80 wire */
>  	return 1;
>  }

    Wrong, wrong as well! The correct value to compare with would be 0x4000.

MBR, Sergei

Re: [PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check, take 3

[Sergei Shtylyov]

Hello, I wrote:

>> The 80c wire bit is bit 13, not 14.  Bit 14 is always 1 if word93 is
>> implemented.  This increases the chance of incorrect wire detection
>> especially because host side cable detection is often unreliable and
>> we sometimes soley depend on drive side cable detection.  Fix the test
>> and add word93 validity check.

>> Signed-off-by: Tejun Heo <htejun@gmail.com>

>> diff --git a/include/linux/ata.h b/include/linux/ata.h
>> index 1df9416..939be94 100644
>> --- a/include/linux/ata.h
>> +++ b/include/linux/ata.h
>> @@ -347,7 +347,7 @@ static inline int ata_drive_40wire(const u16 *dev_id)
>>  {
>>      if (ata_id_major_version(dev_id) >= 5 && ata_id_is_sata(dev_id))
>>          return 0;    /* SATA */
>> -    if (dev_id[93] & 0x4000)
>> +    if ((dev_id[93] & 0xE000) == 0x6000)
>>          return 0;    /* 80 wire */
>>      return 1;
>>  }

>    Wrong, wrong as well! The correct value to compare with would be 0x4000.

    Sorry for the false alarm -- those opposite senses on CBLID- have really 
confused me. :-<

MBR, Sergei

Re: [PATCH libata-dev#upstream-fixes] libata: fix drive side 80c cable check, take 3

[Tejun Heo]

Sergei Shtylyov wrote:
>>    Wrong, wrong as well! The correct value to compare with would be
>> 0x4000.
> 
>    Sorry for the false alarm -- those opposite senses on CBLID- have
> really confused me. :-<

Yeah, all those cable detection mess plays tricks on minds.  PATA is
really screwed up in that area.

-- 
tejun