Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754653Ab3IXUuf (ORCPT ); Tue, 24 Sep 2013 16:50:35 -0400 Received: from mail-ie0-f178.google.com ([209.85.223.178]:48870 "EHLO mail-ie0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754015Ab3IXUub (ORCPT ); Tue, 24 Sep 2013 16:50:31 -0400 MIME-Version: 1.0 In-Reply-To: References: From: Bjorn Helgaas Date: Tue, 24 Sep 2013 14:50:10 -0600 Message-ID: Subject: Re: [PATCH] PCI: add quirk for 3ware 9650SE controller To: Jiri Kosina Cc: "linux-kernel@vger.kernel.org" , "linux-pci@vger.kernel.org" , "linux-scsi@vger.kernel.org" , "Eric W. Biederman" , Jesse Barnes , Adam Radford Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5210 Lines: 136 On Fri, Sep 6, 2013 at 4:47 PM, Bjorn Helgaas wrote: > On Fri, Sep 6, 2013 at 3:51 AM, Jiri Kosina wrote: >>> >> > Commit d5dea7d95 ("PCI: msi: Disable msi interrupts when we initialize a >>> >> > pci device") makes MSIs be forcibly disabled at boot time. >>> >> > >>> >> > It turns out that this breaks 3ware controller -- if MSIs are disabled >>> >> > during PCI discovery of this controller, the device doesn't work properly >>> >> > (it doesn't respond to any commands that are being sent to it after >>> >> > initialization). > > Is there a bug report for this issue? It's nice to have a pointer to, > e.g., a bugzilla.kernel.org bug report with info such as dmesg logs, > lspci output, etc., for future reference. Maybe somebody will figure > out a more generic change that could make this quirk unnecessary, and > details will help in figuring that out. > > I assume the actual PCI discovery done in the PCI core works fine; > it's just that the driver doesn't work if MSIs are disabled on the > device. If that's the case, can this be fixed by some driver change? > Maybe the driver needs to enable MSI before it sends commands to the > device? Ping? Since the complaint is that the device doesn't work unless MSIs are enabled, can this be fixed in the driver instead of in a quirk? > Any description of what this failure looks like to a user? How can a > user or a distro connect a symptom (driver timeout, console message, > or whatever) to this patch? > >>> >> > Reverting d5dea7d95 or not force-disabling MSIs in pci_msi_init_pci_dev() >>> >> > makes the device work properly again. >>> >> > >>> >> > Signed-off-by: Jiri Kosina >>> >> > >>> >> > --- >>> >> > >>> >> > I am adding Adam Radford as a recepient as well, to see whether he is able >>> >> > to provide some more explanation why this device would expose this >>> >> > behavior. >>> > >>> > OK, so Adam Radford's lsi.com address is bouncing, hence I guess we can't >>> > expect any feedback from him. >>> > >>> > Bjorn, Jesse, any word on this please? >>> >>> It's on my list to look at. It's too late to put it in for v3.11, and >>> it's doubtful that it will even make the v3.12 merge window (though >>> possibly it could go in post-merge window). d5dea7d95 is several >>> years old, so hopefully this issue isn't super-urgent. Let me know if >>> otherwise. >> >> I agree that this should be applicable to 3.12-rcX as well, as it's very >> device-specific. >> >> Thanks. >> >>> >>> Bjorn >>> >>> >> > drivers/pci/msi.c | 3 +++ >>> >> > drivers/pci/quirks.c | 10 ++++++++++ >>> >> > include/linux/pci.h | 1 + >>> >> > 3 files changed, 14 insertions(+), 0 deletions(-) >>> >> > >>> >> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c >>> >> > index aca7578..4f36b8b 100644 >>> >> > --- a/drivers/pci/msi.c >>> >> > +++ b/drivers/pci/msi.c >>> >> > @@ -1040,6 +1040,9 @@ void pci_msi_init_pci_dev(struct pci_dev *dev) >>> >> > { >>> >> > INIT_LIST_HEAD(&dev->msi_list); >>> >> > >>> >> > + if (dev->broken_msi_disable) >>> >> > + return; >>> >> > + >>> >> > /* Disable the msi hardware to avoid screaming interrupts >>> >> > * during boot. This is the power on reset default so >>> >> > * usually this should be a noop. >>> >> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c >>> >> > index e85d230..4ba3400 100644 >>> >> > --- a/drivers/pci/quirks.c >>> >> > +++ b/drivers/pci/quirks.c >>> >> > @@ -2890,6 +2890,16 @@ static void quirk_intel_ntb(struct pci_dev *dev) >>> >> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0e08, quirk_intel_ntb); >>> >> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0e0d, quirk_intel_ntb); >>> >> > >>> >> > +/* >>> >> > + * 3ware 9650SE controller doesn't properly initialize if MSI are >>> >> > + * disabled on it during PCI device discovery >>> >> > + */ >>> >> > +static void quirk_broken_msi_disable(struct pci_dev *dev) >>> >> > +{ >>> >> > + dev->broken_msi_disable = 1; >>> >> > +} >>> >> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_3WARE, 0x1004, quirk_broken_msi_disable); >>> >> > + >>> >> > static ktime_t fixup_debug_start(struct pci_dev *dev, >>> >> > void (*fn)(struct pci_dev *dev)) >>> >> > { >>> >> > diff --git a/include/linux/pci.h b/include/linux/pci.h >>> >> > index 0fd1f15..c327d74 100644 >>> >> > --- a/include/linux/pci.h >>> >> > +++ b/include/linux/pci.h >>> >> > @@ -341,6 +341,7 @@ struct pci_dev { >>> >> > #ifdef CONFIG_PCI_MSI >>> >> > struct list_head msi_list; >>> >> > struct kset *msi_kset; >>> >> > + unsigned int broken_msi_disable:1; >>> >> > #endif >>> >> > struct pci_vpd *vpd; >>> >> > #ifdef CONFIG_PCI_ATS >>> >> > >>> >> > -- >>> >> > Jiri Kosina >>> >> > SUSE Labs >>> >> > >>> >> >>> >> -- >>> >> Jiri Kosina >>> >> SUSE Labs >>> >> >>> > >>> > -- >>> > Jiri Kosina >>> > SUSE Labs >>> >> >> -- >> Jiri Kosina >> SUSE Labs -- 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/