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=-2.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED,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 BDD5FC4360F for ; Wed, 3 Apr 2019 17:36:53 +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 68B0020830 for ; Wed, 3 Apr 2019 17:36:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="geDORH0B"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="Db52NSfv" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 68B0020830 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=AwvJ+eO8/IiTUwaaFeNxXSvpg+CX8i1+DtqUlkr4NY0=; b=geDORH0BPtxN/3 rHVXMTtoGWSvhJ17Mpcb8K7jfvwgAA39PjZP5EUIuhPg312TWRD+lNebJJ+SyqQoZomm3HYzwsKtY p4a+mMbwdehfvBNnX2SHjRqbjKpFz1ZfwVfmjj8E/jr2SCem8hlMR7ma9ESDmHXqURFoxOTPNTrP9 0lm9BOTOpp6ENTKzEkxDMY9/8RRh4BLZ4KzISQK9xMjok5MC0cl0aDnbl5tzwVuV5p5bSkKDGaZAT Uya8OgctrC99mIXuC6mjFGfZcyQ/Js7+S7uOxvCqpIzm/GAZqZfN6CgbiSbK6z+OnZ5+XHRCZSzu+ Bi13/lgPDzb4G3TvCKvg==; 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 1hBjod-0001sz-FN; Wed, 03 Apr 2019 17:36:47 +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 1hBjoZ-0001rR-2W for linux-arm-kernel@lists.infradead.org; Wed, 03 Apr 2019 17:36:44 +0000 Received: from localhost (173-25-63-173.client.mchsi.com [173.25.63.173]) (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 3BE2B206DF; Wed, 3 Apr 2019 17:36:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1554313002; bh=AVMXa3cG1N8pImjPVrC4CxhvQ77U3Fimk39UpmGZpnk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Db52NSfvGrnTYCWkR+TYzld6vI+TYwRltWYtH5c1y9xQhK08lUkCr6HEQSvuCaBdU Z2qAw/z3JMUtclMRuaZqyXNVtn9kIyGw52DQLUIyeOBdPpNd7nQDOAkgnULYshuZGU bg/YrCmWJ2Bh2Qdvb5HGOld2+fMATdSERPOqOQGM= Date: Wed, 3 Apr 2019 12:36:41 -0500 From: Bjorn Helgaas To: Vidya Sagar Subject: Re: [PATCH 09/10] PCI: tegra: Add Tegra194 PCIe support Message-ID: <20190403173641.GI141706@google.com> References: <1553613207-3988-1-git-send-email-vidyas@nvidia.com> <1553613207-3988-10-git-send-email-vidyas@nvidia.com> <20190329203159.GG24180@google.com> <5eb9599c-a6d6-d3a3-beef-5225ed7393f9@nvidia.com> <20190402183110.GE141706@google.com> 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-20190403_103643_146952_86C8F695 X-CRM114-Status: GOOD ( 31.39 ) 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, heiko@sntech.de, hayashi.kunihiko@socionext.com, tiwai@suse.de, catalin.marinas@arm.com, spujar@nvidia.com, will.deacon@arm.com, kthota@nvidia.com, mperttunen@nvidia.com, thierry.reding@gmail.com, jonathanh@nvidia.com, stefan.wahren@i2se.com, lorenzo.pieralisi@arm.com, krzk@kernel.org, kishon@ti.com, maxime.ripard@bootlin.com, jagan@amarulasolutions.com, linux-pci@vger.kernel.org, andy.gross@linaro.org, shawn.lin@rock-chips.com, devicetree@vger.kernel.org, mmaddireddy@nvidia.com, marc.w.gonzalez@free.fr, liviu.dudau@arm.com, yue.wang@amlogic.com, enric.balletbo@collabora.com, robh+dt@kernel.org, linux-tegra@vger.kernel.org, horms+renesas@verge.net.au, bjorn.andersson@linaro.org, ezequiel@collabora.com, linux-arm-kernel@lists.infradead.org, xiaowei.bao@nxp.com, gustavo.pimentel@synopsys.com, linux-kernel@vger.kernel.org, skomatineni@nvidia.com, jingoohan1@gmail.com, olof@lixom.net, tpiepho@impinj.com, l.stach@pengutronix.de 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 Wed, Apr 03, 2019 at 03:13:09PM +0530, Vidya Sagar wrote: > On 4/3/2019 12:01 AM, Bjorn Helgaas wrote: > > On Tue, Apr 02, 2019 at 12:47:48PM +0530, Vidya Sagar wrote: > > > On 3/30/2019 2:22 AM, Bjorn Helgaas wrote: > > > > On Tue, Mar 26, 2019 at 08:43:26PM +0530, Vidya Sagar wrote: > > > > > Add support for Synopsys DesignWare core IP based PCIe host controller > > > > > present in Tegra194 SoC. > > > > - Why does this chip require pcie_pme_disable_msi()? The only other > > use is a DMI quirk for "MSI Wind U-100", added by c39fae1416d5 > > ("PCI PM: Make it possible to force using INTx for PCIe PME > > signaling"). > > Because Tegra194 doesn't support raising PME interrupts through MSI line. What does the spec say about this? Is hardware supposed to support MSI for PME? Given that MSI Wind U-100 and Tegra194 are the only two cases we know about where PME via MSI isn't supported, it seems like there must be either a requirement for that or some mechanism for the OS to figure this out, e.g., a capability bit. > > > > I see that an earlier patch added "bus" to struct pcie_port. > > > > I think it would be better to somehow connect to the > > > > pci_host_bridge struct. Several other drivers already do > > > > this; see uses of pci_host_bridge_from_priv(). > > > > > > All non-DesignWare based implementations save their private data > > > structure in 'private' pointer of struct pci_host_bridge and use > > > pci_host_bridge_from_priv() to get it back. But, DesignWare > > > based implementations save pcie_port in 'sysdata' and nothing in > > > 'private' pointer. So, I'm not sure if > > > pci_host_bridge_from_priv() can be used in this case. Please do > > > let me know if you think otherwise. > > > > DesignWare-based drivers should have a way to retrieve the > > pci_host_bridge pointer. It doesn't have to be *exactly* the same > > as non-DesignWare drivers, but it should be similar. > > I gave my reasoning as to why with the current code, it is not > possible to get the pci_host_bridge structure pointer from struct > pcie_port pointer in another thread as a reply to Thierry Reding's > comments. Since Jishen'g changes to support remove functionality are > accepted, I think using bus pointer saved in struct pcie_port > pointer shouldn't be any issue now. Please do let me know if that is > something not acceptable. > > > > > That would give you the bus, as well as flags like > > > > no_ext_tags, native_aer, etc, which this driver, being a host > > > > bridge driver that's responsible for this part of the > > > > firmware/OS interface, may conceivably need. I think saving the pp->root_bus pointer as Jisheng's patch does is a sub-optimal solution. If we figure out how to save the pci_host_bridge pointer, we automatically get the root bus pointer as well. It may require some restructuring to save the pci_host_bridge pointer, but I doubt it's really *impossible*. > > > > > +static int tegra_pcie_dw_runtime_suspend(struct device *dev) > > > > > +{ > > > > > + struct tegra_pcie_dw *pcie = dev_get_drvdata(dev); > > > > > + > > > > > + tegra_pcie_downstream_dev_to_D0(pcie); > > > > > + > > > > > + pci_stop_root_bus(pcie->pci.pp.bus); > > > > > + pci_remove_root_bus(pcie->pci.pp.bus); > > > > > > > > Why are you calling these? No other drivers do this except in > > > > their .remove() methods. Is there something special about > > > > Tegra, or is this something the other drivers *should* be > > > > doing? > > > > > > Since this API is called by remove, I'm removing the hierarchy > > > to safely bring down all the devices. I'll have to re-visit this > > > part as Jisheng Zhang's patches > > > https://patchwork.kernel.org/project/linux-pci/list/?series=98559 > > > are now approved and I need to verify this part after > > > cherry-picking Jisheng's changes. > > > > Tegra194 should do this the same way as other drivers, independent > > of Jisheng's changes. > > When other Designware implementations add remove functionality, even > they should be calling these APIs (Jisheng also mentioned the same > in his commit message) My point is that these APIs should be called from driver .remove() methods, not from .runtime_suspend() methods. Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel