Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161992AbWKPKoo (ORCPT ); Thu, 16 Nov 2006 05:44:44 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1161993AbWKPKoo (ORCPT ); Thu, 16 Nov 2006 05:44:44 -0500 Received: from mail.suse.de ([195.135.220.2]:54177 "EHLO mx1.suse.de") by vger.kernel.org with ESMTP id S1161992AbWKPKon (ORCPT ); Thu, 16 Nov 2006 05:44:43 -0500 Date: Thu, 16 Nov 2006 11:44:37 +0100 Message-ID: From: Takashi Iwai To: Jeff Garzik Cc: "Lu, Yinghai" , "Olivier Nicolas" , David Miller , torvalds@osdl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] ALSA: hda-intel - Disable MSI support by default In-Reply-To: <455B6761.3050700@garzik.org> References: <20061114.190036.30187059.davem@davemloft.net> <20061114.192117.112621278.davem@davemloft.net> <455B5D22.10408@garzik.org> <455B6761.3050700@garzik.org> User-Agent: Wanderlust/2.12.0 (Your Wildest Dreams) SEMI/1.14.6 (Maruoka) FLIM/1.14.7 (=?ISO-8859-4?Q?Sanj=F2?=) APEL/10.6 MULE XEmacs/21.5 (beta27) (fiddleheads) (+CVS-20060704) (i386-suse-linux) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2081 Lines: 61 At Wed, 15 Nov 2006 14:15:45 -0500, Jeff Garzik wrote: > > ACK the pci_intx() calls, NAK the obviously overweight spinlock changes. > The spinlock changes are completely unnecessary. Just look at any > other (non-ALSA) PCI driver. Existing "spin_lock()" is fine for both > PCI shared irq handlers and MSI irq handlers. > > It sounds like you are trying to work around a reentrancy problem that > does not exist. > > Only weird drivers like ps2kbd/mouse or IDE need spin_lock_irqsave(), > where separate interrupt sources call the same function. OK, I revised it, also referring to a similar patch by Yinghai. I think we can simplify the change like below. Olivier, could you test this patch, too? (Here the first chunk (enable_msi=1) is just for testing. The patch isn't for merge yet.) Takashi --- diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c index e35cfd3..130ee12 100644 --- a/sound/pci/hda/hda_intel.c +++ b/sound/pci/hda/hda_intel.c @@ -55,7 +55,7 @@ static char *model; static int position_fix; static int probe_mask = -1; static int single_cmd; -static int enable_msi; +static int enable_msi = 1; module_param(index, int, 0444); MODULE_PARM_DESC(index, "Index value for Intel HD audio interface."); @@ -1380,7 +1380,8 @@ static int __devinit azx_init_stream(str static int azx_acquire_irq(struct azx *chip, int do_disconnect) { - if (request_irq(chip->pci->irq, azx_interrupt, IRQF_DISABLED|IRQF_SHARED, + if (request_irq(chip->pci->irq, azx_interrupt, + chip->msi ? 0 : IRQF_SHARED, "HDA Intel", chip)) { printk(KERN_ERR "hda-intel: unable to grab IRQ %d, " "disabling device\n", chip->pci->irq); @@ -1389,6 +1390,7 @@ static int azx_acquire_irq(struct azx *c return -1; } chip->irq = chip->pci->irq; + pci_intx(chi->pci, !chip->msi); return 0; } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/