On Tue, Jun 10, 2025 at 10:00:20AM +0530, Krishna Chaitanya Chundru wrote: > On 6/10/2025 4:04 AM, Bjorn Helgaas wrote: > > On Mon, Jun 09, 2025 at 05:29:49PM +0530, Manivannan Sadhasivam wrote: > > > + 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@xxxxxxxxxxxxxx > > > > > > 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. > > > > If WAKE# is supported at all, it's a sideband signal independent of > > the link topology. PCIe CEM r6.0, sec 2.3, says WAKE# from multiple > > connectors can be wire-ORed together, or can have individual > > connections to the PM controller. > > I believe they are referring to multi root port where WAKE# can > routed to individual root port where each root port can go D3cold > individually. AFAICT there's no requirement that WAKE# be routed to a Root Port or a Switch Port. The routing is completely implementation specific. > From endpoint perspective they will have single WAKE# signal, the > WAKE# from endpoint will be routed to its DSP's i.e root port in > direct attach and in case of switch they will routed to the USP from > their again they will be connected to the root port only as there is > noway that individual DSP's in the switch can go to D3cold from > linux point of view as linux will not have control over switch > firmware to control D3cold to D0 sequence. > > But still if the firmware in the DSP of a switch can allow device to > go in to D3cold after moving host moving link to D3hot, the DSP in > the switch needs to receive the WAKE# signal first to supply power > and refclk then DSP will propagate WAKE# to host to change device > state to D0. In this case if there is separate WAKE# signal routed > to the host, we can define WAKE# in the device-tree assigned to the > DSP of the switch. As the DSP's are also tied with the portdrv, the > same existing patch will work since this patch is looking for > wake-gpios property assigned to that particular port in the DT. WAKE# is only defined for certain form factors, and Root Ports and Switch Ports have no WAKE#-related behavior defined by the PCIe specs. I don't want to make assumptions about how WAKE# is routed, whether Switches have implementation-specific WAKE# handling, or how D3cold transitions happen. Those things are all implementation specific. My main objections are: - Setting up a wake IRQ should be done on an endpoint, but this patch assumes doing it on a Root Port or Switch Port is enough. We can start a DT search for a wake IRQ at the endpoint and traverse up the hierarchy if necessary, of course. - The code should not be in portdrv.c. Putting it in portdrv means it won't work unless CONFIG_PCIEPORTBUS is enabled, and WAKE# has nothing to do with the rest of portdrv. Bjorn
