iommu.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] dt-bindings: iommu: Convert Arm SMMUv3 to DT schema
@ 2019-10-14 19:12 Rob Herring
  2019-10-22 16:54 ` Robin Murphy
  2019-11-01 17:08 ` Will Deacon
  0 siblings, 2 replies; 5+ messages in thread
From: Rob Herring @ 2019-10-14 19:12 UTC (permalink / raw)
  To: devicetree; +Cc: Mark Rutland, Will Deacon, linux-kernel, iommu, Robin Murphy

Convert the Arm SMMv3 binding to the DT schema format.

Cc: Joerg Roedel <joro@8bytes.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Robin Murphy <Robin.Murphy@arm.com>
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
v2:
- Refine interrupt definition based on Robin's comments

 .../devicetree/bindings/iommu/arm,smmu-v3.txt |  77 --------------
 .../bindings/iommu/arm,smmu-v3.yaml           | 100 ++++++++++++++++++
 2 files changed, 100 insertions(+), 77 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
 create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
deleted file mode 100644
index c9abbf3e4f68..000000000000
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
+++ /dev/null
@@ -1,77 +0,0 @@
-* ARM SMMUv3 Architecture Implementation
-
-The SMMUv3 architecture is a significant departure from previous
-revisions, replacing the MMIO register interface with in-memory command
-and event queues and adding support for the ATS and PRI components of
-the PCIe specification.
-
-** SMMUv3 required properties:
-
-- compatible        : Should include:
-
-                      * "arm,smmu-v3" for any SMMUv3 compliant
-                        implementation. This entry should be last in the
-                        compatible list.
-
-- reg               : Base address and size of the SMMU.
-
-- interrupts        : Non-secure interrupt list describing the wired
-                      interrupt sources corresponding to entries in
-                      interrupt-names. If no wired interrupts are
-                      present then this property may be omitted.
-
-- interrupt-names   : When the interrupts property is present, should
-                      include the following:
-                      * "eventq"    - Event Queue not empty
-                      * "priq"      - PRI Queue not empty
-                      * "cmdq-sync" - CMD_SYNC complete
-                      * "gerror"    - Global Error activated
-                      * "combined"  - The combined interrupt is optional,
-				      and should only be provided if the
-				      hardware supports just a single,
-				      combined interrupt line.
-				      If provided, then the combined interrupt
-				      will be used in preference to any others.
-
-- #iommu-cells      : See the generic IOMMU binding described in
-                        devicetree/bindings/pci/pci-iommu.txt
-                      for details. For SMMUv3, must be 1, with each cell
-                      describing a single stream ID. All possible stream
-                      IDs which a device may emit must be described.
-
-** SMMUv3 optional properties:
-
-- dma-coherent      : Present if DMA operations made by the SMMU (page
-                      table walks, stream table accesses etc) are cache
-                      coherent with the CPU.
-
-                      NOTE: this only applies to the SMMU itself, not
-                      masters connected upstream of the SMMU.
-
-- msi-parent        : See the generic MSI binding described in
-                        devicetree/bindings/interrupt-controller/msi.txt
-                      for a description of the msi-parent property.
-
-- hisilicon,broken-prefetch-cmd
-                    : Avoid sending CMD_PREFETCH_* commands to the SMMU.
-
-- cavium,cn9900-broken-page1-regspace
-                    : Replaces all page 1 offsets used for EVTQ_PROD/CONS,
-		      PRIQ_PROD/CONS register access with page 0 offsets.
-		      Set for Cavium ThunderX2 silicon that doesn't support
-		      SMMU page1 register space.
-
-** Example
-
-        smmu@2b400000 {
-                compatible = "arm,smmu-v3";
-                reg = <0x0 0x2b400000 0x0 0x20000>;
-                interrupts = <GIC_SPI 74 IRQ_TYPE_EDGE_RISING>,
-                             <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>,
-                             <GIC_SPI 77 IRQ_TYPE_EDGE_RISING>,
-                             <GIC_SPI 79 IRQ_TYPE_EDGE_RISING>;
-                interrupt-names = "eventq", "priq", "cmdq-sync", "gerror";
-                dma-coherent;
-                #iommu-cells = <1>;
-                msi-parent = <&its 0xff0000>;
-        };
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
new file mode 100644
index 000000000000..662cbc4592c9
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/arm,smmu-v3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM SMMUv3 Architecture Implementation
+
+maintainers:
+  - Will Deacon <will@kernel.org>
+  - Robin Murphy <Robin.Murphy@arm.com>
+
+description: |+
+  The SMMUv3 architecture is a significant departure from previous
+  revisions, replacing the MMIO register interface with in-memory command
+  and event queues and adding support for the ATS and PRI components of
+  the PCIe specification.
+
+properties:
+  $nodename:
+    pattern: "^iommu@[0-9a-f]*"
+  compatible:
+    const: arm,smmu-v3
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    maxItems: 4
+
+  interrupt-names:
+    oneOf:
+      - const: combined
+        description:
+          The combined interrupt is optional, and should only be provided if the
+          hardware supports just a single, combined interrupt line.
+          If provided, then the combined interrupt will be used in preference to
+          any others.
+      - items:
+          - const: eventq     # Event Queue not empt
+          - const: priq       # PRI Queue not empty
+          - const: cmdq-sync  # CMD_SYNC complete
+          - const: gerror     # Global Error activated
+      - minItems: 2
+        maxItems: 4
+        items:
+          - const: eventq
+          - const: gerror
+          - const: priq
+          - const: cmdq-sync
+
+  '#iommu-cells':
+    const: 1
+
+  dma-coherent:
+    description: |
+      Present if page table walks made by the SMMU are cache coherent with the
+      CPU.
+
+      NOTE: this only applies to the SMMU itself, not masters connected
+      upstream of the SMMU.
+
+  msi-parent: true
+
+  hisilicon,broken-prefetch-cmd:
+    type: boolean
+    description: Avoid sending CMD_PREFETCH_* commands to the SMMU.
+
+  cavium,cn9900-broken-page1-regspace:
+    type: boolean
+    description:
+      Replaces all page 1 offsets used for EVTQ_PROD/CONS, PRIQ_PROD/CONS
+      register access with page 0 offsets. Set for Cavium ThunderX2 silicon that
+      doesn't support SMMU page1 register space.
+
+required:
+  - compatible
+  - reg
+  - '#iommu-cells'
+
+additionalProperties: false
+
+examples:
+  - |+
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    iommu@2b400000 {
+            compatible = "arm,smmu-v3";
+            reg = <0x2b400000 0x20000>;
+            interrupts = <GIC_SPI 74 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 77 IRQ_TYPE_EDGE_RISING>,
+                         <GIC_SPI 79 IRQ_TYPE_EDGE_RISING>;
+            interrupt-names = "eventq", "priq", "cmdq-sync", "gerror";
+            dma-coherent;
+            #iommu-cells = <1>;
+            msi-parent = <&its 0xff0000>;
+    };
-- 
2.20.1

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] dt-bindings: iommu: Convert Arm SMMUv3 to DT schema
  2019-10-14 19:12 [PATCH v2] dt-bindings: iommu: Convert Arm SMMUv3 to DT schema Rob Herring
@ 2019-10-22 16:54 ` Robin Murphy
  2019-10-22 17:09   ` Rob Herring
  2019-11-01 17:08 ` Will Deacon
  1 sibling, 1 reply; 5+ messages in thread
From: Robin Murphy @ 2019-10-22 16:54 UTC (permalink / raw)
  To: Rob Herring, devicetree; +Cc: Mark Rutland, Will Deacon, linux-kernel, iommu

On 14/10/2019 20:12, Rob Herring wrote:
> Convert the Arm SMMv3 binding to the DT schema format.
> 
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Robin Murphy <Robin.Murphy@arm.com>
> Cc: iommu@lists.linux-foundation.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> v2:
> - Refine interrupt definition based on Robin's comments
> 
>   .../devicetree/bindings/iommu/arm,smmu-v3.txt |  77 --------------
>   .../bindings/iommu/arm,smmu-v3.yaml           | 100 ++++++++++++++++++
>   2 files changed, 100 insertions(+), 77 deletions(-)
>   delete mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
>   create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> deleted file mode 100644
> index c9abbf3e4f68..000000000000
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> +++ /dev/null
> @@ -1,77 +0,0 @@
> -* ARM SMMUv3 Architecture Implementation
> -
> -The SMMUv3 architecture is a significant departure from previous
> -revisions, replacing the MMIO register interface with in-memory command
> -and event queues and adding support for the ATS and PRI components of
> -the PCIe specification.
> -
> -** SMMUv3 required properties:
> -
> -- compatible        : Should include:
> -
> -                      * "arm,smmu-v3" for any SMMUv3 compliant
> -                        implementation. This entry should be last in the
> -                        compatible list.
> -
> -- reg               : Base address and size of the SMMU.
> -
> -- interrupts        : Non-secure interrupt list describing the wired
> -                      interrupt sources corresponding to entries in
> -                      interrupt-names. If no wired interrupts are
> -                      present then this property may be omitted.
> -
> -- interrupt-names   : When the interrupts property is present, should
> -                      include the following:
> -                      * "eventq"    - Event Queue not empty
> -                      * "priq"      - PRI Queue not empty
> -                      * "cmdq-sync" - CMD_SYNC complete
> -                      * "gerror"    - Global Error activated
> -                      * "combined"  - The combined interrupt is optional,
> -				      and should only be provided if the
> -				      hardware supports just a single,
> -				      combined interrupt line.
> -				      If provided, then the combined interrupt
> -				      will be used in preference to any others.
> -
> -- #iommu-cells      : See the generic IOMMU binding described in
> -                        devicetree/bindings/pci/pci-iommu.txt
> -                      for details. For SMMUv3, must be 1, with each cell
> -                      describing a single stream ID. All possible stream
> -                      IDs which a device may emit must be described.
> -
> -** SMMUv3 optional properties:
> -
> -- dma-coherent      : Present if DMA operations made by the SMMU (page
> -                      table walks, stream table accesses etc) are cache
> -                      coherent with the CPU.
> -
> -                      NOTE: this only applies to the SMMU itself, not
> -                      masters connected upstream of the SMMU.
> -
> -- msi-parent        : See the generic MSI binding described in
> -                        devicetree/bindings/interrupt-controller/msi.txt
> -                      for a description of the msi-parent property.
> -
> -- hisilicon,broken-prefetch-cmd
> -                    : Avoid sending CMD_PREFETCH_* commands to the SMMU.
> -
> -- cavium,cn9900-broken-page1-regspace
> -                    : Replaces all page 1 offsets used for EVTQ_PROD/CONS,
> -		      PRIQ_PROD/CONS register access with page 0 offsets.
> -		      Set for Cavium ThunderX2 silicon that doesn't support
> -		      SMMU page1 register space.
> -
> -** Example
> -
> -        smmu@2b400000 {
> -                compatible = "arm,smmu-v3";
> -                reg = <0x0 0x2b400000 0x0 0x20000>;
> -                interrupts = <GIC_SPI 74 IRQ_TYPE_EDGE_RISING>,
> -                             <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>,
> -                             <GIC_SPI 77 IRQ_TYPE_EDGE_RISING>,
> -                             <GIC_SPI 79 IRQ_TYPE_EDGE_RISING>;
> -                interrupt-names = "eventq", "priq", "cmdq-sync", "gerror";
> -                dma-coherent;
> -                #iommu-cells = <1>;
> -                msi-parent = <&its 0xff0000>;
> -        };
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> new file mode 100644
> index 000000000000..662cbc4592c9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iommu/arm,smmu-v3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM SMMUv3 Architecture Implementation
> +
> +maintainers:
> +  - Will Deacon <will@kernel.org>
> +  - Robin Murphy <Robin.Murphy@arm.com>
> +
> +description: |+
> +  The SMMUv3 architecture is a significant departure from previous
> +  revisions, replacing the MMIO register interface with in-memory command
> +  and event queues and adding support for the ATS and PRI components of
> +  the PCIe specification.
> +
> +properties:
> +  $nodename:
> +    pattern: "^iommu@[0-9a-f]*"
> +  compatible:
> +    const: arm,smmu-v3
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 4
> +
> +  interrupt-names:
> +    oneOf:
> +      - const: combined
> +        description:
> +          The combined interrupt is optional, and should only be provided if the
> +          hardware supports just a single, combined interrupt line.
> +          If provided, then the combined interrupt will be used in preference to
> +          any others.
> +      - items:
> +          - const: eventq     # Event Queue not empt
> +          - const: priq       # PRI Queue not empty
> +          - const: cmdq-sync  # CMD_SYNC complete
> +          - const: gerror     # Global Error activated

Isn't this effectively redundant with the 4-item case of the version 
below? If it's purely about the ordering, and we can't express "one or 
more of any of:" without spelling out all 64 possible permutations, then 
TBH I'd rather just settle on a single definition that can work for all 
current cases and update the Fast Model DT if necessary.

Otherwise, though, this looks like a fair starting point to me;

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

> +      - minItems: 2
> +        maxItems: 4
> +        items:
> +          - const: eventq
> +          - const: gerror
> +          - const: priq
> +          - const: cmdq-sync
> +
> +  '#iommu-cells':
> +    const: 1
> +
> +  dma-coherent:
> +    description: |
> +      Present if page table walks made by the SMMU are cache coherent with the
> +      CPU.
> +
> +      NOTE: this only applies to the SMMU itself, not masters connected
> +      upstream of the SMMU.
> +
> +  msi-parent: true
> +
> +  hisilicon,broken-prefetch-cmd:
> +    type: boolean
> +    description: Avoid sending CMD_PREFETCH_* commands to the SMMU.
> +
> +  cavium,cn9900-broken-page1-regspace:
> +    type: boolean
> +    description:
> +      Replaces all page 1 offsets used for EVTQ_PROD/CONS, PRIQ_PROD/CONS
> +      register access with page 0 offsets. Set for Cavium ThunderX2 silicon that
> +      doesn't support SMMU page1 register space.
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#iommu-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |+
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    iommu@2b400000 {
> +            compatible = "arm,smmu-v3";
> +            reg = <0x2b400000 0x20000>;
> +            interrupts = <GIC_SPI 74 IRQ_TYPE_EDGE_RISING>,
> +                         <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>,
> +                         <GIC_SPI 77 IRQ_TYPE_EDGE_RISING>,
> +                         <GIC_SPI 79 IRQ_TYPE_EDGE_RISING>;
> +            interrupt-names = "eventq", "priq", "cmdq-sync", "gerror";
> +            dma-coherent;
> +            #iommu-cells = <1>;
> +            msi-parent = <&its 0xff0000>;
> +    };
> 
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] dt-bindings: iommu: Convert Arm SMMUv3 to DT schema
  2019-10-22 16:54 ` Robin Murphy
@ 2019-10-22 17:09   ` Rob Herring
  0 siblings, 0 replies; 5+ messages in thread
From: Rob Herring @ 2019-10-22 17:09 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Mark Rutland, devicetree, linux-kernel, Linux IOMMU, Will Deacon

On Tue, Oct 22, 2019 at 11:54 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 14/10/2019 20:12, Rob Herring wrote:
> > Convert the Arm SMMv3 binding to the DT schema format.
> >
> > Cc: Joerg Roedel <joro@8bytes.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Robin Murphy <Robin.Murphy@arm.com>
> > Cc: iommu@lists.linux-foundation.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > v2:
> > - Refine interrupt definition based on Robin's comments
> >
> >   .../devicetree/bindings/iommu/arm,smmu-v3.txt |  77 --------------
> >   .../bindings/iommu/arm,smmu-v3.yaml           | 100 ++++++++++++++++++
> >   2 files changed, 100 insertions(+), 77 deletions(-)
> >   delete mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> >   create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml

[...]

> > +  interrupt-names:
> > +    oneOf:
> > +      - const: combined
> > +        description:
> > +          The combined interrupt is optional, and should only be provided if the
> > +          hardware supports just a single, combined interrupt line.
> > +          If provided, then the combined interrupt will be used in preference to
> > +          any others.
> > +      - items:
> > +          - const: eventq     # Event Queue not empt
> > +          - const: priq       # PRI Queue not empty
> > +          - const: cmdq-sync  # CMD_SYNC complete
> > +          - const: gerror     # Global Error activated
>
> Isn't this effectively redundant with the 4-item case of the version
> below? If it's purely about the ordering, and we can't express "one or
> more of any of:" without spelling out all 64 possible permutations, then
> TBH I'd rather just settle on a single definition that can work for all
> current cases and update the Fast Model DT if necessary.

We can allow any order if needed (such as already having lots of
permutations), but in general the order is supposed to be defined.

Updating the Fast Model DT seems okay to me. I'll shift the comments
to the below entry and drop this one (and fix the example).

> Otherwise, though, this looks like a fair starting point to me;
>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>

Thanks.

>
> > +      - minItems: 2
> > +        maxItems: 4
> > +        items:
> > +          - const: eventq
> > +          - const: gerror
> > +          - const: priq
> > +          - const: cmdq-sync
> > +
> > +  '#iommu-cells':
> > +    const: 1
> > +
> > +  dma-coherent:
> > +    description: |
> > +      Present if page table walks made by the SMMU are cache coherent with the
> > +      CPU.
> > +
> > +      NOTE: this only applies to the SMMU itself, not masters connected
> > +      upstream of the SMMU.
> > +
> > +  msi-parent: true
> > +
> > +  hisilicon,broken-prefetch-cmd:
> > +    type: boolean
> > +    description: Avoid sending CMD_PREFETCH_* commands to the SMMU.
> > +
> > +  cavium,cn9900-broken-page1-regspace:
> > +    type: boolean
> > +    description:
> > +      Replaces all page 1 offsets used for EVTQ_PROD/CONS, PRIQ_PROD/CONS
> > +      register access with page 0 offsets. Set for Cavium ThunderX2 silicon that
> > +      doesn't support SMMU page1 register space.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - '#iommu-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |+
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    iommu@2b400000 {
> > +            compatible = "arm,smmu-v3";
> > +            reg = <0x2b400000 0x20000>;
> > +            interrupts = <GIC_SPI 74 IRQ_TYPE_EDGE_RISING>,
> > +                         <GIC_SPI 75 IRQ_TYPE_EDGE_RISING>,
> > +                         <GIC_SPI 77 IRQ_TYPE_EDGE_RISING>,
> > +                         <GIC_SPI 79 IRQ_TYPE_EDGE_RISING>;
> > +            interrupt-names = "eventq", "priq", "cmdq-sync", "gerror";
> > +            dma-coherent;
> > +            #iommu-cells = <1>;
> > +            msi-parent = <&its 0xff0000>;
> > +    };
> >
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] dt-bindings: iommu: Convert Arm SMMUv3 to DT schema
  2019-10-14 19:12 [PATCH v2] dt-bindings: iommu: Convert Arm SMMUv3 to DT schema Rob Herring
  2019-10-22 16:54 ` Robin Murphy
