Re: [PATCH] libata-core: Don't have screaming fits over DF/ERR combinations

[Sergei Shtylyov <sshtylyov@ru.mvista.com>]

Alan Cox wrote:

> Some hardware seems to get this wrong in a non-harmful way, and there are
> some devices that seem to do it deliberately for various reasons.

> Just take it as a device error not a catastrophic state machine
> explosion. 

> Signed-off-by: Alan Cox <alan@redhat.com>
> 
> diff -u --exclude-from /usr/src/exclude --new-file --recursive linux.vanilla-2.6.23-mm1/drivers/ata/libata-core.c linux-2.6.23-mm1/drivers/ata/libata-core.c
> --- linux.vanilla-2.6.23-mm1/drivers/ata/libata-core.c	2007-10-15 15:03:26.000000000 +0100
> +++ linux-2.6.23-mm1/drivers/ata/libata-core.c	2007-10-15 15:13:49.000000000 +0100
> @@ -5258,7 +5319,15 @@
>  		if (unlikely(status & (ATA_ERR | ATA_DF))) {
>  			ata_port_printk(ap, KERN_WARNING, "DRQ=1 with device "
>  					"error, dev_stat 0x%X\n", status);
> -			qc->err_mask |= AC_ERR_HSM;
> +			/* Some devices muck this up. Some follow an ATA
> +			   non-standard that permits the damaged sector to
> +			   be retrieved at this point. The ATA spec says
> +			   we should jump up and down on DRQ + ERR, reality

    I've always thought that setting both DRQ and ERR is perfectly valid 
(well, maybe it's become invalid since ATAPI-4 where all these state 
transition flow charts have made its first appearance, to be quickly replaced 
by the state diagrams :-) -- I'm too lazy to check now... :-)

> +			   says we should be a little more relaxed.
> +
> +			   Rather than an HSM error, take it as a device
> +			   error */

    I'm not sure it's an error in the first place.

> +			qc->err_mask |= AC_ERR_DEV;
>  			ap->hsm_task_state = HSM_ST_ERR;
>  			goto fsm_start;
>  		}

MBR, Sergei