[PATCH 1/6] qd65xx: fix PIO mode selection

[PATCH 1/6] qd65xx: fix PIO mode selection

[Bartlomiej Zolnierkiewicz]

PIO4 is a maximum PIO mode supported by a driver.  Using "255" as a max_mode
argument to ide_get_best_pio_mode() could result in wrong timings being used
by a driver (for "pio" equal to 5) or OOPS (for "pio" values > 5 && < 255).

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/legacy/qd65xx.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: b/drivers/ide/legacy/qd65xx.c
===================================================================
--- a/drivers/ide/legacy/qd65xx.c
+++ b/drivers/ide/legacy/qd65xx.c
@@ -258,8 +258,7 @@ static void qd6580_tune_drive (ide_drive
 	int recovery_time = 415; /* worst case values from the dos driver */
 
 	if (drive->id && !qd_find_disk_type(drive, &active_time, &recovery_time)) {
-		pio = ide_get_best_pio_mode(drive, pio, 255, &d);
-		pio = min_t(u8, pio, 4);
+		pio = ide_get_best_pio_mode(drive, pio, 4, &d);
 
 		switch (pio) {
 			case 0: break;

Re: [PATCH 1/6] qd65xx: fix PIO mode selection

[Sergei Shtylyov]

Bartlomiej Zolnierkiewicz wrote:
> PIO4 is a maximum PIO mode supported by a driver.  Using "255" as a max_mode
> argument to ide_get_best_pio_mode() could result in wrong timings being used
> by a driver (for "pio" equal to 5) or OOPS (for "pio" values > 5 && < 255).
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

> ---
>  drivers/ide/legacy/qd65xx.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> Index: b/drivers/ide/legacy/qd65xx.c
> ===================================================================
> --- a/drivers/ide/legacy/qd65xx.c
> +++ b/drivers/ide/legacy/qd65xx.c
> @@ -258,8 +258,7 @@ static void qd6580_tune_drive (ide_drive
>  	int recovery_time = 415; /* worst case values from the dos driver */
>  
>  	if (drive->id && !qd_find_disk_type(drive, &active_time, &recovery_time)) {
> -		pio = ide_get_best_pio_mode(drive, pio, 255, &d);
> -		pio = min_t(u8, pio, 4);
> +		pio = ide_get_best_pio_mode(drive, pio, 4, &d);
>  
>  		switch (pio) {
>  			case 0: break;

MBR, Sergei

Re: [PATCH 1/6] qd65xx: fix PIO mode selection

[Alan Cox]

On Sat, 23 Jun 2007 20:02:07 +0200
Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> wrote:

> 
> PIO4 is a maximum PIO mode supported by a driver.  Using "255" as a max_mode
> argument to ide_get_best_pio_mode() could result in wrong timings being used
> by a driver (for "pio" equal to 5) or OOPS (for "pio" values > 5 && < 255).

The qdi6580 driver is somewhat hosed. The libata one worked on the single
VLB test case we managed to rig up.  Registers are in VLB clocks (usually
33MHz). I need to fix the failed single channel test case then persuade
the VLB using nutter to test again 8)

Registers you need are:

2 * timing:
	bit 0-3 active in VLB clocks (whacky as its 17 - bitvalue)
	bit 4-7 recover (15 - value) [don't use < 2 ]

3
	bit 0 Pri/Sec enabled
	bit 1 Primary enabled only
	bit 2-6 write as 1 
	bit 7 - Prefetch/Postwrite (ATA disk only) [this is global]


The timing stuff is a bit whacko:

If both channels are in use then timing source 0 is channel 0, timing
source 1 is channel 1. If single channel is in use then timing source 0
is channel 0 primary, and timing source 1 is channel 0 slave

Possibly we should just drop support as I know of only one user and thats
purely to test the code

Alan

Re: [PATCH 1/6] qd65xx: fix PIO mode selection

[Sergei Shtylyov]

Hello.

Alan Cox wrote:

>>PIO4 is a maximum PIO mode supported by a driver.  Using "255" as a max_mode
>>argument to ide_get_best_pio_mode() could result in wrong timings being used
>>by a driver (for "pio" equal to 5) or OOPS (for "pio" values > 5 && < 255).

> The qdi6580 driver is somewhat hosed. The libata one worked on the single

[...]

> Possibly we should just drop support as I know of only one user and thats
> purely to test the code

    I remember actually using this piece of [insert yourself] back in 199[45], 
not in Linux though... :-)

> Alan

MBR, Sergei

Re: [PATCH 1/6] qd65xx: fix PIO mode selection

[Bartlomiej Zolnierkiewicz]

On Saturday 23 June 2007, Alan Cox wrote:

> Possibly we should just drop support as I know of only one user and thats
> purely to test the code

Fine with me, please send a patch.

PS Added yours "Reviewed-by:" to this patch as I still need to push it
(I have other patches in the queue depending on it).

Thanks,
Bart