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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y1D/Vaa/3zKP4Cxj@hovoldconsulting.com>
Date:   Thu, 20 Oct 2022 09:57:09 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Johan Hovold <johan+linaro@...nel.org>,
        Stanimir Varbanov <svarbanov@...sol.com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Krishna chaitanya chundru <quic_krichai@...cinc.com>,
        quic_vbadigan@...cinc.com, linux-arm-msm@...r.kernel.org,
        linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] dt-bindings: PCI: qcom: Add SC8280XP/SA8540P
 interconnects

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