2019-09-10 07:47:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: arm: samsung: Convert Samsung Exynos IOMMU H/W, System MMU to dt-schema

On Mon, 9 Sep 2019 at 14:42, Maciej Falkowski <[email protected]> wrote:
>
> Convert Samsung Exynos IOMMU H/W, System Memory Management Unit
> to newer dt-schema format.
>
> Signed-off-by: Maciej Falkowski <[email protected]>
> Signed-off-by: Andrzej Hajda <[email protected]>

Hi Maciej,

Thanks for the patch. Few comments below.

> ---
> .../bindings/iommu/samsung,sysmmu.txt | 67 ------------
> .../bindings/iommu/samsung,sysmmu.yaml | 102 ++++++++++++++++++
> 2 files changed, 102 insertions(+), 67 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
> create mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
>
> diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
> deleted file mode 100644
> index 525ec82615a6..000000000000
> --- a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
> +++ /dev/null
> @@ -1,67 +0,0 @@
> -Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
> -
> -Samsung's Exynos architecture contains System MMUs that enables scattered
> -physical memory chunks visible as a contiguous region to DMA-capable peripheral
> -devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
> -
> -System MMU is an IOMMU and supports identical translation table format to
> -ARMv7 translation tables with minimum set of page properties including access
> -permissions, shareability and security protection. In addition, System MMU has
> -another capabilities like L2 TLB or block-fetch buffers to minimize translation
> -latency.
> -
> -System MMUs are in many to one relation with peripheral devices, i.e. single
> -peripheral device might have multiple System MMUs (usually one for each bus
> -master), but one System MMU can handle transactions from only one peripheral
> -device. The relation between a System MMU and the peripheral device needs to be
> -defined in device node of the peripheral device.
> -
> -MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
> -MMUs.
> -* MFC has one System MMU on its left and right bus.
> -* FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
> - for window 1, 2 and 3.
> -* M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
> - the other System MMU on the write channel.
> -
> -For information on assigning System MMU controller to its peripheral devices,
> -see generic IOMMU bindings.
> -
> -Required properties:
> -- compatible: Should be "samsung,exynos-sysmmu"
> -- reg: A tuple of base address and size of System MMU registers.
> -- #iommu-cells: Should be <0>.
> -- interrupts: An interrupt specifier for interrupt signal of System MMU,
> - according to the format defined by a particular interrupt
> - controller.
> -- clock-names: Should be "sysmmu" or a pair of "aclk" and "pclk" to gate
> - SYSMMU core clocks.
> - Optional "master" if the clock to the System MMU is gated by
> - another gate clock other core (usually main gate clock
> - of peripheral device this SYSMMU belongs to).
> -- clocks: Phandles for respective clocks described by clock-names.
> -- power-domains: Required if the System MMU is needed to gate its power.
> - Please refer to the following document:
> - Documentation/devicetree/bindings/power/pd-samsung.txt
> -
> -Examples:
> - gsc_0: gsc@13e00000 {
> - compatible = "samsung,exynos5-gsc";
> - reg = <0x13e00000 0x1000>;
> - interrupts = <0 85 0>;
> - power-domains = <&pd_gsc>;
> - clocks = <&clock CLK_GSCL0>;
> - clock-names = "gscl";
> - iommus = <&sysmmu_gsc0>;
> - };
> -
> - sysmmu_gsc0: sysmmu@13e80000 {
> - compatible = "samsung,exynos-sysmmu";
> - reg = <0x13E80000 0x1000>;
> - interrupt-parent = <&combiner>;
> - interrupts = <2 0>;
> - clock-names = "sysmmu", "master";
> - clocks = <&clock CLK_SMMU_GSCL0>, <&clock CLK_GSCL0>;
> - power-domains = <&pd_gsc>;
> - #iommu-cells = <0>;
> - };
> diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
> new file mode 100644
> index 000000000000..6c40dfb86899
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iommu/samsung,sysmmu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
> +
> +maintainers:
> + - Marek Szyprowski <[email protected]>
> +
> +description: |+
> + Samsung's Exynos architecture contains System MMUs that enables scattered
> + physical memory chunks visible as a contiguous region to DMA-capable peripheral
> + devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
> +
> + System MMU is an IOMMU and supports identical translation table format to
> + ARMv7 translation tables with minimum set of page properties including access
> + permissions, shareability and security protection. In addition, System MMU has
> + another capabilities like L2 TLB or block-fetch buffers to minimize translation
> + latency.
> +
> + System MMUs are in many to one relation with peripheral devices, i.e. single
> + peripheral device might have multiple System MMUs (usually one for each bus
> + master), but one System MMU can handle transactions from only one peripheral
> + device. The relation between a System MMU and the peripheral device needs to be
> + defined in device node of the peripheral device.
> +
> + MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
> + MMUs.
> + * MFC has one System MMU on its left and right bus.
> + * FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
> + for window 1, 2 and 3.
> + * M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
> + the other System MMU on the write channel.
> +
> + For information on assigning System MMU controller to its peripheral devices,
> + see generic IOMMU bindings.
> +
> +properties:
> + compatible:
> + const: samsung,exynos-sysmmu

Add empty line between properties. Makes it easier to read.

> + reg:
> + description: A tuple of base address and size of System MMU registers.

Description not needed.

> + maxItems: 1
> + interrupts:
> + description: |
> + An interrupt specifier for interrupt signal of System MMU,
> + according to the format defined by a particular interrupt
> + controller.

Description not needed.

> + clocks:
> + description: Phandles for respective clocks described by clock-names.

Description not needed but I think number of elements is.

> + clock-names:
> + description: |
> + Should be "sysmmu" or a pair of "aclk" and "pclk" to gate
> + SYSMMU core clocks.
> + Optional "master" if the clock to the System MMU is gated by
> + another gate clock other core (usually main gate clock
> + of peripheral device this SYSMMU belongs to).
> + minItems: 1
> + maxItems: 2

Based on the description this can be up to three clocks.

Please declare the exact items within each combination (so either
sysmmu or aclk+pclk, plus optional master?). If this does not depend
on compatible, then oneOf could work. Then number of items could be
removed.

> + "#iommu-cells":
> + const: 0
> + description: Should be <0>.

Description not needed.

> + power-domains:

$ref: /schemas/types.yaml#/definitions/phandle

> + description: |
> + Required if the System MMU is needed to gate its power.
> + Please refer to the following document:
> + Documentation/devicetree/bindings/power/pd-samsung.txt
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - "#iommu-cells"
> +
> +examples:
> + - |
> + gsc_0: gsc@13e00000 {
> + compatible = "samsung,exynos5-gsc";
> + reg = <0x13e00000 0x1000>;
> + interrupts = <0 85 0>;
> + power-domains = <&pd_gsc>;
> + clocks = <&clock 0>; //CLK_GSCL0

Missing space after //

> + clock-names = "gscl";
> + iommus = <&sysmmu_gsc0>;
> + };
> + - |

This is one example, not two.

Best regards,
Krzysztof

