* [PATCH 0/3] SOCFPGA Stratix10 SMMU Support
@ 2018-07-13 16:27 ` thor.thayer at linux.intel.com
0 siblings, 0 replies; 20+ messages in thread
From: thor.thayer-VuQAYsv1563Yd54FQh9/CA @ 2018-07-13 16:27 UTC (permalink / raw)
To: dinguyen-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
joro-zLv9SwRftAIdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
robin.murphy-5wv7dgnIgG8
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Thor Thayer,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
From: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
This patch series adds SMMU support to the SOCFPGA Stratix10 family
of parts. The addition of a clock parameter was required to ungate
the SMMU clock.
Thor Thayer (3):
Docs: dt: arm-smmu: Add optional clock parameter
iommu/arm-smmu: Add optional SMMU clock
arm64: dts: stratix10: Add SMMU Node
.../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++++
arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 44 ++++++++++++++++++++++
drivers/iommu/arm-smmu.c | 17 +++++++++
3 files changed, 77 insertions(+)
--
2.7.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3] SOCFPGA Stratix10 SMMU Support
@ 2018-07-13 16:27 ` thor.thayer at linux.intel.com
0 siblings, 0 replies; 20+ messages in thread
From: thor.thayer at linux.intel.com @ 2018-07-13 16:27 UTC (permalink / raw)
To: linux-arm-kernel
From: Thor Thayer <thor.thayer@linux.intel.com>
This patch series adds SMMU support to the SOCFPGA Stratix10 family
of parts. The addition of a clock parameter was required to ungate
the SMMU clock.
Thor Thayer (3):
Docs: dt: arm-smmu: Add optional clock parameter
iommu/arm-smmu: Add optional SMMU clock
arm64: dts: stratix10: Add SMMU Node
.../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++++
arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 44 ++++++++++++++++++++++
drivers/iommu/arm-smmu.c | 17 +++++++++
3 files changed, 77 insertions(+)
--
2.7.4
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] Docs: dt: arm-smmu: Add optional clock parameter
2018-07-13 16:27 ` thor.thayer at linux.intel.com
@ 2018-07-13 16:27 ` thor.thayer at linux.intel.com
-1 siblings, 0 replies; 20+ messages in thread
From: thor.thayer-VuQAYsv1563Yd54FQh9/CA @ 2018-07-13 16:27 UTC (permalink / raw)
To: dinguyen-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
joro-zLv9SwRftAIdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
robin.murphy-5wv7dgnIgG8
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Thor Thayer,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
From: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Add a clock to the SMMU node bindings.
Signed-off-by: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
Documentation/devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 8a6ffce12af5..356fd9f41e1b 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -71,6 +71,8 @@ conditions.
or using stream matching with #iommu-cells = <2>, and
may be ignored if present in such cases.
+- clock: clock provider specifier
+
** Deprecated properties:
- mmu-masters (deprecated in favour of the generic "iommus" binding) :
@@ -137,3 +139,17 @@ conditions.
iommu-map = <0 &smmu3 0 0x400>;
...
};
+
+ /* ARM MMU-500 with clock node */
+ smmu4: iommu {
+ compatible = "arm,mmu-500", "arm,smmu-v2";
+ clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
+ ...
+ #iommu-cells = <1>;
+ };
+
+ /* device with stream ID 1 */
+ master4 {
+ iommus = <&smmu4 1>;
+ };
+
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 1/3] Docs: dt: arm-smmu: Add optional clock parameter
@ 2018-07-13 16:27 ` thor.thayer at linux.intel.com
0 siblings, 0 replies; 20+ messages in thread
From: thor.thayer at linux.intel.com @ 2018-07-13 16:27 UTC (permalink / raw)
To: linux-arm-kernel
From: Thor Thayer <thor.thayer@linux.intel.com>
Add a clock to the SMMU node bindings.
Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
Documentation/devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 8a6ffce12af5..356fd9f41e1b 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -71,6 +71,8 @@ conditions.
or using stream matching with #iommu-cells = <2>, and
may be ignored if present in such cases.
+- clock: clock provider specifier
+
** Deprecated properties:
- mmu-masters (deprecated in favour of the generic "iommus" binding) :
@@ -137,3 +139,17 @@ conditions.
iommu-map = <0 &smmu3 0 0x400>;
...
};
+
+ /* ARM MMU-500 with clock node */
+ smmu4: iommu {
+ compatible = "arm,mmu-500", "arm,smmu-v2";
+ clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
+ ...
+ #iommu-cells = <1>;
+ };
+
+ /* device with stream ID 1 */
+ master4 {
+ iommus = <&smmu4 1>;
+ };
+
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] iommu/arm-smmu: Add optional SMMU clock
2018-07-13 16:27 ` thor.thayer at linux.intel.com
@ 2018-07-13 16:27 ` thor.thayer at linux.intel.com
-1 siblings, 0 replies; 20+ messages in thread
From: thor.thayer-VuQAYsv1563Yd54FQh9/CA @ 2018-07-13 16:27 UTC (permalink / raw)
To: dinguyen-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
joro-zLv9SwRftAIdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
robin.murphy-5wv7dgnIgG8
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Thor Thayer,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
From: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Add a clock to the SMMU structure. In the device tree case,
check for a clock handle and enable the clock if found.
Signed-off-by: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
---
drivers/iommu/arm-smmu.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 69e7c60792a8..0e6dd5019c23 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -212,6 +212,7 @@ struct arm_smmu_device {
/* IOMMU core code handle */
struct iommu_device iommu;
+ struct clk *clk;
};
enum arm_smmu_context_fmt {
@@ -1992,6 +1993,15 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
struct device *dev = &pdev->dev;
bool legacy_binding;
+ /* If a clock is declared, enable it */
+ smmu->clk = devm_clk_get(smmu->dev, NULL);
+ if (IS_ERR(smmu->clk)) {
+ smmu->clk = NULL;
+ dev_dbg(dev, "cannot get smmu clock\n");
+ } else {
+ clk_prepare_enable(smmu->clk);
+ }
+
if (of_property_read_u32(dev->of_node, "#global-interrupts",
&smmu->num_global_irqs)) {
dev_err(dev, "missing #global-interrupts property\n");
@@ -2181,6 +2191,10 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
/* Turn the thing off */
writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
+
+ if (smmu->clk)
+ clk_disable_unprepare(smmu->clk);
+
return 0;
}
@@ -2193,6 +2207,9 @@ static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
{
struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+ if (smmu->clk)
+ clk_prepare_enable(smmu->clk);
+
arm_smmu_device_reset(smmu);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/3] iommu/arm-smmu: Add optional SMMU clock
@ 2018-07-13 16:27 ` thor.thayer at linux.intel.com
0 siblings, 0 replies; 20+ messages in thread
From: thor.thayer at linux.intel.com @ 2018-07-13 16:27 UTC (permalink / raw)
To: linux-arm-kernel
From: Thor Thayer <thor.thayer@linux.intel.com>
Add a clock to the SMMU structure. In the device tree case,
check for a clock handle and enable the clock if found.
Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
drivers/iommu/arm-smmu.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 69e7c60792a8..0e6dd5019c23 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -212,6 +212,7 @@ struct arm_smmu_device {
/* IOMMU core code handle */
struct iommu_device iommu;
+ struct clk *clk;
};
enum arm_smmu_context_fmt {
@@ -1992,6 +1993,15 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
struct device *dev = &pdev->dev;
bool legacy_binding;
+ /* If a clock is declared, enable it */
+ smmu->clk = devm_clk_get(smmu->dev, NULL);
+ if (IS_ERR(smmu->clk)) {
+ smmu->clk = NULL;
+ dev_dbg(dev, "cannot get smmu clock\n");
+ } else {
+ clk_prepare_enable(smmu->clk);
+ }
+
if (of_property_read_u32(dev->of_node, "#global-interrupts",
&smmu->num_global_irqs)) {
dev_err(dev, "missing #global-interrupts property\n");
@@ -2181,6 +2191,10 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
/* Turn the thing off */
writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
+
+ if (smmu->clk)
+ clk_disable_unprepare(smmu->clk);
+
return 0;
}
@@ -2193,6 +2207,9 @@ static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
{
struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+ if (smmu->clk)
+ clk_prepare_enable(smmu->clk);
+
arm_smmu_device_reset(smmu);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] arm64: dts: stratix10: Add SMMU Node
2018-07-13 16:27 ` thor.thayer at linux.intel.com
@ 2018-07-13 16:27 ` thor.thayer at linux.intel.com
-1 siblings, 0 replies; 20+ messages in thread
From: thor.thayer @ 2018-07-13 16:27 UTC (permalink / raw)
To: dinguyen, robh+dt, joro, mark.rutland, robin.murphy
Cc: devicetree, Thor Thayer, catalin.marinas, will.deacon, iommu,
linux-arm-kernel
From: Thor Thayer <thor.thayer@linux.intel.com>
Add the SMMU node and IOMMU parameters to the
Stratix10 Device Tree.
Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 44 +++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index ca67ecb5866e..9b6ead87ae70 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -162,6 +162,8 @@
reset-names = "stmmaceth", "stmmaceth-ocp";
clocks = <&clkmgr STRATIX10_EMAC0_CLK>;
clock-names = "stmmaceth";
+ #stream-id-cells = <1>;
+ iommus = <&smmu 1>;
status = "disabled";
};
@@ -175,6 +177,8 @@
reset-names = "stmmaceth", "stmmaceth-ocp";
clocks = <&clkmgr STRATIX10_EMAC1_CLK>;
clock-names = "stmmaceth";
+ #stream-id-cells = <1>;
+ iommus = <&smmu 2>;
status = "disabled";
};
@@ -188,6 +192,8 @@
reset-names = "stmmaceth", "stmmaceth-ocp";
clocks = <&clkmgr STRATIX10_EMAC2_CLK>;
clock-names = "stmmaceth";
+ #stream-id-cells = <1>;
+ iommus = <&smmu 3>;
status = "disabled";
};
@@ -298,6 +304,8 @@
clocks = <&clkmgr STRATIX10_L4_MP_CLK>,
<&clkmgr STRATIX10_SDMMC_CLK>;
clock-names = "biu", "ciu";
+ #stream-id-cells = <1>;
+ iommus = <&smmu 5>;
status = "disabled";
};
@@ -323,6 +331,8 @@
#dma-requests = <32>;
clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
clock-names = "apb_pclk";
+ #stream-id-cells = <1>;
+ iommus = <&smmu 8>;
};
rst: rstmgr@ffd11000 {
@@ -332,6 +342,36 @@
altr,modrst-offset = <0x20>;
};
+ smmu: iommu@fa000000 {
+ compatible = "arm,mmu-500", "arm,smmu-v2";
+ reg = <0xfa000000 0x40000>;
+ #global-interrupts = <9>;
+ #iommu-cells = <1>;
+ clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
+ clock-names = "smmu_clk";
+ interrupt-parent = <&intc>;
+ interrupts = <0 128 4>, /* Global Secure Fault */
+ <0 129 4>, /* Global Non-secure Fault */
+ <0 130 4>, /* FPGA Performance Counter */
+ <0 131 4>, /* DMA Performance Counter */
+ <0 132 4>, /* EMAC Performance Counter */
+ <0 133 4>, /* IO Performance Counter */
+ <0 134 4>, /* SDM Performance Counter */
+ <0 136 4>, /* Non-secure Combined Interrupt */
+ <0 137 4>, /* Secure Combined Interrupt */
+ /* 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>;
@@ -439,6 +479,8 @@
resets = <&rst USB0_RESET>, <&rst USB0_OCP_RESET>;
reset-names = "dwc2", "dwc2-ecc";
clocks = <&clkmgr STRATIX10_USB_CLK>;
+ #stream-id-cells = <1>;
+ iommus = <&smmu 6>;
status = "disabled";
};
@@ -451,6 +493,8 @@
resets = <&rst USB1_RESET>, <&rst USB1_OCP_RESET>;
reset-names = "dwc2", "dwc2-ecc";
clocks = <&clkmgr STRATIX10_USB_CLK>;
+ #stream-id-cells = <1>;
+ iommus = <&smmu 7>;
status = "disabled";
};
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/3] arm64: dts: stratix10: Add SMMU Node
@ 2018-07-13 16:27 ` thor.thayer at linux.intel.com
0 siblings, 0 replies; 20+ messages in thread
From: thor.thayer at linux.intel.com @ 2018-07-13 16:27 UTC (permalink / raw)
To: linux-arm-kernel
From: Thor Thayer <thor.thayer@linux.intel.com>
Add the SMMU node and IOMMU parameters to the
Stratix10 Device Tree.
Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 44 +++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index ca67ecb5866e..9b6ead87ae70 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -162,6 +162,8 @@
reset-names = "stmmaceth", "stmmaceth-ocp";
clocks = <&clkmgr STRATIX10_EMAC0_CLK>;
clock-names = "stmmaceth";
+ #stream-id-cells = <1>;
+ iommus = <&smmu 1>;
status = "disabled";
};
@@ -175,6 +177,8 @@
reset-names = "stmmaceth", "stmmaceth-ocp";
clocks = <&clkmgr STRATIX10_EMAC1_CLK>;
clock-names = "stmmaceth";
+ #stream-id-cells = <1>;
+ iommus = <&smmu 2>;
status = "disabled";
};
@@ -188,6 +192,8 @@
reset-names = "stmmaceth", "stmmaceth-ocp";
clocks = <&clkmgr STRATIX10_EMAC2_CLK>;
clock-names = "stmmaceth";
+ #stream-id-cells = <1>;
+ iommus = <&smmu 3>;
status = "disabled";
};
@@ -298,6 +304,8 @@
clocks = <&clkmgr STRATIX10_L4_MP_CLK>,
<&clkmgr STRATIX10_SDMMC_CLK>;
clock-names = "biu", "ciu";
+ #stream-id-cells = <1>;
+ iommus = <&smmu 5>;
status = "disabled";
};
@@ -323,6 +331,8 @@
#dma-requests = <32>;
clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
clock-names = "apb_pclk";
+ #stream-id-cells = <1>;
+ iommus = <&smmu 8>;
};
rst: rstmgr at ffd11000 {
@@ -332,6 +342,36 @@
altr,modrst-offset = <0x20>;
};
+ smmu: iommu at fa000000 {
+ compatible = "arm,mmu-500", "arm,smmu-v2";
+ reg = <0xfa000000 0x40000>;
+ #global-interrupts = <9>;
+ #iommu-cells = <1>;
+ clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
+ clock-names = "smmu_clk";
+ interrupt-parent = <&intc>;
+ interrupts = <0 128 4>, /* Global Secure Fault */
+ <0 129 4>, /* Global Non-secure Fault */
+ <0 130 4>, /* FPGA Performance Counter */
+ <0 131 4>, /* DMA Performance Counter */
+ <0 132 4>, /* EMAC Performance Counter */
+ <0 133 4>, /* IO Performance Counter */
+ <0 134 4>, /* SDM Performance Counter */
+ <0 136 4>, /* Non-secure Combined Interrupt */
+ <0 137 4>, /* Secure Combined Interrupt */
+ /* 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 at ffda4000 {
compatible = "snps,dw-apb-ssi";
#address-cells = <1>;
@@ -439,6 +479,8 @@
resets = <&rst USB0_RESET>, <&rst USB0_OCP_RESET>;
reset-names = "dwc2", "dwc2-ecc";
clocks = <&clkmgr STRATIX10_USB_CLK>;
+ #stream-id-cells = <1>;
+ iommus = <&smmu 6>;
status = "disabled";
};
@@ -451,6 +493,8 @@
resets = <&rst USB1_RESET>, <&rst USB1_OCP_RESET>;
reset-names = "dwc2", "dwc2-ecc";
clocks = <&clkmgr STRATIX10_USB_CLK>;
+ #stream-id-cells = <1>;
+ iommus = <&smmu 7>;
status = "disabled";
};
--
2.7.4
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 0/3] SOCFPGA Stratix10 SMMU Support
2018-07-13 16:27 ` thor.thayer at linux.intel.com
@ 2018-07-13 17:05 ` Robin Murphy
-1 siblings, 0 replies; 20+ messages in thread
From: Robin Murphy @ 2018-07-13 17:05 UTC (permalink / raw)
To: thor.thayer, dinguyen, robh+dt, joro, mark.rutland
Cc: catalin.marinas, iommu, will.deacon, linux-arm-kernel, devicetree
Hi Thor,
On 13/07/18 17:27, thor.thayer@linux.intel.com wrote:
> From: Thor Thayer <thor.thayer@linux.intel.com>
>
> This patch series adds SMMU support to the SOCFPGA Stratix10 family
> of parts. The addition of a clock parameter was required to ungate
> the SMMU clock.
Please see the latest version of Vivek's runtime PM series[1] - there's
already been an awful lot of discussion over how to handle clocks, since
they're not part of the SMMU architecture and different implementations
can have very different requirements (and even different configurations
of the same implementation may vary, e.g. MMU-500 could technically have
up to 33 separate clocks).
Thanks,
Robin.
[1]
https://www.mail-archive.com/freedreno@lists.freedesktop.org/msg02422.html
> Thor Thayer (3):
> Docs: dt: arm-smmu: Add optional clock parameter
> iommu/arm-smmu: Add optional SMMU clock
> arm64: dts: stratix10: Add SMMU Node
>
> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++++
> arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 44 ++++++++++++++++++++++
> drivers/iommu/arm-smmu.c | 17 +++++++++
> 3 files changed, 77 insertions(+)
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/3] SOCFPGA Stratix10 SMMU Support
@ 2018-07-13 17:05 ` Robin Murphy
0 siblings, 0 replies; 20+ messages in thread
From: Robin Murphy @ 2018-07-13 17:05 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thor,
On 13/07/18 17:27, thor.thayer at linux.intel.com wrote:
> From: Thor Thayer <thor.thayer@linux.intel.com>
>
> This patch series adds SMMU support to the SOCFPGA Stratix10 family
> of parts. The addition of a clock parameter was required to ungate
> the SMMU clock.
Please see the latest version of Vivek's runtime PM series[1] - there's
already been an awful lot of discussion over how to handle clocks, since
they're not part of the SMMU architecture and different implementations
can have very different requirements (and even different configurations
of the same implementation may vary, e.g. MMU-500 could technically have
up to 33 separate clocks).
Thanks,
Robin.
[1]
https://www.mail-archive.com/freedreno at lists.freedesktop.org/msg02422.html
> Thor Thayer (3):
> Docs: dt: arm-smmu: Add optional clock parameter
> iommu/arm-smmu: Add optional SMMU clock
> arm64: dts: stratix10: Add SMMU Node
>
> .../devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++++
> arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 44 ++++++++++++++++++++++
> drivers/iommu/arm-smmu.c | 17 +++++++++
> 3 files changed, 77 insertions(+)
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] arm64: dts: stratix10: Add SMMU Node
2018-07-13 16:27 ` thor.thayer at linux.intel.com
@ 2018-07-13 18:09 ` Robin Murphy
-1 siblings, 0 replies; 20+ messages in thread
From: Robin Murphy @ 2018-07-13 18:09 UTC (permalink / raw)
To: thor.thayer-VuQAYsv1563Yd54FQh9/CA,
dinguyen-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
joro-zLv9SwRftAIdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
will.deacon-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
catalin.marinas-5wv7dgnIgG8
On 13/07/18 17:27, thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org wrote:
> From: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>
> Add the SMMU node and IOMMU parameters to the
> Stratix10 Device Tree.
>
> Signed-off-by: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
> arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 44 +++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> index ca67ecb5866e..9b6ead87ae70 100644
> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> @@ -162,6 +162,8 @@
> reset-names = "stmmaceth", "stmmaceth-ocp";
> clocks = <&clkmgr STRATIX10_EMAC0_CLK>;
> clock-names = "stmmaceth";
> + #stream-id-cells = <1>;
The #stream-id-cells property is part of the deprecated mmu-masters
binding, so you don't need to add these.
> + iommus = <&smmu 1>;
> status = "disabled";
> };
>
> @@ -175,6 +177,8 @@
> reset-names = "stmmaceth", "stmmaceth-ocp";
> clocks = <&clkmgr STRATIX10_EMAC1_CLK>;
> clock-names = "stmmaceth";
> + #stream-id-cells = <1>;
> + iommus = <&smmu 2>;
> status = "disabled";
> };
>
> @@ -188,6 +192,8 @@
> reset-names = "stmmaceth", "stmmaceth-ocp";
> clocks = <&clkmgr STRATIX10_EMAC2_CLK>;
> clock-names = "stmmaceth";
> + #stream-id-cells = <1>;
> + iommus = <&smmu 3>;
> status = "disabled";
> };
>
> @@ -298,6 +304,8 @@
> clocks = <&clkmgr STRATIX10_L4_MP_CLK>,
> <&clkmgr STRATIX10_SDMMC_CLK>;
> clock-names = "biu", "ciu";
> + #stream-id-cells = <1>;
> + iommus = <&smmu 5>;
> status = "disabled";
> };
>
> @@ -323,6 +331,8 @@
> #dma-requests = <32>;
> clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
> clock-names = "apb_pclk";
> + #stream-id-cells = <1>;
> + iommus = <&smmu 8>;
Just to double-check, all the channel threads and the manager thread
share the one stream ID? (I'm accustomed to seeing DMA-330 integrated
with an SMMU by tapping the AxID outputs off to the stream ID input.)
> };
>
> rst: rstmgr@ffd11000 {
> @@ -332,6 +342,36 @@
> altr,modrst-offset = <0x20>;
> };
>
> + smmu: iommu@fa000000 {
> + compatible = "arm,mmu-500", "arm,smmu-v2";
> + reg = <0xfa000000 0x40000>;
> + #global-interrupts = <9>;
> + #iommu-cells = <1>;
> + clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
> + clock-names = "smmu_clk";
> + interrupt-parent = <&intc>;
> + interrupts = <0 128 4>, /* Global Secure Fault */
> + <0 129 4>, /* Global Non-secure Fault */
> + <0 130 4>, /* FPGA Performance Counter */
> + <0 131 4>, /* DMA Performance Counter */
> + <0 132 4>, /* EMAC Performance Counter */
> + <0 133 4>, /* IO Performance Counter */
> + <0 134 4>, /* SDM Performance Counter */
Note that there isn't much benefit to adding the secure or PMU
interrupts here other than to document the hardware - FWIW I have
actually been working on a PMU driver, and needless to say it turns out
not to be sufficient just having those munged into the SMMU global fault
handler.
> + <0 136 4>, /* Non-secure Combined Interrupt */
> + <0 137 4>, /* Secure Combined Interrupt */
Similarly the combined interrupt; that's literally just all these other
interrupt lines ORed together at the SMMU end, and would generally only
be useful if you *didn't* have the individual lines wired up. As it
stands with everything listed, any event will also generate a spurious
global fault IRQ, which isn't ideal (not that you should get many
interrupts during normal operation, but still...)
Robin.
> + /* 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>;
> @@ -439,6 +479,8 @@
> resets = <&rst USB0_RESET>, <&rst USB0_OCP_RESET>;
> reset-names = "dwc2", "dwc2-ecc";
> clocks = <&clkmgr STRATIX10_USB_CLK>;
> + #stream-id-cells = <1>;
> + iommus = <&smmu 6>;
> status = "disabled";
> };
>
> @@ -451,6 +493,8 @@
> resets = <&rst USB1_RESET>, <&rst USB1_OCP_RESET>;
> reset-names = "dwc2", "dwc2-ecc";
> clocks = <&clkmgr STRATIX10_USB_CLK>;
> + #stream-id-cells = <1>;
> + iommus = <&smmu 7>;
> status = "disabled";
> };
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] arm64: dts: stratix10: Add SMMU Node
@ 2018-07-13 18:09 ` Robin Murphy
0 siblings, 0 replies; 20+ messages in thread
From: Robin Murphy @ 2018-07-13 18:09 UTC (permalink / raw)
To: linux-arm-kernel
On 13/07/18 17:27, thor.thayer at linux.intel.com wrote:
> From: Thor Thayer <thor.thayer@linux.intel.com>
>
> Add the SMMU node and IOMMU parameters to the
> Stratix10 Device Tree.
>
> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
> ---
> arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 44 +++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> index ca67ecb5866e..9b6ead87ae70 100644
> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
> @@ -162,6 +162,8 @@
> reset-names = "stmmaceth", "stmmaceth-ocp";
> clocks = <&clkmgr STRATIX10_EMAC0_CLK>;
> clock-names = "stmmaceth";
> + #stream-id-cells = <1>;
The #stream-id-cells property is part of the deprecated mmu-masters
binding, so you don't need to add these.
> + iommus = <&smmu 1>;
> status = "disabled";
> };
>
> @@ -175,6 +177,8 @@
> reset-names = "stmmaceth", "stmmaceth-ocp";
> clocks = <&clkmgr STRATIX10_EMAC1_CLK>;
> clock-names = "stmmaceth";
> + #stream-id-cells = <1>;
> + iommus = <&smmu 2>;
> status = "disabled";
> };
>
> @@ -188,6 +192,8 @@
> reset-names = "stmmaceth", "stmmaceth-ocp";
> clocks = <&clkmgr STRATIX10_EMAC2_CLK>;
> clock-names = "stmmaceth";
> + #stream-id-cells = <1>;
> + iommus = <&smmu 3>;
> status = "disabled";
> };
>
> @@ -298,6 +304,8 @@
> clocks = <&clkmgr STRATIX10_L4_MP_CLK>,
> <&clkmgr STRATIX10_SDMMC_CLK>;
> clock-names = "biu", "ciu";
> + #stream-id-cells = <1>;
> + iommus = <&smmu 5>;
> status = "disabled";
> };
>
> @@ -323,6 +331,8 @@
> #dma-requests = <32>;
> clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
> clock-names = "apb_pclk";
> + #stream-id-cells = <1>;
> + iommus = <&smmu 8>;
Just to double-check, all the channel threads and the manager thread
share the one stream ID? (I'm accustomed to seeing DMA-330 integrated
with an SMMU by tapping the AxID outputs off to the stream ID input.)
> };
>
> rst: rstmgr at ffd11000 {
> @@ -332,6 +342,36 @@
> altr,modrst-offset = <0x20>;
> };
>
> + smmu: iommu at fa000000 {
> + compatible = "arm,mmu-500", "arm,smmu-v2";
> + reg = <0xfa000000 0x40000>;
> + #global-interrupts = <9>;
> + #iommu-cells = <1>;
> + clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
> + clock-names = "smmu_clk";
> + interrupt-parent = <&intc>;
> + interrupts = <0 128 4>, /* Global Secure Fault */
> + <0 129 4>, /* Global Non-secure Fault */
> + <0 130 4>, /* FPGA Performance Counter */
> + <0 131 4>, /* DMA Performance Counter */
> + <0 132 4>, /* EMAC Performance Counter */
> + <0 133 4>, /* IO Performance Counter */
> + <0 134 4>, /* SDM Performance Counter */
Note that there isn't much benefit to adding the secure or PMU
interrupts here other than to document the hardware - FWIW I have
actually been working on a PMU driver, and needless to say it turns out
not to be sufficient just having those munged into the SMMU global fault
handler.
> + <0 136 4>, /* Non-secure Combined Interrupt */
> + <0 137 4>, /* Secure Combined Interrupt */
Similarly the combined interrupt; that's literally just all these other
interrupt lines ORed together at the SMMU end, and would generally only
be useful if you *didn't* have the individual lines wired up. As it
stands with everything listed, any event will also generate a spurious
global fault IRQ, which isn't ideal (not that you should get many
interrupts during normal operation, but still...)
Robin.
> + /* 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 at ffda4000 {
> compatible = "snps,dw-apb-ssi";
> #address-cells = <1>;
> @@ -439,6 +479,8 @@
> resets = <&rst USB0_RESET>, <&rst USB0_OCP_RESET>;
> reset-names = "dwc2", "dwc2-ecc";
> clocks = <&clkmgr STRATIX10_USB_CLK>;
> + #stream-id-cells = <1>;
> + iommus = <&smmu 6>;
> status = "disabled";
> };
>
> @@ -451,6 +493,8 @@
> resets = <&rst USB1_RESET>, <&rst USB1_OCP_RESET>;
> reset-names = "dwc2", "dwc2-ecc";
> clocks = <&clkmgr STRATIX10_USB_CLK>;
> + #stream-id-cells = <1>;
> + iommus = <&smmu 7>;
> status = "disabled";
> };
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] arm64: dts: stratix10: Add SMMU Node
2018-07-13 18:09 ` Robin Murphy
@ 2018-07-16 18:56 ` Thor Thayer
-1 siblings, 0 replies; 20+ messages in thread
From: Thor Thayer @ 2018-07-16 18:56 UTC (permalink / raw)
To: Robin Murphy, dinguyen-DgEjT+Ai2ygdnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, joro-zLv9SwRftAIdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
will.deacon-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
catalin.marinas-5wv7dgnIgG8
Hi Robin,
On 07/13/2018 01:09 PM, Robin Murphy wrote:
> On 13/07/18 17:27, thor.thayer@linux.intel.com wrote:
>> From: Thor Thayer <thor.thayer@linux.intel.com>
>>
>> Add the SMMU node and IOMMU parameters to the
>> Stratix10 Device Tree.
>>
>> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
>> ---
>> arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 44
>> +++++++++++++++++++++++
>> 1 file changed, 44 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>> b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>> index ca67ecb5866e..9b6ead87ae70 100644
>> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>> @@ -162,6 +162,8 @@
>> reset-names = "stmmaceth", "stmmaceth-ocp";
>> clocks = <&clkmgr STRATIX10_EMAC0_CLK>;
>> clock-names = "stmmaceth";
>> + #stream-id-cells = <1>;
>
> The #stream-id-cells property is part of the deprecated mmu-masters
> binding, so you don't need to add these.
>
OK. Thank you.
>> + iommus = <&smmu 1>;
>> status = "disabled";
>> };
<SNIP>
>> status = "disabled";
>> };
>> @@ -323,6 +331,8 @@
>> #dma-requests = <32>;
>> clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
>> clock-names = "apb_pclk";
>> + #stream-id-cells = <1>;
>> + iommus = <&smmu 8>;
>
> Just to double-check, all the channel threads and the manager thread
> share the one stream ID? (I'm accustomed to seeing DMA-330 integrated
> with an SMMU by tapping the AxID outputs off to the stream ID input.)
>
Yes, we have only one stream ID for the DMA. I'll forward the
differences you noted to our architecture team as something to consider
for future chips.
>> };
>> rst: rstmgr@ffd11000 {
>> @@ -332,6 +342,36 @@
>> altr,modrst-offset = <0x20>;
>> };
>> + smmu: iommu@fa000000 {
>> + compatible = "arm,mmu-500", "arm,smmu-v2";
>> + reg = <0xfa000000 0x40000>;
>> + #global-interrupts = <9>;
>> + #iommu-cells = <1>;
>> + clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
>> + clock-names = "smmu_clk";
>> + interrupt-parent = <&intc>;
>> + interrupts = <0 128 4>, /* Global Secure Fault */
>> + <0 129 4>, /* Global Non-secure Fault */
>> + <0 130 4>, /* FPGA Performance Counter */
>> + <0 131 4>, /* DMA Performance Counter */
>> + <0 132 4>, /* EMAC Performance Counter */
>> + <0 133 4>, /* IO Performance Counter */
>> + <0 134 4>, /* SDM Performance Counter */
>
> Note that there isn't much benefit to adding the secure or PMU
> interrupts here other than to document the hardware - FWIW I have
> actually been working on a PMU driver, and needless to say it turns out
> not to be sufficient just having those munged into the SMMU global fault
> handler.
>
Thanks for pointing this out. I was following other SMMU-500 device
trees. Just to clarify, how should I simplify this? Should I replace all
the above with the following?
<0 129 4>, /* Global Non-secure Fault */
Or will your upcoming PMU driver need the PMU units? It sounded like
using the just Global fault was not sufficient.
>> + <0 136 4>, /* Non-secure Combined Interrupt */
>> + <0 137 4>, /* Secure Combined Interrupt */
>
> Similarly the combined interrupt; that's literally just all these other
> interrupt lines ORed together at the SMMU end, and would generally only
> be useful if you *didn't* have the individual lines wired up. As it
> stands with everything listed, any event will also generate a spurious
> global fault IRQ, which isn't ideal (not that you should get many
> interrupts during normal operation, but still...)
>
And I'd remove both of these then, right?
Thanks for the review and helpful comments (and also pointing out the
existing clock patch in my patch series summary)!
Thor
> Robin.
>
>> + /* 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";
>> + };
>> +
<snip>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] arm64: dts: stratix10: Add SMMU Node
@ 2018-07-16 18:56 ` Thor Thayer
0 siblings, 0 replies; 20+ messages in thread
From: Thor Thayer @ 2018-07-16 18:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi Robin,
On 07/13/2018 01:09 PM, Robin Murphy wrote:
> On 13/07/18 17:27, thor.thayer at linux.intel.com wrote:
>> From: Thor Thayer <thor.thayer@linux.intel.com>
>>
>> Add the SMMU node and IOMMU parameters to the
>> Stratix10 Device Tree.
>>
>> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
>> ---
>> ? arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 44
>> +++++++++++++++++++++++
>> ? 1 file changed, 44 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>> b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>> index ca67ecb5866e..9b6ead87ae70 100644
>> --- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>> +++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
>> @@ -162,6 +162,8 @@
>> ????????????? reset-names = "stmmaceth", "stmmaceth-ocp";
>> ????????????? clocks = <&clkmgr STRATIX10_EMAC0_CLK>;
>> ????????????? clock-names = "stmmaceth";
>> +??????????? #stream-id-cells = <1>;
>
> The #stream-id-cells property is part of the deprecated mmu-masters
> binding, so you don't need to add these.
>
OK. Thank you.
>> +??????????? iommus = <&smmu 1>;
>> ????????????? status = "disabled";
>> ????????? };
<SNIP>
>> ????????????? status = "disabled";
>> ????????? };
>> @@ -323,6 +331,8 @@
>> ????????????? #dma-requests = <32>;
>> ????????????? clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
>> ????????????? clock-names = "apb_pclk";
>> +??????????? #stream-id-cells = <1>;
>> +??????????? iommus = <&smmu 8>;
>
> Just to double-check, all the channel threads and the manager thread
> share the one stream ID? (I'm accustomed to seeing DMA-330 integrated
> with an SMMU by tapping the AxID outputs off to the stream ID input.)
>
Yes, we have only one stream ID for the DMA. I'll forward the
differences you noted to our architecture team as something to consider
for future chips.
>> ????????? };
>> ????????? rst: rstmgr at ffd11000 {
>> @@ -332,6 +342,36 @@
>> ????????????? altr,modrst-offset = <0x20>;
>> ????????? };
>> +??????? smmu: iommu at fa000000 {
>> +??????????? compatible = "arm,mmu-500", "arm,smmu-v2";
>> +??????????? reg = <0xfa000000 0x40000>;
>> +??????????? #global-interrupts = <9>;
>> +??????????? #iommu-cells = <1>;
>> +??????????? clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
>> +??????????? clock-names = "smmu_clk";
>> +??????????? interrupt-parent = <&intc>;
>> +??????????? interrupts = <0 128 4>,??? /* Global Secure Fault */
>> +??????????????? <0 129 4>, /* Global Non-secure Fault */
>> +??????????????? <0 130 4>, /* FPGA Performance Counter */
>> +??????????????? <0 131 4>, /* DMA Performance Counter */
>> +??????????????? <0 132 4>, /* EMAC Performance Counter */
>> +??????????????? <0 133 4>, /* IO Performance Counter */
>> +??????????????? <0 134 4>, /* SDM Performance Counter */
>
> Note that there isn't much benefit to adding the secure or PMU
> interrupts here other than to document the hardware - FWIW I have
> actually been working on a PMU driver, and needless to say it turns out
> not to be sufficient just having those munged into the SMMU global fault
> handler.
>
Thanks for pointing this out. I was following other SMMU-500 device
trees. Just to clarify, how should I simplify this? Should I replace all
the above with the following?
<0 129 4>, /* Global Non-secure Fault */
Or will your upcoming PMU driver need the PMU units? It sounded like
using the just Global fault was not sufficient.
>> +??????????????? <0 136 4>, /* Non-secure Combined Interrupt */
>> +??????????????? <0 137 4>, /* Secure Combined Interrupt */
>
> Similarly the combined interrupt; that's literally just all these other
> interrupt lines ORed together at the SMMU end, and would generally only
> be useful if you *didn't* have the individual lines wired up. As it
> stands with everything listed, any event will also generate a spurious
> global fault IRQ, which isn't ideal (not that you should get many
> interrupts during normal operation, but still...)
>
And I'd remove both of these then, right?
Thanks for the review and helpful comments (and also pointing out the
existing clock patch in my patch series summary)!
Thor
> Robin.
>
>> +??????????????? /* 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";
>> +??????? };
>> +
<snip>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] Docs: dt: arm-smmu: Add optional clock parameter
2018-07-13 16:27 ` thor.thayer at linux.intel.com
@ 2018-07-20 16:15 ` Rob Herring
-1 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2018-07-20 16:15 UTC (permalink / raw)
To: thor.thayer-VuQAYsv1563Yd54FQh9/CA
Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
dinguyen-DgEjT+Ai2ygdnm+yROfE0A,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Fri, Jul 13, 2018 at 11:27:56AM -0500, thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org wrote:
> From: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>
> Add a clock to the SMMU node bindings.
>
> Signed-off-by: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
> Documentation/devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 8a6ffce12af5..356fd9f41e1b 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -71,6 +71,8 @@ conditions.
> or using stream matching with #iommu-cells = <2>, and
> may be ignored if present in such cases.
>
> +- clock: clock provider specifier
> +
The TRM says there is a TCU clock and clock per TBU.
Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] Docs: dt: arm-smmu: Add optional clock parameter
@ 2018-07-20 16:15 ` Rob Herring
0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2018-07-20 16:15 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jul 13, 2018 at 11:27:56AM -0500, thor.thayer at linux.intel.com wrote:
> From: Thor Thayer <thor.thayer@linux.intel.com>
>
> Add a clock to the SMMU node bindings.
>
> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
> ---
> Documentation/devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 8a6ffce12af5..356fd9f41e1b 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -71,6 +71,8 @@ conditions.
> or using stream matching with #iommu-cells = <2>, and
> may be ignored if present in such cases.
>
> +- clock: clock provider specifier
> +
The TRM says there is a TCU clock and clock per TBU.
Rob
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] Docs: dt: arm-smmu: Add optional clock parameter
2018-07-20 16:15 ` Rob Herring
@ 2018-07-24 22:25 ` Thor Thayer
-1 siblings, 0 replies; 20+ messages in thread
From: Thor Thayer @ 2018-07-24 22:25 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
catalin.marinas-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
dinguyen-DgEjT+Ai2ygdnm+yROfE0A,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Hi Rob,
On 07/20/2018 11:15 AM, Rob Herring wrote:
> On Fri, Jul 13, 2018 at 11:27:56AM -0500, thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org wrote:
>> From: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>
>> Add a clock to the SMMU node bindings.
>>
>> Signed-off-by: Thor Thayer <thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>> ---
>> Documentation/devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 8a6ffce12af5..356fd9f41e1b 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -71,6 +71,8 @@ conditions.
>> or using stream matching with #iommu-cells = <2>, and
>> may be ignored if present in such cases.
>>
>> +- clock: clock provider specifier
>> +
>
> The TRM says there is a TCU clock and clock per TBU.
>
> Rob
>
Yes, good point. I'm abandoning this review and will use the bulk clock
suggested in [1].
In our case, the TCU clock is always on and we have 1 clock for the
TBU masters.
Thanks for reviewing!
Thor
[1] https://patchwork.kernel.org/patch/10534089/
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/3] Docs: dt: arm-smmu: Add optional clock parameter
@ 2018-07-24 22:25 ` Thor Thayer
0 siblings, 0 replies; 20+ messages in thread
From: Thor Thayer @ 2018-07-24 22:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rob,
On 07/20/2018 11:15 AM, Rob Herring wrote:
> On Fri, Jul 13, 2018 at 11:27:56AM -0500, thor.thayer at linux.intel.com wrote:
>> From: Thor Thayer <thor.thayer@linux.intel.com>
>>
>> Add a clock to the SMMU node bindings.
>>
>> Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
>> ---
>> Documentation/devicetree/bindings/iommu/arm,smmu.txt | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 8a6ffce12af5..356fd9f41e1b 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -71,6 +71,8 @@ conditions.
>> or using stream matching with #iommu-cells = <2>, and
>> may be ignored if present in such cases.
>>
>> +- clock: clock provider specifier
>> +
>
> The TRM says there is a TCU clock and clock per TBU.
>
> Rob
>
Yes, good point. I'm abandoning this review and will use the bulk clock
suggested in [1].
In our case, the TCU clock is always on and we have 1 clock for the
TBU masters.
Thanks for reviewing!
Thor
[1] https://patchwork.kernel.org/patch/10534089/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 3/3] arm64: dts: stratix10: Add SMMU Node
2018-07-16 18:56 ` Thor Thayer
@ 2018-07-25 13:34 ` Robin Murphy
-1 siblings, 0 replies; 20+ messages in thread
From: Robin Murphy @ 2018-07-25 13:34 UTC (permalink / raw)
To: thor.thayer-VuQAYsv1563Yd54FQh9/CA,
dinguyen-DgEjT+Ai2ygdnm+yROfE0A, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
joro-zLv9SwRftAIdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
will.deacon-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
catalin.marinas-5wv7dgnIgG8
On 16/07/18 19:56, Thor Thayer wrote:
[...]
>>> @@ -332,6 +342,36 @@
>>> altr,modrst-offset = <0x20>;
>>> };
>>> + smmu: iommu@fa000000 {
>>> + compatible = "arm,mmu-500", "arm,smmu-v2";
>>> + reg = <0xfa000000 0x40000>;
>>> + #global-interrupts = <9>;
>>> + #iommu-cells = <1>;
>>> + clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
>>> + clock-names = "smmu_clk";
>>> + interrupt-parent = <&intc>;
>>> + interrupts = <0 128 4>, /* Global Secure Fault */
>>> + <0 129 4>, /* Global Non-secure Fault */
>>> + <0 130 4>, /* FPGA Performance Counter */
>>> + <0 131 4>, /* DMA Performance Counter */
>>> + <0 132 4>, /* EMAC Performance Counter */
>>> + <0 133 4>, /* IO Performance Counter */
>>> + <0 134 4>, /* SDM Performance Counter */
>>
>> Note that there isn't much benefit to adding the secure or PMU
>> interrupts here other than to document the hardware - FWIW I have
>> actually been working on a PMU driver, and needless to say it turns
>> out not to be sufficient just having those munged into the SMMU global
>> fault handler.
>>
> Thanks for pointing this out. I was following other SMMU-500 device
> trees. Just to clarify, how should I simplify this? Should I replace all
> the above with the following?
>
> <0 129 4>, /* Global Non-secure Fault */
>
> Or will your upcoming PMU driver need the PMU units? It sounded like
> using the just Global fault was not sufficient.
No, you had it right - what I meant was it's only worth including the
actual global fault signals themselves here (there's no harm in leaving
the secure one in if you like, it just won't do anything if the GIC is
configured correctly). What I found was that the current SMMU binding
leaves no reasonable way to encode the PMU interrupts in this interrupt
list that doesn't break at least one of the possible old/new driver/DT
combinations, so whatever happens they will need to be specified
separately once a PMU binding is defined.
>>> + <0 136 4>, /* Non-secure Combined Interrupt */
>>> + <0 137 4>, /* Secure Combined Interrupt */
>>
>> Similarly the combined interrupt; that's literally just all these
>> other interrupt lines ORed together at the SMMU end, and would
>> generally only be useful if you *didn't* have the individual lines
>> wired up. As it stands with everything listed, any event will also
>> generate a spurious global fault IRQ, which isn't ideal (not that you
>> should get many interrupts during normal operation, but still...)
>>
> And I'd remove both of these then, right?
Indeed.
> Thanks for the review and helpful comments (and also pointing out the
> existing clock patch in my patch series summary)!
Cheers,
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] arm64: dts: stratix10: Add SMMU Node
@ 2018-07-25 13:34 ` Robin Murphy
0 siblings, 0 replies; 20+ messages in thread
From: Robin Murphy @ 2018-07-25 13:34 UTC (permalink / raw)
To: linux-arm-kernel
On 16/07/18 19:56, Thor Thayer wrote:
[...]
>>> @@ -332,6 +342,36 @@
>>> ????????????? altr,modrst-offset = <0x20>;
>>> ????????? };
>>> +??????? smmu: iommu at fa000000 {
>>> +??????????? compatible = "arm,mmu-500", "arm,smmu-v2";
>>> +??????????? reg = <0xfa000000 0x40000>;
>>> +??????????? #global-interrupts = <9>;
>>> +??????????? #iommu-cells = <1>;
>>> +??????????? clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
>>> +??????????? clock-names = "smmu_clk";
>>> +??????????? interrupt-parent = <&intc>;
>>> +??????????? interrupts = <0 128 4>,??? /* Global Secure Fault */
>>> +??????????????? <0 129 4>, /* Global Non-secure Fault */
>>> +??????????????? <0 130 4>, /* FPGA Performance Counter */
>>> +??????????????? <0 131 4>, /* DMA Performance Counter */
>>> +??????????????? <0 132 4>, /* EMAC Performance Counter */
>>> +??????????????? <0 133 4>, /* IO Performance Counter */
>>> +??????????????? <0 134 4>, /* SDM Performance Counter */
>>
>> Note that there isn't much benefit to adding the secure or PMU
>> interrupts here other than to document the hardware - FWIW I have
>> actually been working on a PMU driver, and needless to say it turns
>> out not to be sufficient just having those munged into the SMMU global
>> fault handler.
>>
> Thanks for pointing this out. I was following other SMMU-500 device
> trees. Just to clarify, how should I simplify this? Should I replace all
> the above with the following?
>
> ????<0 129 4>, /* Global Non-secure Fault */
>
> Or will your upcoming PMU driver need the PMU units? It sounded like
> using the just Global fault was not sufficient.
No, you had it right - what I meant was it's only worth including the
actual global fault signals themselves here (there's no harm in leaving
the secure one in if you like, it just won't do anything if the GIC is
configured correctly). What I found was that the current SMMU binding
leaves no reasonable way to encode the PMU interrupts in this interrupt
list that doesn't break at least one of the possible old/new driver/DT
combinations, so whatever happens they will need to be specified
separately once a PMU binding is defined.
>>> +??????????????? <0 136 4>, /* Non-secure Combined Interrupt */
>>> +??????????????? <0 137 4>, /* Secure Combined Interrupt */
>>
>> Similarly the combined interrupt; that's literally just all these
>> other interrupt lines ORed together at the SMMU end, and would
>> generally only be useful if you *didn't* have the individual lines
>> wired up. As it stands with everything listed, any event will also
>> generate a spurious global fault IRQ, which isn't ideal (not that you
>> should get many interrupts during normal operation, but still...)
>>
> And I'd remove both of these then, right?
Indeed.
> Thanks for the review and helpful comments (and also pointing out the
> existing clock patch in my patch series summary)!
Cheers,
Robin.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-07-25 13:34 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-13 16:27 [PATCH 0/3] SOCFPGA Stratix10 SMMU Support thor.thayer-VuQAYsv1563Yd54FQh9/CA
2018-07-13 16:27 ` thor.thayer at linux.intel.com
[not found] ` <1531499278-32132-1-git-send-email-thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-07-13 16:27 ` [PATCH 1/3] Docs: dt: arm-smmu: Add optional clock parameter thor.thayer-VuQAYsv1563Yd54FQh9/CA
2018-07-13 16:27 ` thor.thayer at linux.intel.com
[not found] ` <1531499278-32132-2-git-send-email-thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-07-20 16:15 ` Rob Herring
2018-07-20 16:15 ` Rob Herring
2018-07-24 22:25 ` Thor Thayer
2018-07-24 22:25 ` Thor Thayer
2018-07-13 16:27 ` [PATCH 2/3] iommu/arm-smmu: Add optional SMMU clock thor.thayer-VuQAYsv1563Yd54FQh9/CA
2018-07-13 16:27 ` thor.thayer at linux.intel.com
2018-07-13 16:27 ` [PATCH 3/3] arm64: dts: stratix10: Add SMMU Node thor.thayer
2018-07-13 16:27 ` thor.thayer at linux.intel.com
[not found] ` <1531499278-32132-4-git-send-email-thor.thayer-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-07-13 18:09 ` Robin Murphy
2018-07-13 18:09 ` Robin Murphy
[not found] ` <2d17b3b1-96a1-8a0a-521e-134de9df72d0-5wv7dgnIgG8@public.gmane.org>
2018-07-16 18:56 ` Thor Thayer
2018-07-16 18:56 ` Thor Thayer
[not found] ` <847f8f94-5108-47a3-bb08-c5f50b64e6e6-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-07-25 13:34 ` Robin Murphy
2018-07-25 13:34 ` Robin Murphy
2018-07-13 17:05 ` [PATCH 0/3] SOCFPGA Stratix10 SMMU Support Robin Murphy
2018-07-13 17:05 ` Robin Murphy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.