linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Mark Lord <liml@rtr.ca>
To: Tejun Heo <htejun@gmail.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Jeff Garzik <jgarzik@pobox.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	michal.k.k.piotrowski@gmail.com, bryan@arbores.ca,
	linux-kernel@vger.kernel.org, linux-ide@vger.kernel.org
Subject: [PATCH] libata drain fifo on stuck DRQ HSM violation (try#2)
Date: Fri, 28 Sep 2007 09:41:26 -0400	[thread overview]
Message-ID: <46FD0486.3060405@rtr.ca> (raw)
In-Reply-To: <20070928112735.264733ad@the-village.bc.nu>

Alan Cox wrote:
>> Drain up to 512 words from host/bridge FIFO on stuck DRQ HSM violation,
>> rather than just getting stuck there forever.
> 
> Why 512 words ?
> 
> 
>>  	ata_altstatus(ap);
>> -	ata_chk_status(ap);
>> +	ata_drain_fifo(ap, qc);
> 
> ap->ops->cleanup();
> 
> might be wiser

Actually, I belileve we should base it on qc->sect_size instead.

Then, if somebody also would like to submit a patch introducing
a cleanup() method, then please do so!

As a separate patch, though (seems to be the "libata way").
* * * *

Tejun Heo wrote:
> Jeff Garzik wrote:
>> Tejun Heo wrote:
>>> Alan Cox wrote:
>>>>> I think there have been enough cases where this draining was necessary.
>>>>>  IIRC, ata_piix was involved in those cases, right?  If so, can you
>>>>> please submit a patch which applies this only to affected controllers?
>>>>> I don't feel too confident about applying this to all SFF controllers.
>>>> Old IDE does it on all controllers bar a couple. So we have a very good
>>>> knowledge of what does/doesn't work. The one that needs care in old ide
>>>> is an ordering issue where a state machine reset done first causes the
>>>> drain of the I/O to hang.
>>> Hmmm... So, do we apply draining to all PATA?  Or is ata_piix SATA
>>> affected too?
>> I would think all SFF controllers, since a lot of first gen SATA are
>> really bridged solutions.  If they are flagging DRQ, I say oblige them :)
>
> Alright, then the posted patch should be good enough.  Mark, can you be
> bothered to regenerate the patch and post it one more time (again)?  It
> seems we all agree the update is needed.

I think this original patch still applies cleanly on at least 2.6.23-rc7.

Drain up to 512 words from host/bridge FIFO on stuck DRQ HSM violation,
rather than just getting stuck there forever.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

--- old/drivers/ata/libata-sff.c	2007-09-28 09:29:22.000000000 -0400
+++ linux/drivers/ata/libata-sff.c	2007-09-28 09:39:44.000000000 -0400
@@ -420,6 +420,28 @@
 	ap->ops->irq_on(ap);
 }
 
+static void ata_drain_fifo(struct ata_port *ap, struct ata_queued_cmd *qc)
+{
+	u8 stat = ata_chk_status(ap);
+	/*
+	 * Try to clear stuck DRQ if necessary,
+	 * by reading/discarding up to two sectors worth of data.
+	 */
+	if ((stat & ATA_DRQ) && (!qc || qc->dma_dir != DMA_TO_DEVICE)) {
+		unsigned int i;
+		unsigned int limit = qc ? qc->sect_size : ATA_SECT_SIZE;
+
+		printk(KERN_WARNING "Draining up to %u words from data FIFO.\n",
+									limit);
+		for (i = 0; i < limit ; ++i) {
+			ioread16(ap->ioaddr.data_addr);
+			if (!(ata_chk_status(ap) & ATA_DRQ))
+				break;
+		}
+		printk(KERN_WARNING "Drained %u/%u words.\n", i, limit);
+	}
+}
+
 /**
  *	ata_bmdma_drive_eh - Perform EH with given methods for BMDMA controller
  *	@ap: port to handle error for
@@ -476,7 +498,7 @@
 	}
 
 	ata_altstatus(ap);
-	ata_chk_status(ap);
+	ata_drain_fifo(ap, qc);
 	ap->ops->irq_clear(ap);
 
 	spin_unlock_irqrestore(ap->lock, flags);

  reply	other threads:[~2007-09-28 13:41 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <46CFA08E.6090604@arbores.ca>
2007-08-26 23:10 ` Stardom SATA HSM violation Michal Piotrowski
2007-09-03  8:53   ` Tejun Heo
2007-09-05 16:53     ` Andrew Morton
2007-09-05 17:23       ` Mark Lord
2007-09-05 19:38         ` Andrew Morton
2007-09-05 23:03           ` Mark Lord
2007-09-07  0:58         ` Tejun Heo
2007-09-07 13:40           ` Mark Lord
2007-09-27  7:05             ` Tejun Heo
2007-09-27 18:37               ` Alan Cox
2007-09-27 23:32                 ` Tejun Heo
2007-09-27 23:42                   ` Jeff Garzik
2007-09-27 23:52                     ` Tejun Heo
2007-09-28  3:56                       ` [PATCH] libata drain fifo on stuck DRQ " Mark Lord
2007-09-28  9:48                         ` Tejun Heo
2007-09-28  9:56                           ` Andrew Morton
2007-09-28 10:01                             ` Tejun Heo
2007-09-28 10:27                         ` Alan Cox
2007-09-28 13:41                           ` Mark Lord [this message]
2007-09-29  6:24                             ` [PATCH] libata drain fifo on stuck DRQ HSM violation (try#2) Jeff Garzik
2007-09-29  1:05                           ` [PATCH] libata drain fifo on stuck DRQ HSM violation Jeff Garzik
2007-09-29  6:28                             ` Alan Cox
2007-09-29 12:34                               ` Mark Lord
2007-09-28  3:52                   ` Stardom SATA " Mark Lord
2007-09-06 15:00     ` Bryan Woods
2007-09-07  0:58       ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46FD0486.3060405@rtr.ca \
    --to=liml@rtr.ca \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=bryan@arbores.ca \
    --cc=htejun@gmail.com \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.k.k.piotrowski@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).