Message-ID: <pl262pfxe5mjtxzvr4qcsxwt4cyrdjzncd3ztsqpb6zuc7gi5b@hu6njospevgk>
Date: Mon, 9 Jun 2025 17:29:49 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Bjorn Helgaas <helgaas@...nel.org>, 
	Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
Cc: Brian Norris <briannorris@...omium.org>, 
	"Rafael J. Wysocki" <rjw@...ysocki.net>, Tony Lindgren <tony@...mide.com>, 
	JeffyChen <jeffy.chen@...k-chips.com>, Bjorn Andersson <andersson@...nel.org>, 
	Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	cros-qcom-dts-watchers@...omium.org, Bjorn Helgaas <bhelgaas@...gle.com>, 
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-pci@...r.kernel.org, quic_vbadigan@...cinc.com, quic_mrana@...cinc.com, 
	Sherry Sun <sherry.sun@....com>
Subject: Re: [PATCH v3 2/2] PCI/portdrv: Add support for PCIe wake interrupt

+ Brian, Rafael, Tony, Jeffy (who were part of the previous attempt to add WAKE#
GPIO/interrupt support:
https://lore.kernel.org/linux-pci/20171225114742.18920-1-jeffy.chen@rock-chips.com

On Mon, Jun 09, 2025 at 11:27:49AM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 6/6/2025 1:56 AM, Bjorn Helgaas wrote:
> > On Thu, Jun 05, 2025 at 10:54:45AM +0530, Krishna Chaitanya Chundru wrote:
> > > PCIe wake interrupt is needed for bringing back PCIe device state
> > > from D3cold to D0.
> > 
> > Does this refer to the WAKE# signal or Beacon or both?  I guess the
> > comments in the patch suggest WAKE#.  Is there any spec section we can
> > cite here?
> > 
> we are referring only WAKE# signal, I will add the PCIe spec r6.0, sec
> 5.3.3.2 in next patch version.
> > > Implement new functions, of_pci_setup_wake_irq() and
> > > of_pci_teardown_wake_irq(), to manage wake interrupts for PCI devices
> > > using the Device Tree.
> > > 
> > >  From the port bus driver call these functions to enable wake support
> > > for bridges.
> > 
> > What is the connection to bridges and portdrv?  WAKE# is described in
> > PCIe r6.0, sec 5.3.3.2, and PCIe CEM r6.0, sec 2.3, but AFAICS neither
> > restricts it to bridges.
> > 

You are right. WAKE# is really a PCIe slot/Endpoint property and doesn't
necessarily belong to a Root Port/Bridge. But the problem is with handling the
Wake interrupt in the host. For instance, below is the DT representation of the
PCIe hierarchy:

	PCIe Host Bridge
		|
		v
	PCIe Root Port/Bridge
		|
		|
		v
PCIe Slot <-------------> PCIe Endpoint

DTs usually define both the WAKE# and PERST# GPIOs ({wake/reset}-gpios property)
in the PCIe Host Bridge node. But we have decided to move atleast the PERST# to
the Root Port node since the PERST# lines are per slot and not per host bridge.

Similar interpretation applies to WAKE# as well, but the major difference is
that it is controlled by the endpoints, not by the host (RC/Host Bridge/Root
Port). The host only cares about the interrupt that rises from the WAKE# GPIO.
The PCIe spec, r6.0, Figure 5-4, tells us that the WAKE# is routed to the PM
controller on the host. In most of the systems that tends to be true as the
WAKE# is not tied to the PCIe IP itself, but to a GPIO controller in the host.

In this case, the PCI core somehow needs to know the IRQ number corresponding to
the WAKE# GPIO, so that it can register an IRQ handler for it to wakeup the
endpoint when an interrupt happens. Previous attempts [1], tried to add a new DT
property for the interrupts:
https://lore.kernel.org/linux-pci/20171225114742.18920-2-jeffy.chen@rock-chips.com

But that doesn't seem to fly. So Krishna came with the proposal to reuse the
WAKE# GPIO defined in the Root Port node (for DTs that have moved the properties
out of the Host Bridge node) and extract the IRQ number from it using
gpiod_to_irq() API.

And he used portdrv as the placeholder for the irq setup code, because the WAKE#
GPIO is going to be defined in the Root Port node irrespective of a PCIe Slot or
an endpoint is connected to the Root Port. And the portdrv driver is the one
controlling the Root Port. Though, this placeholder part can be subject to
discussion.

But from the previous discussions, I could infer that the PCIe Root Port/Bridge
DT node is going to be the placeholder for the WAKE# GPIOs:
https://lore.kernel.org/linux-pci/2806186.IK6EZBC0cX@aspire.rjw.lan

It also sounds sensible to me since we do not want to define the wake-gpios
property in the endpoint node as that would mean that for each device connected,
a separate DT endpoint node needs to be created just for defining the WAKE#
GPIO. Also, if there is only a PCIe slot in the board, then the property has to
be defined in the PCIe Root Port node itself as there is no separate DT node for
PCIe slot.

> The wake# is used by the endpoints to wake host to bring PCIe device
> state to D0 again, in direct attach root port wake# will be connected
> to the root port and in switch cases the wake# will connected to the
> switch Downstream ports and switch will consolidate wake# from different
> downstream ports and sends to the root port. The wake# will be used by
> root port bridges only. portdrv is the driver for root port.

This is not correct. Root Port doesn't have anything to do with the WAKE#
interrupt. We are just merely trying to reuse the Root Port driver because of
how the WAKE# GPIO is wired up in the DT. It might not be applicable for ACPI as
the FW raises GPE for the Wake interrupt and kernel doesn't parse WAKE#, afaik.

> > Unless there's some related functionality in a Root Port, RCEC, or
> > Switch Port, maybe this code should be elsewhere, like
> > set_pcie_port_type(), so we could set this up for any PCIe device that
> > has a WAKE# description?
> > 

I think it should work if we tie the Wake interrupt to the PCI device instead of
the Root Port/Bridge in the kernel, even though the DT representation is
different. In that case, PCI core should parse the Root Port node for each
device to get the WAKE# GPIO and setup an IRQ handler for it. When the Wake
interrupt happens, the PCI core should power ON the PCI hierarchy starting from
the host bridge, till the endpoint (top down approach).

I think we all agree that the WAKE# GPIO should be handled by the PCI core
instead of the PCI controller or client drivers. We should agree on the best
possible place though.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists