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

[Krzysztof Kozlowski]

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.

> 
> 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? Anyway, aren't you
forgetting specific compatible? Please open your internal (quite
comprehensive) guideline on bindings and DTS.


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

> +
>  required:
>    - compatible
>    - reg
> @@ -146,11 +166,22 @@ allOf:
>          reg:
>            maxItems: 1


Best regards,
Krzysztof