All of lore.kernel.org
 help / color / mirror / Atom feed
From: Albert Lee <albertcc@tw.ibm.com>
To: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>,
	Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Linux IDE <linux-ide@vger.kernel.org>,
	"Adam W. Hawks" <awhawks@us.ibm.com>
Subject: [PATCH 2/2] ide: clear bmdma status in ide_intr() (revised)
Date: Thu, 25 Jan 2007 17:31:28 +0800	[thread overview]
Message-ID: <45B878F0.7060707@tw.ibm.com> (raw)
In-Reply-To: <45B7596F.6080504@ru.mvista.com>

patch 2/2 (revised):
- Do the dma status clearing in ide_intr() and add a new hwif->ide_dma_clear_irq such that LLDD can override it.
- Fix drive->waiting_for_dma to work with CDB-intr devices.

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
hwif->dma is not reliable: ide_intr() races with dma_start(). Sometimes we got hwif->dma == 0
in ide_intr() even though it is actually a DMA interrupt. So, drive->waiting_for_dma is used instead.

Also revised per Sergei's comments and let ide_dma_clear_irq return "void".
The "if (hwif->dma_status)" check in __ide_dma_clear_irq() is kept as is since I think
it would be safer for the old ISA/VESA IDE devices that has no BMDMA registers.

For your review, thanks.


diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide-cd.c 02_add_to_ide_intr/drivers/ide/ide-cd.c
--- 01_remove_from_ide_cd/drivers/ide/ide-cd.c	2007-01-24 11:00:03.000000000 +0800
+++ 02_add_to_ide_intr/drivers/ide/ide-cd.c	2007-01-25 16:52:20.000000000 +0800
@@ -923,6 +923,10 @@ static ide_startstop_t cdrom_start_packe
 		HWIF(drive)->OUTB(drive->ctl, IDE_CONTROL_REG);
  
 	if (CDROM_CONFIG_FLAGS (drive)->drq_interrupt) {
+		/* waiting for CDB interrupt, not DMA yet. */
+		if (info->dma)
+			drive->waiting_for_dma = 0;
+
 		/* packet command */
 		ide_execute_command(drive, WIN_PACKETCMD, handler, ATAPI_WAIT_PC, cdrom_timer_expiry);
 		return ide_started;
@@ -965,6 +969,10 @@ static ide_startstop_t cdrom_transfer_pa
 		/* Check for errors. */
 		if (cdrom_decode_status(drive, DRQ_STAT, NULL))
 			return ide_stopped;
+
+		/* Ok, next interrupt will be dma interrupt. */
+		if (info->dma)
+			drive->waiting_for_dma = 1;
 	} else {
 		/* Otherwise, we must wait for DRQ to get set. */
 		if (ide_wait_stat(&startstop, drive, DRQ_STAT,
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide-dma.c 02_add_to_ide_intr/drivers/ide/ide-dma.c
--- 01_remove_from_ide_cd/drivers/ide/ide-dma.c	2006-11-30 05:57:37.000000000 +0800
+++ 02_add_to_ide_intr/drivers/ide/ide-dma.c	2007-01-25 16:37:24.000000000 +0800
@@ -650,6 +650,19 @@ static int __ide_dma_test_irq(ide_drive_
 			drive->name, __FUNCTION__);
 	return 0;
 }
+
+static void __ide_dma_clear_irq(ide_drive_t *drive)
+{
+	ide_hwif_t *hwif = HWIF(drive);
+	u8 dma_stat;
+
+	/* clear the INTR & ERROR bits */
+	if (hwif->dma_status) {
+		dma_stat = hwif->INB(hwif->dma_status);
+		/* Should we force the bit as well ? */
+		hwif->OUTB(dma_stat, hwif->dma_status);
+	}
+}
 #endif /* CONFIG_BLK_DEV_IDEDMA_PCI */
 
 int __ide_dma_bad_drive (ide_drive_t *drive)
@@ -928,6 +941,8 @@ void ide_setup_dma (ide_hwif_t *hwif, un
 		hwif->ide_dma_end = &__ide_dma_end;
 	if (!hwif->ide_dma_test_irq)
 		hwif->ide_dma_test_irq = &__ide_dma_test_irq;
+	if (!hwif->ide_dma_clear_irq)
+		hwif->ide_dma_clear_irq = &__ide_dma_clear_irq;
 	if (!hwif->ide_dma_timeout)
 		hwif->ide_dma_timeout = &__ide_dma_timeout;
 	if (!hwif->ide_dma_lostirq)
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide-io.c 02_add_to_ide_intr/drivers/ide/ide-io.c
--- 01_remove_from_ide_cd/drivers/ide/ide-io.c	2006-11-30 05:57:37.000000000 +0800
+++ 02_add_to_ide_intr/drivers/ide/ide-io.c	2007-01-25 16:27:21.000000000 +0800
@@ -1644,6 +1644,18 @@ irqreturn_t ide_intr (int irq, void *dev
 	}
 	hwgroup->handler = NULL;
 	del_timer(&hwgroup->timer);
+
+	/* Some controllers might set DMA INTR no matter DMA or PIO;
+	 * bmdma status might need to be cleared even for
+	 * PIO interrupts to prevent spurious/lost irq.
+	 */
+	if (hwif->ide_dma_clear_irq && !(drive->waiting_for_dma))
+		/* ide_dma_end() needs bmdma status for error checking.
+		 * So, skip clearing bmdma status here and leave it
+		 * to ide_dma_end() if this is dma interrupt.
+		 */
+		hwif->ide_dma_clear_irq(drive);
+
 	spin_unlock(&ide_lock);
 
 	if (drive->unmask)
diff -Nrup 01_remove_from_ide_cd/drivers/ide/ide.c 02_add_to_ide_intr/drivers/ide/ide.c
--- 01_remove_from_ide_cd/drivers/ide/ide.c	2007-01-23 11:47:42.000000000 +0800
+++ 02_add_to_ide_intr/drivers/ide/ide.c	2007-01-24 11:00:48.000000000 +0800
@@ -503,6 +503,7 @@ static void ide_hwif_restore(ide_hwif_t 
 	hwif->ide_dma_on		= tmp_hwif->ide_dma_on;
 	hwif->ide_dma_off_quietly	= tmp_hwif->ide_dma_off_quietly;
 	hwif->ide_dma_test_irq		= tmp_hwif->ide_dma_test_irq;
+	hwif->ide_dma_clear_irq		= tmp_hwif->ide_dma_clear_irq;
 	hwif->ide_dma_host_on		= tmp_hwif->ide_dma_host_on;
 	hwif->ide_dma_host_off		= tmp_hwif->ide_dma_host_off;
 	hwif->ide_dma_lostirq		= tmp_hwif->ide_dma_lostirq;
diff -Nrup 01_remove_from_ide_cd/include/linux/ide.h 02_add_to_ide_intr/include/linux/ide.h
--- 01_remove_from_ide_cd/include/linux/ide.h	2007-01-24 11:00:03.000000000 +0800
+++ 02_add_to_ide_intr/include/linux/ide.h	2007-01-25 16:10:42.000000000 +0800
@@ -727,6 +727,7 @@ typedef struct hwif_s {
 	int (*ide_dma_on)(ide_drive_t *drive);
 	int (*ide_dma_off_quietly)(ide_drive_t *drive);
 	int (*ide_dma_test_irq)(ide_drive_t *drive);
+	void (*ide_dma_clear_irq)(ide_drive_t *drive);
 	int (*ide_dma_host_on)(ide_drive_t *drive);
 	int (*ide_dma_host_off)(ide_drive_t *drive);
 	int (*ide_dma_lostirq)(ide_drive_t *drive);



  reply	other threads:[~2007-01-25  9:31 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-18 11:19 [PATCH] ide: Fix ATAPI DMA lost irq problem with CDB intr devices Albert Lee
     [not found] ` <58cb370e0701191133m3dd584ffna5b231b00392c13d@mail.gmail.com>
2007-01-19 19:59   ` Bartlomiej Zolnierkiewicz
2007-01-22  7:28     ` Albert Lee
2007-01-22  8:45       ` Albert Lee
2007-01-22 16:05         ` Bartlomiej Zolnierkiewicz
2007-01-24  3:33           ` [PATCH 0/2] ide: clear bmdma status in ide_intr for all commands Albert Lee
2007-01-24  3:36             ` [PATCH 1/2] ide: remove clearing bmdma status from cdrom_decode_status() Albert Lee
2007-01-24  3:42             ` [PATCH 2/2] ide: clear bmdma status in ide_intr() Albert Lee
2007-01-24 13:04               ` Sergei Shtylyov
2007-01-25  9:31                 ` Albert Lee [this message]
2007-01-25 15:17                   ` [PATCH 2/2] ide: clear bmdma status in ide_intr() (revised) Sergei Shtylyov
2007-01-25 16:40                     ` Sergei Shtylyov
2007-01-30  2:49                       ` [PATCH 0/2] ide: clear bmdma status in ide_intr for Intel controllers (revised #3) Albert Lee
2007-01-30  2:52                         ` [PATCH 1/2] ide: remove clearing bmdma status from cdrom_decode_status() (rev #3) Albert Lee
2007-01-30  3:02                         ` [PATCH 2/2] ide: clear bmdma status in ide_intr() for Intel controllers (revised #3) Albert Lee
2007-01-30 14:13                           ` Sergei Shtylyov
2007-01-30 14:27                           ` Alan
     [not found]                           ` <58cb370e0701301117n316a9efax1e0b6299a8f8594a@mail.gmail.com>
2007-01-30 20:21                             ` Bartlomiej Zolnierkiewicz
2007-01-31  5:49                               ` [PATCH 0/2] ide: clear bmdma status in ide_intr for Intel controllers (revised #4) Albert Lee
2007-01-31  5:54                                 ` [PATCH 1/2] ide: clear bmdma status in ide_intr() for ICHx " Albert Lee
2007-01-31  5:57                                 ` [PATCH 2/2] ide: remove clearing bmdma status from cdrom_decode_status() (rev #4) Albert Lee
     [not found]                                 ` <58cb370e0702021307i3d275e84qbeb3b44f58ad6b51@mail.gmail.com>
2007-02-02 21:14                                   ` [PATCH 0/2] ide: clear bmdma status in ide_intr for Intel controllers (revised #4) Bartlomiej Zolnierkiewicz
2007-01-22 14:47       ` [PATCH] ide: Fix ATAPI DMA lost irq problem with CDB intr devices Alan

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=45B878F0.7060707@tw.ibm.com \
    --to=albertcc@tw.ibm.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=albertl@mail.com \
    --cc=awhawks@us.ibm.com \
    --cc=bzolnier@gmail.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=sshtylyov@ru.mvista.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.