Re: [PATCH 3/9] Docs: dt: document qcom iommu bindings

On Fri, Mar 3, 2017 at 1:21 AM, Rob Herring [off-list ref] wrote:

On Wed, Mar 01, 2017 at 12:42:52PM -0500, Rob Clark wrote:

Nit: use "dt-bindings: iommu: ..." for subject. And a commit message
would be nice.

quoted

Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Rob Clark <redacted>
---
 .../devicetree/bindings/iommu/qcom,iommu.txt       | 106 +++++++++++++++++++++
 1 file changed, 106 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iommu/qcom,iommu.txt

diff --git a/Documentation/devicetree/bindings/iommu/qcom,iommu.txt b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt
new file mode 100644
index 0000000..2e69b78
--- /dev/null
+++ b/Documentation/devicetree/bindings/iommu/qcom,iommu.txt

@@ -0,0 +1,106 @@
+* QCOM IOMMU v1 Implementation
+
+Qualcomm "B" family devices which are not compatible with arm-smmu have
+a similar looking IOMMU but without access to the global register space,
+and optionally requiring additional configuration to route context irqs
+to non-secure vs secure interrupt line.
+
+** Required properties:
+
+- compatible       : Should be "qcom,msm-iommu-v1".

Fine as a fallback, but this needs chip specific compatibles.

ok, so maybe:

   compatible = "qcom,msm8916-iommu", "qcom,msm-iommu-v1";

quoted

+- clocks           : The interface clock (iface_clk) and bus clock (bus_clk).

The names need to be documented under clock-names prop.

'_clk' is redundant.

ok

quoted

+- #address-cells   : must be 1.
+- #size-cells      : must be 1.
+- #iommu-cells     : Must be 1.
+- ranges           : Base address and size of the iommu context banks.
+- qcom,iommu-secure-id  : secure-id.
+
+- List of sub-nodes, one per translation context bank.  Each sub-node
+  has the following required properties:
+
+  - compatible     : Should be one of:
+        - "qcom,msm-iommu-v1-ns"  : non-secure context bank
+        - "qcom,msm-iommu-v1-sec" : secure context bank

These are okay without chip specific strings.

quoted

+  - reg            : Base address and size of context bank within the iommu
+  - interrupts     : The context fault irq.
+
+** Optional properties:
+
+- reg              : Base address and size of the SMMU local base, should
+                     be only specified if the iommu requires configuration
+                     for routing of context bank irq's to secure vs non-
+                     secure lines.  (Ie. if the iommu contains secure
+                     context banks)
+
+
+** Examples:
+
+     apps_iommu: msm-iommu-v1@1e20000 {

iommu@...

And this should be the reg address, not the ranges address.

ok.. but I'm not entirely sure what to do w/ gpu_iommu, which doesn't
have a reg address.

I guess I could have a required reg address (which is the unaccessible
global register space), and make the "SMMU local base" thing a 2nd
optional address.  Not sure if that is weird, since we can't actually
do anything with the global register space.

quoted

+             #address-cells = <1>;
+             #size-cells = <1>;
+             #iommu-cells = <1>;
+             compatible = "qcom,msm-iommu-v1";
+             ranges = <0 0x1e20000 0x40000>;
+             reg = <0x1ef0000 0x3000>;
+             clocks = <&gcc GCC_SMMU_CFG_CLK>,
+                      <&gcc GCC_APSS_TCU_CLK>;
+             clock-names = "iface_clk", "bus_clk";
+             qcom,iommu-secure-id = <17>;
+
+             // mdp_0:
+             msm-iommu-v1-ctx@4000 {

iommu@...

it's not weird to have:

  iommu@1e20000 {
    ...
    iommu@4000 {
       ...
    };
  };

??

BR,
-R

quoted

+                     compatible = "qcom,msm-iommu-v1-ns";
+                     reg = <0x4000 0x1000>;
+                     interrupts = <GIC_SPI 70 0>;
+             };
+
+             // venus_ns:
+             msm-iommu-v1-ctx@5000 {
+                     compatible = "qcom,msm-iommu-v1-sec";
+                     reg = <0x5000 0x1000>;
+                     interrupts = <GIC_SPI 70 0>;
+             };
+     };
+
+     gpu_iommu: msm-iommu-v1@1f08000 {

ditto.

quoted

+             #address-cells = <1>;
+             #size-cells = <1>;
+             #iommu-cells = <1>;
+             compatible = "qcom,msm-iommu-v1";
+             ranges = <0 0x1f08000 0x10000>;
+             clocks = <&gcc GCC_SMMU_CFG_CLK>,
+                      <&gcc GCC_GFX_TCU_CLK>;
+             clock-names = "iface_clk", "bus_clk";
+             qcom,iommu-secure-id = <18>;
+
+             // gfx3d_user:
+             msm-iommu-v1-ctx@1f09000 {
+                     compatible = "qcom,msm-iommu-v1-ns";
+                     reg = <0x1000 0x1000>;
+                     interrupts = <GIC_SPI 241 0>;
+             };
+
+             // gfx3d_priv:
+             msm-iommu-v1-ctx@1f0a000 {
+                     compatible = "qcom,msm-iommu-v1-ns";
+                     reg = <0x2000 0x1000>;
+                     interrupts = <GIC_SPI 242 0>;
+             };
+     };
+
+     ...
+
+     venus: video-codec@1d00000 {
+             ...
+             iommus = <&apps_iommu 5>;
+     };
+
+     mdp: mdp@1a01000 {
+             ...
+             iommus = <&apps_iommu 4>;
+     };
+
+     gpu@01c00000 {
+             ...
+             iommus = <&gpu_iommu 1>, <&gpu_iommu 2>;
+     };
--
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html