From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [PATCH 3/5] sata_sil: new interrupt handler Date: Tue, 30 May 2006 00:15:37 -0400 Message-ID: <447BC6E9.104@pobox.com> References: <11488844821123-git-send-email-htejun@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from srv5.dvmed.net ([207.36.208.214]:63971 "EHLO mail.dvmed.net") by vger.kernel.org with ESMTP id S1751477AbWE3EPz (ORCPT ); Tue, 30 May 2006 00:15:55 -0400 In-Reply-To: <11488844821123-git-send-email-htejun@gmail.com> Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: Tejun Heo Cc: mlord@pobox.com, albertcc@tw.ibm.com, alan@lxorguk.ukuu.org.uk, axboe@suse.de, forrest.zhao@intel.com, linux-ide@vger.kernel.org Tejun Heo wrote: > The DMA complete bit of these controllers reflects ATA IRQ status > while no DMA command is in progress. So, we can tell whether the > controller is raising an interrupt or not in deterministic manner. > This patch gives sata_sil its own interrupt handler which behaves much > better than the original one in terms of error detection and handling. > This change is also necessary for later hotplug support. > > Further improvements are possible, in both 2 and 4 ports versions, we > can get all status with only one readl and using custom bmdma > operations can further cut down register accesses. > > Signed-off-by: Tejun Heo > > --- > > drivers/scsi/sata_sil.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 files changed, 92 insertions(+), 1 deletions(-) > > bf8f8bae070d0b05bb8f0d577f1751afcc19a163 > diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c > index f926883..695c06c 100644 > --- a/drivers/scsi/sata_sil.c > +++ b/drivers/scsi/sata_sil.c > @@ -111,6 +111,8 @@ 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_interrupt(int irq, void *dev_instance, > + struct pt_regs *regs); > static void sil_freeze(struct ata_port *ap); > static void sil_thaw(struct ata_port *ap); > > @@ -196,7 +198,7 @@ static const struct ata_port_operations > .thaw = sil_thaw, > .error_handler = ata_bmdma_error_handler, > .post_internal_cmd = ata_bmdma_post_internal_cmd, > - .irq_handler = ata_interrupt, > + .irq_handler = sil_interrupt, > .irq_clear = ata_bmdma_irq_clear, > .scr_read = sil_scr_read, > .scr_write = sil_scr_write, > @@ -336,6 +338,95 @@ static void sil_scr_write (struct ata_po > writel(val, mmio); > } > > +static void sil_host_intr(struct ata_port *ap, u32 bmdma2) > +{ > + struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag); > + u8 status; > + > + if (unlikely(!qc || qc->tf.ctl & ATA_NIEN)) > + goto freeze; > + > + /* Check whether we are expecting interrupt in this state */ > + switch (ap->hsm_task_state) { > + case HSM_ST_FIRST: > + /* Some pre-ATAPI-4 devices assert INTRQ > + * at this state when ready to receive CDB. > + */ > + > + /* Check the ATA_DFLAG_CDB_INTR flag is enough here. > + * The flag was turned on only for atapi devices. > + * No need to check is_atapi_taskfile(&qc->tf) again. > + */ > + if (!(qc->dev->flags & ATA_DFLAG_CDB_INTR)) > + goto err_hsm; > + break; > + case HSM_ST_LAST: > + if (qc->tf.protocol == ATA_PROT_DMA || > + qc->tf.protocol == ATA_PROT_ATAPI_DMA) { > + /* clear DMA-Start bit */ > + ap->ops->bmdma_stop(qc); > + > + if (bmdma2 & SIL_DMA_ERROR) { > + qc->err_mask |= AC_ERR_HOST_BUS; > + ap->hsm_task_state = HSM_ST_ERR; > + } > + } > + break; > + case HSM_ST: > + break; > + default: > + goto err_hsm; > + } > + > + /* check main status, clearing INTRQ */ > + status = ata_chk_status(ap); > + if (unlikely(status & ATA_BUSY)) > + goto err_hsm; > + > + /* ack bmdma irq events */ > + ata_bmdma_irq_clear(ap); > + > + /* kick HSM in the ass */ > + ata_hsm_move(ap, qc, status, 0); > + > + return; > + > + err_hsm: > + qc->err_mask |= AC_ERR_HSM; > + freeze: > + ata_port_freeze(ap); > +} > + > +static irqreturn_t sil_interrupt(int irq, void *dev_instance, > + struct pt_regs *regs) > +{ > + struct ata_host_set *host_set = dev_instance; > + void __iomem *mmio_base = host_set->mmio_base; > + int handled = 0; > + int i; > + > + /* TODO: make _irqsave conditional on x86 PCI IDE legacy mode */ > + spin_lock_irq(&host_set->lock); > + > + for (i = 0; i < host_set->n_ports; i++) { > + struct ata_port *ap = host_set->ports[i]; > + u32 bmdma2 = readl(mmio_base + sil_port[ap->port_no].bmdma2); > + > + if (unlikely(!ap || ap->flags & ATA_FLAG_DISABLED)) > + continue; > + > + if (!(bmdma2 & SIL_DMA_COMPLETE)) > + continue; Is this bit asserted even on error? > + sil_host_intr(ap, bmdma2); > + handled = 1; > + } > + > + spin_unlock_irq(&host_set->lock); use spin_lock() and spin_unlock()