Re: [PATCH V2 4/7] dt-bindings: PCI: host-generic-pci: Add power-domains related binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
---
  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






[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux