Re: [PATCH 1/2] arm64: dts: stratix10: Add Stratix10 SMMU support
From: Thor Thayer
Date: Thu Aug 16 2018 - 23:31:08 EST
Hi Robin,
On 08/08/2018 12:38 PM, Robin Murphy wrote:
On 25/07/18 19:24, thor.thayer@xxxxxxxxxxxxxxx wrote:
From: Thor Thayer <thor.thayer@xxxxxxxxxxxxxxx>
Add SMMU support to the Stratix10 Device Tree which
includes adding the SMMU node and adding IOMMU stream
ids to the SMMU peripherals. Update bindings.
Signed-off-by: Thor Thayer <thor.thayer@xxxxxxxxxxxxxxx>
---
This patch is dependent on the patch series
"iommu/arm-smmu: Add runtime pm/sleep support"
(https://patchwork.ozlabs.org/cover/946160/)
---
 .../devicetree/bindings/iommu/arm,smmu.txt | 25
++++++++++++++++++
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 30
++++++++++++++++++++++
 2 files changed, 55 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 7c71a6ed465a..8e3fe0594e3e 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -18,6 +18,7 @@ conditions.
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "arm,mmu-500"
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "cavium,smmu-v2"
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "qcom,<soc>-smmu-v2", "qcom,smmu-v2"
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "altr,smmu-v2"
Can we guarantee that no Altera SoC will ever exist with a different
SMMU implementation, configuration, or clock tree? If we must have
compatibles for SoC-specific integrations, I'd be a lot happier if they
were actually SoC-specific, i.e. at least "altr,stratix10-smmu", or even
something like "altr,gx5500-smmu" if there's a chance of new
incompatible designs being added to the Stratix 10 family in future.
Good point. I'll get a better compatible string if I pursue this.
I'm still dubious that we actually need this for MMU-500, though, since
we will always need the TCU clock enabled to do anything, and given the
difficulty in associating particular TBU clocks with whichever domains
might cause allocations into which TBU's TLBs, it seems highly unlikely
that it'll ever be feasible to work at a granularity finer than "all of
the clocks". And at that point the names don't really matter, and we
merely need something like the proposed of_clk_bulk_get()[1], which
works fine regardless of how many TBUs and distinct clocks exist for a
particular MMU-500 configuration and integration.
Yes, I would prefer to use the standard arm,mmu-500 but with the changes
proposed by [2] that this patch was dependent on, it seemed I would need
to make a new structure and type.
I like the patch series for devm_clk_bulk_get_all() that includes
of_clk_bulk_get(). That would enable my patch to work with minor changes
to add bulk_clk to arm-smmu.c. However, the patchset doesn't seem to
have been accepted yet.
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ depending on the particular implementation and/or the
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ version of the architecture implemented.
@@ -179,3 +180,27 @@ conditions.
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <&mmcc SMMU_MDP_AHB_CLK>;
ÂÂÂÂÂÂÂÂÂ clock-names = "bus", "iface";
ÂÂÂÂÂ };
+
+ÂÂÂ /* Stratix10 arm,smmu-v2 implementation */
+ÂÂÂ smmu5: iommu@fa000000 {
+ÂÂÂÂÂÂÂ compatible = "altr,smmu-v2", "arm,mmu-500",
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "arm,smmu-v2";
+ÂÂÂÂÂÂÂ reg = <0xfa000000 0x40000>;
+ÂÂÂÂÂÂÂ #global-interrupts = <2>;
+ÂÂÂÂÂÂÂ #iommu-cells = <1>;
+ÂÂÂÂÂÂÂ clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
+ÂÂÂÂÂÂÂ clock-names = "masters";
This isn't documented as an actual property, and if it also clocks the
TCU then I'm not sure it's really the most accurate name.
Robin.
[1] https://patchwork.kernel.org/patch/10427095/
In the patch I'll remove the clock-names.
I'll keep track of the status of this patch (and [3] from the same
patchset). I tested a simple patch that uses devm_clk_bulk_get_all()
from these bulk_clk patches and it works well with the standard
"arm,mmu-500" compatible.
This patch has dependencies on [2]. It seems like [2] could use the
bulk_clk patches in [1] above (in particular [2]'s patch 1/4). The
function arm_smmu_fill_clk_data() wouldn't be needed because everything
happens in devm_clk_bulk_get_all(). However, those bulk_clk patches have
been hanging out there since May.
I'm unclear on how to proceed. Do I continue with dependency on [2] or
create a new patch adding the bulk clocks changes in [1] (and [3])?
Thanks for reviewing!
Thor
[2] https://patchwork.kernel.org/patch/10546677/
[3] https://patchwork.kernel.org/patch/10427079/
+ÂÂÂÂÂÂÂ interrupt-parent = <&intc>;
+ÂÂÂÂÂÂÂ interrupts = <0 128 4>,ÂÂÂ /* Global Secure Fault */
+ÂÂÂÂÂÂÂÂÂÂÂ <0 129 4>, /* Global Non-secure Fault */
+ÂÂÂÂÂÂÂÂÂÂÂ /* Non-secure Context Interrupts (32) */
+ÂÂÂÂÂÂÂÂÂÂÂ <0 138 4>, <0 139 4>, <0 140 4>, <0 141 4>,
+ÂÂÂÂÂÂÂÂÂÂÂ <0 142 4>, <0 143 4>, <0 144 4>, <0 145 4>,
+ÂÂÂÂÂÂÂÂÂÂÂ <0 146 4>, <0 147 4>, <0 148 4>, <0 149 4>,
+ÂÂÂÂÂÂÂÂÂÂÂ <0 150 4>, <0 151 4>, <0 152 4>, <0 153 4>,
+ÂÂÂÂÂÂÂÂÂÂÂ <0 154 4>, <0 155 4>, <0 156 4>, <0 157 4>,
+ÂÂÂÂÂÂÂÂÂÂÂ <0 158 4>, <0 159 4>, <0 160 4>, <0 161 4>,
+ÂÂÂÂÂÂÂÂÂÂÂ <0 162 4>, <0 163 4>, <0 164 4>, <0 165 4>,
+ÂÂÂÂÂÂÂÂÂÂÂ <0 166 4>, <0 167 4>, <0 168 4>, <0 169 4>;
+ÂÂÂÂÂÂÂ stream-match-mask = <0x7ff0>;
+ÂÂÂ };
diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index d033da401c26..e38ca86d48f6 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -137,6 +137,7 @@
ÂÂÂÂÂÂÂÂÂÂÂÂÂ reset-names = "stmmaceth", "stmmaceth-ocp";
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clocks = <&clkmgr STRATIX10_EMAC0_CLK>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clock-names = "stmmaceth";
+ÂÂÂÂÂÂÂÂÂÂÂ iommus = <&smmu 1>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ status = "disabled";
ÂÂÂÂÂÂÂÂÂ };
@@ -150,6 +151,7 @@
ÂÂÂÂÂÂÂÂÂÂÂÂÂ reset-names = "stmmaceth", "stmmaceth-ocp";
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clocks = <&clkmgr STRATIX10_EMAC1_CLK>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clock-names = "stmmaceth";
+ÂÂÂÂÂÂÂÂÂÂÂ iommus = <&smmu 2>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ status = "disabled";
ÂÂÂÂÂÂÂÂÂ };
@@ -163,6 +165,7 @@
ÂÂÂÂÂÂÂÂÂÂÂÂÂ reset-names = "stmmaceth", "stmmaceth-ocp";
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clocks = <&clkmgr STRATIX10_EMAC2_CLK>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clock-names = "stmmaceth";
+ÂÂÂÂÂÂÂÂÂÂÂ iommus = <&smmu 3>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ status = "disabled";
ÂÂÂÂÂÂÂÂÂ };
@@ -273,6 +276,7 @@
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clocks = <&clkmgr STRATIX10_L4_MP_CLK>,
ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <&clkmgr STRATIX10_SDMMC_CLK>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clock-names = "biu", "ciu";
+ÂÂÂÂÂÂÂÂÂÂÂ iommus = <&smmu 5>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ status = "disabled";
ÂÂÂÂÂÂÂÂÂ };
@@ -307,6 +311,30 @@
ÂÂÂÂÂÂÂÂÂÂÂÂÂ altr,modrst-offset = <0x20>;
ÂÂÂÂÂÂÂÂÂ };
+ÂÂÂÂÂÂÂ smmu: iommu@fa000000 {
+ÂÂÂÂÂÂÂÂÂÂÂ compatible = "altr,smmu-v2", "arm,mmu-500",
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ "arm,smmu-v2";
+ÂÂÂÂÂÂÂÂÂÂÂ reg = <0xfa000000 0x40000>;
+ÂÂÂÂÂÂÂÂÂÂÂ #global-interrupts = <2>;
+ÂÂÂÂÂÂÂÂÂÂÂ #iommu-cells = <1>;
+ÂÂÂÂÂÂÂÂÂÂÂ clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
+ÂÂÂÂÂÂÂÂÂÂÂ clock-names = "masters";
+ÂÂÂÂÂÂÂÂÂÂÂ interrupt-parent = <&intc>;
+ÂÂÂÂÂÂÂÂÂÂÂ interrupts = <0 128 4>,ÂÂÂ /* Global Secure Fault */
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <0 129 4>, /* Global Non-secure Fault */
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ /* Non-secure Context Interrupts (32) */
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <0 138 4>, <0 139 4>, <0 140 4>, <0 141 4>,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <0 142 4>, <0 143 4>, <0 144 4>, <0 145 4>,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <0 146 4>, <0 147 4>, <0 148 4>, <0 149 4>,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <0 150 4>, <0 151 4>, <0 152 4>, <0 153 4>,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <0 154 4>, <0 155 4>, <0 156 4>, <0 157 4>,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <0 158 4>, <0 159 4>, <0 160 4>, <0 161 4>,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <0 162 4>, <0 163 4>, <0 164 4>, <0 165 4>,
+ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ <0 166 4>, <0 167 4>, <0 168 4>, <0 169 4>;
+ÂÂÂÂÂÂÂÂÂÂÂ stream-match-mask = <0x7ff0>;
+ÂÂÂÂÂÂÂÂÂÂÂ status = "disabled";
+ÂÂÂÂÂÂÂ };
+
ÂÂÂÂÂÂÂÂÂ spi0: spi@ffda4000 {
ÂÂÂÂÂÂÂÂÂÂÂÂÂ compatible = "snps,dw-apb-ssi";
ÂÂÂÂÂÂÂÂÂÂÂÂÂ #address-cells = <1>;
@@ -416,6 +444,7 @@
ÂÂÂÂÂÂÂÂÂÂÂÂÂ resets = <&rst USB0_RESET>, <&rst USB0_OCP_RESET>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ reset-names = "dwc2", "dwc2-ecc";
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clocks = <&clkmgr STRATIX10_USB_CLK>;
+ÂÂÂÂÂÂÂÂÂÂÂ iommus = <&smmu 6>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ status = "disabled";
ÂÂÂÂÂÂÂÂÂ };
@@ -428,6 +457,7 @@
ÂÂÂÂÂÂÂÂÂÂÂÂÂ resets = <&rst USB1_RESET>, <&rst USB1_OCP_RESET>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ reset-names = "dwc2", "dwc2-ecc";
ÂÂÂÂÂÂÂÂÂÂÂÂÂ clocks = <&clkmgr STRATIX10_USB_CLK>;
+ÂÂÂÂÂÂÂÂÂÂÂ iommus = <&smmu 7>;
ÂÂÂÂÂÂÂÂÂÂÂÂÂ status = "disabled";
ÂÂÂÂÂÂÂÂÂ };