Re: [PATCH v2] dt-bindings: iommu: Convert Arm SMMUv3 to DT schema

[Rob Herring]

On Tue, Oct 22, 2019 at 11:54 AM Robin Murphy <robin.murphy@xxxxxxx> wrote:
>
> On 14/10/2019 20:12, Rob Herring wrote:
> > Convert the Arm SMMv3 binding to the DT schema format.
> >
> > Cc: Joerg Roedel <joro@xxxxxxxxxx>
> > Cc: Mark Rutland <mark.rutland@xxxxxxx>
> > Cc: Will Deacon <will@xxxxxxxxxx>
> > Cc: Robin Murphy <Robin.Murphy@xxxxxxx>
> > Cc: iommu@xxxxxxxxxxxxxxxxxxxxxxxxxx
> > Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> > ---
> > 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@xxxxxxx>

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>;
> > +    };
> >