[PATCH 1/3] dt-bindings: PCI: brcm,iproc-pcie: Fix example indentation

[PATCH 1/3] dt-bindings: PCI: brcm,iproc-pcie: Fix example indentation

[Rob Herring]

The example's indentation is off. While fixing this, the 'bus' node
is unnecessary and can be dropped. It is also preferred to split up
unrelated examples to their own entries.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../bindings/pci/brcm,iproc-pcie.yaml         | 124 +++++++++---------
 1 file changed, 60 insertions(+), 64 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
index 0972868735fc..0cb5bd6cffa1 100644
--- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
@@ -117,68 +117,64 @@ unevaluatedProperties: false
 
 examples:
   - |
-   #include <dt-bindings/interrupt-controller/arm-gic.h>
-
-   bus {
-      #address-cells = <1>;
-      #size-cells = <1>;
-           pcie0: pcie@18012000 {
-              compatible = "brcm,iproc-pcie";
-              reg = <0x18012000 0x1000>;
-
-              #interrupt-cells = <1>;
-              interrupt-map-mask = <0 0 0 0>;
-              interrupt-map = <0 0 0 0 &gic GIC_SPI 100 IRQ_TYPE_NONE>;
-
-              linux,pci-domain = <0>;
-
-              bus-range = <0x00 0xff>;
-
-              #address-cells = <3>;
-              #size-cells = <2>;
-              device_type = "pci";
-              ranges = <0x81000000 0 0     0x28000000 0 0x00010000>,
-                   <0x82000000 0 0x20000000 0x20000000 0 0x04000000>;
-
-              phys = <&phy 0 5>;
-              phy-names = "pcie-phy";
-
-              brcm,pcie-ob;
-              brcm,pcie-ob-axi-offset = <0x00000000>;
-
-              msi-parent = <&msi0>;
-
-              /* iProc event queue based MSI */
-              msi0: msi {
-                 compatible = "brcm,iproc-msi";
-                 msi-controller;
-                 interrupt-parent = <&gic>;
-                 interrupts = <GIC_SPI 96 IRQ_TYPE_NONE>,
-                         <GIC_SPI 97 IRQ_TYPE_NONE>,
-                         <GIC_SPI 98 IRQ_TYPE_NONE>,
-                         <GIC_SPI 99 IRQ_TYPE_NONE>;
-              };
-           };
-
-           pcie1: pcie@18013000 {
-              compatible = "brcm,iproc-pcie";
-              reg = <0x18013000 0x1000>;
-
-              #interrupt-cells = <1>;
-              interrupt-map-mask = <0 0 0 0>;
-              interrupt-map = <0 0 0 0 &gic GIC_SPI 106 IRQ_TYPE_NONE>;
-
-              linux,pci-domain = <1>;
-
-              bus-range = <0x00 0xff>;
-
-              #address-cells = <3>;
-              #size-cells = <2>;
-              device_type = "pci";
-              ranges = <0x81000000 0 0     0x48000000 0 0x00010000>,
-                   <0x82000000 0 0x40000000 0x40000000 0 0x04000000>;
-
-              phys = <&phy 1 6>;
-              phy-names = "pcie-phy";
-           };
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    pcie@18012000 {
+        compatible = "brcm,iproc-pcie";
+        reg = <0x18012000 0x1000>;
+
+        #interrupt-cells = <1>;
+        interrupt-map-mask = <0 0 0 0>;
+        interrupt-map = <0 0 0 0 &gic GIC_SPI 100 IRQ_TYPE_NONE>;
+
+        linux,pci-domain = <0>;
+
+        bus-range = <0x00 0xff>;
+
+        #address-cells = <3>;
+        #size-cells = <2>;
+        device_type = "pci";
+        ranges = <0x81000000 0          0 0x28000000 0 0x00010000>,
+                 <0x82000000 0 0x20000000 0x20000000 0 0x04000000>;
+
+        phys = <&phy 0 5>;
+        phy-names = "pcie-phy";
+
+        brcm,pcie-ob;
+        brcm,pcie-ob-axi-offset = <0x00000000>;
+
+        msi-parent = <&msi0>;
+
+        /* iProc event queue based MSI */
+        msi0: msi {
+            compatible = "brcm,iproc-msi";
+            msi-controller;
+            interrupt-parent = <&gic>;
+            interrupts = <GIC_SPI 96 IRQ_TYPE_NONE>,
+                    <GIC_SPI 97 IRQ_TYPE_NONE>,
+                    <GIC_SPI 98 IRQ_TYPE_NONE>,
+                    <GIC_SPI 99 IRQ_TYPE_NONE>;
+        };
+    };
+  - |
+    pcie@18013000 {
+        compatible = "brcm,iproc-pcie";
+        reg = <0x18013000 0x1000>;
+
+        #interrupt-cells = <1>;
+        interrupt-map-mask = <0 0 0 0>;
+        interrupt-map = <0 0 0 0 &gic GIC_SPI 106 IRQ_TYPE_NONE>;
+
+        linux,pci-domain = <1>;
+
+        bus-range = <0x00 0xff>;
+
+        #address-cells = <3>;
+        #size-cells = <2>;
+        device_type = "pci";
+        ranges = <0x81000000 0          0 0x48000000 0 0x00010000>,
+                 <0x82000000 0 0x40000000 0x40000000 0 0x04000000>;
+
+        phys = <&phy 1 6>;
+        phy-names = "pcie-phy";
     };
-- 
2.40.1

[PATCH 2/3] dt-bindings: PCI: brcm,iproc-pcie: Drop common pci-bus properties

[Rob Herring]

Drop the unnecessary listing of properties already defined in
pci-bus.yaml. Unless there are additional constraints, it is not
necessary.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/pci/brcm,iproc-pcie.yaml  | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
index 0cb5bd6cffa1..6730d68fedc7 100644
--- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
@@ -34,13 +34,6 @@ properties:
     description: >
        Base address and length of the PCIe controller I/O register space
 
-  interrupt-map: true
-
-  interrupt-map-mask: true
-
-  "#interrupt-cells":
-    const: 1
-
   ranges:
     minItems: 1
     maxItems: 2
@@ -54,16 +47,8 @@ properties:
     items:
       - const: pcie-phy
 
-  bus-range: true
-
   dma-coherent: true
 
-  "#address-cells": true
-
-  "#size-cells": true
-
-  device_type: true
-
   brcm,pcie-ob:
     type: boolean
     description: >
-- 
2.40.1

Re: [PATCH 2/3] dt-bindings: PCI: brcm,iproc-pcie: Drop common pci-bus properties

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1401 bytes --]