> + sysmmu_gsc0: sysmmu@13e80000 {
> + compatible = "samsung,exynos-sysmmu";
> + reg = <0x13E80000 0x1000>;
> + interrupt-parent = <&combiner>;
> + interrupts = <2 0>;
> + clock-names = "sysmmu", "master";
> + clocks = <&clock 0>, // CLK_SMMU_GSCL0
> + <&clock 0>; // CLK_GSCL0
> + power-domains = <&pd_gsc>;
> + #iommu-cells = <0>;
> + };
> +
> --
> 2.17.1
>


2019-09-10 19:11:36

by Maciej Falkowski

[permalink] [raw]
Subject: [PATCH v2] dt-bindings: arm: samsung: Convert Samsung Exynos IOMMU H/W, System MMU to dt-schema

Convert Samsung Exynos IOMMU H/W, System Memory Management Unit
to newer dt-schema format.

Update clock description.

Signed-off-by: Maciej Falkowski <[email protected]>
Signed-off-by: Andrzej Hajda <[email protected]>
---
Hi Krzysztof,

Thank you for feedback.

New changes:
- style fixes including missing empty lines,
deletion of unneeded descriptions

- fix mistake where one example was split
into two separete ones.

There are some updates with clock description. I have spoken with
Marek Szyprowski and the right setup for clocks seems to be two pairs:
"sysmmu" with optional "master" or a pair of "aclk" + "pclk".

The option: "aclk" + "pclk" + "master" was never used in any
of device bindings and there are none compilation problems with that.

In so, clock-names are rewritten to handle this version
and maximal clock number is set to two.

Best regards,
Maciej Falkowski
---
.../bindings/iommu/samsung,sysmmu.txt | 67 -----------
.../bindings/iommu/samsung,sysmmu.yaml | 112 ++++++++++++++++++
2 files changed, 112 insertions(+), 67 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
create mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml

diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
deleted file mode 100644
index 525ec82615a6..000000000000
--- a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
+++ /dev/null
@@ -1,67 +0,0 @@
-Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
-
-Samsung's Exynos architecture contains System MMUs that enables scattered
-physical memory chunks visible as a contiguous region to DMA-capable peripheral
-devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
-
-System MMU is an IOMMU and supports identical translation table format to
-ARMv7 translation tables with minimum set of page properties including access
-permissions, shareability and security protection. In addition, System MMU has
-another capabilities like L2 TLB or block-fetch buffers to minimize translation
-latency.
-
-System MMUs are in many to one relation with peripheral devices, i.e. single
-peripheral device might have multiple System MMUs (usually one for each bus
-master), but one System MMU can handle transactions from only one peripheral
-device. The relation between a System MMU and the peripheral device needs to be
-defined in device node of the peripheral device.
-
-MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
-MMUs.
-* MFC has one System MMU on its left and right bus.
-* FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
- for window 1, 2 and 3.
-* M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
- the other System MMU on the write channel.
-
-For information on assigning System MMU controller to its peripheral devices,
-see generic IOMMU bindings.
-
-Required properties:
-- compatible: Should be "samsung,exynos-sysmmu"
-- reg: A tuple of base address and size of System MMU registers.
-- #iommu-cells: Should be <0>.
-- interrupts: An interrupt specifier for interrupt signal of System MMU,
- according to the format defined by a particular interrupt
- controller.
-- clock-names: Should be "sysmmu" or a pair of "aclk" and "pclk" to gate
- SYSMMU core clocks.
- Optional "master" if the clock to the System MMU is gated by
- another gate clock other core (usually main gate clock
- of peripheral device this SYSMMU belongs to).
-- clocks: Phandles for respective clocks described by clock-names.
-- power-domains: Required if the System MMU is needed to gate its power.
- Please refer to the following document:
- Documentation/devicetree/bindings/power/pd-samsung.txt
-
-Examples:
- gsc_0: gsc@13e00000 {
- compatible = "samsung,exynos5-gsc";
- reg = <0x13e00000 0x1000>;
- interrupts = <0 85 0>;
- power-domains = <&pd_gsc>;
- clocks = <&clock CLK_GSCL0>;
- clock-names = "gscl";
- iommus = <&sysmmu_gsc0>;
- };
-
- sysmmu_gsc0: sysmmu@13e80000 {
- compatible = "samsung,exynos-sysmmu";
- reg = <0x13E80000 0x1000>;
- interrupt-parent = <&combiner>;
- interrupts = <2 0>;
- clock-names = "sysmmu", "master";
- clocks = <&clock CLK_SMMU_GSCL0>, <&clock CLK_GSCL0>;
- power-domains = <&pd_gsc>;
- #iommu-cells = <0>;
- };
diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
new file mode 100644
index 000000000000..85d1a251f2ff
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/samsung,sysmmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
+
+maintainers:
+ - Marek Szyprowski <[email protected]>
+
+description: |+
+ Samsung's Exynos architecture contains System MMUs that enables scattered
+ physical memory chunks visible as a contiguous region to DMA-capable peripheral
+ devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
+
+ System MMU is an IOMMU and supports identical translation table format to
+ ARMv7 translation tables with minimum set of page properties including access
+ permissions, shareability and security protection. In addition, System MMU has
+ another capabilities like L2 TLB or block-fetch buffers to minimize translation
+ latency.
+
+ System MMUs are in many to one relation with peripheral devices, i.e. single
+ peripheral device might have multiple System MMUs (usually one for each bus
+ master), but one System MMU can handle transactions from only one peripheral
+ device. The relation between a System MMU and the peripheral device needs to be
+ defined in device node of the peripheral device.
+
+ MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
+ MMUs.
+ * MFC has one System MMU on its left and right bus.
+ * FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
+ for window 1, 2 and 3.
+ * M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
+ the other System MMU on the write channel.
+
+ For information on assigning System MMU controller to its peripheral devices,
+ see generic IOMMU bindings.
+
+properties:
+ compatible:
+ const: samsung,exynos-sysmmu
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ description: |
+ An interrupt specifier for interrupt signal of System MMU,
+ according to the format defined by a particular interrupt
+ controller.
+
+ clocks:
+ minItems: 1
+ maxItems: 2
+
+ clock-names:
+ oneOf:
+ - contains:
+ enum:
+ - sysmmu
+ - master
+ - contains:
+ enum:
+ - aclk
+ - pclk
+ description: |
+ Should be "sysmmu" with optional "master"
+ or a pair "aclk" with "pclk".
+
+ "#iommu-cells":
+ const: 0
+
+ power-domains:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: |
+ Required if the System MMU is needed to gate its power.
+ Please refer to the following document:
+ Documentation/devicetree/bindings/power/pd-samsung.txt
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - "#iommu-cells"
+
+examples:
+ - |
+ gsc_0: gsc@13e00000 {
+ compatible = "samsung,exynos5-gsc";
+ reg = <0x13e00000 0x1000>;
+ interrupts = <0 85 0>;
+ power-domains = <&pd_gsc>;
+ clocks = <&clock 0>; // CLK_GSCL0
+ clock-names = "gscl";
+ iommus = <&sysmmu_gsc0>;
+ };
+
+ sysmmu_gsc0: sysmmu@13e80000 {
+ compatible = "samsung,exynos-sysmmu";
+ reg = <0x13E80000 0x1000>;
+ interrupt-parent = <&combiner>;
+ interrupts = <2 0>;
+ clock-names = "sysmmu", "master";
+ clocks = <&clock 0>, // CLK_SMMU_GSCL0
+ <&clock 0>; // CLK_GSCL0
+ power-domains = <&pd_gsc>;
+ #iommu-cells = <0>;
+ };
+
--
2.17.1