@ 2019-11-01 17:08 ` Will Deacon
  2019-11-01 20:14   ` Rob Herring
  1 sibling, 1 reply; 5+ messages in thread
From: Will Deacon @ 2019-11-01 17:08 UTC (permalink / raw)
  To: Rob Herring; +Cc: Mark Rutland, devicetree, linux-kernel, iommu, Robin Murphy

Hi Rob,

On Mon, Oct 14, 2019 at 02:12:56PM -0500, Rob Herring wrote:
> Convert the Arm SMMv3 binding to the DT schema format.
> 
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Robin Murphy <Robin.Murphy@arm.com>
> Cc: iommu@lists.linux-foundation.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> v2:
> - Refine interrupt definition based on Robin's comments
> 
>  .../devicetree/bindings/iommu/arm,smmu-v3.txt |  77 --------------
>  .../bindings/iommu/arm,smmu-v3.yaml           | 100 ++++++++++++++++++
>  2 files changed, 100 insertions(+), 77 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
>  create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml

[...]

> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> new file mode 100644
> index 000000000000..662cbc4592c9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> @@ -0,0 +1,100 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iommu/arm,smmu-v3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ARM SMMUv3 Architecture Implementation
> +
> +maintainers:
> +  - Will Deacon <will@kernel.org>
> +  - Robin Murphy <Robin.Murphy@arm.com>
> +
> +description: |+
> +  The SMMUv3 architecture is a significant departure from previous
> +  revisions, replacing the MMIO register interface with in-memory command
> +  and event queues and adding support for the ATS and PRI components of
> +  the PCIe specification.
> +
> +properties:
> +  $nodename:
> +    pattern: "^iommu@[0-9a-f]*"
> +  compatible:
> +    const: arm,smmu-v3
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 1
> +    maxItems: 4
> +
> +  interrupt-names:
> +    oneOf:
> +      - const: combined
> +        description:
> +          The combined interrupt is optional, and should only be provided if the
> +          hardware supports just a single, combined interrupt line.
> +          If provided, then the combined interrupt will be used in preference to
> +          any others.
> +      - items:
> +          - const: eventq     # Event Queue not empt
> +          - const: priq       # PRI Queue not empty
> +          - const: cmdq-sync  # CMD_SYNC complete
> +          - const: gerror     # Global Error activated
> +      - minItems: 2
> +        maxItems: 4
> +        items:
> +          - const: eventq
> +          - const: gerror
> +          - const: priq
> +          - const: cmdq-sync

