schemas: pci: bridge: Document WAKE# interrupt properties #126
Conversation
|
Can PCI-PCI bridges also generate legacy interrupts? |
|
I don't think the bridge itself will trigger legacy interrupts, but PCI-PCI bridge specification has below reference: Chapter 9, sec 9.1: |
Mani-Sadhasivam
force-pushed
the
pcie-wake
branch
from
0e95c13 to
ef8b808
Compare
|
@robherring Ping |
| @@ -34,4 +34,10 @@ properties: | |||
| to D3 states. | |||
| type: boolean | |||
|
|
|||
| interrupts-extended: | |||
There was a problem hiding this comment.
First, schemas just define 'interrupts'. The tools fix them up to support either interrupts or interrupts-extended. The fact that you have to use interrupts-extended to express this is outside the scope of the binding.
Second, we already have interrupts pulled in from pci-device.yaml. You can't just add interrupts-extended here. Which one does the OS use?
Also, is your intent WAKE goes in the source node or the parent node? The "correct" place is the source, but it's kind of fuzzy for PCI because we've put slot properties in the parent node.
There was a problem hiding this comment.
First, schemas just define 'interrupts'. The tools fix them up to support either interrupts or interrupts-extended. The fact that you have to use interrupts-extended to express this is outside the scope of the binding.
Okay. Will change it to interrupts.
Also, is your intent WAKE goes in the source node or the parent node? The "correct" place is the source, but it's kind of fuzzy for PCI because we've put slot properties in the parent node.
I believe you are referring to PCIe controller as parent and PCIe bridge as source. If so, yes the WAKE# interrupt belongs to the bridge node. But it is also possible that the interrupt might not be routed to the bridge in some platforms. Like in all Qcom platforms, both PERST# and WAKE# are just regular GPIOs without any PCIe hw control. So they do not strictly belong to the bridge node. Moreover, all Qcom platforms only support a single bridge per controller, so both of these GPIOs were described in the controller binding from the start.
So to conclude, WAKE# interrupt should be described for platforms that route WAKE# to the bridge in hw. This is the case for all of the platforms supporting multiple bridges (slots) per controller.
There was a problem hiding this comment.
I believe you are referring to PCIe controller as parent and PCIe bridge as source.
No. It's either the bridge (or root port) or the device. Putting it in the PCI host would be wrong as there is no link between the host and RP.
Most hosts have only a single RP like QCom, but it is entirely possible there is a PCIe switch on a board for any of those. Then there are multiple PERST# and WAKE# which could be all GPIOs routed to the host SoC. See HiKey 960 (or 970?) PCI. So we need to make sure this can work for something like that.
There was a problem hiding this comment.
Putting it in the PCI host would be wrong as there is no link between the host and RP.
I don't understand how there is no link between the host bridge and RP.
See HiKey 960 (or 970?) PCI. So we need to make sure this can work for something like that.
It is Hikey970. But, I do not see how we can add these properties (both PERST# and WAKE#) to the device node. Because the RC driver on the host needs to control/configure these GPIOs even before enumerating the device (in the case of PERST#). If a design has multiple PERST# and WAKE# for multiple endpoints, then those belong to either bridge (if routed in hw) or RC (for regular GPIOs).
Atleast for this binding, we can define up to 32 interrupts as per the PCIe spec.
There was a problem hiding this comment.
I don't understand how there is no link between the host bridge and RP.
The link is between a RP and the attached, external PCIe device. The RP is just part of the host bridge (DWC or whatever). It's so intertwined that we often haven't even had a node for the RP. But that's wrong because as soon as 1 host bridge has multiple RPs, our bindings break.
See HiKey 960 (or 970?) PCI. So we need to make sure this can work for something like that.
It is Hikey970. But, I do not see how we can add these properties (both PERST# and WAKE#) to the device node. Because the RC driver on the host needs to control/configure these GPIOs even before enumerating the device (in the case of PERST#).
If a design has multiple PERST# and WAKE# for multiple endpoints, then those belong to either bridge (if routed in hw) or RC (for regular GPIOs).
If you had a design with 100 devices behind switches, you would stick 100 GPIOs (or 200 with PERST) in the RC node? That doesn't tell you what the association of GPIO to each device is.
Atleast for this binding, we can define up to 32 interrupts as per the PCIe spec.
Where does 32 come from?
There was a problem hiding this comment.
The link is between a RP and the attached, external PCIe device. The RP is just part of the host bridge (DWC or whatever). It's so intertwined that we often haven't even had a node for the RP. But that's wrong because as soon as 1 host bridge has multiple RPs, our bindings break.
Right. I submitted patches to add bridge nodes for all Qcom SoCs, but I did not add the WAKE# GPIO to the bridge node because it is not connected in the hw. So I'm not sure what would be the apt node. Are you suggesting the device node in this case?
If you had a design with 100 devices behind switches, you would stick 100 GPIOs (or 200 with PERST) in the RC node? That doesn't tell you what the association of GPIO to each device is.
Ideally, the switch will control the PEST# for the downstream devices. But the case applies if all the endpoints are directly connected to the single controller. And I get your point.
Where does 32 come from?
From the BDF identifier. Device has only 5 bits.
WAKE# sideband interrupt is used by the PCIe devices to signal the host to re-establish power and reference clocks while waking from D3Cold/L2 state. This is based on the DT bindings patch proposed in LKML: https://lore.kernel.org/linux-pci/[email protected]/ In that patch, there were 2 interrupts mentioned: "wake" and "pci", where the latter one was described as "legacy PCI interrupt". But those legacy interrupts are already defined as "INT-{A,B,C,D}" in pci-device.yaml. So I removed that one and just kept "wake". Most of the platforms route the WAKE# GPIO to the PCI bridges (slots) in hardware. But some platforms like Qcom SoCs, do not do that and for those platforms the WAKE# interrupt is described in the controller binding itself. Signed-off-by: Manivannan Sadhasivam <[email protected]>
Mani-Sadhasivam
force-pushed
the
pcie-wake
branch
from
ef8b808 to
174127f
Compare
WAKE# sideband interrupt is used by the PCIe devices to signal the host to re-establish power and reference clocks while waking from D3Cold/L2 state.
This is based on the DT bindings patch proposed to LKML: https://lore.kernel.org/linux-pci/[email protected]/
In that patch, there were 2 interrupts mentioned: "wake" and "pci", and the latter one was described as "legacy PCI interrupt". But those legacy interrupts are already defined as "INT-{A,B,C,D}" in pci-device.yaml. So I removed that one and just kept "wake".