2019-09-11 06:30:25

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2] dt-bindings: arm: samsung: Convert Samsung Exynos IOMMU H/W, System MMU to dt-schema

On Tue, 10 Sep 2019 at 17:52, Maciej Falkowski <[email protected]> wrote:
>
> Convert Samsung Exynos IOMMU H/W, System Memory Management Unit
> to newer dt-schema format.
>
> Update clock description.
>
> Signed-off-by: Maciej Falkowski <[email protected]>
> Signed-off-by: Andrzej Hajda <[email protected]>
> ---
> Hi Krzysztof,
>
> Thank you for feedback.
>
> New changes:
> - style fixes including missing empty lines,
> deletion of unneeded descriptions
>
> - fix mistake where one example was split
> into two separete ones.
>
> There are some updates with clock description. I have spoken with
> Marek Szyprowski and the right setup for clocks seems to be two pairs:
> "sysmmu" with optional "master" or a pair of "aclk" + "pclk".
>
> The option: "aclk" + "pclk" + "master" was never used in any
> of device bindings and there are none compilation problems with that.
>
> In so, clock-names are rewritten to handle this version
> and maximal clock number is set to two.
>
> Best regards,
> Maciej Falkowski
> ---
> .../bindings/iommu/samsung,sysmmu.txt | 67 -----------
> .../bindings/iommu/samsung,sysmmu.yaml | 112 ++++++++++++++++++
> 2 files changed, 112 insertions(+), 67 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
> create mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
>
> diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
> deleted file mode 100644
> index 525ec82615a6..000000000000
> --- a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
> +++ /dev/null
> @@ -1,67 +0,0 @@
> -Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
> -
> -Samsung's Exynos architecture contains System MMUs that enables scattered
> -physical memory chunks visible as a contiguous region to DMA-capable peripheral
> -devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
> -
> -System MMU is an IOMMU and supports identical translation table format to
> -ARMv7 translation tables with minimum set of page properties including access
> -permissions, shareability and security protection. In addition, System MMU has
> -another capabilities like L2 TLB or block-fetch buffers to minimize translation
> -latency.
> -
> -System MMUs are in many to one relation with peripheral devices, i.e. single
> -peripheral device might have multiple System MMUs (usually one for each bus
> -master), but one System MMU can handle transactions from only one peripheral
> -device. The relation between a System MMU and the peripheral device needs to be
> -defined in device node of the peripheral device.
> -
> -MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
> -MMUs.
> -* MFC has one System MMU on its left and right bus.
> -* FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
> - for window 1, 2 and 3.
> -* M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
> - the other System MMU on the write channel.
> -
> -For information on assigning System MMU controller to its peripheral devices,
> -see generic IOMMU bindings.
> -
> -Required properties:
> -- compatible: Should be "samsung,exynos-sysmmu"
> -- reg: A tuple of base address and size of System MMU registers.
> -- #iommu-cells: Should be <0>.
> -- interrupts: An interrupt specifier for interrupt signal of System MMU,
> - according to the format defined by a particular interrupt
> - controller.
> -- clock-names: Should be "sysmmu" or a pair of "aclk" and "pclk" to gate
> - SYSMMU core clocks.
> - Optional "master" if the clock to the System MMU is gated by
> - another gate clock other core (usually main gate clock
> - of peripheral device this SYSMMU belongs to).
> -- clocks: Phandles for respective clocks described by clock-names.
> -- power-domains: Required if the System MMU is needed to gate its power.
> - Please refer to the following document:
> - Documentation/devicetree/bindings/power/pd-samsung.txt
> -
> -Examples:
> - gsc_0: gsc@13e00000 {
> - compatible = "samsung,exynos5-gsc";
> - reg = <0x13e00000 0x1000>;
> - interrupts = <0 85 0>;
> - power-domains = <&pd_gsc>;
> - clocks = <&clock CLK_GSCL0>;
> - clock-names = "gscl";
> - iommus = <&sysmmu_gsc0>;
> - };
> -
> - sysmmu_gsc0: sysmmu@13e80000 {
> - compatible = "samsung,exynos-sysmmu";
> - reg = <0x13E80000 0x1000>;
> - interrupt-parent = <&combiner>;
> - interrupts = <2 0>;
> - clock-names = "sysmmu", "master";
> - clocks = <&clock CLK_SMMU_GSCL0>, <&clock CLK_GSCL0>;
> - power-domains = <&pd_gsc>;
> - #iommu-cells = <0>;
> - };
> diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
> new file mode 100644
> index 000000000000..85d1a251f2ff
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
> @@ -0,0 +1,112 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iommu/samsung,sysmmu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
> +
> +maintainers:
> + - Marek Szyprowski <[email protected]>
> +
> +description: |+
> + Samsung's Exynos architecture contains System MMUs that enables scattered
> + physical memory chunks visible as a contiguous region to DMA-capable peripheral
> + devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
> +
> + System MMU is an IOMMU and supports identical translation table format to
> + ARMv7 translation tables with minimum set of page properties including access
> + permissions, shareability and security protection. In addition, System MMU has
> + another capabilities like L2 TLB or block-fetch buffers to minimize translation
> + latency.
> +
> + System MMUs are in many to one relation with peripheral devices, i.e. single
> + peripheral device might have multiple System MMUs (usually one for each bus
> + master), but one System MMU can handle transactions from only one peripheral
> + device. The relation between a System MMU and the peripheral device needs to be
> + defined in device node of the peripheral device.
> +
> + MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
> + MMUs.
> + * MFC has one System MMU on its left and right bus.
> + * FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
> + for window 1, 2 and 3.
> + * M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
> + the other System MMU on the write channel.
> +
> + For information on assigning System MMU controller to its peripheral devices,
> + see generic IOMMU bindings.
> +
> +properties:
> + compatible:
> + const: samsung,exynos-sysmmu
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + description: |
> + An interrupt specifier for interrupt signal of System MMU,
> + according to the format defined by a particular interrupt
> + controller.

You left this description and it is not needed - does not bring any information.
Add also maxItems as I believe there should be only one interrupt.

> +
> + clocks:
> + minItems: 1
> + maxItems: 2
> +
> + clock-names:
> + oneOf:
> + - contains:
> + enum:
> + - sysmmu
> + - master

This is not specific enough, because it accepts:
clock-names = "master";
clock-names = "aclk";

Instead I think this could work:
oneOf:
- items:
- const: sysmmu
- items:
- const: sysmmu
- const: master
- items:
- const: aclk
- const: pclk

Best regards,
Krzysztof

2019-09-11 11:08:05

