diff mbox series

[03/10] dt-bindings: PCI: qcom: Enumerate platforms with single msi interrupt

Message ID 20220629141000.18111-4-johan+linaro@kernel.org
State New
Headers show
Series PCI: qcom: Add support for SC8280XP and SA8540P | expand

Commit Message

Johan Hovold June 29, 2022, 2:09 p.m. UTC
Explicitly enumerate the older platforms that have a single msi host
interrupt. This allows for adding further platforms without resorting
to nested conditionals.

Drop the redundant comment about older chipsets instead of moving it.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../devicetree/bindings/pci/qcom,pcie.yaml      | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Krzysztof Kozlowski July 1, 2022, 8:33 a.m. UTC | #1
On 29/06/2022 16:09, Johan Hovold wrote:
> Explicitly enumerate the older platforms that have a single msi host
> interrupt. This allows for adding further platforms without resorting
> to nested conditionals.
> 
> Drop the redundant comment about older chipsets instead of moving it.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

This does not exist in linux-next, so it should be squashed it with the
previous series.

> ---
>  .../devicetree/bindings/pci/qcom,pcie.yaml      | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index a1b4fc70e162..8560c65e6f0b 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -625,7 +625,6 @@ allOf:
>          - reset-names
>  
>      # On newer chipsets support either 1 or 8 msi interrupts
> -    # On older chipsets it's always 1 msi interrupt
>    - if:
>        properties:
>          compatible:
> @@ -660,7 +659,21 @@ allOf:
>                  - const: msi5
>                  - const: msi6
>                  - const: msi7
> -    else:
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-apq8064
> +              - qcom,pcie-apq8084
> +              - qcom,pcie-ipq4019
> +              - qcom,pcie-ipq6018
> +              - qcom,pcie-ipq8064
> +              - qcom,pcie-ipq8064-v2
> +              - qcom,pcie-ipq8074
> +              - qcom,pcie-qcs404

Otherwise I cannot even check the context...

> +    then:
>        properties:
>          interrupts:
>            maxItems: 1


Best regards,
Krzysztof
Krzysztof Kozlowski July 1, 2022, 8:35 a.m. UTC | #2
On 29/06/2022 16:09, Johan Hovold wrote:
> Explicitly enumerate the older platforms that have a single msi host
> interrupt. This allows for adding further platforms without resorting
> to nested conditionals.

How does it allow it? New platform if not explicitly added to first
"if:" will fall into the "else:", so will be handled and there is no
need for nested if.

Best regards,
Krzysztof
Johan Hovold July 1, 2022, 8:38 a.m. UTC | #3
On Fri, Jul 01, 2022 at 10:33:35AM +0200, Krzysztof Kozlowski wrote:
> On 29/06/2022 16:09, Johan Hovold wrote:
> > Explicitly enumerate the older platforms that have a single msi host
> > interrupt. This allows for adding further platforms without resorting
> > to nested conditionals.
> > 
> > Drop the redundant comment about older chipsets instead of moving it.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> 
> This does not exist in linux-next, so it should be squashed it with the
> previous series.

As mentioned in the cover letter this depends on the MSI series that has
unfortunately not yet been merged.

That series is self-contained and ready to be merged, so this follow-up
does not need to be squashed in.

> > ---
> >  .../devicetree/bindings/pci/qcom,pcie.yaml      | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > index a1b4fc70e162..8560c65e6f0b 100644
> > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> > @@ -625,7 +625,6 @@ allOf:
> >          - reset-names
> >  
> >      # On newer chipsets support either 1 or 8 msi interrupts
> > -    # On older chipsets it's always 1 msi interrupt
> >    - if:
> >        properties:
> >          compatible:
> > @@ -660,7 +659,21 @@ allOf:
> >                  - const: msi5
> >                  - const: msi6
> >                  - const: msi7
> > -    else:
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - qcom,pcie-apq8064
> > +              - qcom,pcie-apq8084
> > +              - qcom,pcie-ipq4019
> > +              - qcom,pcie-ipq6018
> > +              - qcom,pcie-ipq8064
> > +              - qcom,pcie-ipq8064-v2
> > +              - qcom,pcie-ipq8074
> > +              - qcom,pcie-qcs404
> 
> Otherwise I cannot even check the context...

Yeah, I realise that makes reviewing a bit harder, but hopefully the
maintainer will pick up the MSI series soon.

> 
> > +    then:
> >        properties:
> >          interrupts:
> >            maxItems: 1

