Re: [PATCH 1/2] arm64: dts: stratix10: Add Stratix10 SMMU support

2018-08-16 Thread Thor Thayer

Hi Robin,

On 08/08/2018 12:38 PM, Robin Murphy wrote:

On 25/07/18 19:24, thor.tha...@linux.intel.com wrote:

From: Thor Thayer 

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 
---
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,-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.
   < SMMU_MDP_AHB_CLK>;
  clock-names = "bus", "iface";
  };
+
+    /* Stratix10 arm,smmu-v2 implementation */
+    smmu5: iommu@fa00 {
+    compatible = "altr,smmu-v2", "arm,mmu-500",
+ "arm,smmu-v2";
+    reg = <0xfa00 0x4>;
+    #global-interrupts = <2>;
+    #iommu-cells = <1>;
+    clocks = < 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 = <>;
+    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 

Re: [PATCH 1/2] arm64: dts: stratix10: Add Stratix10 SMMU support

2018-08-08 Thread Robin Murphy

On 25/07/18 19:24, thor.tha...@linux.intel.com wrote:

From: Thor Thayer 

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 
---
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,-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.


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.


  
depending on the particular implementation and/or the

version of the architecture implemented.
@@ -179,3 +180,27 @@ conditions.
 < SMMU_MDP_AHB_CLK>;
clock-names = "bus", "iface";
};
+
+   /* Stratix10 arm,smmu-v2 implementation */
+   smmu5: iommu@fa00 {
+   compatible = "altr,smmu-v2", "arm,mmu-500",
+"arm,smmu-v2";
+   reg = <0xfa00 0x4>;
+   #global-interrupts = <2>;
+   #iommu-cells = <1>;
+   clocks = < 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/


+   interrupt-parent = <>;
+   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 = < STRATIX10_EMAC0_CLK>;
clock-names = "stmmaceth";
+   iommus = < 1>;
status = "disabled";
};
  
@@ -150,6 +151,7 @@

reset-names = "stmmaceth", "stmmaceth-ocp";
clocks = < STRATIX10_EMAC1_CLK>;
clock-names = "stmmaceth";
+   iommus = < 2>;
status = "disabled";
};
  
@@ -163,6 +165,7 @@

reset-names = "stmmaceth", "stmmaceth-ocp";
clocks = < STRATIX10_EMAC2_CLK>;
clock-names = "stmmaceth";
+   iommus = < 3>;
status = "disabled";
  

Re: [PATCH 1/2] arm64: dts: stratix10: Add Stratix10 SMMU support

2018-08-07 Thread Rob Herring
On Wed, Jul 25, 2018 at 01:24:36PM -0500, thor.tha...@linux.intel.com wrote:
> From: Thor Thayer 
> 
> 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 
> ---
> This patch is dependent on the patch series
> "iommu/arm-smmu: Add runtime pm/sleep support"
> (https://patchwork.ozlabs.org/cover/946160/)

I don't think so.

> ---
>  .../devicetree/bindings/iommu/arm,smmu.txt | 25 ++
>  arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi  | 30 
> ++
>  2 files changed, 55 insertions(+)

Reviewed-by: Rob Herring 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH 1/2] arm64: dts: stratix10: Add Stratix10 SMMU support

2018-07-25 Thread thor . thayer
From: Thor Thayer 

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 
---
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,-smmu-v2", "qcom,smmu-v2"
+"altr,smmu-v2"
 
   depending on the particular implementation and/or the
   version of the architecture implemented.
@@ -179,3 +180,27 @@ conditions.
 < SMMU_MDP_AHB_CLK>;
clock-names = "bus", "iface";
};
+
+   /* Stratix10 arm,smmu-v2 implementation */
+   smmu5: iommu@fa00 {
+   compatible = "altr,smmu-v2", "arm,mmu-500",
+"arm,smmu-v2";
+   reg = <0xfa00 0x4>;
+   #global-interrupts = <2>;
+   #iommu-cells = <1>;
+   clocks = < STRATIX10_L4_MAIN_CLK>;
+   clock-names = "masters";
+   interrupt-parent = <>;
+   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 = < STRATIX10_EMAC0_CLK>;
clock-names = "stmmaceth";
+   iommus = < 1>;
status = "disabled";
};
 
@@ -150,6 +151,7 @@
reset-names = "stmmaceth", "stmmaceth-ocp";
clocks = < STRATIX10_EMAC1_CLK>;
clock-names = "stmmaceth";
+   iommus = < 2>;
status = "disabled";
};
 
@@ -163,6 +165,7 @@
reset-names = "stmmaceth", "stmmaceth-ocp";
clocks = < STRATIX10_EMAC2_CLK>;
clock-names = "stmmaceth";
+   iommus = < 3>;
status = "disabled";
};
 
@@ -273,6 +276,7 @@
clocks = < STRATIX10_L4_MP_CLK>,
 < STRATIX10_SDMMC_CLK>;
clock-names = "biu", "ciu";
+   iommus = < 5>;
status = "disabled";
};
 
@@ -307,6 +311,30 @@
altr,modrst-offset = <0x20>;
};
 
+   smmu: iommu@fa00 {
+   compatible = "altr,smmu-v2", "arm,mmu-500",
+"arm,smmu-v2";
+   reg = <0xfa00 0x4>;
+   #global-interrupts = <2>;
+   #iommu-cells = <1>;
+   clocks = < STRATIX10_L4_MAIN_CLK>;
+   clock-names = "masters";
+   interrupt-parent = <>;
+   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