[PATCH] ide: fix drive side 80c cable check

[PATCH] ide: fix drive side 80c cable check

[Tejun Heo]

The 80c wire bit is bit 13, not 14.  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.

 drivers/ide/ide-iops.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: work/drivers/ide/ide-iops.c
===================================================================
--- work.orig/drivers/ide/ide-iops.c
+++ work/drivers/ide/ide-iops.c
@@ -604,7 +604,7 @@ u8 eighty_ninty_three (ide_drive_t *driv
 	if (!(drive->id->hw_config & 0x6000))
 		return 0;
 #ifndef CONFIG_IDEDMA_IVB
-	if(!(drive->id->hw_config & 0x4000))
+	if (!(drive->id->hw_config & 0x2000))
 		return 0;
 #endif /* CONFIG_IDEDMA_IVB */
 	return 1;

Re: [PATCH] ide: fix drive side 80c cable check

[Alan]

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

> The 80c wire bit is bit 13, not 14.  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.
> 
>  drivers/ide/ide-iops.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: work/drivers/ide/ide-iops.c
> ===================================================================
> --- work.orig/drivers/ide/ide-iops.c
> +++ work/drivers/ide/ide-iops.c
> @@ -604,7 +604,7 @@ u8 eighty_ninty_three (ide_drive_t *driv
>  	if (!(drive->id->hw_config & 0x6000))
>  		return 0;
>  #ifndef CONFIG_IDEDMA_IVB
> -	if(!(drive->id->hw_config & 0x4000))
> +	if (!(drive->id->hw_config & 0x2000))
>  		return 0;
>  #endif /* CONFIG_IDEDMA_IVB */

NAK

While the old code is a mess, your changes don't fix it. The code above
is correct before you touch it as far as I can tell. Incomplete but
correct as far as it went.

The logic in the function as far as it goes is correct

If neither valid bit nor 80pin bit set -> 40pin		[00]
If checking valid bit && valid bit clear -> 40pin	[0x]

The 0x2000 test is needed as an additional test (as per the
ide_ata66_check function directly below)		[10] v [11]

Without this an id value of 0x2000 will trigger 80pin but is not valid.

(the IDEDMA_IVB check is more relaxted to handle some confused ATA4
drives, and is probably something we don't want in libata anyway, or
should blacklist the afflicted for this)

Alan

[PATCH] ide: fix drive side 80c cable check, take 2

[Tejun Heo]

eighty_ninty_three() had word 93 validitity check but not the 80c bit
test itself (bit 12).  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>
---
Ah... thanks.  That explains the code much better.  Fixed accordingly.

diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index badde63..6558055 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -607,6 +607,8 @@ u8 eighty_ninty_three (ide_drive_t *drive)
 	if(!(drive->id->hw_config & 0x4000))
 		return 0;
 #endif /* CONFIG_IDEDMA_IVB */
+	if (!(drive->id->hw_config & 0x2000))
+		return 0;
 	return 1;
 }
 

Re: [PATCH] ide: fix drive side 80c cable check, take 2

[Alan]

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

> eighty_ninty_three() had word 93 validitity check but not the 80c bit
> test itself (bit 12).  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>
> ---
> Ah... thanks.  That explains the code much better.  Fixed accordingly.
> 
> diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
> index badde63..6558055 100644
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -607,6 +607,8 @@ u8 eighty_ninty_three (ide_drive_t *drive)
>  	if(!(drive->id->hw_config & 0x4000))
>  		return 0;
>  #endif /* CONFIG_IDEDMA_IVB */
> +	if (!(drive->id->hw_config & 0x2000))
> +		return 0;
>  	return 1;

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

Re: [PATCH] ide: fix drive side 80c cable check, take 2

[Bartlomiej Zolnierkiewicz]

Alan wrote:
> On Mon, 5 Feb 2007 21:47:13 +0900
> Tejun Heo <htejun@gmail.com> wrote:
> 
>> eighty_ninty_three() had word 93 validitity check but not the 80c bit
>> test itself (bit 12).  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>
>> ---
>> Ah... thanks.  That explains the code much better.  Fixed accordingly.
>>
>> diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
>> index badde63..6558055 100644
>> --- a/drivers/ide/ide-iops.c
>> +++ b/drivers/ide/ide-iops.c
>> @@ -607,6 +607,8 @@ u8 eighty_ninty_three (ide_drive_t *drive)
>>       if(!(drive->id->hw_config & 0x4000))
>>               return 0;
>>  #endif /* CONFIG_IDEDMA_IVB */
>> +     if (!(drive->id->hw_config & 0x2000))
>> +             return 0;
>>       return 1;
> 
> Acked-by: Alan Cox <alan@redhat.com>

applied

Re: [PATCH] ide: fix drive side 80c cable check, take 2

[Sergei Shtylyov]

Hello.

Tejun Heo wrote:

> eighty_ninty_three() had word 93 validitity check but not the 80c bit
> test itself (bit 12).  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>

> diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
> index badde63..6558055 100644
> --- a/drivers/ide/ide-iops.c
> +++ b/drivers/ide/ide-iops.c
> @@ -607,6 +607,8 @@ u8 eighty_ninty_three (ide_drive_t *drive)
>  	if(!(drive->id->hw_config & 0x4000))
>  		return 0;
>  #endif /* CONFIG_IDEDMA_IVB */
> +	if (!(drive->id->hw_config & 0x2000))
> +		return 0;

    Haha, you know just *why* this was wrong? Bit 13 of the word 93 when *set* 
means 40c cable, not 80c!  Look at the table 9 in ATA/PI-6, for example, and 
then into the bit description in the table 27.

>  	return 1;
>  }

MBR, Sergei

Re: [PATCH] ide: fix drive side 80c cable check, take 2

[Sergei Shtylyov]

Hello, I wrote:

>> eighty_ninty_three() had word 93 validitity check but not the 80c bit
>> test itself (bit 12).  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>

>> diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
>> index badde63..6558055 100644
>> --- a/drivers/ide/ide-iops.c
>> +++ b/drivers/ide/ide-iops.c
>> @@ -607,6 +607,8 @@ u8 eighty_ninty_three (ide_drive_t *drive)
>>      if(!(drive->id->hw_config & 0x4000))
>>          return 0;
>>  #endif /* CONFIG_IDEDMA_IVB */
>> +    if (!(drive->id->hw_config & 0x2000))
>> +        return 0;

>    Haha, you know just *why* this was wrong? Bit 13 of the word 93 when 
> *set* means 40c cable, not 80c!  Look at the table 9 in ATA/PI-6, for 
> example, and then into the bit description in the table 27.

    Oops, that's a host side detection, the device side CBLID- seems to have 
the opposite levels. Should have really be looking at the table 8. :-<

>>      return 1;
>>  }

    Actually, trying to find out why 'hdparm -i' clips the modes to udma2 
sometimes...

MBR, Sergei