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/17/2024 11:05 PM, Krzysztof Kozlowski wrote:
On 17/07/2024 19:20, Mayank Rana wrote:
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.

Well, it is not. If you read their bindings or any reviews related to
such cores, you would see that single Designware compatible is always
never appropriate. Such cores always have customization per user.

You can also look at the binding you are changing. Do you see Designware
alone? No.
I found reference in this binding as below:
 79         items:
 80           - enum:
 81               - marvell,armada8k-pcie-ecam
 82               - socionext,synquacer-pcie-ecam
 83           - const: snps,dw-pcie-ecam

And as you mentioned in previous emails about how to add such usage, I ACKed it but let me put reason why I tried to add differently to start with it.

This specific driver under-discussion is really not vendor specific driver. It can work with any PCIe controller which is already configured in ECAM mode by firmware (i.e. PCIe controller from SNPS or any other vendor). There are few quirks only added to get specific vendor based SOC configuration for SNPS PCIe controller by different SOC vendors.

 90       - description:
 91           CAM or ECAM compliant PCI host controllers without any quirks
 92         enum:
 93           - pci-host-cam-generic
 94           - pci-host-ecam-generic

Above enum based usage works for SA8775P platform which is having SNPS PCIe controller which doesn't need any quirks, and firmware is able to configure PCIe controller into ECAM mode. Here I tried adding PCIe SNPS controller based MSI support as SA8775P doesn't support LPI/ITS for MSI. I need to differentiate it hence added generic enum as MSI controller is part of SNPS PCIe controller, and not separate MSI IP here. Although how many MSI can be supported it depends on how many SPIs are available/used with MSI controller depend on particular SOC. Hence put as snps,dw-pcie-ecam-msi as usage and variable number of MSI. hopefully I am able to convey why this driver binding modified differently.

Regards,
Mayank




[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