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

[Mayank Rana]

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 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.
> 
> 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