Re: [RFC PATCH v11 4/5] PCI / PM: Add support for the PCIe WAKE# signal for OF

[Brian Norris]

Hi,

On Thu, Jan 25, 2018 at 05:54:23PM +0100, Rafael J. Wysocki wrote:
> On Thu, Jan 25, 2018 at 5:40 PM, Tony Lindgren <tony@xxxxxxxxxxx> wrote:
> > * Brian Norris <briannorris@xxxxxxxxxxxx> [180125 01:22]:
> >> On Fri, Jan 05, 2018 at 02:13:33AM +0100, Rafael J. Wysocki wrote:
> >> > The "wakeup-interrupt" property at the bridge level basically has to be defined
> >> > as the wakeup interrupt for all devices on the bus under the bridge.
> >>
> >> The only thing I'm at a loss for is whether this goes in (referring to
> >> rk3399-gru.dtsi) &pcie or &pci_rootport. I think some versions of this
> >> series have been aiming for the former, and some the latter.
> >
> > I'd keep the wakeup interrupt property at the rootport level. That way
> > it can work with whatever pcie card that might be plugged into that
> > slot. That is in case it's just a slot and not hardwired pcie device :)

^^ Right, and that's what I believe this series was doing. Previous
versions put it in &pcie, which might have had a similar effect. The
existing behavior is the misguided bindings that put it in &mvl_wifi
(the endpoint device).

> Do I understand correctly that &pcie is the device and the
> &pci_rootport is the port that device is connected to?

No. (Assuming "device" means "endpoint PCIe device".)

&pcie: The top-level representation of the host bridge

&pci_rootport: a virtual representation of a root port. I don't think
this corresponds to a Linux device in the end, but I thought it
corresponded most closely to a "slot" [1]

&mvl_wifi: an endpoint device

Brian

[1] As another example, consider
arch/arm/boot/dts/armada-370{.dtsi,-dlink-dns327l.dts}
It has a structure of:

  pciec: pcie@82000000 {
    pcie0: pcie@1,0 /* Port 0, Lane 0 */ {
      // Brian: no subnode, since we don't generally want to describe
      // specific endpoints in DT, when they should be autodetectable
    };
    pcie2: pcie@2,0 /* Port 1, Lane 0 */ {
      // Brian: no subnode, since we don't generally want to describe
      // specific endpoints in DT, when they should be autodetectable
    };
  };

Where Gru's &pcie is equivalent to Armada's &pciec, and Gru's
&pci_rootport is equivalent to &pcie0 and &pcie2.