by Maciej Falkowski

[permalink] [raw]
Subject: [PATCH v3] dt-bindings: arm: samsung: Convert Samsung Exynos IOMMU H/W, System MMU to dt-schema

Convert Samsung Exynos IOMMU H/W, System Memory Management Unit
to newer dt-schema format.

Update clock description.

Signed-off-by: Maciej Falkowski <[email protected]>
Signed-off-by: Andrzej Hajda <[email protected]>
---
Hi Krzysztof,

Thank you for feedback.

v3:

- remove obsolete interrupts description and
set its maxItems to one. There are some incompatible
files which will be fixed with another patch.

- clock-names pattern is changed to your more precise
version. I also added option "pclk" + "aclk" as some
bindings are also using it.

Best regards,
Maciej Falkowski
---
.../bindings/iommu/samsung,sysmmu.txt | 67 -----------
.../bindings/iommu/samsung,sysmmu.yaml | 112 ++++++++++++++++++
2 files changed, 112 insertions(+), 67 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
create mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml

diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
deleted file mode 100644
index 525ec82615a6..000000000000
--- a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
+++ /dev/null
@@ -1,67 +0,0 @@
-Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
-
-Samsung's Exynos architecture contains System MMUs that enables scattered
-physical memory chunks visible as a contiguous region to DMA-capable peripheral
-devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
-
-System MMU is an IOMMU and supports identical translation table format to
-ARMv7 translation tables with minimum set of page properties including access
-permissions, shareability and security protection. In addition, System MMU has
-another capabilities like L2 TLB or block-fetch buffers to minimize translation
-latency.
-
-System MMUs are in many to one relation with peripheral devices, i.e. single
-peripheral device might have multiple System MMUs (usually one for each bus
-master), but one System MMU can handle transactions from only one peripheral
-device. The relation between a System MMU and the peripheral device needs to be
-defined in device node of the peripheral device.
-
-MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
-MMUs.
-* MFC has one System MMU on its left and right bus.
-* FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
- for window 1, 2 and 3.
-* M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
- the other System MMU on the write channel.
-
-For information on assigning System MMU controller to its peripheral devices,
-see generic IOMMU bindings.
-
-Required properties:
-- compatible: Should be "samsung,exynos-sysmmu"
-- reg: A tuple of base address and size of System MMU registers.
-- #iommu-cells: Should be <0>.
-- interrupts: An interrupt specifier for interrupt signal of System MMU,
- according to the format defined by a particular interrupt
- controller.
-- clock-names: Should be "sysmmu" or a pair of "aclk" and "pclk" to gate
- SYSMMU core clocks.
- Optional "master" if the clock to the System MMU is gated by
- another gate clock other core (usually main gate clock
- of peripheral device this SYSMMU belongs to).
-- clocks: Phandles for respective clocks described by clock-names.
-- power-domains: Required if the System MMU is needed to gate its power.
- Please refer to the following document:
- Documentation/devicetree/bindings/power/pd-samsung.txt
-
-Examples:
- gsc_0: gsc@13e00000 {
- compatible = "samsung,exynos5-gsc";
- reg = <0x13e00000 0x1000>;
- interrupts = <0 85 0>;
- power-domains = <&pd_gsc>;
- clocks = <&clock CLK_GSCL0>;
- clock-names = "gscl";
- iommus = <&sysmmu_gsc0>;
- };
-
- sysmmu_gsc0: sysmmu@13e80000 {
- compatible = "samsung,exynos-sysmmu";
- reg = <0x13E80000 0x1000>;
- interrupt-parent = <&combiner>;
- interrupts = <2 0>;
- clock-names = "sysmmu", "master";
- clocks = <&clock CLK_SMMU_GSCL0>, <&clock CLK_GSCL0>;
- power-domains = <&pd_gsc>;
- #iommu-cells = <0>;
- };
diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
new file mode 100644
index 000000000000..a8141d6c326a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/samsung,sysmmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
+
+maintainers:
+ - Marek Szyprowski <[email protected]>
+
+description: |+
+ Samsung's Exynos architecture contains System MMUs that enables scattered
+ physical memory chunks visible as a contiguous region to DMA-capable peripheral
+ devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
+
+ System MMU is an IOMMU and supports identical translation table format to
+ ARMv7 translation tables with minimum set of page properties including access
+ permissions, shareability and security protection. In addition, System MMU has
+ another capabilities like L2 TLB or block-fetch buffers to minimize translation
+ latency.
+
+ System MMUs are in many to one relation with peripheral devices, i.e. single
+ peripheral device might have multiple System MMUs (usually one for each bus
+ master), but one System MMU can handle transactions from only one peripheral
+ device. The relation between a System MMU and the peripheral device needs to be
+ defined in device node of the peripheral device.
+
+ MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
+ MMUs.
+ * MFC has one System MMU on its left and right bus.
+ * FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
+ for window 1, 2 and 3.
+ * M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
+ the other System MMU on the write channel.
+
+ For information on assigning System MMU controller to its peripheral devices,
+ see generic IOMMU bindings.
+
+properties:
+ compatible:
+ const: samsung,exynos-sysmmu
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ minItems: 1
+ maxItems: 2
+
+ clock-names:
+ oneOf:
+ - items:
+ - const: sysmmu
+ - items:
+ - const: sysmmu
+ - const: master
+ - items:
+ - const: aclk
+ - const: pclk
+ - items:
+ - const: pclk
+ - const: aclk
+ description: |
+ Should be "sysmmu" with optional "master"
+ or a pair "aclk" with "pclk".
+
+ "#iommu-cells":
+ const: 0
+
+ power-domains:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: |
+ Required if the System MMU is needed to gate its power.
+ Please refer to the following document:
+ Documentation/devicetree/bindings/power/pd-samsung.txt
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - "#iommu-cells"
+
+examples:
+ - |
+ gsc_0: gsc@13e00000 {
+ compatible = "samsung,exynos5-gsc";
+ reg = <0x13e00000 0x1000>;
+ interrupts = <0 85 0>;
+ power-domains = <&pd_gsc>;
+ clocks = <&clock 0>; // CLK_GSCL0
+ clock-names = "gscl";
+ iommus = <&sysmmu_gsc0>;
+ };
+
+ sysmmu_gsc0: sysmmu@13e80000 {
+ compatible = "samsung,exynos-sysmmu";
+ reg = <0x13E80000 0x1000>;
+ interrupt-parent = <&combiner>;
+ interrupts = <2 0>;
+ clock-names = "sysmmu", "master";
+ clocks = <&clock 0>, // CLK_SMMU_GSCL0
+ <&clock 0>; // CLK_GSCL0
+ power-domains = <&pd_gsc>;
+ #iommu-cells = <0>;
+ };
+
--
2.17.1

2019-09-11 11:39:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3] dt-bindings: arm: samsung: Convert Samsung Exynos IOMMU H/W, System MMU to dt-schema

On Wed, 11 Sep 2019 at 13:05, Maciej Falkowski <[email protected]> wrote:
>
> Convert Samsung Exynos IOMMU H/W, System Memory Management Unit
> to newer dt-schema format.
>
> Update clock description.
>
> Signed-off-by: Maciej Falkowski <[email protected]>
> Signed-off-by: Andrzej Hajda <[email protected]>
> ---
> Hi Krzysztof,
>
> Thank you for feedback.
>
> v3:
>
> - remove obsolete interrupts description and
> set its maxItems to one. There are some incompatible
> files which will be fixed with another patch.

Driver stopped supporting two IRQ lines in commit
7222e8db2d506197ee183de0f9b76b3ad97e8c18 (iommu/exynos: Fix build
errors). The second IRQ line in Exynos3250 DTS seems to be ignored.

The patch now looks good to me:
Reviewed-by: Krzysztof Kozlowski <[email protected]>

However for some reasons you did not CC the IOMMU maintainers. Please
use scripts/get_maintainer.pl to get the list of folks to CC.

Best regards,
Krzysztof

2019-09-11 12:01:41

by Marek Szyprowski

[permalink] [raw]
Subject: Re: [PATCH v3] dt-bindings: arm: samsung: Convert Samsung Exynos IOMMU H/W, System MMU to dt-schema

Hi Krzyszotf,

On 2019-09-11 13:36, Krzysztof Kozlowski wrote:
> On Wed, 11 Sep 2019 at 13:05, Maciej Falkowski <[email protected]> wrote:
>> Convert Samsung Exynos IOMMU H/W, System Memory Management Unit
>> to newer dt-schema format.
>>
>> Update clock description.
>>
>> Signed-off-by: Maciej Falkowski <[email protected]>
>> Signed-off-by: Andrzej Hajda <[email protected]>
>> ---
>> Hi Krzysztof,
>>
>> Thank you for feedback.
>>
>> v3:
>>
>> - remove obsolete interrupts description and
>> set its maxItems to one. There are some incompatible
>> files which will be fixed with another patch.
> Driver stopped supporting two IRQ lines in commit
> 7222e8db2d506197ee183de0f9b76b3ad97e8c18 (iommu/exynos: Fix build
> errors). The second IRQ line in Exynos3250 DTS seems to be ignored.
>
> The patch now looks good to me:
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
>
> However for some reasons you did not CC the IOMMU maintainers. Please
> use scripts/get_maintainer.pl to get the list of folks to CC.

Frankly I don't see any reason to spam IOMMU ml or maintainer with this
discussion about dt-binding conversion. This patch will be merged via dt
tree if I got it right.

Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

2019-09-11 12:31:11

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v3] dt-bindings: arm: samsung: Convert Samsung Exynos IOMMU H/W, System MMU to dt-schema

On Wed, 11 Sep 2019 at 13:57, Marek Szyprowski <[email protected]> wrote:
>
> Hi Krzyszotf,
>
> On 2019-09-11 13:36, Krzysztof Kozlowski wrote:
> > On Wed, 11 Sep 2019 at 13:05, Maciej Falkowski <[email protected]> wrote:
> >> Convert Samsung Exynos IOMMU H/W, System Memory Management Unit
> >> to newer dt-schema format.
> >>
> >> Update clock description.
> >>
> >> Signed-off-by: Maciej Falkowski <[email protected]>
> >> Signed-off-by: Andrzej Hajda <[email protected]>
> >> ---
> >> Hi Krzysztof,
> >>
> >> Thank you for feedback.
> >>
> >> v3:
> >>
> >> - remove obsolete interrupts description and
> >> set its maxItems to one. There are some incompatible
> >> files which will be fixed with another patch.
> > Driver stopped supporting two IRQ lines in commit
> > 7222e8db2d506197ee183de0f9b76b3ad97e8c18 (iommu/exynos: Fix build
> > errors). The second IRQ line in Exynos3250 DTS seems to be ignored.
> >
> > The patch now looks good to me:
> > Reviewed-by: Krzysztof Kozlowski <[email protected]>
> >
> > However for some reasons you did not CC the IOMMU maintainers. Please
> > use scripts/get_maintainer.pl to get the list of folks to CC.
>
> Frankly I don't see any reason to spam IOMMU ml or maintainer with this
> discussion about dt-binding conversion. This patch will be merged via dt
> tree if I got it right.

Indeed usually subsystem maintainers are not interested in DT schema
conversion although they are mentioned as maintainers for this file so
it is nice to CC them... I would not call spamming when there is
explicit pattern for CCing.

Best regards,
Krzysztof

2019-09-11 13:35:51

by Maciej Falkowski

[permalink] [raw]
Subject: [PATCH] dt-bindings: arm: samsung: Exynos 3250: iommu: remove obsolete IRQ lines

In commit 7222e8db2d506197ee183de0f9b76b3ad97e8c18 (iommu/exynos: Fix build
errors) Exynos3250 iommu driver stopped supporting two IRQ lines.
The second IRQ line in DTS is ignored and is not needed.

