| Message ID | 1721067215-5832-5-git-send-email-quic_mrana@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Manivannan Sadhasivam |
| Headers | show |
| Series | Add power domain and MSI functionality with PCIe host generic ECAM driver | expand |
On 15/07/2024 20:13, Mayank Rana wrote: > Add "power-domains" usage (optional) related binding to power up ECAM > compliant PCIe root complex. > > Signed-off-by: Mayank Rana <quic_mrana@quicinc.com> > --- > Documentation/devicetree/bindings/pci/host-generic-pci.yaml | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml > index 3484e0b..9c714fa 100644 > --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml > +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml > @@ -110,6 +110,12 @@ properties: > iommu-map-mask: true > msi-parent: true > > + power-domains: > + maxItems: 1 > + description: > + A phandle to the node that controls power or/and system resource or interface to firmware Wrap how Coding Style asks (so 80). I am sorry, but power domains are power domains, not interface to firmware to enable your hardware. Rephrase it to actually describe the hardware. Also, drop all redundant information. It cannot be anything else than phandle to the node... > + to enable ECAM compliant PCIe root complex. > + Anyway, there are no DTS users with such power domain. Look at the binding and its compatibles. Does any of these devices have power domain? No. Best regards, Krzysztof
Hi Krzysztof Appreciate your quick review comments. On 7/16/2024 12:25 AM, Krzysztof Kozlowski wrote: > On 15/07/2024 20:13, Mayank Rana wrote: >> Add "power-domains" usage (optional) related binding to power up ECAM >> compliant PCIe root complex. >> >> Signed-off-by: Mayank Rana <quic_mrana@quicinc.com> >> --- >> Documentation/devicetree/bindings/pci/host-generic-pci.yaml | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml >> index 3484e0b..9c714fa 100644 >> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml >> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml >> @@ -110,6 +110,12 @@ properties: >> iommu-map-mask: true >> msi-parent: true >> >> + power-domains: >> + maxItems: 1 >> + description: >> + A phandle to the node that controls power or/and system resource or interface to firmware > > Wrap how Coding Style asks (so 80). My bad, I shall fix it into next patchset. > I am sorry, but power domains are power domains, not interface to > firmware to enable your hardware. Rephrase it to actually describe the > hardware. Agree with your above first part of comment. I understood that you are suggesting to describe all steps in terms of what happen with usage of power domain instead of mentioning generic abstraction with proposed solution here. I mentioned generic way as power domain is not tied with specific compatible string or platform as optional usage with this generic driver. Power domain shall be doing possible below implementation: 1. controls power -> can be just regulators 2. system resource -> can be PCIe related all system resource like GDSC/Clock/regulators/gpio 3. Interface to firmware -> including all system resource handling in addition to PCIe PHY and controller programming to PCIe ECAM RC mode with D0 state. Cover letter is showing high level architecture and usage here although it would be lost. So to document here I can add more information. Below are steps which is being performed: 1. Handle all system resources (GDSC/CLOCKs/regulators/bus (interconnect voting)) 2. Bring PCIe PHY and Controller out of reset 3. Program PCIe PHY and controller to get PCIe link up Power domain interface is also used to perform D3cold based functionality with system suspend case to turn off resources after performing PCIe level handshake (i.e. PME turn off and L23 ready). I can rework/reword above provided power domain binding information but not sure shall I keep it generic information or capture specific above usage with proposed solution. Please let me know what do suggest or prefer here ? > Also, drop all redundant information. It cannot be anything else than > phandle to the node... ACK >> + to enable ECAM compliant PCIe root complex. >> + > > Anyway, there are no DTS users with such power domain. Look at the > binding and its compatibles. Does any of these devices have power > domain? No. Agree. Work in progress, and based on outcome of that I shall add user of it as part of next patchset. > > Best regards, > Krzysztof > >
diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml index 3484e0b..9c714fa 100644 --- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml @@ -110,6 +110,12 @@ properties: iommu-map-mask: true msi-parent: true + power-domains: + maxItems: 1 + description: + A phandle to the node that controls power or/and system resource or interface to firmware + to enable ECAM compliant PCIe root complex. + required: - compatible - reg @@ -172,6 +178,7 @@ examples: // PCI_DEVICE(3) INT#(1) interrupt-map-mask = <0xf800 0x0 0x0 0x7>; + power-domains = <&scmi5_pd 0>; }; }; ...
Add "power-domains" usage (optional) related binding to power up ECAM compliant PCIe root complex. Signed-off-by: Mayank Rana <quic_mrana@quicinc.com> --- Documentation/devicetree/bindings/pci/host-generic-pci.yaml | 7 +++++++ 1 file changed, 7 insertions(+)