From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ric Wheeler Subject: Re: libata EH appears to be NFG up to 2.6.17 (at least). Date: Thu, 06 Jul 2006 19:22:17 -0400 Message-ID: <44AD9B29.9090508@emc.com> References: <44AD749C.1070208@rtr.ca> <44AD76CA.40100@pobox.com> <44AD77D5.90905@rtr.ca> <200607061724.13448.liml@rtr.ca> <44AD8A25.7020006@pobox.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from mexforward.lss.emc.com ([128.222.32.20]:44857 "EHLO mexforward.lss.emc.com") by vger.kernel.org with ESMTP id S1751035AbWGFXWx (ORCPT ); Thu, 6 Jul 2006 19:22:53 -0400 In-Reply-To: <44AD8A25.7020006@pobox.com> Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: Jeff Garzik Cc: Mark Lord , IDE/ATA development list , Jens Axboe Jeff Garzik wrote: > Mark Lord wrote: > >> Enable libata-scsi to report correct sense data on errors. >> >> Signed-off-by: Mark Lord >> --- >> --- linux/drivers/scsi/libata-scsi.c.orig 2006-07-06 >> 17:09:54.000000000 -0400 >> +++ linux/drivers/scsi/libata-scsi.c 2006-07-06 17:17:43.000000000 >> -0400 >> @@ -667,6 +667,13 @@ >> qc->ap->ops->tf_read(qc->ap, tf); >> >> /* >> + * Restore the error bit, which got cleared when the >> + * interrupt handler first read the ata_status. >> + */ >> + if (qc->err_mask & AC_ERR_DEV) >> + tf->command |= ATA_ERR; >> + >> + /* >> * Use ata_to_sense_error() to map status register bits > > > > Again it's an LLDD issue. Your answer of "all LLDDs" sounds a bit > suspicious. > > AC_ERR_DEV should not be set unless (a) some code noticed that ATA_ERR > was already present in the Status register, or (b) some code set > AC_ERR_DEV for some reason other than ATA_ERR presence. Both of those > factors depend heavily on LLDD behavior. > > ANYWAY, your above patch is not wrong, but it begs the question of > where the problem REALLY lies. Which LLDD sets AC_ERR_DEV when > ATA_ERR is not present? > > Jeff > > I believe that Mark was testing this on an ahci (ich6r) based box. A quick (and inexpert) look at ahci.c in 2.6.17 shows that ahci_host_intr() does set AC_ERR_DEV - I did not see in my quick look where ATA_ERR was set. Tejun's updated EH code changed the code path pretty substantially. ric