Signed-off-by: Maciej Falkowski <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
---
arch/arm/boot/dts/exynos3250.dtsi | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
index 784818490376..190d9160a5d1 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -314,8 +314,7 @@
sysmmu_jpeg: sysmmu@11a60000 {
compatible = "samsung,exynos-sysmmu";
reg = <0x11a60000 0x1000>;
- interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 161 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>;
clock-names = "sysmmu", "master";
clocks = <&cmu CLK_SMMUJPEG>, <&cmu CLK_JPEG>;
power-domains = <&pd_cam>;
@@ -355,8 +354,7 @@
sysmmu_fimd0: sysmmu@11e20000 {
compatible = "samsung,exynos-sysmmu";
reg = <0x11e20000 0x1000>;
- interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
clock-names = "sysmmu", "master";
clocks = <&cmu CLK_SMMUFIMD0>, <&cmu CLK_FIMD0>;
power-domains = <&pd_lcd0>;
@@ -507,8 +505,7 @@
sysmmu_mfc: sysmmu@13620000 {
compatible = "samsung,exynos-sysmmu";
reg = <0x13620000 0x1000>;
- interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
clock-names = "sysmmu", "master";
clocks = <&cmu CLK_SMMUMFC_L>, <&cmu CLK_MFC>;
power-domains = <&pd_mfc>;
--
2.17.1

2019-09-12 20:48:16

by Maciej Falkowski

[permalink] [raw]
Subject: [PATCH v2] ARM: dts: exynos: remove obsolete IRQ lines

In commit 7222e8db2d506197ee183de0f9b76b3ad97e8c18 (iommu/exynos: Fix build
errors) Exynos3250 iommu driver stopped supporting two IRQ lines.
The second IRQ line in DTS is ignored and is not needed.

Signed-off-by: Maciej Falkowski <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
---
v2:
- change commit message to proper version
- add proper recipients
---
arch/arm/boot/dts/exynos3250.dtsi | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/exynos3250.dtsi b/arch/arm/boot/dts/exynos3250.dtsi
index 784818490376..190d9160a5d1 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -314,8 +314,7 @@
sysmmu_jpeg: sysmmu@11a60000 {
compatible = "samsung,exynos-sysmmu";
reg = <0x11a60000 0x1000>;
- interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 161 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH>;
clock-names = "sysmmu", "master";
clocks = <&cmu CLK_SMMUJPEG>, <&cmu CLK_JPEG>;
power-domains = <&pd_cam>;
@@ -355,8 +354,7 @@
sysmmu_fimd0: sysmmu@11e20000 {
compatible = "samsung,exynos-sysmmu";
reg = <0x11e20000 0x1000>;
- interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 81 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 80 IRQ_TYPE_LEVEL_HIGH>;
clock-names = "sysmmu", "master";
clocks = <&cmu CLK_SMMUFIMD0>, <&cmu CLK_FIMD0>;
power-domains = <&pd_lcd0>;
@@ -507,8 +505,7 @@
sysmmu_mfc: sysmmu@13620000 {
compatible = "samsung,exynos-sysmmu";
reg = <0x13620000 0x1000>;
- interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
clock-names = "sysmmu", "master";
clocks = <&cmu CLK_SMMUMFC_L>, <&cmu CLK_MFC>;
power-domains = <&pd_mfc>;
--
2.17.1

2019-09-18 00:28:57

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v3] dt-bindings: arm: samsung: Convert Samsung Exynos IOMMU H/W, System MMU to dt-schema

On Wed, Sep 11, 2019 at 01:04:46PM +0200, Maciej Falkowski wrote:
> Convert Samsung Exynos IOMMU H/W, System Memory Management Unit
> to newer dt-schema format.
>
> Update clock description.
>
> Signed-off-by: Maciej Falkowski <[email protected]>
> Signed-off-by: Andrzej Hajda <[email protected]>
> ---
> Hi Krzysztof,
>
> Thank you for feedback.
>
> v3:
>
> - remove obsolete interrupts description and
> set its maxItems to one. There are some incompatible
> files which will be fixed with another patch.
>
> - clock-names pattern is changed to your more precise
> version. I also added option "pclk" + "aclk" as some
> bindings are also using it.
>
> Best regards,
> Maciej Falkowski
> ---
> .../bindings/iommu/samsung,sysmmu.txt | 67 -----------
> .../bindings/iommu/samsung,sysmmu.yaml | 112 ++++++++++++++++++
> 2 files changed, 112 insertions(+), 67 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
> create mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml


> diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
> new file mode 100644
> index 000000000000..a8141d6c326a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
> @@ -0,0 +1,112 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iommu/samsung,sysmmu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
> +
> +maintainers:
> + - Marek Szyprowski <[email protected]>
> +
> +description: |+
> + Samsung's Exynos architecture contains System MMUs that enables scattered
> + physical memory chunks visible as a contiguous region to DMA-capable peripheral
> + devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
> +
> + System MMU is an IOMMU and supports identical translation table format to
> + ARMv7 translation tables with minimum set of page properties including access
> + permissions, shareability and security protection. In addition, System MMU has
> + another capabilities like L2 TLB or block-fetch buffers to minimize translation
> + latency.
> +
> + System MMUs are in many to one relation with peripheral devices, i.e. single
> + peripheral device might have multiple System MMUs (usually one for each bus
> + master), but one System MMU can handle transactions from only one peripheral
> + device. The relation between a System MMU and the peripheral device needs to be
> + defined in device node of the peripheral device.
> +
> + MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
> + MMUs.
> + * MFC has one System MMU on its left and right bus.
> + * FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
> + for window 1, 2 and 3.
> + * M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
> + the other System MMU on the write channel.
> +
> + For information on assigning System MMU controller to its peripheral devices,
> + see generic IOMMU bindings.
> +
> +properties:
> + compatible:
> + const: samsung,exynos-sysmmu
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + minItems: 1
> + maxItems: 2
> +
> + clock-names:
> + oneOf:
> + - items:
> + - const: sysmmu
> + - items:
> + - const: sysmmu
> + - const: master
> + - items:
> + - const: aclk
> + - const: pclk
> + - items:
> + - const: pclk
> + - const: aclk

Sigh. I'd prefer you fix the order in whichever case is less common.

> + description: |
> + Should be "sysmmu" with optional "master"
> + or a pair "aclk" with "pclk".

No need to describe what the schema already says.

> +
> + "#iommu-cells":
> + const: 0
> +
> + power-domains:
> + $ref: /schemas/types.yaml#/definitions/phandle

No need to define common property types.

Just 'maxItems: 1' is enough.

> + description: |
> + Required if the System MMU is needed to gate its power.
> + Please refer to the following document:
> + Documentation/devicetree/bindings/power/pd-samsung.txt
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - "#iommu-cells"
> +
> +examples:
> + - |
> + gsc_0: gsc@13e00000 {
> + compatible = "samsung,exynos5-gsc";
> + reg = <0x13e00000 0x1000>;
> + interrupts = <0 85 0>;
> + power-domains = <&pd_gsc>;
> + clocks = <&clock 0>; // CLK_GSCL0
> + clock-names = "gscl";
> + iommus = <&sysmmu_gsc0>;
> + };
> +
> + sysmmu_gsc0: sysmmu@13e80000 {

This should be:

iommu@...

> + compatible = "samsung,exynos-sysmmu";
> + reg = <0x13E80000 0x1000>;
> + interrupt-parent = <&combiner>;
> + interrupts = <2 0>;
> + clock-names = "sysmmu", "master";
> + clocks = <&clock 0>, // CLK_SMMU_GSCL0
> + <&clock 0>; // CLK_GSCL0
> + power-domains = <&pd_gsc>;
> + #iommu-cells = <0>;
> + };
> +
> --
> 2.17.1
>

2019-09-19 13:56:48

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH v4] dt-bindings: iommu: Convert Samsung Exynos IOMMU H/W, System MMU to dt-schema

From: Maciej Falkowski <[email protected]>

Convert Samsung Exynos IOMMU H/W, System Memory Management Unit
to newer dt-schema format.

Signed-off-by: Maciej Falkowski <[email protected]>
Signed-off-by: Andrzej Hajda <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
---
v4:
- Rename commit message to match common style
- Remove '"pclk", "aclk"' option from clock-names property.
Some of bindings are incompatible with that and they will be fixed with
another patch.
- Remove unneeded description of clock-names property.
- Remove type description from power-domains property as it is
already a common property.
- Rename node names to match generic names,
specifically: sysmmu -> iommu, gsc -> scaler

- Add include directive in examples to include
clock macros. This increases readability of the example
as clock macros do not have to be substituted with numerical values
which makes examples more self-explanatory and natural.

Best regards,
Maciej Falkowski
---
.../bindings/iommu/samsung,sysmmu.txt | 67 -----------
.../bindings/iommu/samsung,sysmmu.yaml | 108 ++++++++++++++++++
2 files changed, 108 insertions(+), 67 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
create mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml

diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
deleted file mode 100644
index 525ec82615a6..000000000000
--- a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
+++ /dev/null
@@ -1,67 +0,0 @@
-Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
-
-Samsung's Exynos architecture contains System MMUs that enables scattered
-physical memory chunks visible as a contiguous region to DMA-capable peripheral
-devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
-
-System MMU is an IOMMU and supports identical translation table format to
-ARMv7 translation tables with minimum set of page properties including access
-permissions, shareability and security protection. In addition, System MMU has
-another capabilities like L2 TLB or block-fetch buffers to minimize translation
-latency.
-
-System MMUs are in many to one relation with peripheral devices, i.e. single
-peripheral device might have multiple System MMUs (usually one for each bus
-master), but one System MMU can handle transactions from only one peripheral
-device. The relation between a System MMU and the peripheral device needs to be
-defined in device node of the peripheral device.
-
-MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
-MMUs.
-* MFC has one System MMU on its left and right bus.
-* FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
- for window 1, 2 and 3.
-* M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
- the other System MMU on the write channel.
-
-For information on assigning System MMU controller to its peripheral devices,
-see generic IOMMU bindings.
-
-Required properties:
-- compatible: Should be "samsung,exynos-sysmmu"
-- reg: A tuple of base address and size of System MMU registers.
-- #iommu-cells: Should be <0>.
-- interrupts: An interrupt specifier for interrupt signal of System MMU,
- according to the format defined by a particular interrupt
- controller.
-- clock-names: Should be "sysmmu" or a pair of "aclk" and "pclk" to gate
- SYSMMU core clocks.
- Optional "master" if the clock to the System MMU is gated by
- another gate clock other core (usually main gate clock
- of peripheral device this SYSMMU belongs to).
-- clocks: Phandles for respective clocks described by clock-names.
-- power-domains: Required if the System MMU is needed to gate its power.
- Please refer to the following document:
- Documentation/devicetree/bindings/power/pd-samsung.txt
-
-Examples:
- gsc_0: gsc@13e00000 {
- compatible = "samsung,exynos5-gsc";
- reg = <0x13e00000 0x1000>;
- interrupts = <0 85 0>;
- power-domains = <&pd_gsc>;
- clocks = <&clock CLK_GSCL0>;
- clock-names = "gscl";
- iommus = <&sysmmu_gsc0>;
- };
-
- sysmmu_gsc0: sysmmu@13e80000 {
- compatible = "samsung,exynos-sysmmu";
- reg = <0x13E80000 0x1000>;
- interrupt-parent = <&combiner>;
- interrupts = <2 0>;
- clock-names = "sysmmu", "master";
- clocks = <&clock CLK_SMMU_GSCL0>, <&clock CLK_GSCL0>;
- power-domains = <&pd_gsc>;
- #iommu-cells = <0>;
- };
diff --git a/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
new file mode 100644
index 000000000000..ecde98da5b72
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iommu/samsung,sysmmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung Exynos IOMMU H/W, System MMU (System Memory Management Unit)
+
+maintainers:
+ - Marek Szyprowski <[email protected]>
+
+description: |+
+ Samsung's Exynos architecture contains System MMUs that enables scattered
+ physical memory chunks visible as a contiguous region to DMA-capable peripheral
+ devices like MFC, FIMC, FIMD, GScaler, FIMC-IS and so forth.
+
+ System MMU is an IOMMU and supports identical translation table format to
+ ARMv7 translation tables with minimum set of page properties including access
+ permissions, shareability and security protection. In addition, System MMU has
+ another capabilities like L2 TLB or block-fetch buffers to minimize translation
+ latency.
+
+ System MMUs are in many to one relation with peripheral devices, i.e. single
+ peripheral device might have multiple System MMUs (usually one for each bus
+ master), but one System MMU can handle transactions from only one peripheral
+ device. The relation between a System MMU and the peripheral device needs to be
+ defined in device node of the peripheral device.
+
+ MFC in all Exynos SoCs and FIMD, M2M Scalers and G2D in Exynos5420 has 2 System
+ MMUs.
+ * MFC has one System MMU on its left and right bus.
+ * FIMD in Exynos5420 has one System MMU for window 0 and 4, the other system MMU
+ for window 1, 2 and 3.
+ * M2M Scalers and G2D in Exynos5420 has one System MMU on the read channel and
+ the other System MMU on the write channel.
+
+ For information on assigning System MMU controller to its peripheral devices,
+ see generic IOMMU bindings.
+
+properties:
+ compatible:
+ const: samsung,exynos-sysmmu
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ minItems: 1
+ maxItems: 2
+
+ clock-names:
+ oneOf:
+ - items:
+ - const: sysmmu
+ - items:
+ - const: sysmmu
+ - const: master
+ - items:
+ - const: aclk
+ - const: pclk
+
+ "#iommu-cells":
+ const: 0
+
+ power-domains:
+ description: |
+ Required if the System MMU is needed to gate its power.
+ Please refer to the following document:
+ Documentation/devicetree/bindings/power/pd-samsung.txt
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+ - "#iommu-cells"
+
+examples:
+ - |
+ #include <dt-bindings/clock/exynos5250.h>
+
+ gsc_0: scaler@13e00000 {
+ compatible = "samsung,exynos5-gsc";
+ reg = <0x13e00000 0x1000>;
+ interrupts = <0 85 0>;
+ power-domains = <&pd_gsc>;
+ clocks = <&clock CLK_GSCL0>;
+ clock-names = "gscl";
+ iommus = <&sysmmu_gsc0>;
+ };
+
+ sysmmu_gsc0: iommu@13e80000 {
+ compatible = "samsung,exynos-sysmmu";
+ reg = <0x13E80000 0x1000>;
+ interrupt-parent = <&combiner>;
+ interrupts = <2 0>;
+ clock-names = "sysmmu", "master";
+ clocks = <&clock CLK_SMMU_GSCL0>,
+ <&clock CLK_GSCL0>;
+ power-domains = <&pd_gsc>;
+ #iommu-cells = <0>;
+ };
+
--
2.17.1



2019-09-19 19:54:40

by Marek Szyprowski

[permalink] [raw]
Subject: [PATCH] arm64: dts: exynos: Exynos5433: swap clock order of sysmmu

From: Maciej Falkowski <[email protected]>

dt-schema supports only order of names "aclk", "pclk".
Swap some sysmmu definitions to make them compatible with schema.

Signed-off-by: Maciej Falkowski <[email protected]>
Signed-off-by: Marek Szyprowski <[email protected]>
---
arch/arm64/boot/dts/exynos/exynos5433.dtsi | 54 +++++++++++-----------
1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
index a76f620f7f35..ba66ea906f60 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi
+++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi
@@ -1179,9 +1179,9 @@
compatible = "samsung,exynos-sysmmu";
reg = <0x13a00000 0x1000>;
interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "pclk", "aclk";
- clocks = <&cmu_disp CLK_PCLK_SMMU_DECON0X>,
- <&cmu_disp CLK_ACLK_SMMU_DECON0X>;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_disp CLK_ACLK_SMMU_DECON0X>,
+ <&cmu_disp CLK_PCLK_SMMU_DECON0X>;
power-domains = <&pd_disp>;
#iommu-cells = <0>;
};
@@ -1190,9 +1190,9 @@
compatible = "samsung,exynos-sysmmu";
reg = <0x13a10000 0x1000>;
interrupts = <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "pclk", "aclk";
- clocks = <&cmu_disp CLK_PCLK_SMMU_DECON1X>,
- <&cmu_disp CLK_ACLK_SMMU_DECON1X>;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_disp CLK_ACLK_SMMU_DECON1X>,
+ <&cmu_disp CLK_PCLK_SMMU_DECON1X>;
#iommu-cells = <0>;
power-domains = <&pd_disp>;
};
@@ -1201,9 +1201,9 @@
compatible = "samsung,exynos-sysmmu";
reg = <0x13a20000 0x1000>;
interrupts = <GIC_SPI 214 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "pclk", "aclk";
- clocks = <&cmu_disp CLK_PCLK_SMMU_TV0X>,
- <&cmu_disp CLK_ACLK_SMMU_TV0X>;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_disp CLK_ACLK_SMMU_TV0X>,
+ <&cmu_disp CLK_PCLK_SMMU_TV0X>;
#iommu-cells = <0>;
power-domains = <&pd_disp>;
};
@@ -1212,9 +1212,9 @@
compatible = "samsung,exynos-sysmmu";
reg = <0x13a30000 0x1000>;
interrupts = <GIC_SPI 216 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "pclk", "aclk";
- clocks = <&cmu_disp CLK_PCLK_SMMU_TV1X>,
- <&cmu_disp CLK_ACLK_SMMU_TV1X>;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_disp CLK_ACLK_SMMU_TV1X>,
+ <&cmu_disp CLK_PCLK_SMMU_TV1X>;
#iommu-cells = <0>;
power-domains = <&pd_disp>;
};
@@ -1256,9 +1256,9 @@
compatible = "samsung,exynos-sysmmu";
reg = <0x15040000 0x1000>;
interrupts = <GIC_SPI 404 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "pclk", "aclk";
- clocks = <&cmu_mscl CLK_PCLK_SMMU_M2MSCALER0>,
- <&cmu_mscl CLK_ACLK_SMMU_M2MSCALER0>;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_mscl CLK_ACLK_SMMU_M2MSCALER0>,
+ <&cmu_mscl CLK_PCLK_SMMU_M2MSCALER0>;
#iommu-cells = <0>;
power-domains = <&pd_mscl>;
};
@@ -1267,9 +1267,9 @@
compatible = "samsung,exynos-sysmmu";
reg = <0x15050000 0x1000>;
interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "pclk", "aclk";
- clocks = <&cmu_mscl CLK_PCLK_SMMU_M2MSCALER1>,
- <&cmu_mscl CLK_ACLK_SMMU_M2MSCALER1>;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_mscl CLK_ACLK_SMMU_M2MSCALER1>,
+ <&cmu_mscl CLK_PCLK_SMMU_M2MSCALER1>;
#iommu-cells = <0>;
power-domains = <&pd_mscl>;
};
@@ -1278,9 +1278,9 @@
compatible = "samsung,exynos-sysmmu";
reg = <0x15060000 0x1000>;
interrupts = <GIC_SPI 408 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "pclk", "aclk";
- clocks = <&cmu_mscl CLK_PCLK_SMMU_JPEG>,
- <&cmu_mscl CLK_ACLK_SMMU_JPEG>;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_mscl CLK_ACLK_SMMU_JPEG>,
+ <&cmu_mscl CLK_PCLK_SMMU_JPEG>;
#iommu-cells = <0>;
power-domains = <&pd_mscl>;
};
@@ -1289,9 +1289,9 @@
compatible = "samsung,exynos-sysmmu";
reg = <0x15200000 0x1000>;
interrupts = <GIC_SPI 352 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "pclk", "aclk";
- clocks = <&cmu_mfc CLK_PCLK_SMMU_MFC_0>,
- <&cmu_mfc CLK_ACLK_SMMU_MFC_0>;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_mfc CLK_ACLK_SMMU_MFC_0>,
+ <&cmu_mfc CLK_PCLK_SMMU_MFC_0>;
#iommu-cells = <0>;
power-domains = <&pd_mfc>;
};
@@ -1300,9 +1300,9 @@
compatible = "samsung,exynos-sysmmu";
reg = <0x15210000 0x1000>;
interrupts = <GIC_SPI 354 IRQ_TYPE_LEVEL_HIGH>;
- clock-names = "pclk", "aclk";
- clocks = <&cmu_mfc CLK_PCLK_SMMU_MFC_1>,
- <&cmu_mfc CLK_ACLK_SMMU_MFC_1>;
+ clock-names = "aclk", "pclk";
+ clocks = <&cmu_mfc CLK_ACLK_SMMU_MFC_1>,
+ <&cmu_mfc CLK_PCLK_SMMU_MFC_1>;
#iommu-cells = <0>;
power-domains = <&pd_mfc>;
};
--
2.17.1