On Tue, Sep 26, 2023 at 10:53:41AM -0500, Rob Herring wrote:
> Drop the unnecessary listing of properties already defined in
> pci-bus.yaml. Unless there are additional constraints, it is not
> necessary.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> ---
>  .../devicetree/bindings/pci/brcm,iproc-pcie.yaml  | 15 ---------------
>  1 file changed, 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
> index 0cb5bd6cffa1..6730d68fedc7 100644
> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
> @@ -34,13 +34,6 @@ properties:
>      description: >
>         Base address and length of the PCIe controller I/O register space
>  
> -  interrupt-map: true
> -
> -  interrupt-map-mask: true
> -
> -  "#interrupt-cells":
> -    const: 1
> -
>    ranges:
>      minItems: 1
>      maxItems: 2
> @@ -54,16 +47,8 @@ properties:
>      items:
>        - const: pcie-phy
>  
> -  bus-range: true
> -
>    dma-coherent: true
>  
> -  "#address-cells": true
> -
> -  "#size-cells": true
> -
> -  device_type: true
> -
>    brcm,pcie-ob:
>      type: boolean
>      description: >
> -- 
> 2.40.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/3] dt-bindings: PCI: brcm,iproc-pcie: Drop common pci-bus properties

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 312 bytes --]



On 9/26/2023 5:53 PM, Rob Herring wrote:
> Drop the unnecessary listing of properties already defined in
> pci-bus.yaml. Unless there are additional constraints, it is not
> necessary.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH 2/3] dt-bindings: PCI: brcm,iproc-pcie: Drop common pci-bus properties

[Rob Herring]

On Tue, 26 Sep 2023 10:53:41 -0500, Rob Herring wrote:
> Drop the unnecessary listing of properties already defined in
> pci-bus.yaml. Unless there are additional constraints, it is not
> necessary.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../devicetree/bindings/pci/brcm,iproc-pcie.yaml  | 15 ---------------
>  1 file changed, 15 deletions(-)
> 

Applied, thanks!

[PATCH 3/3] dt-bindings: PCI: brcm,iproc-pcie: Fix 'msi' child node schema

[Rob Herring]