Johan
Rob Herring July 1, 2022, 6:38 p.m. UTC | #4
On Fri, Jul 01, 2022 at 10:38:42AM +0200, Johan Hovold wrote:
> On Fri, Jul 01, 2022 at 10:33:35AM +0200, Krzysztof Kozlowski wrote:
> > On 29/06/2022 16:09, Johan Hovold wrote:
> > > Explicitly enumerate the older platforms that have a single msi host
> > > interrupt. This allows for adding further platforms without resorting
> > > to nested conditionals.
> > > 
> > > Drop the redundant comment about older chipsets instead of moving it.
> > > 
> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > 
> > This does not exist in linux-next, so it should be squashed it with the
> > previous series.
> 
> As mentioned in the cover letter this depends on the MSI series that has
> unfortunately not yet been merged.
> 
> That series is self-contained and ready to be merged, so this follow-up
> does not need to be squashed in.

I suspect that Bjorn would rather squash these in.

Rob
Johan Hovold July 4, 2022, 2:21 p.m. UTC | #5
On Fri, Jul 01, 2022 at 12:38:19PM -0600, Rob Herring wrote:
> On Fri, Jul 01, 2022 at 10:38:42AM +0200, Johan Hovold wrote:
> > On Fri, Jul 01, 2022 at 10:33:35AM +0200, Krzysztof Kozlowski wrote:
> > > On 29/06/2022 16:09, Johan Hovold wrote:
> > > > Explicitly enumerate the older platforms that have a single msi host
> > > > interrupt. This allows for adding further platforms without resorting
> > > > to nested conditionals.
> > > > 
> > > > Drop the redundant comment about older chipsets instead of moving it.
> > > > 
> > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > > 
> > > This does not exist in linux-next, so it should be squashed it with the
> > > previous series.
> > 
> > As mentioned in the cover letter this depends on the MSI series that has
> > unfortunately not yet been merged.
> > 
> > That series is self-contained and ready to be merged, so this follow-up
> > does not need to be squashed in.
> 
> I suspect that Bjorn would rather squash these in.

Sure. Squashing in the compatible-conditional fix makes sense, but the
motivation for this one is the SoC added by this series so I'd argue
that it belongs here. But either way is fine with me.

Johan
Manivannan Sadhasivam July 9, 2022, 7:58 a.m. UTC | #6
On Wed, Jun 29, 2022 at 04:09:53PM +0200, Johan Hovold wrote:
> Explicitly enumerate the older platforms that have a single msi host
> interrupt. This allows for adding further platforms without resorting
> to nested conditionals.
> 
> Drop the redundant comment about older chipsets instead of moving it.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>

With the comment from Krzysztof on wording mentioned in patch 4/10
addressed,

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani


> ---
>  .../devicetree/bindings/pci/qcom,pcie.yaml      | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> index a1b4fc70e162..8560c65e6f0b 100644
> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
> @@ -625,7 +625,6 @@ allOf:
>          - reset-names
>  
>      # On newer chipsets support either 1 or 8 msi interrupts
> -    # On older chipsets it's always 1 msi interrupt
>    - if:
>        properties:
>          compatible:
> @@ -660,7 +659,21 @@ allOf:
>                  - const: msi5
>                  - const: msi6
>                  - const: msi7
> -    else:
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,pcie-apq8064
> +              - qcom,pcie-apq8084
> +              - qcom,pcie-ipq4019
> +              - qcom,pcie-ipq6018
> +              - qcom,pcie-ipq8064
> +              - qcom,pcie-ipq8064-v2
> +              - qcom,pcie-ipq8074
> +              - qcom,pcie-qcs404
> +    then:
>        properties:
>          interrupts:
>            maxItems: 1
> -- 
> 2.35.1
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
index a1b4fc70e162..8560c65e6f0b 100644
--- a/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/qcom,pcie.yaml
@@ -625,7 +625,6 @@  allOf:
         - reset-names
 
     # On newer chipsets support either 1 or 8 msi interrupts
-    # On older chipsets it's always 1 msi interrupt
   - if:
       properties:
         compatible:
@@ -660,7 +659,21 @@  allOf:
                 - const: msi5
                 - const: msi6
                 - const: msi7
-    else:
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,pcie-apq8064
+              - qcom,pcie-apq8084
+              - qcom,pcie-ipq4019
+              - qcom,pcie-ipq6018
+              - qcom,pcie-ipq8064
+              - qcom,pcie-ipq8064-v2
+              - qcom,pcie-ipq8074
+              - qcom,pcie-qcs404
+    then:
       properties:
         interrupts:
           maxItems: 1