Re: [PATCH] libata: Fix (hopefully) all the remaining problems with devices failing setup/identify

Re: [PATCH] libata: Fix (hopefully) all the remaining problems with devices failing setup/identify

[Jeff Garzik]

Alan wrote:
> @@ -5142,6 +5174,20 @@
>  	status = ata_chk_status(ap);
>  	if (unlikely(status & ATA_BUSY))
>  		goto idle_irq;
> +		
> +	if (unlikely(qc->tf.command == ATA_CMD_SET_FEATURES && 
> +			qc->tf.feature == SETFEATURES_XFER)) {
> +		/* Let the timings change settle and the drive catch up as
> +		   some hardware needs up to 10uS to get its brain back in
> +		   gear. Taken from the workarounds in drivers/ide done by
> +		   Matthew Faupel/Niccolo Rigacci */
> +		for (i = 0; i < 10; i++) {
> +			if ((status & (ATA_BUSY | ATA_DRQ | ATA_ERR)) == 0)
> +				break;
> +			udelay(1);
> +			status = ata_chk_status(ap);
> +		}


Looks like you should use ata_busy_wait() here, rather than reproducing 
the same code again.

	Jeff

[PATCH] libata: Fix (hopefully) all the remaining problems with devices failing setup/identify

[Alan]

Two fixes in this test patch. One of them allows old CF cards to refuse
pio mode setting, and one to wait for the drive to settle after a set
features changes the speed settings, which is based upon the workarounds
used by drivers/ide.

Please test and report back if you have an afflicted system. This patch
isn't for merging just testing.

The CF card fix will still display errors when the card works (got fixes
for that once I know the cure works) but should then be found/usable.

Not signed off by anyone, not for merging

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-rc6-mm3/drivers/ata/libata-core.c linux-2.6.20-rc6-mm3/drivers/ata/libata-core.c
--- linux.vanilla-2.6.20-rc6-mm3/drivers/ata/libata-core.c	2007-01-31 14:20:39.000000000 +0000
+++ linux-2.6.20-rc6-mm3/drivers/ata/libata-core.c	2007-01-31 16:25:43.000000000 +0000
@@ -2404,6 +2418,10 @@
 		dev->flags |= ATA_DFLAG_PIO;
 
 	err_mask = ata_dev_set_xfermode(dev);
+	/* Old CFA may refuse this command, which is just fine */
+	if (dev->xfer_shift == ATA_SHIFT_PIO && ata_id_is_cfa(dev->id))
+        	err_mask &= ~AC_ERR_DEV;
+
 	if (err_mask) {
 		ata_dev_printk(dev, KERN_ERR, "failed to set xfermode "
 			       "(err_mask=0x%x)\n", err_mask);
@@ -5088,6 +5119,7 @@
 {
 	struct ata_eh_info *ehi = &ap->eh_info;
 	u8 status, host_stat = 0;
+	int i;
 
 	VPRINTK("ata%u: protocol %d task_state %d\n",
 		ap->id, qc->tf.protocol, ap->hsm_task_state);
@@ -5142,6 +5174,20 @@
 	status = ata_chk_status(ap);
 	if (unlikely(status & ATA_BUSY))
 		goto idle_irq;
+		
+	if (unlikely(qc->tf.command == ATA_CMD_SET_FEATURES && 
+			qc->tf.feature == SETFEATURES_XFER)) {
+		/* Let the timings change settle and the drive catch up as
+		   some hardware needs up to 10uS to get its brain back in
+		   gear. Taken from the workarounds in drivers/ide done by
+		   Matthew Faupel/Niccolo Rigacci */
+		for (i = 0; i < 10; i++) {
+			if ((status & (ATA_BUSY | ATA_DRQ | ATA_ERR)) == 0)
+				break;
+			udelay(1);
+			status = ata_chk_status(ap);
+		}
+	}
 
 	/* ack bmdma irq events */
 	ap->ops->irq_clear(ap);

Re: [PATCH] libata: Fix (hopefully) all the remaining problems with devices failing setup/identify

[Alan]

> Looks like you should use ata_busy_wait() here, rather than reproducing 
> the same code again.

It waits in 10uS chunks while 1uS chunks were used in the workaround.
Could indeed do that once I know the fix is right. While I'm at it the
ata_busy_wait kerneldoc is borked so here's a fix

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

--- linux.vanilla-2.6.20-rc6-mm3/include/linux/libata.h	2007-01-31 14:20:43.000000000 +0000
+++ linux-2.6.20-rc6-mm3/include/linux/libata.h	2007-01-31 17:19:04.703229512 +0000
@@ -1052,6 +1053,8 @@
 /**
  *	ata_busy_wait - Wait for a port status register
  *	@ap: Port to wait for.
+ *	@bits: bits that must be clear
+ *	@max: number of 10uS waits to perform
  *
  *	Waits up to max*10 microseconds for the selected bits in the port's
  *	status register to be cleared.

Re: [PATCH] libata: Fix (hopefully) all the remaining problems with devices failing setup/identify

[Jeff Garzik]

Alan wrote:
>> Looks like you should use ata_busy_wait() here, rather than reproducing 
>> the same code again.
> 
> It waits in 10uS chunks while 1uS chunks were used in the workaround.
> Could indeed do that once I know the fix is right. While I'm at it the
> ata_busy_wait kerneldoc is borked so here's a fix
> 
> Signed-off-by: Alan Cox <alan@redhat.com>

applied

Re: [PATCH] libata: Fix (hopefully) all the remaining problems with devices failing setup/identify

[Andrey Borzenkov]

Alan wrote:

> Two fixes in this test patch. One of them allows old CF cards to refuse
> pio mode setting, and one to wait for the drive to settle after a set
> features changes the speed settings, which is based upon the workarounds
> used by drivers/ide.
> 
> Please test and report back if you have an afflicted system. This patch
> isn't for merging just testing.
> 

Was it supposed to fix pata_ali failing to detect CD-ROM in DMA? Because it
did not (2.6.20-rc7 + this patch).

Thank you

-andrey

Re: [PATCH] libata: Fix (hopefully) all the remaining problems with devices failing setup/identify

[Haavard Skinnemoen]

On Wed, 31 Jan 2007 17:26:05 +0000
Alan <alan@lxorguk.ukuu.org.uk> wrote:

> Two fixes in this test patch. One of them allows old CF cards to refuse
> pio mode setting, and one to wait for the drive to settle after a set
> features changes the speed settings, which is based upon the workarounds
> used by drivers/ide.
> 
> Please test and report back if you have an afflicted system. This patch
> isn't for merging just testing.

Just ran into the CF problem you describe when implementing support for
CompactFlash on AVR32 using an old 32 MB TwinMos card. Works fine with
this patch applied; without it, libata gives up after failing to set
xfermode (err_mask=0x1).

> The CF card fix will still display errors when the card works (got fixes
> for that once I know the cure works) but should then be found/usable.

Actually, I don't see any errors, but the card comes up in PIO0 mode
(which is correct behaviour as far as I can tell):

ata1: PATA max PIO0 cmd 0xA4800000 ctl 0xA480000E bmdma 0x0 irq 67
scsi0 : pata_pcmcia
ata1.00: CFA, max PIO0, 62592 sectors: LBA
ata1.00: ata1: dev 0 multi count 0
ata1.00: configured for PIO0
scsi 0:0:0:0: Direct-Access     ATA      32MB CHH         Rev  PQ: 0 ANSI: 5

Thanks for the patch. I would probably have spent a long time debugging
this without it.

Haavard