The 'msi' child node schema is missing constraints on additional properties.
It turns out it is incomplete and properties for it are documented in the
parent node by mistake. Move the reference to msi-controller.yaml and
the custom properties to the 'msi' node. Adding 'unevaluatedProperties'
ensures all the properties in the 'msi' node are documented.

With the schema corrected, a minimal interrupt controller node is needed
to properly decode the interrupt properties since the example has
multiple interrupt parents.

Signed-off-by: Rob Herring <robh@kernel.org>
---
 .../bindings/pci/brcm,iproc-pcie.yaml         | 24 ++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
index 6730d68fedc7..0e07ab61a48d 100644
--- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
+++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
@@ -12,7 +12,6 @@ maintainers:
 
 allOf:
   - $ref: /schemas/pci/pci-bus.yaml#
-  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
 
 properties:
   compatible:
@@ -63,20 +62,24 @@ properties:
 
   msi:
     type: object
+    $ref: /schemas/interrupt-controller/msi-controller.yaml#
+    unevaluatedProperties: false
+
     properties:
       compatible:
         items:
           - const: brcm,iproc-msi
 
-  msi-parent: true
+      interrupts:
+        maxItems: 4
 
-  msi-controller: true
+      brcm,pcie-msi-inten:
+        type: boolean
+        description:
+          Needs to be present for some older iProc platforms that require the
+          interrupt enable registers to be set explicitly to enable MSI
 
-  brcm,pcie-msi-inten:
-    type: boolean
-    description: >
-      Needs to be present for some older iProc platforms that require the
-      interrupt enable registers to be set explicitly to enable MSI
+  msi-parent: true
 
 dependencies:
   brcm,pcie-ob-axi-offset: ["brcm,pcie-ob"]
@@ -104,6 +107,11 @@ examples:
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
 
+    gic: interrupt-controller {
+        interrupt-controller;
+        #interrupt-cells = <3>;
+    };
+
     pcie@18012000 {
         compatible = "brcm,iproc-pcie";
         reg = <0x18012000 0x1000>;
-- 
2.40.1

Re: [PATCH 3/3] dt-bindings: PCI: brcm,iproc-pcie: Fix 'msi' child node schema

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2991 bytes --]

On Tue, Sep 26, 2023 at 10:56:09AM -0500, Rob Herring wrote:
> The 'msi' child node schema is missing constraints on additional properties.
> It turns out it is incomplete and properties for it are documented in the
> parent node by mistake. Move the reference to msi-controller.yaml and
> the custom properties to the 'msi' node. Adding 'unevaluatedProperties'
> ensures all the properties in the 'msi' node are documented.
> 
> With the schema corrected, a minimal interrupt controller node is needed
> to properly decode the interrupt properties since the example has
> multiple interrupt parents.

I suppose this is an ABI break, but the patch just makes the binding
match the example and intent. Feels like of all the patches doing the
unevaluatedProperty additions, this one is the most deserving of a fixes
tag, since the original binding just seems to be completely wrong?

Otherwise,
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/pci/brcm,iproc-pcie.yaml         | 24 ++++++++++++-------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
> index 6730d68fedc7..0e07ab61a48d 100644
> --- a/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
> +++ b/Documentation/devicetree/bindings/pci/brcm,iproc-pcie.yaml
> @@ -12,7 +12,6 @@ maintainers:
>  
>  allOf:
>    - $ref: /schemas/pci/pci-bus.yaml#
> -  - $ref: /schemas/interrupt-controller/msi-controller.yaml#
>  
>  properties:
>    compatible:
> @@ -63,20 +62,24 @@ properties:
>  
>    msi:
>      type: object
> +    $ref: /schemas/interrupt-controller/msi-controller.yaml#
> +    unevaluatedProperties: false
> +
>      properties:
>        compatible:
>          items:
>            - const: brcm,iproc-msi
>  
> -  msi-parent: true
> +      interrupts:
> +        maxItems: 4
>  
> -  msi-controller: true
> +      brcm,pcie-msi-inten:
> +        type: boolean
> +        description:
> +          Needs to be present for some older iProc platforms that require the
> +          interrupt enable registers to be set explicitly to enable MSI
>  
> -  brcm,pcie-msi-inten:
> -    type: boolean
> -    description: >
> -      Needs to be present for some older iProc platforms that require the
> -      interrupt enable registers to be set explicitly to enable MSI
> +  msi-parent: true
>  
>  dependencies:
>    brcm,pcie-ob-axi-offset: ["brcm,pcie-ob"]
> @@ -104,6 +107,11 @@ examples:
>    - |
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
>  
> +    gic: interrupt-controller {
> +        interrupt-controller;
> +        #interrupt-cells = <3>;
> +    };
> +
>      pcie@18012000 {
>          compatible = "brcm,iproc-pcie";
>          reg = <0x18012000 0x1000>;
> -- 
> 2.40.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 3/3] dt-bindings: PCI: brcm,iproc-pcie: Fix 'msi' child node schema

