From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bjorn Helgaas Subject: Re: [PATCH V7 02/15] PCI: Disable MSI for Tegra194 root port Date: Tue, 21 May 2019 14:36:16 -0500 Message-ID: <20190521193616.GE57618@google.com> References: <20190517123846.3708-1-vidyas@nvidia.com> <20190517123846.3708-3-vidyas@nvidia.com> <20190521102729.GB29166@ulmo> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Vidya Sagar Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, lorenzo.pieralisi@arm.com, mperttunen@nvidia.com, mmaddireddy@nvidia.com, linux-pci@vger.kernel.org, catalin.marinas@arm.com, will.deacon@arm.com, linux-kernel@vger.kernel.org, robh+dt@kernel.org, kishon@ti.com, kthota@nvidia.com, Thierry Reding , gustavo.pimentel@synopsys.com, jingoohan1@gmail.com, linux-tegra@vger.kernel.org, jonathanh@nvidia.com, linux-arm-kernel@lists.infradead.org, sagar.tv@gmail.com List-Id: linux-tegra@vger.kernel.org On Tue, May 21, 2019 at 10:17:26PM +0530, Vidya Sagar wrote: > On 5/21/2019 3:57 PM, Thierry Reding wrote: > > On Fri, May 17, 2019 at 06:08:33PM +0530, Vidya Sagar wrote: > > > Tegra194 rootports don't generate MSI interrupts for PME events and hence > > > MSI needs to be disabled for them to avoid root ports service drivers > > > registering their respective ISRs with MSI interrupt. The service drivers (AER, hotplug, etc) don't know whether the interrupt is INTx or MSI; they just register their ISRs with pcie_device.irq. The point of this patch is that the PCI core doesn't support devices that use MSI and INTx at the same time, and since this device can't generate MSI for PME, we have to use INTx for *all* its interrupts. > > > Signed-off-by: Vidya Sagar > > > --- > > > Changes since [v6]: > > > * This is a new patch > > > > > > drivers/pci/quirks.c | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > index 0f16acc323c6..28f9a0380df5 100644 > > > --- a/drivers/pci/quirks.c > > > +++ b/drivers/pci/quirks.c > > > @@ -2592,6 +2592,20 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, > > > PCI_DEVICE_ID_NVIDIA_NVENET_15, > > > nvenet_msi_disable); > > > +/* > > > + * Tegra194's PCIe root ports don't generate MSI interrupts for PME events > > > + * instead legacy interrupts are generated. Hence, to avoid service drivers > > > + * registering their respective ISRs for MSIs, need to disable MSI interrupts > > > + * for root ports. > > > + */ > > > +static void disable_tegra194_rp_msi(struct pci_dev *dev) > > > +{ > > > + dev->no_msi = 1; > > > +} > > > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad0, disable_tegra194_rp_msi); > > > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad1, disable_tegra194_rp_msi); > > > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad2, disable_tegra194_rp_msi); > > > + > > > > Later functions in this file seem to use a more consistent naming > > pattern, according to which the name for this would become: > > > > pci_quirk_nvidia_tegra194_disable_rp_msi > > > > Might be worth considering making this consistent. > > > > This could also be moved to the DWC driver to restrict this to where it > > is needed. In either case, this seems like a good solution, so: > > > > Reviewed-by: Thierry Reding > > > Ok. I'll move it to DWC driver along with name change for the quirk API. Is there any possibility this hardware would be used in an ACPI system? If so, the quirk should probably stay in drivers/pci/quirks.c because the DWC driver would not be present. Either way, please also add some PCIe spec references about this in the changelog and a comment in the code about working around this issue. I think the MSI/MSI-X sections that prohibit a device from using both INTx and MSI/MSI-X are probably the most pertinent. The reason I want a comment about this is to discourage future hardware from following this example because every device that *does* follow this example requires a kernel update that would be otherwise unnecessary. Bjorn From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.6 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id CA1E6C46460 for ; Tue, 21 May 2019 19:36:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A20852183F for ; Tue, 21 May 2019 19:36:23 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1558467383; bh=KIETrny8+uJ6mb/6sbywMXsqjLr/B13v3e2H5fLDq/Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=tZDYiFrTklXUoYg7RYh4Rd2x2/xhVIKAzNQvYtM1ejRSBZquWBArSIYmeAYY8g2zn 9bGMgkEhJDuyXj2BNp8wDoOIQhM/gXw2/PmRnhik1ECRRhW/ljPhDscEP/s4vXd3d5 77lmokWAt5tWHuctGEw4P6hOU2LZOXLIuQSwkiew= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727662AbfEUTgW (ORCPT ); Tue, 21 May 2019 15:36:22 -0400 Received: from mail.kernel.org ([198.145.29.99]:46618 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727408AbfEUTgW (ORCPT ); Tue, 21 May 2019 15:36:22 -0400 Received: from localhost (unknown [69.71.4.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BA0CB217D9; Tue, 21 May 2019 19:36:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1558467381; bh=KIETrny8+uJ6mb/6sbywMXsqjLr/B13v3e2H5fLDq/Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=xsAf94Br7UIrfKZQZlaeMGdzgOt2IN/9tfSepnhZ0KrezAUPzlKQ/O4r+xK/7dBYH luMzlO9OxVdclvvHY7utbK1jvz+7ovtu4aW5LS1rc3VTYJc9zEvrfQAuQDqx1tqDma pc60VB7NdUlB4TT4YKcs782g29IIsk8H6GaSeUA4= Date: Tue, 21 May 2019 14:36:16 -0500 From: Bjorn Helgaas To: Vidya Sagar Cc: Thierry Reding , lorenzo.pieralisi@arm.com, robh+dt@kernel.org, mark.rutland@arm.com, jonathanh@nvidia.com, kishon@ti.com, catalin.marinas@arm.com, will.deacon@arm.com, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, mperttunen@nvidia.com, linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kthota@nvidia.com, mmaddireddy@nvidia.com, sagar.tv@gmail.com Subject: Re: [PATCH V7 02/15] PCI: Disable MSI for Tegra194 root port Message-ID: <20190521193616.GE57618@google.com> References: <20190517123846.3708-1-vidyas@nvidia.com> <20190517123846.3708-3-vidyas@nvidia.com> <20190521102729.GB29166@ulmo> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 21, 2019 at 10:17:26PM +0530, Vidya Sagar wrote: > On 5/21/2019 3:57 PM, Thierry Reding wrote: > > On Fri, May 17, 2019 at 06:08:33PM +0530, Vidya Sagar wrote: > > > Tegra194 rootports don't generate MSI interrupts for PME events and hence > > > MSI needs to be disabled for them to avoid root ports service drivers > > > registering their respective ISRs with MSI interrupt. The service drivers (AER, hotplug, etc) don't know whether the interrupt is INTx or MSI; they just register their ISRs with pcie_device.irq. The point of this patch is that the PCI core doesn't support devices that use MSI and INTx at the same time, and since this device can't generate MSI for PME, we have to use INTx for *all* its interrupts. > > > Signed-off-by: Vidya Sagar > > > --- > > > Changes since [v6]: > > > * This is a new patch > > > > > > drivers/pci/quirks.c | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > index 0f16acc323c6..28f9a0380df5 100644 > > > --- a/drivers/pci/quirks.c > > > +++ b/drivers/pci/quirks.c > > > @@ -2592,6 +2592,20 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, > > > PCI_DEVICE_ID_NVIDIA_NVENET_15, > > > nvenet_msi_disable); > > > +/* > > > + * Tegra194's PCIe root ports don't generate MSI interrupts for PME events > > > + * instead legacy interrupts are generated. Hence, to avoid service drivers > > > + * registering their respective ISRs for MSIs, need to disable MSI interrupts > > > + * for root ports. > > > + */ > > > +static void disable_tegra194_rp_msi(struct pci_dev *dev) > > > +{ > > > + dev->no_msi = 1; > > > +} > > > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad0, disable_tegra194_rp_msi); > > > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad1, disable_tegra194_rp_msi); > > > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad2, disable_tegra194_rp_msi); > > > + > > > > Later functions in this file seem to use a more consistent naming > > pattern, according to which the name for this would become: > > > > pci_quirk_nvidia_tegra194_disable_rp_msi > > > > Might be worth considering making this consistent. > > > > This could also be moved to the DWC driver to restrict this to where it > > is needed. In either case, this seems like a good solution, so: > > > > Reviewed-by: Thierry Reding > > > Ok. I'll move it to DWC driver along with name change for the quirk API. Is there any possibility this hardware would be used in an ACPI system? If so, the quirk should probably stay in drivers/pci/quirks.c because the DWC driver would not be present. Either way, please also add some PCIe spec references about this in the changelog and a comment in the code about working around this issue. I think the MSI/MSI-X sections that prohibit a device from using both INTx and MSI/MSI-X are probably the most pertinent. The reason I want a comment about this is to discourage future hardware from following this example because every device that *does* follow this example requires a kernel update that would be otherwise unnecessary. Bjorn From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS,USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 728E2C04E87 for ; Tue, 21 May 2019 19:36:30 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 49241217F9 for ; Tue, 21 May 2019 19:36:30 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="QVPGca5q"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="xsAf94Br" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 49241217F9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=TaN6gLWRv1HEqpM7BuX7KNTpYjkJ4INfL3U/tighpBo=; b=QVPGca5qen56mS HDLwb/7KqE5CNYsd3+T1YWCBNNuVJHTxSZUs/ku7HzneTEax42v33f8RyZKDpi7usj8yr2vYFE6CM BBPTQtnO6y+kteRzADEHpwS2749FbNnaxloiZbXYE6I7+luZxnbxe4Td5QQaXgSOFzWv76APAjqcJ 5ZWnUntvdpuTxn/C624bnOH+bFvDSOWgfFKtKuyKJ/owbZlIndf1rtVvuIjszH17dvvp5SEshBZzR VbIgb395f1sYEcw3Xyb9M5hhejDAf/IBalzoryBGqgeeDdXlLW9WogZxjv0PdZvL8v3Qgk66VEkRZ KlNTtqRL6VeMyj9eAYpA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hTAYi-0000Oy-Ta; Tue, 21 May 2019 19:36:24 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1hTAYf-0000OV-Sd for linux-arm-kernel@lists.infradead.org; Tue, 21 May 2019 19:36:23 +0000 Received: from localhost (unknown [69.71.4.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id BA0CB217D9; Tue, 21 May 2019 19:36:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1558467381; bh=KIETrny8+uJ6mb/6sbywMXsqjLr/B13v3e2H5fLDq/Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=xsAf94Br7UIrfKZQZlaeMGdzgOt2IN/9tfSepnhZ0KrezAUPzlKQ/O4r+xK/7dBYH luMzlO9OxVdclvvHY7utbK1jvz+7ovtu4aW5LS1rc3VTYJc9zEvrfQAuQDqx1tqDma pc60VB7NdUlB4TT4YKcs782g29IIsk8H6GaSeUA4= Date: Tue, 21 May 2019 14:36:16 -0500 From: Bjorn Helgaas To: Vidya Sagar Subject: Re: [PATCH V7 02/15] PCI: Disable MSI for Tegra194 root port Message-ID: <20190521193616.GE57618@google.com> References: <20190517123846.3708-1-vidyas@nvidia.com> <20190517123846.3708-3-vidyas@nvidia.com> <20190521102729.GB29166@ulmo> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190521_123621_960696_31A6C300 X-CRM114-Status: GOOD ( 23.33 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, lorenzo.pieralisi@arm.com, mperttunen@nvidia.com, mmaddireddy@nvidia.com, linux-pci@vger.kernel.org, catalin.marinas@arm.com, will.deacon@arm.com, linux-kernel@vger.kernel.org, robh+dt@kernel.org, kishon@ti.com, kthota@nvidia.com, Thierry Reding , gustavo.pimentel@synopsys.com, jingoohan1@gmail.com, linux-tegra@vger.kernel.org, jonathanh@nvidia.com, linux-arm-kernel@lists.infradead.org, sagar.tv@gmail.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, May 21, 2019 at 10:17:26PM +0530, Vidya Sagar wrote: > On 5/21/2019 3:57 PM, Thierry Reding wrote: > > On Fri, May 17, 2019 at 06:08:33PM +0530, Vidya Sagar wrote: > > > Tegra194 rootports don't generate MSI interrupts for PME events and hence > > > MSI needs to be disabled for them to avoid root ports service drivers > > > registering their respective ISRs with MSI interrupt. The service drivers (AER, hotplug, etc) don't know whether the interrupt is INTx or MSI; they just register their ISRs with pcie_device.irq. The point of this patch is that the PCI core doesn't support devices that use MSI and INTx at the same time, and since this device can't generate MSI for PME, we have to use INTx for *all* its interrupts. > > > Signed-off-by: Vidya Sagar > > > --- > > > Changes since [v6]: > > > * This is a new patch > > > > > > drivers/pci/quirks.c | 14 ++++++++++++++ > > > 1 file changed, 14 insertions(+) > > > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > > index 0f16acc323c6..28f9a0380df5 100644 > > > --- a/drivers/pci/quirks.c > > > +++ b/drivers/pci/quirks.c > > > @@ -2592,6 +2592,20 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, > > > PCI_DEVICE_ID_NVIDIA_NVENET_15, > > > nvenet_msi_disable); > > > +/* > > > + * Tegra194's PCIe root ports don't generate MSI interrupts for PME events > > > + * instead legacy interrupts are generated. Hence, to avoid service drivers > > > + * registering their respective ISRs for MSIs, need to disable MSI interrupts > > > + * for root ports. > > > + */ > > > +static void disable_tegra194_rp_msi(struct pci_dev *dev) > > > +{ > > > + dev->no_msi = 1; > > > +} > > > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad0, disable_tegra194_rp_msi); > > > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad1, disable_tegra194_rp_msi); > > > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_NVIDIA, 0x1ad2, disable_tegra194_rp_msi); > > > + > > > > Later functions in this file seem to use a more consistent naming > > pattern, according to which the name for this would become: > > > > pci_quirk_nvidia_tegra194_disable_rp_msi > > > > Might be worth considering making this consistent. > > > > This could also be moved to the DWC driver to restrict this to where it > > is needed. In either case, this seems like a good solution, so: > > > > Reviewed-by: Thierry Reding > > > Ok. I'll move it to DWC driver along with name change for the quirk API. Is there any possibility this hardware would be used in an ACPI system? If so, the quirk should probably stay in drivers/pci/quirks.c because the DWC driver would not be present. Either way, please also add some PCIe spec references about this in the changelog and a comment in the code about working around this issue. I think the MSI/MSI-X sections that prohibit a device from using both INTx and MSI/MSI-X are probably the most pertinent. The reason I want a comment about this is to discourage future hardware from following this example because every device that *does* follow this example requires a kernel update that would be otherwise unnecessary. Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel