Re: [PATCH v2 1/4] dt-bindings: iommu: rockchip: Convert IOMMU to DT schema

[Benjamin Gaignard]

Le 01/05/2021 à 00:14, Rob Herring a écrit :
> On Thu, Apr 22, 2021 at 02:16:53PM -0300, Ezequiel Garcia wrote:
> > (Adding Kever)
> >
> > Hi Benjamin,
> >
> > Thanks a lot for working on this, it looks amazing. Together with the great work
> > that Rockchip is doing, it seems RK3566/RK3568 will have decent support very soon.
> >
> > One comment here:
> >
> > On Thu, 2021-04-22 at 16:15 +0200, Benjamin Gaignard wrote:
> > > Convert Rockchip IOMMU to DT schema
> > >
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
> > > ---
> > > version 2:
> > >   - Change maintainer
> > >   - Change reg maxItems
> > >   - Change interrupt maxItems
> > >
> > >   .../bindings/iommu/rockchip,iommu.txt         | 38 ---------
> > >   .../bindings/iommu/rockchip,iommu.yaml        | 79 +++++++++++++++++++
> > >   2 files changed, 79 insertions(+), 38 deletions(-)
> > >   delete mode 100644 Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
> > >   create mode 100644 Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt b/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
> > > deleted file mode 100644
> > > index 6ecefea1c6f9..000000000000
> > > --- a/Documentation/devicetree/bindings/iommu/rockchip,iommu.txt
> > > +++ /dev/null
> > > @@ -1,38 +0,0 @@
> > > -Rockchip IOMMU
> > > -==============
> > > -
> > > -A Rockchip DRM iommu translates io virtual addresses to physical addresses for
> > > -its master device.  Each slave device is bound to a single master device, and
> > > -shares its clocks, power domain and irq.
> > > -
> > > -Required properties:
> > > -- compatible      : Should be "rockchip,iommu"
> > > -- reg             : Address space for the configuration registers
> > > -- interrupts      : Interrupt specifier for the IOMMU instance
> > > -- interrupt-names : Interrupt name for the IOMMU instance
> > > -- #iommu-cells    : Should be <0>.  This indicates the iommu is a
> > > -                    "single-master" device, and needs no additional information
> > > -                    to associate with its master device.  See:
> > > -                    Documentation/devicetree/bindings/iommu/iommu.txt
> > > -- clocks          : A list of clocks required for the IOMMU to be accessible by
> > > -                    the host CPU.
> > > -- clock-names     : Should contain the following:
> > > -       "iface" - Main peripheral bus clock (PCLK/HCL) (required)
> > > -       "aclk"  - AXI bus clock (required)
> > > -
> > > -Optional properties:
> > > -- rockchip,disable-mmu-reset : Don't use the mmu reset operation.
> > > -                              Some mmu instances may produce unexpected results
> > > -                              when the reset operation is used.
> > > -
> > > -Example:
> > > -
> > > -       vopl_mmu: iommu@ff940300 {
> > > -               compatible = "rockchip,iommu";
> > > -               reg = <0xff940300 0x100>;
> > > -               interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>;
> > > -               interrupt-names = "vopl_mmu";
> > > -               clocks = <&cru ACLK_VOP1>, <&cru HCLK_VOP1>;
> > > -               clock-names = "aclk", "iface";
> > > -               #iommu-cells = <0>;
> > > -       };
> > > diff --git a/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> > > new file mode 100644
> > > index 000000000000..0db208cf724a
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iommu/rockchip,iommu.yaml
> > > @@ -0,0 +1,79 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iommu/rockchip,iommu.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Rockchip IOMMU
> > > +
> > > +maintainers:
> > > +  - Heiko Stuebner <heiko@xxxxxxxxx>
> > > +
> > > +description: |+
> > > +  A Rockchip DRM iommu translates io virtual addresses to physical addresses for
> > > +  its master device. Each slave device is bound to a single master device and
> > > +  shares its clocks, power domain and irq.
> > > +
> > > +  For information on assigning IOMMU controller to its peripheral devices,
> > > +  see generic IOMMU bindings.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: rockchip,iommu
> > > +
> > > +  reg:
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +
> > > +  interrupts:
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +
> > > +  interrupt-names:
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +
> > AFAICS, the driver supports handling multiple MMUs, and there's one reg and
> > interrupt cell for each MMU. IOW, there's no requirement that maxItems is 2.
> >
> > Is there any way we can describe that? Or maybe just allow a bigger maximum?
> With #iommu-cells == 0, how would one distinguish which IOMMU is
> associated with a device? IOW, is more that 1 really usable?
>
> If you need more just pick a maxItems value that's either the most seen
> or 'should be enough'TM. If the entries are just multiple instances of
> the same thing, please note that here.

In current dts files it is up to two interruptions per IOMMU hardware blocks
so I will keep it to this value.

Benjamin

> Rob