[Rob Herring]

On Wed, Sep 27, 2023 at 10:35 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Sep 26, 2023 at 10:56:09AM -0500, Rob Herring wrote:
> > The 'msi' child node schema is missing constraints on additional properties.
> > It turns out it is incomplete and properties for it are documented in the
> > parent node by mistake. Move the reference to msi-controller.yaml and
> > the custom properties to the 'msi' node. Adding 'unevaluatedProperties'
> > ensures all the properties in the 'msi' node are documented.
> >
> > With the schema corrected, a minimal interrupt controller node is needed
> > to properly decode the interrupt properties since the example has
> > multiple interrupt parents.
>
> I suppose this is an ABI break, but the patch just makes the binding
> match the example and intent.

It also matches what the in tree users do, so not an ABI break. I
imagine the .txt binding just listed out properties and the conversion
carried that over.

> Feels like of all the patches doing the
> unevaluatedProperty additions, this one is the most deserving of a fixes
> tag, since the original binding just seems to be completely wrong?

Yes, though the example fix is a dependency, so probably not worth backporting.

Fixes: 905b986d099c ("dt-bindings: pci: Convert iProc PCIe to YAML")

Rob

Re: [PATCH 3/3] dt-bindings: PCI: brcm,iproc-pcie: Fix 'msi' child node schema

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 708 bytes --]



On 9/26/2023 5:56 PM, Rob Herring wrote:
> The 'msi' child node schema is missing constraints on additional properties.
> It turns out it is incomplete and properties for it are documented in the
> parent node by mistake. Move the reference to msi-controller.yaml and
> the custom properties to the 'msi' node. Adding 'unevaluatedProperties'
> ensures all the properties in the 'msi' node are documented.
> 
> With the schema corrected, a minimal interrupt controller node is needed
> to properly decode the interrupt properties since the example has
> multiple interrupt parents.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH 3/3] dt-bindings: PCI: brcm,iproc-pcie: Fix 'msi' child node schema

[Rob Herring]

On Tue, 26 Sep 2023 10:56:09 -0500, Rob Herring wrote:
> The 'msi' child node schema is missing constraints on additional properties.
> It turns out it is incomplete and properties for it are documented in the
> parent node by mistake. Move the reference to msi-controller.yaml and
> the custom properties to the 'msi' node. Adding 'unevaluatedProperties'
> ensures all the properties in the 'msi' node are documented.
> 
> With the schema corrected, a minimal interrupt controller node is needed
> to properly decode the interrupt properties since the example has
> multiple interrupt parents.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/pci/brcm,iproc-pcie.yaml         | 24 ++++++++++++-------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 

Applied, thanks!

Re: [PATCH 1/3] dt-bindings: PCI: brcm,iproc-pcie: Fix example indentation

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

On Tue, Sep 26, 2023 at 10:53:40AM -0500, Rob Herring wrote:
> The example's indentation is off. While fixing this, the 'bus' node
> is unnecessary and can be dropped. It is also preferred to split up
> unrelated examples to their own entries.

Damn, it really was all over the shop..

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/3] dt-bindings: PCI: brcm,iproc-pcie: Fix example indentation

[Florian Fainelli]

[-- Attachment #1: Type: text/plain, Size: 351 bytes --]



On 9/26/2023 5:53 PM, Rob Herring wrote:
> The example's indentation is off. While fixing this, the 'bus' node
> is unnecessary and can be dropped. It is also preferred to split up
> unrelated examples to their own entries.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
-- 
Florian

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4221 bytes --]

Re: [PATCH 1/3] dt-bindings: PCI: brcm,iproc-pcie: Fix example indentation

[Rob Herring]

On Tue, 26 Sep 2023 10:53:40 -0500, Rob Herring wrote:
> The example's indentation is off. While fixing this, the 'bus' node
> is unnecessary and can be dropped. It is also preferred to split up
> unrelated examples to their own entries.
> 
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/pci/brcm,iproc-pcie.yaml         | 124 +++++++++---------
>  1 file changed, 60 insertions(+), 64 deletions(-)
> 

Applied, thanks!