Re: [PATCH v3 1/2] dt-bindings: iommu: add bindings for sprd iommu
Hi Robin, On Tue, 16 Feb 2021 at 23:10, Robin Murphy wrote: > > >>> > >>> On Wed, Feb 03, 2021 at 05:07:26PM +0800, Chunyan Zhang wrote: > From: Chunyan Zhang > > This iommu module can be used by Unisoc's multimedia devices, such as > display, Image codec(jpeg) and a few signal processors, including > VSP(video), GSP(graphic), ISP(image), and CPP(camera pixel processor), > etc. > > Signed-off-by: Chunyan Zhang > --- > .../devicetree/bindings/iommu/sprd,iommu.yaml | 72 +++ > 1 file changed, 72 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/iommu/sprd,iommu.yaml > > diff --git a/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml > b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml > new file mode 100644 > index ..4fc99e81fa66 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml > @@ -0,0 +1,72 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +# Copyright 2020 Unisoc Inc. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iommu/sprd,iommu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Unisoc IOMMU and Multi-media MMU > + > +maintainers: > + - Chunyan Zhang > + > +properties: > + compatible: > +enum: > + - sprd,iommu-v1 > + > + "#iommu-cells": > +const: 0 > +description: > + Unisoc IOMMUs are all single-master IOMMU devices, therefore no > + additional information needs to associate with its master device. > + Please refer to the generic bindings document for more details, > + Documentation/devicetree/bindings/iommu/iommu.txt > + > + reg: > +maxItems: 1 > +description: > + Not required if 'sprd,iommu-regs' is defined. > + > + clocks: > +description: > + Reference to a gate clock phandle, since access to some of IOMMUs > are > + controlled by gate clock, but this is not required. > + > + sprd,iommu-regs: > +$ref: /schemas/types.yaml#/definitions/phandle-array > +description: > + Reference to a syscon phandle plus 1 cell, the syscon defines the > + register range used by the iommu and the media device, the cell > + defines the offset for iommu registers. Since iommu module shares > + the same register range with the media device which uses it. > + > +required: > + - compatible > + - "#iommu-cells" > > OK, so apparently the hardware is not quite as trivial as my initial > impression, and you should have interrupts as well. I've checked with my colleagues for this issue. And like I explained before, one sprd IOMMU serves one master device only, so interrupts are handled by master devices rather than IOMMUs. Chunyan
Re: [PATCH v3 1/2] dt-bindings: iommu: add bindings for sprd iommu
On Tue, 16 Feb 2021 at 23:10, Robin Murphy wrote:
>
> On 2021-02-10 19:21, Rob Herring wrote:
> > On Fri, Feb 5, 2021 at 1:21 AM Chunyan Zhang wrote:
> >>
> >> Hi Rob,
> >>
> >> On Fri, 5 Feb 2021 at 07:25, Rob Herring wrote:
> >>>
> >>> On Wed, Feb 03, 2021 at 05:07:26PM +0800, Chunyan Zhang wrote:
> From: Chunyan Zhang
>
> This iommu module can be used by Unisoc's multimedia devices, such as
> display, Image codec(jpeg) and a few signal processors, including
> VSP(video), GSP(graphic), ISP(image), and CPP(camera pixel processor),
> etc.
>
> Signed-off-by: Chunyan Zhang
> ---
> .../devicetree/bindings/iommu/sprd,iommu.yaml | 72 +++
> 1 file changed, 72 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
>
> diff --git a/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> new file mode 100644
> index ..4fc99e81fa66
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2020 Unisoc Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iommu/sprd,iommu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Unisoc IOMMU and Multi-media MMU
> +
> +maintainers:
> + - Chunyan Zhang
> +
> +properties:
> + compatible:
> +enum:
> + - sprd,iommu-v1
> +
> + "#iommu-cells":
> +const: 0
> +description:
> + Unisoc IOMMUs are all single-master IOMMU devices, therefore no
> + additional information needs to associate with its master device.
> + Please refer to the generic bindings document for more details,
> + Documentation/devicetree/bindings/iommu/iommu.txt
> +
> + reg:
> +maxItems: 1
> +description:
> + Not required if 'sprd,iommu-regs' is defined.
> +
> + clocks:
> +description:
> + Reference to a gate clock phandle, since access to some of IOMMUs
> are
> + controlled by gate clock, but this is not required.
> +
> + sprd,iommu-regs:
> +$ref: /schemas/types.yaml#/definitions/phandle-array
> +description:
> + Reference to a syscon phandle plus 1 cell, the syscon defines the
> + register range used by the iommu and the media device, the cell
> + defines the offset for iommu registers. Since iommu module shares
> + the same register range with the media device which uses it.
> +
> +required:
> + - compatible
> + - "#iommu-cells"
>
> OK, so apparently the hardware is not quite as trivial as my initial
> impression, and you should have interrupts as well.
Ok, I will have a look.
>
> +
> +oneOf:
> + - required:
> + - reg
> + - required:
> + - sprd,iommu-regs
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +iommu_disp: iommu-disp {
> + compatible = "sprd,iommu-v1";
> + sprd,iommu-regs = <_regs 0x800>;
> >>>
> >>> If the IOMMU is contained within another device, then it should just be
> >>> a child node of that device.
> >>
> >> Yes, actually IOMMU can be seen as a child of multimedia devices, I
> >> considered moving IOMMU under into multimedia device node, but
> >> multimedia devices need IOMMU when probe[1], so I dropped that idea.
> >
> > Don't design your binding around working-around linux issues.
>
> Having stumbled across the DRM driver patches the other day, I now see
> where this is coming from, and it's even worse than that - this whole
> binding seems to be largely working around bad driver design.
>
I guess you mean bad h/w design (not bad driver design)?
Having this syscon node just because I don't want a same register
range to be mapped to multiple virtual address ranges, and that's the
case for many media devices and their IOMMUs. If this issue exsists
for one device only, I can even endure, but that's not unfortunately.
But anyway, as you all think this is not a good way, I will change to
use reg property.
> >> And they share the same register base, e.g.
> >>
> >> + mm {
> >> + compatible = "simple-bus";
> >> + #address-cells = <2>;
> >> + #size-cells = <2>;
> >> + ranges;
> >> +
> >> + dpu_regs: syscon@6300 {
> >
> > Drop this node.
> >
> >> + compatible = "sprd,sc9863a-dpuregs",
> >> "syscon";
> >> + reg = <0 0x6300 0 0x1000>;
> >> + };
> >> +
> >> +
Re: [PATCH v3 1/2] dt-bindings: iommu: add bindings for sprd iommu
On Thu, 11 Feb 2021 at 03:21, Rob Herring wrote:
>
> On Fri, Feb 5, 2021 at 1:21 AM Chunyan Zhang wrote:
> >
> > Hi Rob,
> >
> > On Fri, 5 Feb 2021 at 07:25, Rob Herring wrote:
> > >
> > > On Wed, Feb 03, 2021 at 05:07:26PM +0800, Chunyan Zhang wrote:
> > > > From: Chunyan Zhang
> > > >
> > > > This iommu module can be used by Unisoc's multimedia devices, such as
> > > > display, Image codec(jpeg) and a few signal processors, including
> > > > VSP(video), GSP(graphic), ISP(image), and CPP(camera pixel processor),
> > > > etc.
> > > >
> > > > Signed-off-by: Chunyan Zhang
> > > > ---
> > > > .../devicetree/bindings/iommu/sprd,iommu.yaml | 72 +++
> > > > 1 file changed, 72 insertions(+)
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > > > b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > > > new file mode 100644
> > > > index ..4fc99e81fa66
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > > > @@ -0,0 +1,72 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +# Copyright 2020 Unisoc Inc.
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/iommu/sprd,iommu.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Unisoc IOMMU and Multi-media MMU
> > > > +
> > > > +maintainers:
> > > > + - Chunyan Zhang
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > +enum:
> > > > + - sprd,iommu-v1
> > > > +
> > > > + "#iommu-cells":
> > > > +const: 0
> > > > +description:
> > > > + Unisoc IOMMUs are all single-master IOMMU devices, therefore no
> > > > + additional information needs to associate with its master device.
> > > > + Please refer to the generic bindings document for more details,
> > > > + Documentation/devicetree/bindings/iommu/iommu.txt
> > > > +
> > > > + reg:
> > > > +maxItems: 1
> > > > +description:
> > > > + Not required if 'sprd,iommu-regs' is defined.
> > > > +
> > > > + clocks:
> > > > +description:
> > > > + Reference to a gate clock phandle, since access to some of
> > > > IOMMUs are
> > > > + controlled by gate clock, but this is not required.
> > > > +
> > > > + sprd,iommu-regs:
> > > > +$ref: /schemas/types.yaml#/definitions/phandle-array
> > > > +description:
> > > > + Reference to a syscon phandle plus 1 cell, the syscon defines the
> > > > + register range used by the iommu and the media device, the cell
> > > > + defines the offset for iommu registers. Since iommu module shares
> > > > + the same register range with the media device which uses it.
> > > > +
> > > > +required:
> > > > + - compatible
> > > > + - "#iommu-cells"
> > > > +
> > > > +oneOf:
> > > > + - required:
> > > > + - reg
> > > > + - required:
> > > > + - sprd,iommu-regs
> > > > +
> > > > +additionalProperties: false
> > > > +
> > > > +examples:
> > > > + - |
> > > > +iommu_disp: iommu-disp {
> > > > + compatible = "sprd,iommu-v1";
> > > > + sprd,iommu-regs = <_regs 0x800>;
> > >
> > > If the IOMMU is contained within another device, then it should just be
> > > a child node of that device.
> >
> > Yes, actually IOMMU can be seen as a child of multimedia devices, I
> > considered moving IOMMU under into multimedia device node, but
> > multimedia devices need IOMMU when probe[1], so I dropped that idea.
>
> Don't design your binding around working-around linux issues.
>
> > And they share the same register base, e.g.
> >
> > + mm {
> > + compatible = "simple-bus";
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > +
> > + dpu_regs: syscon@6300 {
>
> Drop this node.
>
> > + compatible = "sprd,sc9863a-dpuregs",
> > "syscon";
> > + reg = <0 0x6300 0 0x1000>;
> > + };
> > +
> > + dpu: dpu@6300 {
> > + compatible = "sprd,sharkl3-dpu";
> > + sprd,disp-regs = <_regs>;
>
> reg = <0 0x6300 0 0x800>;
>
> > + iommus = <_dispc>;
> > + };
> > +
> > + iommu_dispc: iommu@6300 {
> > + compatible = "sprd,iommu-v1";
> > + sprd,iommu-regs = <_regs 0x800>;
>
> reg = <0 0x63000800 0 0x800>;
Alright, considering you deprecate using syscon to map registers here,
I will drop that.
But that would cause the same physical address to be mapped two times at least.
And this is not a single case, since there are a few media devices and
their IOMMUs which
Re: [PATCH v3 1/2] dt-bindings: iommu: add bindings for sprd iommu
On 2021-02-10 19:21, Rob Herring wrote:
On Fri, Feb 5, 2021 at 1:21 AM Chunyan Zhang wrote:
Hi Rob,
On Fri, 5 Feb 2021 at 07:25, Rob Herring wrote:
On Wed, Feb 03, 2021 at 05:07:26PM +0800, Chunyan Zhang wrote:
From: Chunyan Zhang
This iommu module can be used by Unisoc's multimedia devices, such as
display, Image codec(jpeg) and a few signal processors, including
VSP(video), GSP(graphic), ISP(image), and CPP(camera pixel processor), etc.
Signed-off-by: Chunyan Zhang
---
.../devicetree/bindings/iommu/sprd,iommu.yaml | 72 +++
1 file changed, 72 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
diff --git a/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
new file mode 100644
index ..4fc99e81fa66
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2020 Unisoc Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/sprd,iommu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Unisoc IOMMU and Multi-media MMU
+
+maintainers:
+ - Chunyan Zhang
+
+properties:
+ compatible:
+enum:
+ - sprd,iommu-v1
+
+ "#iommu-cells":
+const: 0
+description:
+ Unisoc IOMMUs are all single-master IOMMU devices, therefore no
+ additional information needs to associate with its master device.
+ Please refer to the generic bindings document for more details,
+ Documentation/devicetree/bindings/iommu/iommu.txt
+
+ reg:
+maxItems: 1
+description:
+ Not required if 'sprd,iommu-regs' is defined.
+
+ clocks:
+description:
+ Reference to a gate clock phandle, since access to some of IOMMUs are
+ controlled by gate clock, but this is not required.
+
+ sprd,iommu-regs:
+$ref: /schemas/types.yaml#/definitions/phandle-array
+description:
+ Reference to a syscon phandle plus 1 cell, the syscon defines the
+ register range used by the iommu and the media device, the cell
+ defines the offset for iommu registers. Since iommu module shares
+ the same register range with the media device which uses it.
+
+required:
+ - compatible
+ - "#iommu-cells"
OK, so apparently the hardware is not quite as trivial as my initial
impression, and you should have interrupts as well.
+
+oneOf:
+ - required:
+ - reg
+ - required:
+ - sprd,iommu-regs
+
+additionalProperties: false
+
+examples:
+ - |
+iommu_disp: iommu-disp {
+ compatible = "sprd,iommu-v1";
+ sprd,iommu-regs = <_regs 0x800>;
If the IOMMU is contained within another device, then it should just be
a child node of that device.
Yes, actually IOMMU can be seen as a child of multimedia devices, I
considered moving IOMMU under into multimedia device node, but
multimedia devices need IOMMU when probe[1], so I dropped that idea.
Don't design your binding around working-around linux issues.
Having stumbled across the DRM driver patches the other day, I now see
where this is coming from, and it's even worse than that - this whole
binding seems to be largely working around bad driver design.
And they share the same register base, e.g.
+ mm {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ dpu_regs: syscon@6300 {
Drop this node.
+ compatible = "sprd,sc9863a-dpuregs", "syscon";
+ reg = <0 0x6300 0 0x1000>;
+ };
+
+ dpu: dpu@6300 {
+ compatible = "sprd,sharkl3-dpu";
+ sprd,disp-regs = <_regs>;
reg = <0 0x6300 0 0x800>;
In fact judging by the other driver it looks like the length only needs
to be 0x200 here (but maybe there's more to come in future).
+ iommus = <_dispc>;
+ };
+
+ iommu_dispc: iommu@6300 {
+ compatible = "sprd,iommu-v1";
+ sprd,iommu-regs = <_regs 0x800>;
reg = <0 0x63000800 0 0x800>;
...and this one looks to need less than 0x80, even :)
+ #iommu-cells = <0>;
Though given it seems there is only 1 client and this might really be
just 1 h/w block, you don't really need to use the iommu binding at
all. The DPU should be able to instantiate it's own IOMMU device.
There's other examples of this such as mali GPU though that is all one
driver, but that's a Linux implementation detail.
FWIW that's really a very different situation - the MMUs in a Mali GPU
are fundamental parts of its internal pipelines and would never make
sense to handle as
Re: [PATCH v3 1/2] dt-bindings: iommu: add bindings for sprd iommu
On Fri, Feb 5, 2021 at 1:21 AM Chunyan Zhang wrote:
>
> Hi Rob,
>
> On Fri, 5 Feb 2021 at 07:25, Rob Herring wrote:
> >
> > On Wed, Feb 03, 2021 at 05:07:26PM +0800, Chunyan Zhang wrote:
> > > From: Chunyan Zhang
> > >
> > > This iommu module can be used by Unisoc's multimedia devices, such as
> > > display, Image codec(jpeg) and a few signal processors, including
> > > VSP(video), GSP(graphic), ISP(image), and CPP(camera pixel processor),
> > > etc.
> > >
> > > Signed-off-by: Chunyan Zhang
> > > ---
> > > .../devicetree/bindings/iommu/sprd,iommu.yaml | 72 +++
> > > 1 file changed, 72 insertions(+)
> > > create mode 100644
> > > Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > > b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > > new file mode 100644
> > > index ..4fc99e81fa66
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > > @@ -0,0 +1,72 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +# Copyright 2020 Unisoc Inc.
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iommu/sprd,iommu.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Unisoc IOMMU and Multi-media MMU
> > > +
> > > +maintainers:
> > > + - Chunyan Zhang
> > > +
> > > +properties:
> > > + compatible:
> > > +enum:
> > > + - sprd,iommu-v1
> > > +
> > > + "#iommu-cells":
> > > +const: 0
> > > +description:
> > > + Unisoc IOMMUs are all single-master IOMMU devices, therefore no
> > > + additional information needs to associate with its master device.
> > > + Please refer to the generic bindings document for more details,
> > > + Documentation/devicetree/bindings/iommu/iommu.txt
> > > +
> > > + reg:
> > > +maxItems: 1
> > > +description:
> > > + Not required if 'sprd,iommu-regs' is defined.
> > > +
> > > + clocks:
> > > +description:
> > > + Reference to a gate clock phandle, since access to some of IOMMUs
> > > are
> > > + controlled by gate clock, but this is not required.
> > > +
> > > + sprd,iommu-regs:
> > > +$ref: /schemas/types.yaml#/definitions/phandle-array
> > > +description:
> > > + Reference to a syscon phandle plus 1 cell, the syscon defines the
> > > + register range used by the iommu and the media device, the cell
> > > + defines the offset for iommu registers. Since iommu module shares
> > > + the same register range with the media device which uses it.
> > > +
> > > +required:
> > > + - compatible
> > > + - "#iommu-cells"
> > > +
> > > +oneOf:
> > > + - required:
> > > + - reg
> > > + - required:
> > > + - sprd,iommu-regs
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > + - |
> > > +iommu_disp: iommu-disp {
> > > + compatible = "sprd,iommu-v1";
> > > + sprd,iommu-regs = <_regs 0x800>;
> >
> > If the IOMMU is contained within another device, then it should just be
> > a child node of that device.
>
> Yes, actually IOMMU can be seen as a child of multimedia devices, I
> considered moving IOMMU under into multimedia device node, but
> multimedia devices need IOMMU when probe[1], so I dropped that idea.
Don't design your binding around working-around linux issues.
> And they share the same register base, e.g.
>
> + mm {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + dpu_regs: syscon@6300 {
Drop this node.
> + compatible = "sprd,sc9863a-dpuregs", "syscon";
> + reg = <0 0x6300 0 0x1000>;
> + };
> +
> + dpu: dpu@6300 {
> + compatible = "sprd,sharkl3-dpu";
> + sprd,disp-regs = <_regs>;
reg = <0 0x6300 0 0x800>;
> + iommus = <_dispc>;
> + };
> +
> + iommu_dispc: iommu@6300 {
> + compatible = "sprd,iommu-v1";
> + sprd,iommu-regs = <_regs 0x800>;
reg = <0 0x63000800 0 0x800>;
> + #iommu-cells = <0>;
Though given it seems there is only 1 client and this might really be
just 1 h/w block, you don't really need to use the iommu binding at
all. The DPU should be able to instantiate it's own IOMMU device.
There's other examples of this such as mali GPU though that is all one
driver, but that's a Linux implementation detail.
Rob
Re: [PATCH v3 1/2] dt-bindings: iommu: add bindings for sprd iommu
Hi Rob,
On Fri, 5 Feb 2021 at 07:25, Rob Herring wrote:
>
> On Wed, Feb 03, 2021 at 05:07:26PM +0800, Chunyan Zhang wrote:
> > From: Chunyan Zhang
> >
> > This iommu module can be used by Unisoc's multimedia devices, such as
> > display, Image codec(jpeg) and a few signal processors, including
> > VSP(video), GSP(graphic), ISP(image), and CPP(camera pixel processor), etc.
> >
> > Signed-off-by: Chunyan Zhang
> > ---
> > .../devicetree/bindings/iommu/sprd,iommu.yaml | 72 +++
> > 1 file changed, 72 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > new file mode 100644
> > index ..4fc99e81fa66
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> > @@ -0,0 +1,72 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright 2020 Unisoc Inc.
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iommu/sprd,iommu.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Unisoc IOMMU and Multi-media MMU
> > +
> > +maintainers:
> > + - Chunyan Zhang
> > +
> > +properties:
> > + compatible:
> > +enum:
> > + - sprd,iommu-v1
> > +
> > + "#iommu-cells":
> > +const: 0
> > +description:
> > + Unisoc IOMMUs are all single-master IOMMU devices, therefore no
> > + additional information needs to associate with its master device.
> > + Please refer to the generic bindings document for more details,
> > + Documentation/devicetree/bindings/iommu/iommu.txt
> > +
> > + reg:
> > +maxItems: 1
> > +description:
> > + Not required if 'sprd,iommu-regs' is defined.
> > +
> > + clocks:
> > +description:
> > + Reference to a gate clock phandle, since access to some of IOMMUs are
> > + controlled by gate clock, but this is not required.
> > +
> > + sprd,iommu-regs:
> > +$ref: /schemas/types.yaml#/definitions/phandle-array
> > +description:
> > + Reference to a syscon phandle plus 1 cell, the syscon defines the
> > + register range used by the iommu and the media device, the cell
> > + defines the offset for iommu registers. Since iommu module shares
> > + the same register range with the media device which uses it.
> > +
> > +required:
> > + - compatible
> > + - "#iommu-cells"
> > +
> > +oneOf:
> > + - required:
> > + - reg
> > + - required:
> > + - sprd,iommu-regs
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > +iommu_disp: iommu-disp {
> > + compatible = "sprd,iommu-v1";
> > + sprd,iommu-regs = <_regs 0x800>;
>
> If the IOMMU is contained within another device, then it should just be
> a child node of that device.
Yes, actually IOMMU can be seen as a child of multimedia devices, I
considered moving IOMMU under into multimedia device node, but
multimedia devices need IOMMU when probe[1], so I dropped that idea.
And they share the same register base, e.g.
+ mm {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ dpu_regs: syscon@6300 {
+ compatible = "sprd,sc9863a-dpuregs", "syscon";
+ reg = <0 0x6300 0 0x1000>;
+ };
+
+ dpu: dpu@6300 {
+ compatible = "sprd,sharkl3-dpu";
+ sprd,disp-regs = <_regs>;
+ iommus = <_dispc>;
+ };
+
+ iommu_dispc: iommu@6300 {
+ compatible = "sprd,iommu-v1";
+ sprd,iommu-regs = <_regs 0x800>;
+ #iommu-cells = <0>;
+};
DPU use the registers from 0, IOMMU from 0x800, the purpose of using
syscon node was to avoid remapping register physical address.
> Or just make 'dpu_regs' an IOMMU provider
> (i.e. just add #iommu-cells to it).
xxx_regs(syscon node) defines the register range for IOMMU and a
multimedia device (such as dpu, image codec, etc.)
Hope I've explained the relationship of xxx_regs, multimedia device,
and iommu clearly :)
Any suggestion for this kind of cases?
Thanks,
Chunyan
[1]
https://elixir.bootlin.com/linux/v5.11-rc6/source/drivers/iommu/of_iommu.c#L145
>
> > + #iommu-cells = <0>;
> > +};
> > +
> > + - |
> > +iommu_jpg: iommu-jpg {
> > + compatible = "sprd,iommu-v1";
> > + sprd,iommu-regs = <_regs 0x300>;
> > + #iommu-cells = <0>;
> > + clocks = <_gate 1>;
> > +};
> > +
> > +...
> > --
> > 2.25.1
> >
Re: [PATCH v3 1/2] dt-bindings: iommu: add bindings for sprd iommu
On Wed, Feb 03, 2021 at 05:07:26PM +0800, Chunyan Zhang wrote:
> From: Chunyan Zhang
>
> This iommu module can be used by Unisoc's multimedia devices, such as
> display, Image codec(jpeg) and a few signal processors, including
> VSP(video), GSP(graphic), ISP(image), and CPP(camera pixel processor), etc.
>
> Signed-off-by: Chunyan Zhang
> ---
> .../devicetree/bindings/iommu/sprd,iommu.yaml | 72 +++
> 1 file changed, 72 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
>
> diff --git a/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> new file mode 100644
> index ..4fc99e81fa66
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2020 Unisoc Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iommu/sprd,iommu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Unisoc IOMMU and Multi-media MMU
> +
> +maintainers:
> + - Chunyan Zhang
> +
> +properties:
> + compatible:
> +enum:
> + - sprd,iommu-v1
> +
> + "#iommu-cells":
> +const: 0
> +description:
> + Unisoc IOMMUs are all single-master IOMMU devices, therefore no
> + additional information needs to associate with its master device.
> + Please refer to the generic bindings document for more details,
> + Documentation/devicetree/bindings/iommu/iommu.txt
> +
> + reg:
> +maxItems: 1
> +description:
> + Not required if 'sprd,iommu-regs' is defined.
> +
> + clocks:
> +description:
> + Reference to a gate clock phandle, since access to some of IOMMUs are
> + controlled by gate clock, but this is not required.
> +
> + sprd,iommu-regs:
> +$ref: /schemas/types.yaml#/definitions/phandle-array
> +description:
> + Reference to a syscon phandle plus 1 cell, the syscon defines the
> + register range used by the iommu and the media device, the cell
> + defines the offset for iommu registers. Since iommu module shares
> + the same register range with the media device which uses it.
> +
> +required:
> + - compatible
> + - "#iommu-cells"
> +
> +oneOf:
> + - required:
> + - reg
> + - required:
> + - sprd,iommu-regs
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +iommu_disp: iommu-disp {
> + compatible = "sprd,iommu-v1";
> + sprd,iommu-regs = <_regs 0x800>;
If the IOMMU is contained within another device, then it should just be
a child node of that device. Or just make 'dpu_regs' an IOMMU provider
(i.e. just add #iommu-cells to it).
> + #iommu-cells = <0>;
> +};
> +
> + - |
> +iommu_jpg: iommu-jpg {
> + compatible = "sprd,iommu-v1";
> + sprd,iommu-regs = <_regs 0x300>;
> + #iommu-cells = <0>;
> + clocks = <_gate 1>;
> +};
> +
> +...
> --
> 2.25.1
>
[PATCH v3 1/2] dt-bindings: iommu: add bindings for sprd iommu
From: Chunyan Zhang
This iommu module can be used by Unisoc's multimedia devices, such as
display, Image codec(jpeg) and a few signal processors, including
VSP(video), GSP(graphic), ISP(image), and CPP(camera pixel processor), etc.
Signed-off-by: Chunyan Zhang
---
.../devicetree/bindings/iommu/sprd,iommu.yaml | 72 +++
1 file changed, 72 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
diff --git a/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
new file mode 100644
index ..4fc99e81fa66
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/sprd,iommu.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2020 Unisoc Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/sprd,iommu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Unisoc IOMMU and Multi-media MMU
+
+maintainers:
+ - Chunyan Zhang
+
+properties:
+ compatible:
+enum:
+ - sprd,iommu-v1
+
+ "#iommu-cells":
+const: 0
+description:
+ Unisoc IOMMUs are all single-master IOMMU devices, therefore no
+ additional information needs to associate with its master device.
+ Please refer to the generic bindings document for more details,
+ Documentation/devicetree/bindings/iommu/iommu.txt
+
+ reg:
+maxItems: 1
+description:
+ Not required if 'sprd,iommu-regs' is defined.
+
+ clocks:
+description:
+ Reference to a gate clock phandle, since access to some of IOMMUs are
+ controlled by gate clock, but this is not required.
+
+ sprd,iommu-regs:
+$ref: /schemas/types.yaml#/definitions/phandle-array
+description:
+ Reference to a syscon phandle plus 1 cell, the syscon defines the
+ register range used by the iommu and the media device, the cell
+ defines the offset for iommu registers. Since iommu module shares
+ the same register range with the media device which uses it.
+
+required:
+ - compatible
+ - "#iommu-cells"
+
+oneOf:
+ - required:
+ - reg
+ - required:
+ - sprd,iommu-regs
+
+additionalProperties: false
+
+examples:
+ - |
+iommu_disp: iommu-disp {
+ compatible = "sprd,iommu-v1";
+ sprd,iommu-regs = <_regs 0x800>;
+ #iommu-cells = <0>;
+};
+
+ - |
+iommu_jpg: iommu-jpg {
+ compatible = "sprd,iommu-v1";
+ sprd,iommu-regs = <_regs 0x300>;
+ #iommu-cells = <0>;
+ clocks = <_gate 1>;
+};
+
+...
--
2.25.1
