From patchwork Thu Jan 14 07:53:37 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tejun Heo X-Patchwork-Id: 42853 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id BCFA7B7CC3 for ; Thu, 14 Jan 2010 18:48:13 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756159Ab0ANHsK (ORCPT ); Thu, 14 Jan 2010 02:48:10 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756161Ab0ANHsK (ORCPT ); Thu, 14 Jan 2010 02:48:10 -0500 Received: from hera.kernel.org ([140.211.167.34]:54452 "EHLO hera.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756159Ab0ANHsJ (ORCPT ); Thu, 14 Jan 2010 02:48:09 -0500 Received: from htj.dyndns.org (localhost [127.0.0.1]) by hera.kernel.org (8.14.3/8.14.3) with ESMTP id o0E7lhac022636 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO); Thu, 14 Jan 2010 07:47:44 GMT X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.95.2 at hera.kernel.org Received: from [127.0.0.2] (htj.dyndns.org [127.0.0.2]) by htj.dyndns.org (Postfix) with ESMTPSA id 230691005AA62; Thu, 14 Jan 2010 16:53:38 +0900 (KST) Message-ID: <4B4ECD81.8020205@kernel.org> Date: Thu, 14 Jan 2010 16:53:37 +0900 From: Tejun Heo User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.5) Gecko/20091130 SUSE/3.0.0-1.1.1 Thunderbird/3.0 MIME-Version: 1.0 To: Jeff Garzik , "linux-ide@vger.kernel.org" , Alan Cox , Hans Werner , Sergei Shtylyov Subject: [PATCH #upstream-fixes 2/2] libata: implement spurious irq handling for SFF and apply it to piix References: <4B4ECCCD.1040902@kernel.org> In-Reply-To: <4B4ECCCD.1040902@kernel.org> X-Enigmail-Version: 1.0 X-Spam-Status: No, score=1.0 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00, FH_DATE_PAST_20XX autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on hera.kernel.org X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.2.3 (hera.kernel.org [127.0.0.1]); Thu, 14 Jan 2010 07:47:45 +0000 (UTC) Sender: linux-ide-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ide@vger.kernel.org Traditional IDE interface sucks in that it doesn't have a reliable IRQ pending bit, so if the controller raises IRQ while the driver is expecting it not to, the IRQ won't be cleared and eventually the IRQ line will be killed by interrupt subsystem. Some controllers have non-standard mechanism to indicate IRQ pending so that this condition can be detected and worked around. This patch adds an optional operation ->sff_irq_check() which will be called for each port from the ata_sff_interrupt() if an unexpected interrupt is received. If the operation returns %true, ->sff_check_status() and ->sff_irq_clear() will be cleared for the port. Note that this doesn't mark the interrupt as handled so it won't prevent IRQ subsystem from killing the IRQ if this mechanism fails to clear the spurious IRQ. This patch also implements ->sff_irq_check() for ata_piix. Note that this adds slight overhead to shared IRQ operation as IRQs which are destined for other controllers will trigger extra register accesses to check whether IDE interrupt is pending but this solves rare screaming IRQ cases and for some curious reason also helps weird BIOS related glitch on Samsung n130 as reported in bko#14314. http://bugzilla.kernel.org/show_bug.cgi?id=14314 Signed-off-by: Tejun Heo Reported-by: Johannes Stezenbach Reported-by: Hans Werner Cc: Alan Cox Cc: Sergei Shtylyov --- This is generalized safe version of the previous patch which only worked for ata_piix and marked the interrupt as handled. Thanks. drivers/ata/ata_piix.c | 18 ++++++++++++++++-- drivers/ata/libata-sff.c | 22 ++++++++++++++++++++++ include/linux/libata.h | 1 + 3 files changed, 39 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Index: ata/include/linux/libata.h =================================================================== --- ata.orig/include/linux/libata.h +++ ata/include/linux/libata.h @@ -857,6 +857,7 @@ struct ata_port_operations { unsigned int (*sff_data_xfer)(struct ata_device *dev, unsigned char *buf, unsigned int buflen, int rw); u8 (*sff_irq_on)(struct ata_port *); + bool (*sff_irq_check)(struct ata_port *); void (*sff_irq_clear)(struct ata_port *); void (*bmdma_setup)(struct ata_queued_cmd *qc); Index: ata/drivers/ata/ata_piix.c =================================================================== --- ata.orig/drivers/ata/ata_piix.c +++ ata/drivers/ata/ata_piix.c @@ -173,6 +173,7 @@ static int piix_sidpr_scr_read(struct at unsigned int reg, u32 *val); static int piix_sidpr_scr_write(struct ata_link *link, unsigned int reg, u32 val); +static bool piix_irq_check(struct ata_port *ap); #ifdef CONFIG_PM static int piix_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg); static int piix_pci_device_resume(struct pci_dev *pdev); @@ -309,8 +310,13 @@ static struct scsi_host_template piix_sh ATA_BMDMA_SHT(DRV_NAME), }; -static struct ata_port_operations piix_pata_ops = { +static struct ata_port_operations piix_base_ops = { .inherits = &ata_bmdma32_port_ops, + .sff_irq_check = piix_irq_check, +}; + +static struct ata_port_operations piix_pata_ops = { + .inherits = &piix_base_ops, .cable_detect = ata_cable_40wire, .set_piomode = piix_set_piomode, .set_dmamode = piix_set_dmamode, @@ -329,7 +335,7 @@ static struct ata_port_operations ich_pa }; static struct ata_port_operations piix_sata_ops = { - .inherits = &ata_bmdma32_port_ops, + .inherits = &piix_base_ops, }; static struct ata_port_operations piix_sidpr_sata_ops = { @@ -962,6 +968,14 @@ static int piix_sidpr_scr_write(struct a return 0; } +static bool piix_irq_check(struct ata_port *ap) +{ + if (unlikely(!ap->ioaddr.bmdma_addr)) + return false; + + return ap->ops->bmdma_status(ap) & ATA_DMA_INTR; +} + #ifdef CONFIG_PM static int piix_broken_suspend(void) { Index: ata/drivers/ata/libata-sff.c =================================================================== --- ata.orig/drivers/ata/libata-sff.c +++ ata/drivers/ata/libata-sff.c @@ -1778,6 +1778,28 @@ irqreturn_t ata_sff_interrupt(int irq, v handled |= ata_sff_host_intr(ap, qc); } + /* + * If no port was expecting IRQ but the controller is actually + * asserting IRQ line, nobody cared will ensue. Check IRQ + * pending status if available and clear spurious IRQ. + */ + if (!handled) { + for (i = 0; i < host->n_ports; i++) { + struct ata_port *ap = host->ports[i]; + + if (!ap->ops->sff_irq_check || + !ap->ops->sff_irq_check(ap)) + continue; + + if (printk_ratelimit()) + ata_port_printk(ap, KERN_INFO, + "clearing spurious IRQ\n"); + + ap->ops->sff_check_status(ap); + ap->ops->sff_irq_clear(ap); + } + } + spin_unlock_irqrestore(&host->lock, flags); return IRQ_RETVAL(handled);