Re: [PATCH 1/2] dt-bindings: PCI: qcom: Add SC8280XP/SA8540P interconnects

[Johan Hovold <johan@kernel.org>]

On Wed, Oct 19, 2022 at 10:37:31AM -0400, Krzysztof Kozlowski wrote:
> On 17/10/2022 07:24, Johan Hovold wrote:
> > Add the missing SC8280XP/SA8540P "pcie-mem" and "cpu-pcie" interconnect
> > paths to the bindings.
> > 
> > Fixes: 76d777ae045e ("dt-bindings: PCI: qcom: Add SC8280XP to binding")
> > Fixes: 76c4207f4085 ("dt-bindings: PCI: qcom: Add SA8540P to binding")
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  .../devicetree/bindings/pci/qcom,pcie.yaml    | 25 +++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > index 22a2aac4c23f..a55434f95edd 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -62,6 +62,12 @@ properties:
> >      minItems: 3
> >      maxItems: 12
> >  
> > +  interconnects:
> > +    maxItems: 2
> > +
> > +  interconnect-names:
> > +    maxItems: 2
> > +
> >    resets:
> >      minItems: 1
> >      maxItems: 12
> > @@ -629,6 +635,25 @@ allOf:
> >            items:
> >              - const: pci # PCIe core reset
> >  
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,pcie-sa8540p
> > +              - qcom,pcie-sc8280xp
> > +    then:
> > +      properties:
> > +        interconnects:
> > +          maxItems: 2
> 
> No need for this.
> 
> > +        interconnect-names:
> > +          items:
> > +            - const: pcie-mem
> > +            - const: cpu-pcie
> > +      required:
> > +        - interconnects
> > +        - interconnect-names
> 
> else:
>   ??
> 
> Otherwise, you allow any names for other variants.

Are you suggesting something like moving the names to the common
constraints for now:

  interconnects:
    maxItems: 2

  interconnect-names:
    items:
      - const: pcie-mem
      - const: cpu-pcie

and then in the allOf:

  - if:
      properties:
        compatible:
          contains:
            enum:
              - qcom,pcie-sa8540p
              - qcom,pcie-sc8280xp
    then:
      required:
        - interconnects
        - interconnect-names
    else:
      properties:
        interconnects: false
        interconnect-names: false

This way we'd catch anyone adding interconnects to a DTS without first
updating the bindings, but it also seems to go against the idea of
bindings fully describing the hardware by saying that no other platforms
have interconnects (when they actually do even if we don't describe it
just yet).

Or should we do the above but without the else clause to have some
constraints in place on the names at least?

Johan