Re: [PATCH V226/7] dt-bindings: PCI: host-generic-pci: Add snps,dw-pcie-ecam-msi binding

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

 



Hi Krzysztof

On 7/16/2024 11:47 PM, Krzysztof Kozlowski wrote:
On 17/07/2024 00:09, Mayank Rana wrote:
Hi Krzysztof

On 7/16/2024 12:28 AM, Krzysztof Kozlowski wrote:
On 15/07/2024 20:13, Mayank Rana wrote:
To support MSI functionality using Synopsys DesignWare PCIe controller
based MSI controller with ECAM driver, add "snps,dw-pcie-ecam-msi
compatible binding which uses provided SPIs to support MSI functionality.

To support MSI, you add MSI support... That's a tautology. Describe
hardware instead.
Ok. let me repharse it to provide more useful information.

Signed-off-by: Mayank Rana <quic_mrana@xxxxxxxxxxx>
---
   .../devicetree/bindings/pci/host-generic-pci.yaml  | 57 ++++++++++++++++++++++
   1 file changed, 57 insertions(+)

diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
index 9c714fa..9e860d5 100644
--- a/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
+++ b/Documentation/devicetree/bindings/pci/host-generic-pci.yaml
@@ -81,6 +81,12 @@ properties:
                 - marvell,armada8k-pcie-ecam
                 - socionext,synquacer-pcie-ecam
             - const: snps,dw-pcie-ecam
+      - description: |
+         Firmware is configuring Synopsys DesignWare PCIe controller in RC mode with
+         ECAM compatible fashion. To use MSI controller of Synopsys DesignWare PCIe
+         controller for MSI functionality, this compatible is used.
+        items:
+          - const: snps,dw-pcie-ecam-msi

MSI is already present in the binding, isn't it?
It is mentioning as msi-parent usage which could be different MSI
controller (GIC or vendor specific one) not related to designware PCIe
controller based MSI controller.

Anyway, aren't you
forgetting specific compatible? Please open your internal (quite
comprehensive) guideline on bindings and DTS.
Here I am trying to define Designware based PCIe ECAM controller
supporting MSIcontroller based device. Hence I am not mentioning vendor
specific compatible usage
and keeping generic compatible binding for such device.

I know what you try, yet it feels simply wrong. Read your guideline.
Are you sure you work on Designware core itself, not on one used in
Qualcomm? I would expect people from Designware to design Designware
cores and people from Qualcomm only to design licensed cores.
Ok. let me not make generic comment here. I refereed how it is done with other
snps based IP usage for example USB, and would follow same.

         - description:
             CAM or ECAM compliant PCI host controllers without any quirks
           enum:
@@ -116,6 +122,20 @@ properties:
         A phandle to the node that controls power or/and system resource or interface to firmware
         to enable ECAM compliant PCIe root complex.
+ interrupts:
+    description:
+      DWC PCIe Root Port/Complex specific MSI interrupt/IRQs.
+    minItems: 1
+    maxItems: 8
+
+  interrupt-names:
+    description:
+      MSI interrupt names
+    minItems: 1
+    maxItems: 8
+    items:
+        pattern: '^msi[0-9]+$'

Why the same devices have variable numbers?
Max supported MSI with designware PCIe controller is 8 Only, and it
depends if those all are
used or some of used on specific vendor based device. Hence I have kept
it here variable names. Although here it should be [0 - 7] instead of
[0 - 9].

Wait, you just said there is no specific vendor device.

Sorry, bring some sanity to this
I understood that you are having concern about generic vs platform specific usage here in binding document. I would try avoid mixing those, and will try to do just platform specific usage. Thanks for having patience with me, and helping me to here.

Regards,
Mayank




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux