From: Bjorn Helgaas <helgaas@kernel.org>
To: Vidya Sagar <vidyas@nvidia.com>
Cc: 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,
lorenzo.pieralisi@arm.com, jingoohan1@gmail.com,
gustavo.pimentel@synopsys.com, mperttunen@nvidia.com,
tiwai@suse.de, spujar@nvidia.com, skomatineni@nvidia.com,
liviu.dudau@arm.com, krzk@kernel.org, heiko@sntech.de,
horms+renesas@verge.net.au, olof@lixom.net,
maxime.ripard@bootlin.com, andy.gross@linaro.org,
bjorn.andersson@linaro.org, jagan@amarulasolutions.com,
enric.balletbo@collabora.com, ezequiel@collabora.com,
stefan.wahren@i2se.com, marc.w.gonzalez@free.fr,
l.stach@pengutronix.de, tpiepho@impinj.com,
hayashi.kunihiko@socionext.com, yue.wang@amlogic.com,
shawn.lin@rock-chips.com, xiaowei.bao@nxp.com,
devicetree@vger.kernel.org, mmaddireddy@nvidia.com,
kthota@nvidia.com, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 09/10] PCI: tegra: Add Tegra194 PCIe support
Date: Wed, 3 Apr 2019 12:36:41 -0500 [thread overview]
Message-ID: <20190403173641.GI141706@google.com> (raw)
In-Reply-To: <dcbfbb32-9980-7ee4-89cd-baedfe624ce5@nvidia.com>
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
next prev parent reply other threads:[~2019-04-03 17:36 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-26 15:13 [PATCH 00/10] Add Tegra194 PCIe support Vidya Sagar
2019-03-26 15:13 ` [PATCH 01/10] PCI: save pci_bus pointer in pcie_port structure Vidya Sagar
2019-03-26 15:13 ` [PATCH 02/10] PCI: perform dbi regs write lock towards the end Vidya Sagar
2019-03-26 15:13 ` [PATCH 03/10] PCI: dwc: Move config space capability search API Vidya Sagar
2019-03-28 12:33 ` Thierry Reding
2019-04-01 11:46 ` Vidya Sagar
2019-03-26 15:13 ` [PATCH 04/10] PCI: Add #defines for PCIe spec r4.0 features Vidya Sagar
2019-03-26 15:13 ` [PATCH 05/10] dt-bindings: PCI: tegra: Add device tree support for T194 Vidya Sagar
2019-03-27 10:10 ` Jon Hunter
2019-03-27 10:53 ` Vidya Sagar
2019-03-28 13:15 ` Thierry Reding
2019-04-01 10:01 ` Vidya Sagar
2019-04-01 15:07 ` Thierry Reding
2019-04-02 11:41 ` Vidya Sagar
2019-04-02 14:35 ` Thierry Reding
2019-04-03 6:22 ` Vidya Sagar
2019-04-02 19:21 ` Bjorn Helgaas
2019-03-31 6:42 ` Rob Herring
2019-04-01 11:18 ` Vidya Sagar
2019-04-01 14:31 ` Thierry Reding
2019-04-02 9:16 ` Vidya Sagar
2019-04-02 14:20 ` Thierry Reding
2019-04-03 5:29 ` Vidya Sagar
2019-03-26 15:13 ` [PATCH 06/10] arm64: tegra: Add P2U and PCIe controller nodes to Tegra194 DT Vidya Sagar
2019-03-28 16:59 ` Thierry Reding
2019-04-01 12:37 ` Vidya Sagar
2019-03-26 15:13 ` [PATCH 07/10] arm64: tegra: Enable PCIe slots in P2972-0000 board Vidya Sagar
2019-03-26 15:13 ` [PATCH 08/10] phy: tegra: Add PCIe PIPE2UPHY support Vidya Sagar
2019-04-03 8:05 ` Kishon Vijay Abraham I
2019-04-03 10:45 ` Vidya Sagar
2019-03-26 15:13 ` [PATCH 09/10] PCI: tegra: Add Tegra194 PCIe support Vidya Sagar
2019-03-27 10:07 ` Jon Hunter
2019-03-29 20:52 ` Bjorn Helgaas
2019-04-02 7:17 ` Vidya Sagar
2019-04-02 14:14 ` Thierry Reding
2019-04-03 9:15 ` Vidya Sagar
2019-04-02 18:31 ` Bjorn Helgaas
2019-04-03 9:43 ` Vidya Sagar
2019-04-03 17:36 ` Bjorn Helgaas [this message]
2019-04-04 19:53 ` Vidya Sagar
2019-04-05 18:58 ` Bjorn Helgaas
2019-04-09 11:30 ` Vidya Sagar
2019-04-09 13:26 ` Bjorn Helgaas
2019-04-10 6:10 ` Vidya Sagar
2019-04-10 8:14 ` Liviu Dudau
2019-04-10 9:53 ` Vidya Sagar
2019-04-10 11:35 ` Liviu Dudau
2019-03-26 15:13 ` [PATCH 10/10] arm64: Add Tegra194 PCIe driver to defconfig Vidya Sagar
2019-03-27 10:08 ` Jon Hunter
2019-03-27 10:12 ` Vidya Sagar
2019-03-27 12:26 ` Jon Hunter
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=20190403173641.GI141706@google.com \
--to=helgaas@kernel.org \
--cc=andy.gross@linaro.org \
--cc=bjorn.andersson@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=devicetree@vger.kernel.org \
--cc=enric.balletbo@collabora.com \
--cc=ezequiel@collabora.com \
--cc=gustavo.pimentel@synopsys.com \
--cc=hayashi.kunihiko@socionext.com \
--cc=heiko@sntech.de \
--cc=horms+renesas@verge.net.au \
--cc=jagan@amarulasolutions.com \
--cc=jingoohan1@gmail.com \
--cc=jonathanh@nvidia.com \
--cc=kishon@ti.com \
--cc=krzk@kernel.org \
--cc=kthota@nvidia.com \
--cc=l.stach@pengutronix.de \
--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=liviu.dudau@arm.com \
--cc=lorenzo.pieralisi@arm.com \
--cc=marc.w.gonzalez@free.fr \
--cc=mark.rutland@arm.com \
--cc=maxime.ripard@bootlin.com \
--cc=mmaddireddy@nvidia.com \
--cc=mperttunen@nvidia.com \
--cc=olof@lixom.net \
--cc=robh+dt@kernel.org \
--cc=shawn.lin@rock-chips.com \
--cc=skomatineni@nvidia.com \
--cc=spujar@nvidia.com \
--cc=stefan.wahren@i2se.com \
--cc=thierry.reding@gmail.com \
--cc=tiwai@suse.de \
--cc=tpiepho@impinj.com \
--cc=vidyas@nvidia.com \
--cc=will.deacon@arm.com \
--cc=xiaowei.bao@nxp.com \
--cc=yue.wang@amlogic.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).