I find it a bit odd to say "minItems: 2" here since, for example, if you
have an SMMU that supports PRI then you really want the PRIQ interrupt
hooked up. The only one never care about in the current driver is cmdq-sync,
but that's just a driver quirk.

Also, if the thing supports MSIs then it might not have any wired interrupts
at all. Hmm.

> +
> +  '#iommu-cells':
> +    const: 1
> +
> +  dma-coherent:
> +    description: |
> +      Present if page table walks made by the SMMU are cache coherent with the
> +      CPU.

This looks like you've taken the text from SMMUv2 by accident. For SMMUv3,
it's not just about page table walks, but *any* DMA operations made by the
SMMU (e.g. STE lookup). I don't see the need to change the current text tbh.

Will
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] dt-bindings: iommu: Convert Arm SMMUv3 to DT schema
  2019-11-01 17:08 ` Will Deacon
@ 2019-11-01 20:14   ` Rob Herring
  0 siblings, 0 replies; 5+ messages in thread
From: Rob Herring @ 2019-11-01 20:14 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, devicetree, linux-kernel, Linux IOMMU, Robin Murphy

On Fri, Nov 1, 2019 at 12:08 PM Will Deacon <will@kernel.org> wrote:
>
> Hi Rob,
>
> On Mon, Oct 14, 2019 at 02:12:56PM -0500, Rob Herring wrote:
> > Convert the Arm SMMv3 binding to the DT schema format.
> >
> > Cc: Joerg Roedel <joro@8bytes.org>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: Robin Murphy <Robin.Murphy@arm.com>
> > Cc: iommu@lists.linux-foundation.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > v2:
> > - Refine interrupt definition based on Robin's comments
> >
> >  .../devicetree/bindings/iommu/arm,smmu-v3.txt |  77 --------------
> >  .../bindings/iommu/arm,smmu-v3.yaml           | 100 ++++++++++++++++++
> >  2 files changed, 100 insertions(+), 77 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.txt
> >  create mode 100644 Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
>
> [...]
>
> > diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> > new file mode 100644
> > index 000000000000..662cbc4592c9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
> > @@ -0,0 +1,100 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iommu/arm,smmu-v3.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ARM SMMUv3 Architecture Implementation
> > +
> > +maintainers:
> > +  - Will Deacon <will@kernel.org>
> > +  - Robin Murphy <Robin.Murphy@arm.com>
> > +
> > +description: |+
> > +  The SMMUv3 architecture is a significant departure from previous
> > +  revisions, replacing the MMIO register interface with in-memory command
> > +  and event queues and adding support for the ATS and PRI components of
> > +  the PCIe specification.
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^iommu@[0-9a-f]*"
> > +  compatible:
> > +    const: arm,smmu-v3
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    minItems: 1
> > +    maxItems: 4
> > +
> > +  interrupt-names:
> > +    oneOf:
> > +      - const: combined
> > +        description:
> > +          The combined interrupt is optional, and should only be provided if the
> > +          hardware supports just a single, combined interrupt line.
> > +          If provided, then the combined interrupt will be used in preference to
> > +          any others.
> > +      - items:
> > +          - const: eventq     # Event Queue not empt
> > +          - const: priq       # PRI Queue not empty
> > +          - const: cmdq-sync  # CMD_SYNC complete
> > +          - const: gerror     # Global Error activated
> > +      - minItems: 2
> > +        maxItems: 4
> > +        items:
> > +          - const: eventq
> > +          - const: gerror
> > +          - const: priq
> > +          - const: cmdq-sync
>
> I find it a bit odd to say "minItems: 2" here since, for example, if you
> have an SMMU that supports PRI then you really want the PRIQ interrupt
> hooked up. The only one never care about in the current driver is cmdq-sync,
> but that's just a driver quirk.