2019-09-27 16:22:43

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4] dt-bindings: iommu: Convert Samsung Exynos IOMMU H/W, System MMU to dt-schema

On Thu, 19 Sep 2019 15:19:44 +0200, Marek Szyprowski wrote:
> From: Maciej Falkowski <[email protected]>
>
> Convert Samsung Exynos IOMMU H/W, System Memory Management Unit
> to newer dt-schema format.
>
> Signed-off-by: Maciej Falkowski <[email protected]>
> Signed-off-by: Andrzej Hajda <[email protected]>
> Signed-off-by: Marek Szyprowski <[email protected]>
> ---
> v4:
> - Rename commit message to match common style
> - Remove '"pclk", "aclk"' option from clock-names property.
> Some of bindings are incompatible with that and they will be fixed with
> another patch.
> - Remove unneeded description of clock-names property.
> - Remove type description from power-domains property as it is
> already a common property.
> - Rename node names to match generic names,
> specifically: sysmmu -> iommu, gsc -> scaler
>
> - Add include directive in examples to include
> clock macros. This increases readability of the example
> as clock macros do not have to be substituted with numerical values
> which makes examples more self-explanatory and natural.
>
> Best regards,
> Maciej Falkowski
> ---
> .../bindings/iommu/samsung,sysmmu.txt | 67 -----------
> .../bindings/iommu/samsung,sysmmu.yaml | 108 ++++++++++++++++++
> 2 files changed, 108 insertions(+), 67 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.txt
> create mode 100644 Documentation/devicetree/bindings/iommu/samsung,sysmmu.yaml
>

Applied, thanks.

Rob

2019-10-01 19:15:48

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: exynos: Exynos5433: swap clock order of sysmmu

On Thu, Sep 19, 2019 at 03:50:53PM +0200, Marek Szyprowski wrote:
> From: Maciej Falkowski <[email protected]>
>
> dt-schema supports only order of names "aclk", "pclk".
> Swap some sysmmu definitions to make them compatible with schema.
>
> Signed-off-by: Maciej Falkowski <[email protected]>
> Signed-off-by: Marek Szyprowski <[email protected]>
> ---
> arch/arm64/boot/dts/exynos/exynos5433.dtsi | 54 +++++++++++-----------

Thanks, applied.

Best regards,
Krzysztof