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

[Krzysztof Kozlowski]

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

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.

Best regards,
Krzysztof