From: Bjorn Helgaas <helgaas@kernel.org> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Vidya Sagar <vidyas@nvidia.com>, robh+dt@kernel.org, mark.rutland@arm.com, thierry.reding@gmail.com, jonathanh@nvidia.com, kishon@ti.com, catalin.marinas@arm.com, will.deacon@arm.com, jingoohan1@gmail.com, gustavo.pimentel@synopsys.com, digetx@gmail.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 V13 12/12] PCI: tegra: Add Tegra194 PCIe support Date: Tue, 16 Jul 2019 14:00:13 -0500 [thread overview] Message-ID: <20190716190013.GB4470@google.com> (raw) In-Reply-To: <20190716112225.GA24335@e121166-lin.cambridge.arm.com> On Tue, Jul 16, 2019 at 12:22:25PM +0100, Lorenzo Pieralisi wrote: > On Sat, Jul 13, 2019 at 12:34:34PM +0530, Vidya Sagar wrote: > > > > > So if the link is not up we still go ahead and make probe > > > > > succeed. What for ? > > > > We may need root port to be available to support hot-plugging of > > > > endpoint devices, so, we don't fail the probe. > > > > > > We need it or we don't. If you do support hotplugging of endpoint > > > devices point me at the code, otherwise link up failure means > > > failure to probe. > > Currently hotplugging of endpoint is not supported, but it is one of > > the use cases that we may add support for in future. > > You should elaborate on this, I do not understand what you mean, > either the root port(s) supports hotplug or it does not. > > > But, why should we fail probe if link up doesn't happen? As such, > > nothing went wrong in terms of root port initialization right? I > > checked other DWC based implementations and following are not failing > > the probe pci-dra7xx.c, pcie-armada8k.c, pcie-artpec6.c, pcie-histb.c, > > pcie-kirin.c, pcie-spear13xx.c, pci-exynos.c, pci-imx6.c, > > pci-keystone.c, pci-layerscape.c > > > > Although following do fail the probe if link is not up. pcie-qcom.c, > > pcie-uniphier.c, pci-meson.c > > > > So, to me, it looks more like a choice we can make whether to fail the > > probe or not and in this case we are choosing not to fail. > > I disagree. I had an offline chat with Bjorn and whether link-up should > fail the probe or not depends on whether the root port(s) is hotplug > capable or not and this in turn relies on the root port "Slot > implemented" bit in the PCI Express capabilities register. There might be a little more we can talk about in this regard. I did bring up the "Slot implemented" bit, but after thinking about it more, I don't really think the host bridge driver should be looking at that. That's a PCIe concept, and it's really *downstream* from the host bridge itself. The host bridge is logically a device on the CPU bus, not the PCI bus. I'm starting to think that the host bridge driver probe should be disconnected from question of whether the root port links are up. Logically, the host bridge driver connects the CPU bus to a PCI root bus, so it converts CPU-side accesses to PCI config, memory, or I/O port transactions. Given that, the PCI core can enumerate devices on the root bus and downstream buses. Devices on the root bus typically include Root Ports, but might also include endpoints, Root Complex Integrated Endpoints, Root Complex Event Collectors, etc. I think in principle, we would want the host bridge probe to succeed so we can use these devices even if none of the Root Ports have a link. If a Root Port is present, I think users will expect to see it in the "lspci" output, even if its downstream link is not up. That will enable things like manually poking the Root Port via "setpci" for debug. And if it has a connector, the generic pciehp should be able to handle hot-add events without any special help from the host bridge driver. On ACPI systems there is no concept of the host bridge driver probe failing because of lack of link on a Root Port. If a Root Port doesn't have an operational link, we still keep the pci_root.c driver, and we'll enumerate the Root Port itself. So I tend to think DT systems should behave the same way, i.e., the driver probe should succeed unless it fails to allocate resources or something similar. I think this is analogous to a NIC or USB adapter driver, where the probe succeeds even if there's no network cable or USB device attached. Bjorn
WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <helgaas@kernel.org> To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, mperttunen@nvidia.com, mmaddireddy@nvidia.com, linux-pci@vger.kernel.org, catalin.marinas@arm.com, Vidya Sagar <vidyas@nvidia.com>, will.deacon@arm.com, linux-kernel@vger.kernel.org, kishon@ti.com, kthota@nvidia.com, robh+dt@kernel.org, thierry.reding@gmail.com, gustavo.pimentel@synopsys.com, jingoohan1@gmail.com, linux-tegra@vger.kernel.org, digetx@gmail.com, jonathanh@nvidia.com, linux-arm-kernel@lists.infradead.org, sagar.tv@gmail.com Subject: Re: [PATCH V13 12/12] PCI: tegra: Add Tegra194 PCIe support Date: Tue, 16 Jul 2019 14:00:13 -0500 [thread overview] Message-ID: <20190716190013.GB4470@google.com> (raw) In-Reply-To: <20190716112225.GA24335@e121166-lin.cambridge.arm.com> On Tue, Jul 16, 2019 at 12:22:25PM +0100, Lorenzo Pieralisi wrote: > On Sat, Jul 13, 2019 at 12:34:34PM +0530, Vidya Sagar wrote: > > > > > So if the link is not up we still go ahead and make probe > > > > > succeed. What for ? > > > > We may need root port to be available to support hot-plugging of > > > > endpoint devices, so, we don't fail the probe. > > > > > > We need it or we don't. If you do support hotplugging of endpoint > > > devices point me at the code, otherwise link up failure means > > > failure to probe. > > Currently hotplugging of endpoint is not supported, but it is one of > > the use cases that we may add support for in future. > > You should elaborate on this, I do not understand what you mean, > either the root port(s) supports hotplug or it does not. > > > But, why should we fail probe if link up doesn't happen? As such, > > nothing went wrong in terms of root port initialization right? I > > checked other DWC based implementations and following are not failing > > the probe pci-dra7xx.c, pcie-armada8k.c, pcie-artpec6.c, pcie-histb.c, > > pcie-kirin.c, pcie-spear13xx.c, pci-exynos.c, pci-imx6.c, > > pci-keystone.c, pci-layerscape.c > > > > Although following do fail the probe if link is not up. pcie-qcom.c, > > pcie-uniphier.c, pci-meson.c > > > > So, to me, it looks more like a choice we can make whether to fail the > > probe or not and in this case we are choosing not to fail. > > I disagree. I had an offline chat with Bjorn and whether link-up should > fail the probe or not depends on whether the root port(s) is hotplug > capable or not and this in turn relies on the root port "Slot > implemented" bit in the PCI Express capabilities register. There might be a little more we can talk about in this regard. I did bring up the "Slot implemented" bit, but after thinking about it more, I don't really think the host bridge driver should be looking at that. That's a PCIe concept, and it's really *downstream* from the host bridge itself. The host bridge is logically a device on the CPU bus, not the PCI bus. I'm starting to think that the host bridge driver probe should be disconnected from question of whether the root port links are up. Logically, the host bridge driver connects the CPU bus to a PCI root bus, so it converts CPU-side accesses to PCI config, memory, or I/O port transactions. Given that, the PCI core can enumerate devices on the root bus and downstream buses. Devices on the root bus typically include Root Ports, but might also include endpoints, Root Complex Integrated Endpoints, Root Complex Event Collectors, etc. I think in principle, we would want the host bridge probe to succeed so we can use these devices even if none of the Root Ports have a link. If a Root Port is present, I think users will expect to see it in the "lspci" output, even if its downstream link is not up. That will enable things like manually poking the Root Port via "setpci" for debug. And if it has a connector, the generic pciehp should be able to handle hot-add events without any special help from the host bridge driver. On ACPI systems there is no concept of the host bridge driver probe failing because of lack of link on a Root Port. If a Root Port doesn't have an operational link, we still keep the pci_root.c driver, and we'll enumerate the Root Port itself. So I tend to think DT systems should behave the same way, i.e., the driver probe should succeed unless it fails to allocate resources or something similar. I think this is analogous to a NIC or USB adapter driver, where the probe succeeds even if there's no network cable or USB device attached. Bjorn _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-07-16 19:00 UTC|newest] Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-10 6:22 [PATCH V13 00/12] PCI: tegra: Add Tegra194 PCIe support Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` [PATCH V13 01/12] PCI: Add #defines for some of PCIe spec r4.0 features Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 20:14 ` Bjorn Helgaas 2019-07-10 20:14 ` Bjorn Helgaas 2019-07-10 6:22 ` [PATCH V13 02/12] PCI: Disable MSI for Tegra root ports Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` [PATCH V13 03/12] PCI: dwc: Perform dbi regs write lock towards the end Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` [PATCH V13 04/12] PCI: dwc: Move config space capability search API Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` [PATCH V13 05/12] PCI: dwc: Add ext " Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 10:37 ` Lorenzo Pieralisi 2019-07-10 10:37 ` Lorenzo Pieralisi 2019-07-10 11:27 ` Vidya Sagar 2019-07-10 11:27 ` Vidya Sagar 2019-07-10 11:27 ` Vidya Sagar 2019-07-10 14:19 ` Lorenzo Pieralisi 2019-07-10 14:19 ` Lorenzo Pieralisi 2019-07-10 6:22 ` [PATCH V13 06/12] dt-bindings: PCI: designware: Add binding for CDM register check Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` [PATCH V13 07/12] PCI: dwc: Add support to enable " Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` [PATCH V13 08/12] dt-bindings: Add PCIe supports-clkreq property Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 15:28 ` Lorenzo Pieralisi 2019-07-10 15:28 ` Lorenzo Pieralisi 2019-07-10 17:14 ` Vidya Sagar 2019-07-10 17:14 ` Vidya Sagar 2019-07-10 17:14 ` Vidya Sagar 2019-07-10 6:22 ` [PATCH V13 09/12] dt-bindings: PCI: tegra: Add device tree support for Tegra194 Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` [PATCH V13 10/12] dt-bindings: PHY: P2U: Add Tegra194 P2U block Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` [PATCH V13 11/12] phy: tegra: Add PCIe PIPE2UPHY support Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` [PATCH V13 12/12] PCI: tegra: Add Tegra194 PCIe support Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 6:22 ` Vidya Sagar 2019-07-10 17:02 ` Lorenzo Pieralisi 2019-07-10 17:02 ` Lorenzo Pieralisi 2019-07-10 17:26 ` Vidya Sagar 2019-07-10 17:26 ` Vidya Sagar 2019-07-10 17:26 ` Vidya Sagar 2019-07-11 12:54 ` Lorenzo Pieralisi 2019-07-11 12:54 ` Lorenzo Pieralisi 2019-07-12 15:32 ` Vidya Sagar 2019-07-12 15:32 ` Vidya Sagar 2019-07-12 15:32 ` Vidya Sagar 2019-07-12 16:07 ` Lorenzo Pieralisi 2019-07-12 16:07 ` Lorenzo Pieralisi 2019-07-13 7:04 ` Vidya Sagar 2019-07-13 7:04 ` Vidya Sagar 2019-07-13 7:04 ` Vidya Sagar 2019-07-16 11:22 ` Lorenzo Pieralisi 2019-07-16 11:22 ` Lorenzo Pieralisi 2019-07-16 19:00 ` Bjorn Helgaas [this message] 2019-07-16 19:00 ` Bjorn Helgaas 2019-07-23 14:28 ` Vidya Sagar 2019-07-23 14:28 ` Vidya Sagar 2019-07-23 14:28 ` Vidya Sagar 2019-07-23 14:44 ` Vidya Sagar 2019-07-23 14:44 ` Vidya Sagar 2019-07-23 14:44 ` Vidya Sagar 2019-07-30 15:49 ` Lorenzo Pieralisi 2019-07-30 15:49 ` Lorenzo Pieralisi 2019-08-02 12:06 ` Vidya Sagar 2019-08-02 12:06 ` Vidya Sagar 2019-08-02 12:06 ` Vidya Sagar 2019-08-05 14:01 ` Lorenzo Pieralisi 2019-08-05 14:01 ` Lorenzo Pieralisi 2019-08-05 16:54 ` Vidya Sagar 2019-08-05 16:54 ` Vidya Sagar 2019-08-05 16:54 ` Vidya Sagar 2019-08-06 14:51 ` Lorenzo Pieralisi 2019-08-06 14:51 ` Lorenzo Pieralisi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190716190013.GB4470@google.com \ --to=helgaas@kernel.org \ --cc=catalin.marinas@arm.com \ --cc=devicetree@vger.kernel.org \ --cc=digetx@gmail.com \ --cc=gustavo.pimentel@synopsys.com \ --cc=jingoohan1@gmail.com \ --cc=jonathanh@nvidia.com \ --cc=kishon@ti.com \ --cc=kthota@nvidia.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linux-tegra@vger.kernel.org \ --cc=lorenzo.pieralisi@arm.com \ --cc=mark.rutland@arm.com \ --cc=mmaddireddy@nvidia.com \ --cc=mperttunen@nvidia.com \ --cc=robh+dt@kernel.org \ --cc=sagar.tv@gmail.com \ --cc=thierry.reding@gmail.com \ --cc=vidyas@nvidia.com \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.