linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Lukas Wunner <lukas@wunner.de>
Cc: Jianjun Wang <jianjun.wang@mediatek.com>,
	Rob Herring <robh+dt@kernel.org>,
	Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
	Ryder Lee <ryder.lee@mediatek.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Mauro Carvalho Chehab <mchehab+huawei@kernel.org>,
	linux-pci@vger.kernel.org, linux-mediatek@lists.infradead.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Sj Huang <sj.huang@mediatek.com>,
	youlin.pei@mediatek.com, chuanjia.liu@mediatek.com,
	qizhong.cheng@mediatek.com, sin_jieyang@mediatek.com
Subject: Re: [v4,2/3] PCI: mediatek: Add new generation controller support
Date: Fri, 4 Dec 2020 12:30:52 -0600	[thread overview]
Message-ID: <20201204183052.GA1929838@bjorn-Precision-5520> (raw)
In-Reply-To: <20201204073909.GA17699@wunner.de>

On Fri, Dec 04, 2020 at 08:39:09AM +0100, Lukas Wunner wrote:
> On Mon, Nov 30, 2020 at 11:30:05AM -0600, Bjorn Helgaas wrote:
> > On Mon, Nov 23, 2020 at 02:45:13PM +0800, Jianjun Wang wrote:
> > > On Thu, 2020-11-19 at 14:28 -0600, Bjorn Helgaas wrote:
> > > > > +static int mtk_pcie_setup(struct mtk_pcie_port *port)
> > > > > +{
> [...]
> > > > > +	/* Try link up */
> > > > > +	err = mtk_pcie_startup_port(port);
> > > > > +	if (err) {
> > > > > +		dev_notice(dev, "PCIe link down\n");
> > > > > +		goto err_setup;
> > > > 
> > > > Generally it should not be a fatal error if the link is not up at
> > > > probe-time.  You may be able to hot-add a device, or the device may
> > > > have some external power control that will power it up later.
> > > 
> > > This is for the power saving requirement. If there is no device
> > > connected with the PCIe slot, the PCIe MAC and PHY should be powered
> > > off.
> > > 
> > > Is there any standard flow to support power down the hardware at
> > > probe-time if no device is connected and power it up when hot-add a
> > > device?
> > 
> > That's a good question.  I assume this looks like a standard PCIe
> > hot-add event?
> > 
> > When you hot-add a device, does the Root Port generate a Presence
> > Detect Changed interrupt?  The pciehp driver should field that
> > interrupt and turn on power to the slot via the Power Controller
> > Control bit in the Slot Control register.
> > 
> > Does your hardware require something more than that to control the MAC
> > and PHY power?
> 
> Power saving of unused PCIe ports is generally achieved through the
> runtime PM framework.  When a PCIe port runtime suspends, the PCIe
> core will transition it to D3hot.  On top of that, the platform
> may be able to transition the port to D3cold.  Currently only the
> ACPI platform supports that.  Conceivably, devicetree-based systems
> may want to disable certain clocks or regulators when a PCIe port
> runtime suspends.  I think we do not support that yet but it could
> be added to drivers/pci/pcie/portdrv*.
> 
> A hotplug port is expected to signal PDC and DLLSC interrupts even
> when in D3hot.  At least that's our experience with Thunderbolt.
> To support hotplug interrupts in D3cold, some external mechanism
> (such as a PME) is necessary to wake up the port on hotplug.
> This is also supported with recent Thunderbolt systems.
> 
> Because we've seen various incompatibilities when runtime suspending
> PCIe ports, certain conditions must be satisfied for runtime PM
> to be enabled.  They're encoded in pci_bridge_d3_possible().
> Generally, hotplug ports only runtime suspend if they belong to
> a Thunderbolt controller or if the ACPI platform explicitly allows
> runtime PM (through presence of a _PR3 method or a device property).
> Non-hotplug ports runtime suspend if the BIOS is newer than 2015
> (as specified by DMI).
> 
> Obviously, this policy is very x86-focussed because both Thunderbolt
> and DMI are only really a thing on x86.  That's about to change though
> because Apple's new arm64-based Macs have Thunderbolt integrated into
> the SoC and arm64 SoCs are making inroads in the datacenter, which is
> an important use case for PCIe hotplug (hot-swappable NVMe drives).
> So we may have to amend pci_bridge_d3_possible() to whitelist
> PCIe ports for runtime PM on specific arches or systems.

Thanks for all this very useful information!

My interpretation for the mediatek situation:

  - I assume this patch leaves or puts the Root Port in D3cold if no
    downstream devices are present.

  - I don't see any support for PME or similar mechanisms to signal a
    hot-add while the RP is in D3cold.

  - So I assume you don't support hot-add if the slot was empty at
    boot and that's acceptable for your platform.

  reply	other threads:[~2020-12-04 18:31 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-18  8:29 [v4,0/3] PCI: mediatek: Add new generation controller support Jianjun Wang
2020-11-18  8:29 ` [v4,1/3] dt-bindings: PCI: mediatek: Add YAML schema Jianjun Wang
2020-12-01  0:17   ` Rob Herring
2020-11-18  8:29 ` [v4,2/3] PCI: mediatek: Add new generation controller support Jianjun Wang
2020-11-19 15:22   ` Rob Herring
2020-11-23  5:59     ` Jianjun Wang
2020-11-19 20:28   ` Bjorn Helgaas
2020-11-23  6:45     ` Jianjun Wang
2020-11-30 16:05       ` Rob Herring
2020-11-30 17:33         ` Bjorn Helgaas
2020-11-30 17:30       ` Bjorn Helgaas
2020-12-01  3:06         ` Jianjun Wang
2020-12-04  7:39         ` Lukas Wunner
2020-12-04 18:30           ` Bjorn Helgaas [this message]
2020-12-08  1:27             ` Jianjun Wang
2020-11-18  8:29 ` [v4,3/3] MAINTAINERS: update entry for MediaTek PCIe controller Jianjun Wang
2020-11-19 19:56   ` Bjorn Helgaas

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=20201204183052.GA1929838@bjorn-Precision-5520 \
    --to=helgaas@kernel.org \
    --cc=chuanjia.liu@mediatek.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jianjun.wang@mediatek.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=lukas@wunner.de \
    --cc=matthias.bgg@gmail.com \
    --cc=mchehab+huawei@kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=qizhong.cheng@mediatek.com \
    --cc=robh+dt@kernel.org \
    --cc=ryder.lee@mediatek.com \
    --cc=sin_jieyang@mediatek.com \
    --cc=sj.huang@mediatek.com \
    --cc=youlin.pei@mediatek.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).