Re: [PATCH] sata_sil: improved interrupt handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Tejun Heo wrote:
Hi, Jeff.

Jeff Garzik wrote:

Just committed the following to the 'sii-irq' branch of libata-dev.git,
and verified it on an Adaptec 1210SA (3112).

Haven't decided whether I will push it upstream or not, but I think I
will.  It does a bit better job of handling handling errors, and should
be more efficient (less CPU usage) than the standard ATA interrupt
handler as well.

For users seeing sata_sil problems, this may make them happy.


This patch doesn't make any difference on my sil3114 controller. I'll write about it in the m15w thread.

"doesn't make any difference" I will interpret to mean there are no regressions.


Also, this patch doesn't implement proper handling of PIO protocols and thus breaks ALL branch.

PIO should work fine, modulo the obvious changes for ATA_FLAG_NOINTR disappearance.


It seems to me that the changes made by the new interrupt handler is not very sil3112 specific. Is there any reason this change is sil3112 specific?

There is nothing 3112-specific about this new code.


commit b6abf7755a79383f0e5f108d23a0394f156c54c1
Author: Jeff Garzik <[email protected]>
Date:   Sat Dec 3 00:30:57 2005 -0500

    [libata sata_sil] improved interrupt handling

drivers/scsi/sata_sil.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 117 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 3609186..37398a5 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -85,6 +85,7 @@ static void sil_dev_config(struct ata_po
 static u32 sil_scr_read (struct ata_port *ap, unsigned int sc_reg);
static void sil_scr_write (struct ata_port *ap, unsigned int sc_reg, u32 val);
 static void sil_post_set_mode (struct ata_port *ap);
+static irqreturn_t sil_irq (int irq, void *dev_instance, struct pt_regs *regs); static const struct pci_device_id sil_pci_tbl[] = { @@ -167,7 +168,7 @@ static const struct ata_port_operations .qc_prep = ata_qc_prep,
     .qc_issue        = ata_qc_issue_prot,
     .eng_timeout        = ata_eng_timeout,
-    .irq_handler        = ata_interrupt,
+    .irq_handler        = sil_irq,
     .irq_clear        = ata_bmdma_irq_clear,
     .scr_read        = sil_scr_read,
     .scr_write        = sil_scr_write,
@@ -233,6 +234,121 @@ MODULE_DEVICE_TABLE(pci, sil_pci_tbl);
 MODULE_VERSION(DRV_VERSION);
+static inline void sil_port_irq(struct ata_port *ap, void __iomem *mmio,
+                u8 dma_stat, u8 dma_stat_mask)
+{
+    struct ata_queued_cmd *qc = NULL;
+    unsigned int err_mask = AC_ERR_OTHER;
+    int complete = 1;
+    u8 dev_stat;
+
+    /* Exit now, if port or port's irqs are disabled */
+    if (ap->flags & (ATA_FLAG_PORT_DISABLED | ATA_FLAG_NOINTR)) {
+        complete = 0;
+        goto out;
+    }


Hmmm... By performing this test here, we end up reading bmdma status registers of all ports everytime an interrupt occurs. Is this to prevent screaming IRQ problem?

That's the preferred way to handle interrupts on this hardware. Normal ATA is broken due to the lack of a way to ask "did I receive an interrupt?" without unduly affecting state. 311x, like AHCI, sata_sil24 and other hardware, provides a method to easily determine if a PCI interrupt is owned by the hardware or not.

The code should eliminate all screaming interrupt problems, yes.

As an aside, 3114 should use a single 32-bit read of the Interrupt Summary register, rather than the four separate 8-bit reads that this code does.

	Jeff


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux