Message-ID: <Z_WAKDjIeOjlghVs@google.com>
Date: Tue, 8 Apr 2025 12:59:36 -0700
From: Brian Norris <briannorris@...omium.org>
To: Bartosz Golaszewski <brgl@...ev.pl>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Jingoo Han <jingoohan1@...il.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>
Cc: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>,
	Rob Herring <robh@...nel.org>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	dmitry.baryshkov@...aro.org, Tsai Sung-Fu <danielsftsai@...gle.com>
Subject: [RFC] PCI: pwrctrl and link-up dependencies

TL;DR: PCIe link-up may depend on pwrctrl; however, link-startup is
often run before pwrctrl gets involved. I'm exploring options to resolve
this.

Hi all,

I'm currently looking at reworking how some (currently out-of-tree, but I'm
hoping to change that) pcie-designware based drivers integrate power sequencing
for their endpoint devices, as well as the corresponding start_link()
functionality.

For power sequencing, drivers/pci/pwrctrl/ looks like a very good start at what
we need, since we have various device-specific regulators, GPIOs, and
sequencing requirements, which we'd prefer not to encode directly in the
controller driver.

For link startup, pcie-designware-host.c currently
(a) starts the link via platform-specific means (dw_pcie::ops::start_link()) and
(b) waits for the link training to complete.

However, (b) will fail if the other end of the link is not powered up --
e.g., if the appropriate pwrctrl driver has not yet loaded, or its
device hasn't finished probing. Today, this can mean the designware
driver will either fail to probe, or at least waste time for a condition
that we can't achieve (link up), depending on the HW/driver
implementation.

I'm wondering how any designware-based platforms (on which I believe pwrctrl
was developed) actually support this, and how I should look to integrate
additional platforms/drivers. From what I can tell, the only way things would
work today would either be if:
(1) a given platform uses the dw_pcie_rp::use_linkup_irq==true functionality,
    which means pcie-designware-host will only start the link, but not wait for
    training to succeed. (And presumably the controller will receive its
    link-up IRQ after power sequencing is done, at which point both pwrctrl and
    the IRQ may rescan the PCI bus.) Or:
(2) pci/pwrctrl sequencing only brings up some non-critical power rails for the
    device in question, so link-up can actually succeed even without
    pwrctrl.

My guess is that (1) is the case, and specifically that the relevant folks are
using the pcie-qcom.c, with its "global" IRQ used for link-up events.

So how should I replicate this on other designware-based platforms? I suppose
if the platform in question has a link-up IRQ, I can imitate (1). But what if
it doesn't? (Today, we don't validate and utilize a link-up IRQ, although it's
possible there is one available. Additionally, we use various retry mechanisms
today, which don't trivially fit into this framework, as we won't know when
precisely to retry, if power sequencing is controlled entirely by some other
entity.)

Would it make sense to introduce some sort of pwrctrl -> start_link()
dependency? For example, I see similar work done in this series [1], for
slightly different reasons. In short, that series adds new
pci_ops::{start,stop}_link() callbacks, and teaches a single pwrctrl driver to
stop and restart the bridge link before/after powering things up.

I also see that Manivannan has a proposal out [2] to add semi-generic
link-down + retraining support to core code. It treads somewhat similar
ground, and I could even imagine that its pci_ops::retrain_link()
callback could even be reimplemented in terms of the aforementioned
pci_ops::{start,stop}_link(), or possibly vice versa.

Any thoughts here? Sorry for a lot of text and no patch, but I didn't just want
to start off by throwing a 3rd set of patches on top of the existing ones that
tread similar ground[1][2].

Regards,
Brian

[1] [PATCH v4 00/10] PCI: Enable Power and configure the TC956x PCIe switch
https://lore.kernel.org/linux-pci/20250225-qps615_v4_1-v4-0-e08633a7bdf8@oss.qualcomm.com/

[PATCH v4 03/10] PCI: Add new start_link() & stop_link function ops
https://lore.kernel.org/linux-pci/20250225-qps615_v4_1-v4-3-e08633a7bdf8@oss.qualcomm.com/

[...]
[
[PATCH v4 08/10] PCI: pwrctrl: Add power control driver for tc956x
https://lore.kernel.org/linux-pci/20250225-qps615_v4_1-v4-8-e08633a7bdf8@oss.qualcomm.com/

[2] [PATCH 0/2] PCI: Add support for handling link down event from host bridge drivers
https://lore.kernel.org/linux-pci/20250221172309.120009-1-manivannan.sadhasivam@linaro.org/

Powered by blists - more mailing lists