On Fri, Mar 3, 2017 at 1:21 AM, Rob Herring <robh@xxxxxxxxxx> 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. > >> Cc: devicetree@xxxxxxxxxxxxxxx >> Signed-off-by: Rob Clark <robdclark@xxxxxxxxx> >> --- >> .../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"; >> +- 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 >> +- #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. > >> + - 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. >> + #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 >> + 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. > >> + #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@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html