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

[Mayank Rana]

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 at quicinc.com>
>> ---
>>   .../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.
> 
>>         - 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].
>> +
>>   required:
>>     - compatible
>>     - reg
>> @@ -146,11 +166,22 @@ allOf:
>>           reg:
>>             maxItems: 1
> 
> 
> Best regards,
> Krzysztof
>