I don't know. I'm just documenting what exists and doesn't seem like
an outright error. The one case is TI:

arch/arm64/boot/dts/ti/k3-j721e-main.dtsi:
interrupt-names = "eventq", "gerror";

If we want to make priq conditionally required, then I need to know
which compatibles would imply supporting PRI. If that's discoverable,
then we can't really enforce the interrupt being present in the
schema.

> Also, if the thing supports MSIs then it might not have any wired interrupts
> at all. Hmm.

That would be why 'interrupts' is optional. The schema only applies if
a property is present.

> > +  '#iommu-cells':
> > +    const: 1
> > +
> > +  dma-coherent:
> > +    description: |
> > +      Present if page table walks made by the SMMU are cache coherent with the
> > +      CPU.
>
> This looks like you've taken the text from SMMUv2 by accident. For SMMUv3,
> it's not just about page table walks, but *any* DMA operations made by the
> SMMU (e.g. STE lookup). I don't see the need to change the current text tbh.

Indeed. I'll do a follow-up as I've already applied these 2 patches.

Rob
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-11-01 20:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 19:12 [PATCH v2] dt-bindings: iommu: Convert Arm SMMUv3 to DT schema Rob Herring
2019-10-22 16:54 ` Robin Murphy
2019-10-22 17:09   ` Rob Herring
2019-11-01 17:08 ` Will Deacon
2019-11-01 20:14   ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).