* [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-03-09 15:35 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: linux-arm-kernel
This series provides the support for turning on the arm-smmu's
clocks/power domains using runtime pm. This is done using the
recently introduced device links patches, which lets the symmu's
runtime to follow the master's runtime pm, so the smmu remains
powered only when the masters use it.
Took some reference from the exynos runtime patches [2].
Tested this with MDP, GPU, VENUS devices on apq8096-db820c board.
Previous version of the patchset [1].
[V3]
* Reworked the patches to keep the clocks init/enabling function
seperately for each compatible.
* Added clocks bindings for MMU40x/500.
* Added a new compatible for qcom,smmu-v2 implementation and
the clock bindings for the same.
* Rebased on top of 4.11-rc1
[V2]
* Split the patches little differently.
* Addressed comments.
* Removed the patch #4 [3] from previous post
for arm-smmu context save restore. Planning to
post this separately after reworking/addressing Robin's
feedback.
* Reversed the sequence to disable clocks than enabling.
This was required for those cases where the
clocks are populated in a dependent order from DT.
[1] https://www.spinics.net/lists/linux-arm-msm/msg23870.html
[2] https://lkml.org/lkml/2016/10/20/70
[3] https://patchwork.kernel.org/patch/9389717/
Sricharan R (5):
iommu/arm-smmu: Add pm_runtime/sleep ops
iommu/arm-smmu: Add support for MMU40x/500 clocks
drivers: arm-smmu: Add clock support for QCOM_SMMUV2
iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
iommu/arm-smmu: Add the device_link between masters and smmu
.../devicetree/bindings/iommu/arm,smmu.txt | 35 +++
drivers/iommu/arm-smmu.c | 349 ++++++++++++++++++++-
2 files changed, 373 insertions(+), 11 deletions(-)
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH V3 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops
2017-03-09 15:35 ` Sricharan R
@ 2017-03-09 15:35 ` Sricharan R
-1 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: robin.murphy, will.deacon, joro, iommu, linux-arm-kernel,
linux-arm-msm, m.szyprowski, linux-clk, sboyd, devicetree,
robh+dt, mathieu.poirier, mark.rutland
Cc: sricharan
The smmu needs to be functional only when the respective
master's using it are active. The device_link feature
helps to track such functional dependencies, so that the
iommu gets powered when the master device enables itself
using pm_runtime. So by adapting the smmu driver for
runtime pm, above said dependency can be addressed.
This patch adds the pm runtime/sleep callbacks to the
driver and also the functions to parse the smmu clocks
from DT and enable them in resume/suspend.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
drivers/iommu/arm-smmu.c | 74 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 65 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index abf6496..f7e11d3 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,6 +48,7 @@
#include <linux/of_iommu.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
@@ -340,6 +341,13 @@ struct arm_smmu_master_cfg {
#define for_each_cfg_sme(fw, i, idx) \
for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
+struct arm_smmu_clks {
+ void *clks;
+ int (*init_clocks)(struct arm_smmu_device *smmu);
+ int (*enable_clocks)(struct arm_smmu_device *smmu);
+ void (*disable_clocks)(struct arm_smmu_device *smmu);
+};
+
struct arm_smmu_device {
struct device *dev;
@@ -387,7 +395,7 @@ struct arm_smmu_device {
u32 num_global_irqs;
u32 num_context_irqs;
unsigned int *irqs;
-
+ struct arm_smmu_clks smmu_clks;
u32 cavium_id_base; /* Specific to Cavium */
/* IOMMU core code handle */
@@ -1958,16 +1966,24 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
struct arm_smmu_match_data {
enum arm_smmu_arch_version version;
enum arm_smmu_implementation model;
+ struct arm_smmu_clks smmu_clks;
};
-#define ARM_SMMU_MATCH_DATA(name, ver, imp) \
-static struct arm_smmu_match_data name = { .version = ver, .model = imp }
-
-ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
-ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
-ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU);
-ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500);
-ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2);
+#define ARM_SMMU_MATCH_DATA(name, ver, imp, init, enable, disable) \
+static struct arm_smmu_match_data name = { .version = ver, .model = imp, \
+ .smmu_clks.init_clocks = init, \
+ .smmu_clks.enable_clocks = enable, \
+ .smmu_clks.disable_clocks = disable }
+
+ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU,
+ NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU,
+ NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
+ NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
+ NULL, NULL, NULL);
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = &smmu_generic_v1 },
@@ -2054,6 +2070,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
data = of_device_get_match_data(dev);
smmu->version = data->version;
smmu->model = data->model;
+ smmu->smmu_clks = data->smmu_clks;
parse_driver_options(smmu);
@@ -2135,6 +2152,13 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
smmu->irqs[i] = irq;
}
+ if (smmu->smmu_clks.init_clocks) {
+ err = smmu->smmu_clks.init_clocks(smmu);
+ if (err)
+ return err;
+ }
+
+ pm_runtime_enable(dev);
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;
@@ -2211,10 +2235,42 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM
+static int arm_smmu_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
+ int ret = 0;
+
+ if (smmu->smmu_clks.enable_clocks)
+ ret = smmu->smmu_clks.enable_clocks(smmu);
+
+ return ret;
+}
+
+static int arm_smmu_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
+
+ if (smmu->smmu_clks.disable_clocks)
+ smmu->smmu_clks.disable_clocks(smmu);
+
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops arm_smmu_pm_ops = {
+ SET_RUNTIME_PM_OPS(arm_smmu_suspend, arm_smmu_resume, NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+};
+
static struct platform_driver arm_smmu_driver = {
.driver = {
.name = "arm-smmu",
.of_match_table = of_match_ptr(arm_smmu_of_match),
+ .pm = &arm_smmu_pm_ops,
},
.probe = arm_smmu_device_probe,
.remove = arm_smmu_device_remove,
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH V3 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops
@ 2017-03-09 15:35 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: linux-arm-kernel
The smmu needs to be functional only when the respective
master's using it are active. The device_link feature
helps to track such functional dependencies, so that the
iommu gets powered when the master device enables itself
using pm_runtime. So by adapting the smmu driver for
runtime pm, above said dependency can be addressed.
This patch adds the pm runtime/sleep callbacks to the
driver and also the functions to parse the smmu clocks
from DT and enable them in resume/suspend.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
drivers/iommu/arm-smmu.c | 74 ++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 65 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index abf6496..f7e11d3 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,6 +48,7 @@
#include <linux/of_iommu.h>
#include <linux/pci.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
@@ -340,6 +341,13 @@ struct arm_smmu_master_cfg {
#define for_each_cfg_sme(fw, i, idx) \
for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
+struct arm_smmu_clks {
+ void *clks;
+ int (*init_clocks)(struct arm_smmu_device *smmu);
+ int (*enable_clocks)(struct arm_smmu_device *smmu);
+ void (*disable_clocks)(struct arm_smmu_device *smmu);
+};
+
struct arm_smmu_device {
struct device *dev;
@@ -387,7 +395,7 @@ struct arm_smmu_device {
u32 num_global_irqs;
u32 num_context_irqs;
unsigned int *irqs;
-
+ struct arm_smmu_clks smmu_clks;
u32 cavium_id_base; /* Specific to Cavium */
/* IOMMU core code handle */
@@ -1958,16 +1966,24 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
struct arm_smmu_match_data {
enum arm_smmu_arch_version version;
enum arm_smmu_implementation model;
+ struct arm_smmu_clks smmu_clks;
};
-#define ARM_SMMU_MATCH_DATA(name, ver, imp) \
-static struct arm_smmu_match_data name = { .version = ver, .model = imp }
-
-ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
-ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
-ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU);
-ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500);
-ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2);
+#define ARM_SMMU_MATCH_DATA(name, ver, imp, init, enable, disable) \
+static struct arm_smmu_match_data name = { .version = ver, .model = imp, \
+ .smmu_clks.init_clocks = init, \
+ .smmu_clks.enable_clocks = enable, \
+ .smmu_clks.disable_clocks = disable }
+
+ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU,
+ NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU,
+ NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
+ NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
+ NULL, NULL, NULL);
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = &smmu_generic_v1 },
@@ -2054,6 +2070,7 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
data = of_device_get_match_data(dev);
smmu->version = data->version;
smmu->model = data->model;
+ smmu->smmu_clks = data->smmu_clks;
parse_driver_options(smmu);
@@ -2135,6 +2152,13 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
smmu->irqs[i] = irq;
}
+ if (smmu->smmu_clks.init_clocks) {
+ err = smmu->smmu_clks.init_clocks(smmu);
+ if (err)
+ return err;
+ }
+
+ pm_runtime_enable(dev);
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;
@@ -2211,10 +2235,42 @@ static int arm_smmu_device_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM
+static int arm_smmu_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
+ int ret = 0;
+
+ if (smmu->smmu_clks.enable_clocks)
+ ret = smmu->smmu_clks.enable_clocks(smmu);
+
+ return ret;
+}
+
+static int arm_smmu_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
+
+ if (smmu->smmu_clks.disable_clocks)
+ smmu->smmu_clks.disable_clocks(smmu);
+
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops arm_smmu_pm_ops = {
+ SET_RUNTIME_PM_OPS(arm_smmu_suspend, arm_smmu_resume, NULL)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
+};
+
static struct platform_driver arm_smmu_driver = {
.driver = {
.name = "arm-smmu",
.of_match_table = of_match_ptr(arm_smmu_of_match),
+ .pm = &arm_smmu_pm_ops,
},
.probe = arm_smmu_device_probe,
.remove = arm_smmu_device_remove,
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
2017-03-09 15:35 ` Sricharan R
@ 2017-03-09 15:35 ` Sricharan R
-1 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: robin.murphy, will.deacon, joro, iommu, linux-arm-kernel,
linux-arm-msm, m.szyprowski, linux-clk, sboyd, devicetree,
robh+dt, mathieu.poirier, mark.rutland
Cc: sricharan
The MMU400x/500 is the implementation of the SMMUv2
arch specification. It is split in to two blocks
TBU, TCU. TBU caches the page table, instantiated
for each master locally, clocked by the TBUn_clk.
TCU manages the address translation with PTW and has
the programming interface as well, clocked using the
TCU_CLK. The TBU can also be sharing the same clock
domain as TCU, in which case both are clocked using
the TCU_CLK.
This defines the clock bindings for the same and adds the
init, enable and disable functions for handling the
clocks.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
.../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
2 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 6cdf32d..b369c13 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -60,6 +60,28 @@ conditions.
aliases of secure registers have to be used during
SMMU configuration.
+- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
+ "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
+
+ "tcu_clk" is required for smmu's register access using the
+ programming interface and ptw for downstream bus access.
+
+ "tbu_clk" is required for access to the TBU connected to the
+ master locally. This clock is optional and not required when
+ TBU is in the same clock domain as the TCU or when the TBU is
+ clocked along with the master.
+
+ "cfg_clk" is optional if required to access the TCU's programming
+ interface, apart from the "tcu_clk".
+
+- clocks: Phandles for respective clocks described by clock-names.
+
+- power-domains: Phandles to SMMU's power domain specifier. This is
+ required even if SMMU belongs to the master's power
+ domain, as the SMMU will have to be enabled and
+ accessed before master gets enabled and linked to its
+ SMMU.
+
** Deprecated properties:
- mmu-masters (deprecated in favour of the generic "iommus" binding) :
@@ -84,6 +106,11 @@ conditions.
<0 36 4>,
<0 37 4>;
#iommu-cells = <1>;
+ clocks = <&gcc GCC_SMMU_CFG_CLK>,
+ <&gcc GCC_APSS_TCU_CLK>,
+ <&gcc GCC_MDP_TBU_CLK>;
+
+ clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
};
/* device with two stream IDs, 0 and 7 */
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f7e11d3..720a1ef 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
#define for_each_cfg_sme(fw, i, idx) \
for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
+struct mmu500_clk {
+ struct clk *cfg_clk;
+ struct clk *tcu_clk;
+ struct clk *tbu_clk;
+};
+
struct arm_smmu_clks {
void *clks;
int (*init_clocks)(struct arm_smmu_device *smmu);
@@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
return container_of(dom, struct arm_smmu_domain, domain);
}
+static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
+{
+ int ret = 0;
+ struct mmu500_clk *sclks = smmu->smmu_clks.clks;
+
+ if (!sclks)
+ return 0;
+
+ ret = clk_prepare_enable(sclks->cfg_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couldn't enable cfg_clk");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(sclks->tcu_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couldn't enable tcu_clk");
+ clk_disable_unprepare(sclks->cfg_clk);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(sclks->tbu_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable tbu_clk");
+ clk_disable_unprepare(sclks->tcu_clk);
+ clk_disable_unprepare(sclks->cfg_clk);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
+{
+ struct mmu500_clk *sclks = smmu->smmu_clks.clks;
+
+ if (!sclks) {
+ clk_disable_unprepare(sclks->tbu_clk);
+ clk_disable_unprepare(sclks->tcu_clk);
+ clk_disable_unprepare(sclks->cfg_clk);
+ }
+}
+
+static int mmu500_init_clocks(struct arm_smmu_device *smmu)
+{
+ struct device *dev = smmu->dev;
+ struct mmu500_clk *sclks;
+ int err;
+
+ if (!of_find_property(dev->of_node, "clocks", NULL))
+ return 0;
+
+ sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
+ if (!sclks)
+ return -ENOMEM;
+
+ sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
+ if (IS_ERR(sclks->cfg_clk)) {
+ err = PTR_ERR(sclks->cfg_clk);
+ /* Ignore all, except -EPROBE_DEFER for optional clocks */
+ if (err == -EPROBE_DEFER)
+ return err;
+ else
+ sclks->cfg_clk = NULL;
+ }
+
+ sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
+ if (IS_ERR(sclks->tcu_clk)) {
+ dev_err(dev, "Couldn't get tcu_clk");
+ return PTR_ERR(sclks->tcu_clk);
+ }
+
+ sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
+ if (IS_ERR(sclks->tbu_clk)) {
+ err = PTR_ERR(sclks->tbu_clk);
+ /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
+ if (err == -EPROBE_DEFER)
+ return err;
+ else
+ sclks->tbu_clk = NULL;
+ }
+
+ smmu->smmu_clks.clks = sclks;
+ return 0;
+}
+
static void parse_driver_options(struct arm_smmu_device *smmu)
{
int i = 0;
@@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
NULL, NULL, NULL);
ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
NULL, NULL, NULL);
-ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
+ mmu500_enable_clocks, mmu500_disable_clocks);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
NULL, NULL, NULL);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
@ 2017-03-09 15:35 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: linux-arm-kernel
The MMU400x/500 is the implementation of the SMMUv2
arch specification. It is split in to two blocks
TBU, TCU. TBU caches the page table, instantiated
for each master locally, clocked by the TBUn_clk.
TCU manages the address translation with PTW and has
the programming interface as well, clocked using the
TCU_CLK. The TBU can also be sharing the same clock
domain as TCU, in which case both are clocked using
the TCU_CLK.
This defines the clock bindings for the same and adds the
init, enable and disable functions for handling the
clocks.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
.../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
2 files changed, 121 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index 6cdf32d..b369c13 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -60,6 +60,28 @@ conditions.
aliases of secure registers have to be used during
SMMU configuration.
+- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
+ "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
+
+ "tcu_clk" is required for smmu's register access using the
+ programming interface and ptw for downstream bus access.
+
+ "tbu_clk" is required for access to the TBU connected to the
+ master locally. This clock is optional and not required when
+ TBU is in the same clock domain as the TCU or when the TBU is
+ clocked along with the master.
+
+ "cfg_clk" is optional if required to access the TCU's programming
+ interface, apart from the "tcu_clk".
+
+- clocks: Phandles for respective clocks described by clock-names.
+
+- power-domains: Phandles to SMMU's power domain specifier. This is
+ required even if SMMU belongs to the master's power
+ domain, as the SMMU will have to be enabled and
+ accessed before master gets enabled and linked to its
+ SMMU.
+
** Deprecated properties:
- mmu-masters (deprecated in favour of the generic "iommus" binding) :
@@ -84,6 +106,11 @@ conditions.
<0 36 4>,
<0 37 4>;
#iommu-cells = <1>;
+ clocks = <&gcc GCC_SMMU_CFG_CLK>,
+ <&gcc GCC_APSS_TCU_CLK>,
+ <&gcc GCC_MDP_TBU_CLK>;
+
+ clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
};
/* device with two stream IDs, 0 and 7 */
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f7e11d3..720a1ef 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
#define for_each_cfg_sme(fw, i, idx) \
for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
+struct mmu500_clk {
+ struct clk *cfg_clk;
+ struct clk *tcu_clk;
+ struct clk *tbu_clk;
+};
+
struct arm_smmu_clks {
void *clks;
int (*init_clocks)(struct arm_smmu_device *smmu);
@@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
return container_of(dom, struct arm_smmu_domain, domain);
}
+static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
+{
+ int ret = 0;
+ struct mmu500_clk *sclks = smmu->smmu_clks.clks;
+
+ if (!sclks)
+ return 0;
+
+ ret = clk_prepare_enable(sclks->cfg_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couldn't enable cfg_clk");
+ return ret;
+ }
+
+ ret = clk_prepare_enable(sclks->tcu_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couldn't enable tcu_clk");
+ clk_disable_unprepare(sclks->cfg_clk);
+ return ret;
+ }
+
+ ret = clk_prepare_enable(sclks->tbu_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable tbu_clk");
+ clk_disable_unprepare(sclks->tcu_clk);
+ clk_disable_unprepare(sclks->cfg_clk);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
+{
+ struct mmu500_clk *sclks = smmu->smmu_clks.clks;
+
+ if (!sclks) {
+ clk_disable_unprepare(sclks->tbu_clk);
+ clk_disable_unprepare(sclks->tcu_clk);
+ clk_disable_unprepare(sclks->cfg_clk);
+ }
+}
+
+static int mmu500_init_clocks(struct arm_smmu_device *smmu)
+{
+ struct device *dev = smmu->dev;
+ struct mmu500_clk *sclks;
+ int err;
+
+ if (!of_find_property(dev->of_node, "clocks", NULL))
+ return 0;
+
+ sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
+ if (!sclks)
+ return -ENOMEM;
+
+ sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
+ if (IS_ERR(sclks->cfg_clk)) {
+ err = PTR_ERR(sclks->cfg_clk);
+ /* Ignore all, except -EPROBE_DEFER for optional clocks */
+ if (err == -EPROBE_DEFER)
+ return err;
+ else
+ sclks->cfg_clk = NULL;
+ }
+
+ sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
+ if (IS_ERR(sclks->tcu_clk)) {
+ dev_err(dev, "Couldn't get tcu_clk");
+ return PTR_ERR(sclks->tcu_clk);
+ }
+
+ sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
+ if (IS_ERR(sclks->tbu_clk)) {
+ err = PTR_ERR(sclks->tbu_clk);
+ /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
+ if (err == -EPROBE_DEFER)
+ return err;
+ else
+ sclks->tbu_clk = NULL;
+ }
+
+ smmu->smmu_clks.clks = sclks;
+ return 0;
+}
+
static void parse_driver_options(struct arm_smmu_device *smmu)
{
int i = 0;
@@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
NULL, NULL, NULL);
ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
NULL, NULL, NULL);
-ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
+ mmu500_enable_clocks, mmu500_disable_clocks);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
NULL, NULL, NULL);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 47+ messages in thread[parent not found: <1489073748-3659-3-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* Re: [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
2017-03-09 15:35 ` Sricharan R
(?)
@ 2017-03-16 21:03 ` Rob Herring
-1 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2017-03-16 21:03 UTC (permalink / raw)
To: Sricharan R
Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Thu, Mar 09, 2017 at 09:05:45PM +0530, Sricharan R wrote:
> The MMU400x/500 is the implementation of the SMMUv2
> arch specification. It is split in to two blocks
> TBU, TCU. TBU caches the page table, instantiated
> for each master locally, clocked by the TBUn_clk.
> TCU manages the address translation with PTW and has
> the programming interface as well, clocked using the
> TCU_CLK. The TBU can also be sharing the same clock
> domain as TCU, in which case both are clocked using
> the TCU_CLK.
>
> This defines the clock bindings for the same and adds the
> init, enable and disable functions for handling the
> clocks.
>
> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
> 2 files changed, 121 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 6cdf32d..b369c13 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -60,6 +60,28 @@ conditions.
> aliases of secure registers have to be used during
> SMMU configuration.
>
> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
_clk is redundant
> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
> +
> + "tcu_clk" is required for smmu's register access using the
> + programming interface and ptw for downstream bus access.
> +
> + "tbu_clk" is required for access to the TBU connected to the
> + master locally. This clock is optional and not required when
> + TBU is in the same clock domain as the TCU or when the TBU is
> + clocked along with the master.
> +
> + "cfg_clk" is optional if required to access the TCU's programming
> + interface, apart from the "tcu_clk".
Clocks generally shouldn't be optional. Either the h/w has a clock or it
doesn't.
> +
> +- clocks: Phandles for respective clocks described by clock-names.
> +
> +- power-domains: Phandles to SMMU's power domain specifier. This is
> + required even if SMMU belongs to the master's power
> + domain, as the SMMU will have to be enabled and
> + accessed before master gets enabled and linked to its
> + SMMU.
> +
> ** Deprecated properties:
>
> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> @@ -84,6 +106,11 @@ conditions.
> <0 36 4>,
> <0 37 4>;
> #iommu-cells = <1>;
> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
> + <&gcc GCC_APSS_TCU_CLK>,
> + <&gcc GCC_MDP_TBU_CLK>;
> +
> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
> };
>
> /* device with two stream IDs, 0 and 7 */
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f7e11d3..720a1ef 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
> #define for_each_cfg_sme(fw, i, idx) \
> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>
> +struct mmu500_clk {
> + struct clk *cfg_clk;
> + struct clk *tcu_clk;
> + struct clk *tbu_clk;
> +};
> +
> struct arm_smmu_clks {
> void *clks;
> int (*init_clocks)(struct arm_smmu_device *smmu);
> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> return container_of(dom, struct arm_smmu_domain, domain);
> }
>
> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
> +{
> + int ret = 0;
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks)
> + return 0;
> +
> + ret = clk_prepare_enable(sclks->cfg_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tcu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tbu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couln't enable tbu_clk");
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
> +{
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks) {
> + clk_disable_unprepare(sclks->tbu_clk);
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + }
> +}
> +
> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
> +{
> + struct device *dev = smmu->dev;
> + struct mmu500_clk *sclks;
> + int err;
> +
> + if (!of_find_property(dev->of_node, "clocks", NULL))
> + return 0;
> +
> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
> + if (!sclks)
> + return -ENOMEM;
> +
> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
> + if (IS_ERR(sclks->cfg_clk)) {
> + err = PTR_ERR(sclks->cfg_clk);
> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->cfg_clk = NULL;
> + }
> +
> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
> + if (IS_ERR(sclks->tcu_clk)) {
> + dev_err(dev, "Couldn't get tcu_clk");
> + return PTR_ERR(sclks->tcu_clk);
> + }
> +
> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
> + if (IS_ERR(sclks->tbu_clk)) {
> + err = PTR_ERR(sclks->tbu_clk);
> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->tbu_clk = NULL;
> + }
> +
> + smmu->smmu_clks.clks = sclks;
> + return 0;
> +}
> +
> static void parse_driver_options(struct arm_smmu_device *smmu)
> {
> int i = 0;
> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
> NULL, NULL, NULL);
> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
> NULL, NULL, NULL);
> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
> + mmu500_enable_clocks, mmu500_disable_clocks);
> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
> NULL, NULL, NULL);
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
@ 2017-03-16 21:03 ` Rob Herring
0 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2017-03-16 21:03 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 09, 2017 at 09:05:45PM +0530, Sricharan R wrote:
> The MMU400x/500 is the implementation of the SMMUv2
> arch specification. It is split in to two blocks
> TBU, TCU. TBU caches the page table, instantiated
> for each master locally, clocked by the TBUn_clk.
> TCU manages the address translation with PTW and has
> the programming interface as well, clocked using the
> TCU_CLK. The TBU can also be sharing the same clock
> domain as TCU, in which case both are clocked using
> the TCU_CLK.
>
> This defines the clock bindings for the same and adds the
> init, enable and disable functions for handling the
> clocks.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
> 2 files changed, 121 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 6cdf32d..b369c13 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -60,6 +60,28 @@ conditions.
> aliases of secure registers have to be used during
> SMMU configuration.
>
> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
_clk is redundant
> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
> +
> + "tcu_clk" is required for smmu's register access using the
> + programming interface and ptw for downstream bus access.
> +
> + "tbu_clk" is required for access to the TBU connected to the
> + master locally. This clock is optional and not required when
> + TBU is in the same clock domain as the TCU or when the TBU is
> + clocked along with the master.
> +
> + "cfg_clk" is optional if required to access the TCU's programming
> + interface, apart from the "tcu_clk".
Clocks generally shouldn't be optional. Either the h/w has a clock or it
doesn't.
> +
> +- clocks: Phandles for respective clocks described by clock-names.
> +
> +- power-domains: Phandles to SMMU's power domain specifier. This is
> + required even if SMMU belongs to the master's power
> + domain, as the SMMU will have to be enabled and
> + accessed before master gets enabled and linked to its
> + SMMU.
> +
> ** Deprecated properties:
>
> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> @@ -84,6 +106,11 @@ conditions.
> <0 36 4>,
> <0 37 4>;
> #iommu-cells = <1>;
> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
> + <&gcc GCC_APSS_TCU_CLK>,
> + <&gcc GCC_MDP_TBU_CLK>;
> +
> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
> };
>
> /* device with two stream IDs, 0 and 7 */
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f7e11d3..720a1ef 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
> #define for_each_cfg_sme(fw, i, idx) \
> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>
> +struct mmu500_clk {
> + struct clk *cfg_clk;
> + struct clk *tcu_clk;
> + struct clk *tbu_clk;
> +};
> +
> struct arm_smmu_clks {
> void *clks;
> int (*init_clocks)(struct arm_smmu_device *smmu);
> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> return container_of(dom, struct arm_smmu_domain, domain);
> }
>
> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
> +{
> + int ret = 0;
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks)
> + return 0;
> +
> + ret = clk_prepare_enable(sclks->cfg_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tcu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tbu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couln't enable tbu_clk");
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
> +{
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks) {
> + clk_disable_unprepare(sclks->tbu_clk);
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + }
> +}
> +
> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
> +{
> + struct device *dev = smmu->dev;
> + struct mmu500_clk *sclks;
> + int err;
> +
> + if (!of_find_property(dev->of_node, "clocks", NULL))
> + return 0;
> +
> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
> + if (!sclks)
> + return -ENOMEM;
> +
> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
> + if (IS_ERR(sclks->cfg_clk)) {
> + err = PTR_ERR(sclks->cfg_clk);
> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->cfg_clk = NULL;
> + }
> +
> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
> + if (IS_ERR(sclks->tcu_clk)) {
> + dev_err(dev, "Couldn't get tcu_clk");
> + return PTR_ERR(sclks->tcu_clk);
> + }
> +
> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
> + if (IS_ERR(sclks->tbu_clk)) {
> + err = PTR_ERR(sclks->tbu_clk);
> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->tbu_clk = NULL;
> + }
> +
> + smmu->smmu_clks.clks = sclks;
> + return 0;
> +}
> +
> static void parse_driver_options(struct arm_smmu_device *smmu)
> {
> int i = 0;
> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
> NULL, NULL, NULL);
> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
> NULL, NULL, NULL);
> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
> + mmu500_enable_clocks, mmu500_disable_clocks);
> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
> NULL, NULL, NULL);
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
@ 2017-03-16 21:03 ` Rob Herring
0 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2017-03-16 21:03 UTC (permalink / raw)
To: Sricharan R
Cc: robin.murphy, will.deacon, joro, iommu, linux-arm-kernel,
linux-arm-msm, m.szyprowski, linux-clk, sboyd, devicetree,
mathieu.poirier, mark.rutland
On Thu, Mar 09, 2017 at 09:05:45PM +0530, Sricharan R wrote:
> The MMU400x/500 is the implementation of the SMMUv2
> arch specification. It is split in to two blocks
> TBU, TCU. TBU caches the page table, instantiated
> for each master locally, clocked by the TBUn_clk.
> TCU manages the address translation with PTW and has
> the programming interface as well, clocked using the
> TCU_CLK. The TBU can also be sharing the same clock
> domain as TCU, in which case both are clocked using
> the TCU_CLK.
>
> This defines the clock bindings for the same and adds the
> init, enable and disable functions for handling the
> clocks.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
> 2 files changed, 121 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 6cdf32d..b369c13 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -60,6 +60,28 @@ conditions.
> aliases of secure registers have to be used during
> SMMU configuration.
>
> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
_clk is redundant
> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
> +
> + "tcu_clk" is required for smmu's register access using the
> + programming interface and ptw for downstream bus access.
> +
> + "tbu_clk" is required for access to the TBU connected to the
> + master locally. This clock is optional and not required when
> + TBU is in the same clock domain as the TCU or when the TBU is
> + clocked along with the master.
> +
> + "cfg_clk" is optional if required to access the TCU's programming
> + interface, apart from the "tcu_clk".
Clocks generally shouldn't be optional. Either the h/w has a clock or it
doesn't.
> +
> +- clocks: Phandles for respective clocks described by clock-names.
> +
> +- power-domains: Phandles to SMMU's power domain specifier. This is
> + required even if SMMU belongs to the master's power
> + domain, as the SMMU will have to be enabled and
> + accessed before master gets enabled and linked to its
> + SMMU.
> +
> ** Deprecated properties:
>
> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> @@ -84,6 +106,11 @@ conditions.
> <0 36 4>,
> <0 37 4>;
> #iommu-cells = <1>;
> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
> + <&gcc GCC_APSS_TCU_CLK>,
> + <&gcc GCC_MDP_TBU_CLK>;
> +
> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
> };
>
> /* device with two stream IDs, 0 and 7 */
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f7e11d3..720a1ef 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
> #define for_each_cfg_sme(fw, i, idx) \
> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>
> +struct mmu500_clk {
> + struct clk *cfg_clk;
> + struct clk *tcu_clk;
> + struct clk *tbu_clk;
> +};
> +
> struct arm_smmu_clks {
> void *clks;
> int (*init_clocks)(struct arm_smmu_device *smmu);
> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> return container_of(dom, struct arm_smmu_domain, domain);
> }
>
> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
> +{
> + int ret = 0;
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks)
> + return 0;
> +
> + ret = clk_prepare_enable(sclks->cfg_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tcu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tbu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couln't enable tbu_clk");
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
> +{
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks) {
> + clk_disable_unprepare(sclks->tbu_clk);
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + }
> +}
> +
> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
> +{
> + struct device *dev = smmu->dev;
> + struct mmu500_clk *sclks;
> + int err;
> +
> + if (!of_find_property(dev->of_node, "clocks", NULL))
> + return 0;
> +
> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
> + if (!sclks)
> + return -ENOMEM;
> +
> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
> + if (IS_ERR(sclks->cfg_clk)) {
> + err = PTR_ERR(sclks->cfg_clk);
> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->cfg_clk = NULL;
> + }
> +
> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
> + if (IS_ERR(sclks->tcu_clk)) {
> + dev_err(dev, "Couldn't get tcu_clk");
> + return PTR_ERR(sclks->tcu_clk);
> + }
> +
> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
> + if (IS_ERR(sclks->tbu_clk)) {
> + err = PTR_ERR(sclks->tbu_clk);
> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->tbu_clk = NULL;
> + }
> +
> + smmu->smmu_clks.clks = sclks;
> + return 0;
> +}
> +
> static void parse_driver_options(struct arm_smmu_device *smmu)
> {
> int i = 0;
> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
> NULL, NULL, NULL);
> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
> NULL, NULL, NULL);
> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
> + mmu500_enable_clocks, mmu500_disable_clocks);
> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
> NULL, NULL, NULL);
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
2017-03-16 21:03 ` Rob Herring
@ 2017-03-20 14:48 ` Sricharan R
-1 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-20 14:48 UTC (permalink / raw)
To: Rob Herring
Cc: robin.murphy, will.deacon, joro, iommu, linux-arm-kernel,
linux-arm-msm, m.szyprowski, linux-clk, sboyd, devicetree,
mathieu.poirier, mark.rutland
Hi Rob,
On 3/17/2017 2:33 AM, Rob Herring wrote:
> On Thu, Mar 09, 2017 at 09:05:45PM +0530, Sricharan R wrote:
>> The MMU400x/500 is the implementation of the SMMUv2
>> arch specification. It is split in to two blocks
>> TBU, TCU. TBU caches the page table, instantiated
>> for each master locally, clocked by the TBUn_clk.
>> TCU manages the address translation with PTW and has
>> the programming interface as well, clocked using the
>> TCU_CLK. The TBU can also be sharing the same clock
>> domain as TCU, in which case both are clocked using
>> the TCU_CLK.
>>
>> This defines the clock bindings for the same and adds the
>> init, enable and disable functions for handling the
>> clocks.
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
>> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
>> 2 files changed, 121 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 6cdf32d..b369c13 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -60,6 +60,28 @@ conditions.
>> aliases of secure registers have to be used during
>> SMMU configuration.
>>
>> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
>
> _clk is redundant
ok, will remove it.
>
>> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
>> +
>> + "tcu_clk" is required for smmu's register access using the
>> + programming interface and ptw for downstream bus access.
>> +
>> + "tbu_clk" is required for access to the TBU connected to the
>> + master locally. This clock is optional and not required when
>> + TBU is in the same clock domain as the TCU or when the TBU is
>> + clocked along with the master.
>> +
>> + "cfg_clk" is optional if required to access the TCU's programming
>> + interface, apart from the "tcu_clk".
>
> Clocks generally shouldn't be optional. Either the h/w has a clock or it
> doesn't.
>
ok, will not define it as 'optional'. I will define it to be soc
specific. Also will change the driver to ignore the clk not present,
just to reuse/have common clock enable functions for MMU-500
for other socs as well. Only problem being unable to catch a case
where a clk DT entry is not populated when it should have been.
Regards,
Sricharan
>> +
>> +- clocks: Phandles for respective clocks described by clock-names.
>> +
>> +- power-domains: Phandles to SMMU's power domain specifier. This is
>> + required even if SMMU belongs to the master's power
>> + domain, as the SMMU will have to be enabled and
>> + accessed before master gets enabled and linked to its
>> + SMMU.
>> +
>> ** Deprecated properties:
>>
>> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
>> @@ -84,6 +106,11 @@ conditions.
>> <0 36 4>,
>> <0 37 4>;
>> #iommu-cells = <1>;
>> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
>> + <&gcc GCC_APSS_TCU_CLK>,
>> + <&gcc GCC_MDP_TBU_CLK>;
>> +
>> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
>> };
>>
>> /* device with two stream IDs, 0 and 7 */
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index f7e11d3..720a1ef 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
>> #define for_each_cfg_sme(fw, i, idx) \
>> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>>
>> +struct mmu500_clk {
>> + struct clk *cfg_clk;
>> + struct clk *tcu_clk;
>> + struct clk *tbu_clk;
>> +};
>> +
>> struct arm_smmu_clks {
>> void *clks;
>> int (*init_clocks)(struct arm_smmu_device *smmu);
>> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>> return container_of(dom, struct arm_smmu_domain, domain);
>> }
>>
>> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
>> +{
>> + int ret = 0;
>> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
>> +
>> + if (!sclks)
>> + return 0;
>> +
>> + ret = clk_prepare_enable(sclks->cfg_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(sclks->tcu_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(sclks->tbu_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couln't enable tbu_clk");
>> + clk_disable_unprepare(sclks->tcu_clk);
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
>> +{
>> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
>> +
>> + if (!sclks) {
>> + clk_disable_unprepare(sclks->tbu_clk);
>> + clk_disable_unprepare(sclks->tcu_clk);
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + }
>> +}
>> +
>> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
>> +{
>> + struct device *dev = smmu->dev;
>> + struct mmu500_clk *sclks;
>> + int err;
>> +
>> + if (!of_find_property(dev->of_node, "clocks", NULL))
>> + return 0;
>> +
>> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
>> + if (!sclks)
>> + return -ENOMEM;
>> +
>> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
>> + if (IS_ERR(sclks->cfg_clk)) {
>> + err = PTR_ERR(sclks->cfg_clk);
>> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
>> + if (err == -EPROBE_DEFER)
>> + return err;
>> + else
>> + sclks->cfg_clk = NULL;
>> + }
>> +
>> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
>> + if (IS_ERR(sclks->tcu_clk)) {
>> + dev_err(dev, "Couldn't get tcu_clk");
>> + return PTR_ERR(sclks->tcu_clk);
>> + }
>> +
>> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
>> + if (IS_ERR(sclks->tbu_clk)) {
>> + err = PTR_ERR(sclks->tbu_clk);
>> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
>> + if (err == -EPROBE_DEFER)
>> + return err;
>> + else
>> + sclks->tbu_clk = NULL;
>> + }
>> +
>> + smmu->smmu_clks.clks = sclks;
>> + return 0;
>> +}
>> +
>> static void parse_driver_options(struct arm_smmu_device *smmu)
>> {
>> int i = 0;
>> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
>> NULL, NULL, NULL);
>> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
>> NULL, NULL, NULL);
>> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
>> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
>> + mmu500_enable_clocks, mmu500_disable_clocks);
>> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
>> NULL, NULL, NULL);
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
@ 2017-03-20 14:48 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-20 14:48 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rob,
On 3/17/2017 2:33 AM, Rob Herring wrote:
> On Thu, Mar 09, 2017 at 09:05:45PM +0530, Sricharan R wrote:
>> The MMU400x/500 is the implementation of the SMMUv2
>> arch specification. It is split in to two blocks
>> TBU, TCU. TBU caches the page table, instantiated
>> for each master locally, clocked by the TBUn_clk.
>> TCU manages the address translation with PTW and has
>> the programming interface as well, clocked using the
>> TCU_CLK. The TBU can also be sharing the same clock
>> domain as TCU, in which case both are clocked using
>> the TCU_CLK.
>>
>> This defines the clock bindings for the same and adds the
>> init, enable and disable functions for handling the
>> clocks.
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
>> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
>> 2 files changed, 121 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 6cdf32d..b369c13 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -60,6 +60,28 @@ conditions.
>> aliases of secure registers have to be used during
>> SMMU configuration.
>>
>> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
>
> _clk is redundant
ok, will remove it.
>
>> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
>> +
>> + "tcu_clk" is required for smmu's register access using the
>> + programming interface and ptw for downstream bus access.
>> +
>> + "tbu_clk" is required for access to the TBU connected to the
>> + master locally. This clock is optional and not required when
>> + TBU is in the same clock domain as the TCU or when the TBU is
>> + clocked along with the master.
>> +
>> + "cfg_clk" is optional if required to access the TCU's programming
>> + interface, apart from the "tcu_clk".
>
> Clocks generally shouldn't be optional. Either the h/w has a clock or it
> doesn't.
>
ok, will not define it as 'optional'. I will define it to be soc
specific. Also will change the driver to ignore the clk not present,
just to reuse/have common clock enable functions for MMU-500
for other socs as well. Only problem being unable to catch a case
where a clk DT entry is not populated when it should have been.
Regards,
Sricharan
>> +
>> +- clocks: Phandles for respective clocks described by clock-names.
>> +
>> +- power-domains: Phandles to SMMU's power domain specifier. This is
>> + required even if SMMU belongs to the master's power
>> + domain, as the SMMU will have to be enabled and
>> + accessed before master gets enabled and linked to its
>> + SMMU.
>> +
>> ** Deprecated properties:
>>
>> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
>> @@ -84,6 +106,11 @@ conditions.
>> <0 36 4>,
>> <0 37 4>;
>> #iommu-cells = <1>;
>> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
>> + <&gcc GCC_APSS_TCU_CLK>,
>> + <&gcc GCC_MDP_TBU_CLK>;
>> +
>> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
>> };
>>
>> /* device with two stream IDs, 0 and 7 */
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index f7e11d3..720a1ef 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
>> #define for_each_cfg_sme(fw, i, idx) \
>> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>>
>> +struct mmu500_clk {
>> + struct clk *cfg_clk;
>> + struct clk *tcu_clk;
>> + struct clk *tbu_clk;
>> +};
>> +
>> struct arm_smmu_clks {
>> void *clks;
>> int (*init_clocks)(struct arm_smmu_device *smmu);
>> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>> return container_of(dom, struct arm_smmu_domain, domain);
>> }
>>
>> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
>> +{
>> + int ret = 0;
>> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
>> +
>> + if (!sclks)
>> + return 0;
>> +
>> + ret = clk_prepare_enable(sclks->cfg_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(sclks->tcu_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(sclks->tbu_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couln't enable tbu_clk");
>> + clk_disable_unprepare(sclks->tcu_clk);
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
>> +{
>> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
>> +
>> + if (!sclks) {
>> + clk_disable_unprepare(sclks->tbu_clk);
>> + clk_disable_unprepare(sclks->tcu_clk);
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + }
>> +}
>> +
>> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
>> +{
>> + struct device *dev = smmu->dev;
>> + struct mmu500_clk *sclks;
>> + int err;
>> +
>> + if (!of_find_property(dev->of_node, "clocks", NULL))
>> + return 0;
>> +
>> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
>> + if (!sclks)
>> + return -ENOMEM;
>> +
>> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
>> + if (IS_ERR(sclks->cfg_clk)) {
>> + err = PTR_ERR(sclks->cfg_clk);
>> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
>> + if (err == -EPROBE_DEFER)
>> + return err;
>> + else
>> + sclks->cfg_clk = NULL;
>> + }
>> +
>> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
>> + if (IS_ERR(sclks->tcu_clk)) {
>> + dev_err(dev, "Couldn't get tcu_clk");
>> + return PTR_ERR(sclks->tcu_clk);
>> + }
>> +
>> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
>> + if (IS_ERR(sclks->tbu_clk)) {
>> + err = PTR_ERR(sclks->tbu_clk);
>> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
>> + if (err == -EPROBE_DEFER)
>> + return err;
>> + else
>> + sclks->tbu_clk = NULL;
>> + }
>> +
>> + smmu->smmu_clks.clks = sclks;
>> + return 0;
>> +}
>> +
>> static void parse_driver_options(struct arm_smmu_device *smmu)
>> {
>> int i = 0;
>> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
>> NULL, NULL, NULL);
>> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
>> NULL, NULL, NULL);
>> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
>> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
>> + mmu500_enable_clocks, mmu500_disable_clocks);
>> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
>> NULL, NULL, NULL);
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
2017-03-09 15:35 ` Sricharan R
(?)
@ 2017-03-16 22:52 ` Rob Clark
-1 siblings, 0 replies; 47+ messages in thread
From: Rob Clark @ 2017-03-16 22:52 UTC (permalink / raw)
To: Sricharan R
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Stephen Boyd,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A, linux-arm-msm,
Will Deacon, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Rob Herring, linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Thu, Mar 9, 2017 at 10:35 AM, Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> The MMU400x/500 is the implementation of the SMMUv2
> arch specification. It is split in to two blocks
> TBU, TCU. TBU caches the page table, instantiated
> for each master locally, clocked by the TBUn_clk.
> TCU manages the address translation with PTW and has
> the programming interface as well, clocked using the
> TCU_CLK. The TBU can also be sharing the same clock
> domain as TCU, in which case both are clocked using
> the TCU_CLK.
>
> This defines the clock bindings for the same and adds the
> init, enable and disable functions for handling the
> clocks.
>
> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
> 2 files changed, 121 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 6cdf32d..b369c13 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -60,6 +60,28 @@ conditions.
> aliases of secure registers have to be used during
> SMMU configuration.
>
> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
I guess that should be: "Should be "tbu_clk" *or* "tcu_clk" and
"cfg_clk" for..."
Also, possibly we should define our own compat strings for various
SoC's that require these clks so we can properly describe when they
are required? I guess that would address Rob H's comment.
BR,
-R
> + "tcu_clk" is required for smmu's register access using the
> + programming interface and ptw for downstream bus access.
> +
> + "tbu_clk" is required for access to the TBU connected to the
> + master locally. This clock is optional and not required when
> + TBU is in the same clock domain as the TCU or when the TBU is
> + clocked along with the master.
> +
> + "cfg_clk" is optional if required to access the TCU's programming
> + interface, apart from the "tcu_clk".
> +
> +- clocks: Phandles for respective clocks described by clock-names.
> +
> +- power-domains: Phandles to SMMU's power domain specifier. This is
> + required even if SMMU belongs to the master's power
> + domain, as the SMMU will have to be enabled and
> + accessed before master gets enabled and linked to its
> + SMMU.
> +
> ** Deprecated properties:
>
> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> @@ -84,6 +106,11 @@ conditions.
> <0 36 4>,
> <0 37 4>;
> #iommu-cells = <1>;
> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
> + <&gcc GCC_APSS_TCU_CLK>,
> + <&gcc GCC_MDP_TBU_CLK>;
> +
> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
> };
>
> /* device with two stream IDs, 0 and 7 */
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f7e11d3..720a1ef 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
> #define for_each_cfg_sme(fw, i, idx) \
> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>
> +struct mmu500_clk {
> + struct clk *cfg_clk;
> + struct clk *tcu_clk;
> + struct clk *tbu_clk;
> +};
> +
> struct arm_smmu_clks {
> void *clks;
> int (*init_clocks)(struct arm_smmu_device *smmu);
> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> return container_of(dom, struct arm_smmu_domain, domain);
> }
>
> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
> +{
> + int ret = 0;
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks)
> + return 0;
> +
> + ret = clk_prepare_enable(sclks->cfg_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tcu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tbu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couln't enable tbu_clk");
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
> +{
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks) {
> + clk_disable_unprepare(sclks->tbu_clk);
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + }
> +}
> +
> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
> +{
> + struct device *dev = smmu->dev;
> + struct mmu500_clk *sclks;
> + int err;
> +
> + if (!of_find_property(dev->of_node, "clocks", NULL))
> + return 0;
> +
> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
> + if (!sclks)
> + return -ENOMEM;
> +
> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
> + if (IS_ERR(sclks->cfg_clk)) {
> + err = PTR_ERR(sclks->cfg_clk);
> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->cfg_clk = NULL;
> + }
> +
> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
> + if (IS_ERR(sclks->tcu_clk)) {
> + dev_err(dev, "Couldn't get tcu_clk");
> + return PTR_ERR(sclks->tcu_clk);
> + }
> +
> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
> + if (IS_ERR(sclks->tbu_clk)) {
> + err = PTR_ERR(sclks->tbu_clk);
> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->tbu_clk = NULL;
> + }
> +
> + smmu->smmu_clks.clks = sclks;
> + return 0;
> +}
> +
> static void parse_driver_options(struct arm_smmu_device *smmu)
> {
> int i = 0;
> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
> NULL, NULL, NULL);
> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
> NULL, NULL, NULL);
> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
> + mmu500_enable_clocks, mmu500_disable_clocks);
> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
> NULL, NULL, NULL);
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
@ 2017-03-16 22:52 ` Rob Clark
0 siblings, 0 replies; 47+ messages in thread
From: Rob Clark @ 2017-03-16 22:52 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 9, 2017 at 10:35 AM, Sricharan R <sricharan@codeaurora.org> wrote:
> The MMU400x/500 is the implementation of the SMMUv2
> arch specification. It is split in to two blocks
> TBU, TCU. TBU caches the page table, instantiated
> for each master locally, clocked by the TBUn_clk.
> TCU manages the address translation with PTW and has
> the programming interface as well, clocked using the
> TCU_CLK. The TBU can also be sharing the same clock
> domain as TCU, in which case both are clocked using
> the TCU_CLK.
>
> This defines the clock bindings for the same and adds the
> init, enable and disable functions for handling the
> clocks.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
> 2 files changed, 121 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 6cdf32d..b369c13 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -60,6 +60,28 @@ conditions.
> aliases of secure registers have to be used during
> SMMU configuration.
>
> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
I guess that should be: "Should be "tbu_clk" *or* "tcu_clk" and
"cfg_clk" for..."
Also, possibly we should define our own compat strings for various
SoC's that require these clks so we can properly describe when they
are required? I guess that would address Rob H's comment.
BR,
-R
> + "tcu_clk" is required for smmu's register access using the
> + programming interface and ptw for downstream bus access.
> +
> + "tbu_clk" is required for access to the TBU connected to the
> + master locally. This clock is optional and not required when
> + TBU is in the same clock domain as the TCU or when the TBU is
> + clocked along with the master.
> +
> + "cfg_clk" is optional if required to access the TCU's programming
> + interface, apart from the "tcu_clk".
> +
> +- clocks: Phandles for respective clocks described by clock-names.
> +
> +- power-domains: Phandles to SMMU's power domain specifier. This is
> + required even if SMMU belongs to the master's power
> + domain, as the SMMU will have to be enabled and
> + accessed before master gets enabled and linked to its
> + SMMU.
> +
> ** Deprecated properties:
>
> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> @@ -84,6 +106,11 @@ conditions.
> <0 36 4>,
> <0 37 4>;
> #iommu-cells = <1>;
> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
> + <&gcc GCC_APSS_TCU_CLK>,
> + <&gcc GCC_MDP_TBU_CLK>;
> +
> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
> };
>
> /* device with two stream IDs, 0 and 7 */
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f7e11d3..720a1ef 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
> #define for_each_cfg_sme(fw, i, idx) \
> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>
> +struct mmu500_clk {
> + struct clk *cfg_clk;
> + struct clk *tcu_clk;
> + struct clk *tbu_clk;
> +};
> +
> struct arm_smmu_clks {
> void *clks;
> int (*init_clocks)(struct arm_smmu_device *smmu);
> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> return container_of(dom, struct arm_smmu_domain, domain);
> }
>
> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
> +{
> + int ret = 0;
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks)
> + return 0;
> +
> + ret = clk_prepare_enable(sclks->cfg_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tcu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tbu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couln't enable tbu_clk");
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
> +{
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks) {
> + clk_disable_unprepare(sclks->tbu_clk);
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + }
> +}
> +
> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
> +{
> + struct device *dev = smmu->dev;
> + struct mmu500_clk *sclks;
> + int err;
> +
> + if (!of_find_property(dev->of_node, "clocks", NULL))
> + return 0;
> +
> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
> + if (!sclks)
> + return -ENOMEM;
> +
> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
> + if (IS_ERR(sclks->cfg_clk)) {
> + err = PTR_ERR(sclks->cfg_clk);
> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->cfg_clk = NULL;
> + }
> +
> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
> + if (IS_ERR(sclks->tcu_clk)) {
> + dev_err(dev, "Couldn't get tcu_clk");
> + return PTR_ERR(sclks->tcu_clk);
> + }
> +
> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
> + if (IS_ERR(sclks->tbu_clk)) {
> + err = PTR_ERR(sclks->tbu_clk);
> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->tbu_clk = NULL;
> + }
> +
> + smmu->smmu_clks.clks = sclks;
> + return 0;
> +}
> +
> static void parse_driver_options(struct arm_smmu_device *smmu)
> {
> int i = 0;
> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
> NULL, NULL, NULL);
> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
> NULL, NULL, NULL);
> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
> + mmu500_enable_clocks, mmu500_disable_clocks);
> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
> NULL, NULL, NULL);
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
> _______________________________________________
> iommu mailing list
> iommu at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
@ 2017-03-16 22:52 ` Rob Clark
0 siblings, 0 replies; 47+ messages in thread
From: Rob Clark @ 2017-03-16 22:52 UTC (permalink / raw)
To: Sricharan R
Cc: Mark Rutland, devicetree, Stephen Boyd, mathieu.poirier,
linux-arm-msm, Joerg Roedel, Will Deacon, iommu, Rob Herring,
Robin Murphy, linux-clk, linux-arm-kernel, Marek Szyprowski
On Thu, Mar 9, 2017 at 10:35 AM, Sricharan R <sricharan@codeaurora.org> wrote:
> The MMU400x/500 is the implementation of the SMMUv2
> arch specification. It is split in to two blocks
> TBU, TCU. TBU caches the page table, instantiated
> for each master locally, clocked by the TBUn_clk.
> TCU manages the address translation with PTW and has
> the programming interface as well, clocked using the
> TCU_CLK. The TBU can also be sharing the same clock
> domain as TCU, in which case both are clocked using
> the TCU_CLK.
>
> This defines the clock bindings for the same and adds the
> init, enable and disable functions for handling the
> clocks.
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
> 2 files changed, 121 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index 6cdf32d..b369c13 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -60,6 +60,28 @@ conditions.
> aliases of secure registers have to be used during
> SMMU configuration.
>
> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
I guess that should be: "Should be "tbu_clk" *or* "tcu_clk" and
"cfg_clk" for..."
Also, possibly we should define our own compat strings for various
SoC's that require these clks so we can properly describe when they
are required? I guess that would address Rob H's comment.
BR,
-R
> + "tcu_clk" is required for smmu's register access using the
> + programming interface and ptw for downstream bus access.
> +
> + "tbu_clk" is required for access to the TBU connected to the
> + master locally. This clock is optional and not required when
> + TBU is in the same clock domain as the TCU or when the TBU is
> + clocked along with the master.
> +
> + "cfg_clk" is optional if required to access the TCU's programming
> + interface, apart from the "tcu_clk".
> +
> +- clocks: Phandles for respective clocks described by clock-names.
> +
> +- power-domains: Phandles to SMMU's power domain specifier. This is
> + required even if SMMU belongs to the master's power
> + domain, as the SMMU will have to be enabled and
> + accessed before master gets enabled and linked to its
> + SMMU.
> +
> ** Deprecated properties:
>
> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
> @@ -84,6 +106,11 @@ conditions.
> <0 36 4>,
> <0 37 4>;
> #iommu-cells = <1>;
> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
> + <&gcc GCC_APSS_TCU_CLK>,
> + <&gcc GCC_MDP_TBU_CLK>;
> +
> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
> };
>
> /* device with two stream IDs, 0 and 7 */
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index f7e11d3..720a1ef 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
> #define for_each_cfg_sme(fw, i, idx) \
> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>
> +struct mmu500_clk {
> + struct clk *cfg_clk;
> + struct clk *tcu_clk;
> + struct clk *tbu_clk;
> +};
> +
> struct arm_smmu_clks {
> void *clks;
> int (*init_clocks)(struct arm_smmu_device *smmu);
> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
> return container_of(dom, struct arm_smmu_domain, domain);
> }
>
> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
> +{
> + int ret = 0;
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks)
> + return 0;
> +
> + ret = clk_prepare_enable(sclks->cfg_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tcu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(sclks->tbu_clk);
> + if (ret) {
> + dev_err(smmu->dev, "Couln't enable tbu_clk");
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
> +{
> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
> +
> + if (!sclks) {
> + clk_disable_unprepare(sclks->tbu_clk);
> + clk_disable_unprepare(sclks->tcu_clk);
> + clk_disable_unprepare(sclks->cfg_clk);
> + }
> +}
> +
> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
> +{
> + struct device *dev = smmu->dev;
> + struct mmu500_clk *sclks;
> + int err;
> +
> + if (!of_find_property(dev->of_node, "clocks", NULL))
> + return 0;
> +
> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
> + if (!sclks)
> + return -ENOMEM;
> +
> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
> + if (IS_ERR(sclks->cfg_clk)) {
> + err = PTR_ERR(sclks->cfg_clk);
> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->cfg_clk = NULL;
> + }
> +
> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
> + if (IS_ERR(sclks->tcu_clk)) {
> + dev_err(dev, "Couldn't get tcu_clk");
> + return PTR_ERR(sclks->tcu_clk);
> + }
> +
> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
> + if (IS_ERR(sclks->tbu_clk)) {
> + err = PTR_ERR(sclks->tbu_clk);
> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
> + if (err == -EPROBE_DEFER)
> + return err;
> + else
> + sclks->tbu_clk = NULL;
> + }
> +
> + smmu->smmu_clks.clks = sclks;
> + return 0;
> +}
> +
> static void parse_driver_options(struct arm_smmu_device *smmu)
> {
> int i = 0;
> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
> NULL, NULL, NULL);
> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
> NULL, NULL, NULL);
> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
> + mmu500_enable_clocks, mmu500_disable_clocks);
> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
> NULL, NULL, NULL);
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 47+ messages in thread[parent not found: <CAF6AEGv3FAyWfXGT_8=bcn+UL8Ug1pE8EGu=MQC70U++wUOioA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
2017-03-16 22:52 ` Rob Clark
(?)
@ 2017-03-17 4:43 ` Sricharan R
-1 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-17 4:43 UTC (permalink / raw)
To: Rob Clark
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Stephen Boyd,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A, linux-arm-msm,
Will Deacon, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Rob Herring, linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Hi,
On 3/17/2017 4:22 AM, Rob Clark wrote:
> On Thu, Mar 9, 2017 at 10:35 AM, Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> The MMU400x/500 is the implementation of the SMMUv2
>> arch specification. It is split in to two blocks
>> TBU, TCU. TBU caches the page table, instantiated
>> for each master locally, clocked by the TBUn_clk.
>> TCU manages the address translation with PTW and has
>> the programming interface as well, clocked using the
>> TCU_CLK. The TBU can also be sharing the same clock
>> domain as TCU, in which case both are clocked using
>> the TCU_CLK.
>>
>> This defines the clock bindings for the same and adds the
>> init, enable and disable functions for handling the
>> clocks.
>>
>> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
>> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
>> 2 files changed, 121 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 6cdf32d..b369c13 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -60,6 +60,28 @@ conditions.
>> aliases of secure registers have to be used during
>> SMMU configuration.
>>
>> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
>> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
>
> I guess that should be: "Should be "tbu_clk" *or* "tcu_clk" and
> "cfg_clk" for..."
>
> Also, possibly we should define our own compat strings for various
> SoC's that require these clks so we can properly describe when they
> are required? I guess that would address Rob H's comment.
>
Ok, calling "optional" does not look correct. But i was trying to define
something which would be right for all implementations of MMU-500, but
looks like that's not the right way. Making it soc specific would give a
exact description, except for the number of compatibles and have to
change the code to not consider any of tbu , tcu, cfg clk as mandatory
for adding support for other compatibles later which are mostly similar.
So i am trying to setup the clocks using functions specific for each
compatible. Not sure, if at this point, some soc specific callback from
the driver where these things can be initialized might be good.
Regards,
Sricharan
Regards,
Sricharan
> BR,
> -R
>
>> + "tcu_clk" is required for smmu's register access using the
>> + programming interface and ptw for downstream bus access.
>> +
>> + "tbu_clk" is required for access to the TBU connected to the
>> + master locally. This clock is optional and not required when
>> + TBU is in the same clock domain as the TCU or when the TBU is
>> + clocked along with the master.
>> +
>> + "cfg_clk" is optional if required to access the TCU's programming
>> + interface, apart from the "tcu_clk".
>> +
>> +- clocks: Phandles for respective clocks described by clock-names.
>> +
>> +- power-domains: Phandles to SMMU's power domain specifier. This is
>> + required even if SMMU belongs to the master's power
>> + domain, as the SMMU will have to be enabled and
>> + accessed before master gets enabled and linked to its
>> + SMMU.
>> +
>> ** Deprecated properties:
>>
>> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
>> @@ -84,6 +106,11 @@ conditions.
>> <0 36 4>,
>> <0 37 4>;
>> #iommu-cells = <1>;
>> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
>> + <&gcc GCC_APSS_TCU_CLK>,
>> + <&gcc GCC_MDP_TBU_CLK>;
>> +
>> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
>> };
>>
>> /* device with two stream IDs, 0 and 7 */
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index f7e11d3..720a1ef 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
>> #define for_each_cfg_sme(fw, i, idx) \
>> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>>
>> +struct mmu500_clk {
>> + struct clk *cfg_clk;
>> + struct clk *tcu_clk;
>> + struct clk *tbu_clk;
>> +};
>> +
>> struct arm_smmu_clks {
>> void *clks;
>> int (*init_clocks)(struct arm_smmu_device *smmu);
>> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>> return container_of(dom, struct arm_smmu_domain, domain);
>> }
>>
>> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
>> +{
>> + int ret = 0;
>> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
>> +
>> + if (!sclks)
>> + return 0;
>> +
>> + ret = clk_prepare_enable(sclks->cfg_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(sclks->tcu_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(sclks->tbu_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couln't enable tbu_clk");
>> + clk_disable_unprepare(sclks->tcu_clk);
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
>> +{
>> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
>> +
>> + if (!sclks) {
>> + clk_disable_unprepare(sclks->tbu_clk);
>> + clk_disable_unprepare(sclks->tcu_clk);
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + }
>> +}
>> +
>> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
>> +{
>> + struct device *dev = smmu->dev;
>> + struct mmu500_clk *sclks;
>> + int err;
>> +
>> + if (!of_find_property(dev->of_node, "clocks", NULL))
>> + return 0;
>> +
>> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
>> + if (!sclks)
>> + return -ENOMEM;
>> +
>> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
>> + if (IS_ERR(sclks->cfg_clk)) {
>> + err = PTR_ERR(sclks->cfg_clk);
>> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
>> + if (err == -EPROBE_DEFER)
>> + return err;
>> + else
>> + sclks->cfg_clk = NULL;
>> + }
>> +
>> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
>> + if (IS_ERR(sclks->tcu_clk)) {
>> + dev_err(dev, "Couldn't get tcu_clk");
>> + return PTR_ERR(sclks->tcu_clk);
>> + }
>> +
>> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
>> + if (IS_ERR(sclks->tbu_clk)) {
>> + err = PTR_ERR(sclks->tbu_clk);
>> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
>> + if (err == -EPROBE_DEFER)
>> + return err;
>> + else
>> + sclks->tbu_clk = NULL;
>> + }
>> +
>> + smmu->smmu_clks.clks = sclks;
>> + return 0;
>> +}
>> +
>> static void parse_driver_options(struct arm_smmu_device *smmu)
>> {
>> int i = 0;
>> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
>> NULL, NULL, NULL);
>> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
>> NULL, NULL, NULL);
>> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
>> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
>> + mmu500_enable_clocks, mmu500_disable_clocks);
>> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
>> NULL, NULL, NULL);
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>
>> _______________________________________________
>> iommu mailing list
>> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread* [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
@ 2017-03-17 4:43 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-17 4:43 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 3/17/2017 4:22 AM, Rob Clark wrote:
> On Thu, Mar 9, 2017 at 10:35 AM, Sricharan R <sricharan@codeaurora.org> wrote:
>> The MMU400x/500 is the implementation of the SMMUv2
>> arch specification. It is split in to two blocks
>> TBU, TCU. TBU caches the page table, instantiated
>> for each master locally, clocked by the TBUn_clk.
>> TCU manages the address translation with PTW and has
>> the programming interface as well, clocked using the
>> TCU_CLK. The TBU can also be sharing the same clock
>> domain as TCU, in which case both are clocked using
>> the TCU_CLK.
>>
>> This defines the clock bindings for the same and adds the
>> init, enable and disable functions for handling the
>> clocks.
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
>> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
>> 2 files changed, 121 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 6cdf32d..b369c13 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -60,6 +60,28 @@ conditions.
>> aliases of secure registers have to be used during
>> SMMU configuration.
>>
>> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
>> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
>
> I guess that should be: "Should be "tbu_clk" *or* "tcu_clk" and
> "cfg_clk" for..."
>
> Also, possibly we should define our own compat strings for various
> SoC's that require these clks so we can properly describe when they
> are required? I guess that would address Rob H's comment.
>
Ok, calling "optional" does not look correct. But i was trying to define
something which would be right for all implementations of MMU-500, but
looks like that's not the right way. Making it soc specific would give a
exact description, except for the number of compatibles and have to
change the code to not consider any of tbu , tcu, cfg clk as mandatory
for adding support for other compatibles later which are mostly similar.
So i am trying to setup the clocks using functions specific for each
compatible. Not sure, if at this point, some soc specific callback from
the driver where these things can be initialized might be good.
Regards,
Sricharan
Regards,
Sricharan
> BR,
> -R
>
>> + "tcu_clk" is required for smmu's register access using the
>> + programming interface and ptw for downstream bus access.
>> +
>> + "tbu_clk" is required for access to the TBU connected to the
>> + master locally. This clock is optional and not required when
>> + TBU is in the same clock domain as the TCU or when the TBU is
>> + clocked along with the master.
>> +
>> + "cfg_clk" is optional if required to access the TCU's programming
>> + interface, apart from the "tcu_clk".
>> +
>> +- clocks: Phandles for respective clocks described by clock-names.
>> +
>> +- power-domains: Phandles to SMMU's power domain specifier. This is
>> + required even if SMMU belongs to the master's power
>> + domain, as the SMMU will have to be enabled and
>> + accessed before master gets enabled and linked to its
>> + SMMU.
>> +
>> ** Deprecated properties:
>>
>> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
>> @@ -84,6 +106,11 @@ conditions.
>> <0 36 4>,
>> <0 37 4>;
>> #iommu-cells = <1>;
>> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
>> + <&gcc GCC_APSS_TCU_CLK>,
>> + <&gcc GCC_MDP_TBU_CLK>;
>> +
>> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
>> };
>>
>> /* device with two stream IDs, 0 and 7 */
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index f7e11d3..720a1ef 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
>> #define for_each_cfg_sme(fw, i, idx) \
>> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>>
>> +struct mmu500_clk {
>> + struct clk *cfg_clk;
>> + struct clk *tcu_clk;
>> + struct clk *tbu_clk;
>> +};
>> +
>> struct arm_smmu_clks {
>> void *clks;
>> int (*init_clocks)(struct arm_smmu_device *smmu);
>> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>> return container_of(dom, struct arm_smmu_domain, domain);
>> }
>>
>> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
>> +{
>> + int ret = 0;
>> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
>> +
>> + if (!sclks)
>> + return 0;
>> +
>> + ret = clk_prepare_enable(sclks->cfg_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(sclks->tcu_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(sclks->tbu_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couln't enable tbu_clk");
>> + clk_disable_unprepare(sclks->tcu_clk);
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
>> +{
>> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
>> +
>> + if (!sclks) {
>> + clk_disable_unprepare(sclks->tbu_clk);
>> + clk_disable_unprepare(sclks->tcu_clk);
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + }
>> +}
>> +
>> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
>> +{
>> + struct device *dev = smmu->dev;
>> + struct mmu500_clk *sclks;
>> + int err;
>> +
>> + if (!of_find_property(dev->of_node, "clocks", NULL))
>> + return 0;
>> +
>> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
>> + if (!sclks)
>> + return -ENOMEM;
>> +
>> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
>> + if (IS_ERR(sclks->cfg_clk)) {
>> + err = PTR_ERR(sclks->cfg_clk);
>> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
>> + if (err == -EPROBE_DEFER)
>> + return err;
>> + else
>> + sclks->cfg_clk = NULL;
>> + }
>> +
>> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
>> + if (IS_ERR(sclks->tcu_clk)) {
>> + dev_err(dev, "Couldn't get tcu_clk");
>> + return PTR_ERR(sclks->tcu_clk);
>> + }
>> +
>> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
>> + if (IS_ERR(sclks->tbu_clk)) {
>> + err = PTR_ERR(sclks->tbu_clk);
>> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
>> + if (err == -EPROBE_DEFER)
>> + return err;
>> + else
>> + sclks->tbu_clk = NULL;
>> + }
>> +
>> + smmu->smmu_clks.clks = sclks;
>> + return 0;
>> +}
>> +
>> static void parse_driver_options(struct arm_smmu_device *smmu)
>> {
>> int i = 0;
>> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
>> NULL, NULL, NULL);
>> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
>> NULL, NULL, NULL);
>> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
>> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
>> + mmu500_enable_clocks, mmu500_disable_clocks);
>> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
>> NULL, NULL, NULL);
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>
>> _______________________________________________
>> iommu mailing list
>> iommu at lists.linux-foundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH V3 2/5] iommu/arm-smmu: Add support for MMU40x/500 clocks
@ 2017-03-17 4:43 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-17 4:43 UTC (permalink / raw)
To: Rob Clark
Cc: Robin Murphy, Will Deacon, Joerg Roedel, iommu, linux-arm-kernel,
linux-arm-msm, Marek Szyprowski, linux-clk, Stephen Boyd,
devicetree, Rob Herring, mathieu.poirier, Mark Rutland
Hi,
On 3/17/2017 4:22 AM, Rob Clark wrote:
> On Thu, Mar 9, 2017 at 10:35 AM, Sricharan R <sricharan@codeaurora.org> wrote:
>> The MMU400x/500 is the implementation of the SMMUv2
>> arch specification. It is split in to two blocks
>> TBU, TCU. TBU caches the page table, instantiated
>> for each master locally, clocked by the TBUn_clk.
>> TCU manages the address translation with PTW and has
>> the programming interface as well, clocked using the
>> TCU_CLK. The TBU can also be sharing the same clock
>> domain as TCU, in which case both are clocked using
>> the TCU_CLK.
>>
>> This defines the clock bindings for the same and adds the
>> init, enable and disable functions for handling the
>> clocks.
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 27 ++++++
>> drivers/iommu/arm-smmu.c | 95 +++++++++++++++++++++-
>> 2 files changed, 121 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index 6cdf32d..b369c13 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -60,6 +60,28 @@ conditions.
>> aliases of secure registers have to be used during
>> SMMU configuration.
>>
>> +- clock-names: Should be "tbu_clk" and "tcu_clk" and "cfg_clk" for
>> + "arm,mmu-400", "arm,mmu-401" and "arm,mmu-500"
>
> I guess that should be: "Should be "tbu_clk" *or* "tcu_clk" and
> "cfg_clk" for..."
>
> Also, possibly we should define our own compat strings for various
> SoC's that require these clks so we can properly describe when they
> are required? I guess that would address Rob H's comment.
>
Ok, calling "optional" does not look correct. But i was trying to define
something which would be right for all implementations of MMU-500, but
looks like that's not the right way. Making it soc specific would give a
exact description, except for the number of compatibles and have to
change the code to not consider any of tbu , tcu, cfg clk as mandatory
for adding support for other compatibles later which are mostly similar.
So i am trying to setup the clocks using functions specific for each
compatible. Not sure, if at this point, some soc specific callback from
the driver where these things can be initialized might be good.
Regards,
Sricharan
Regards,
Sricharan
> BR,
> -R
>
>> + "tcu_clk" is required for smmu's register access using the
>> + programming interface and ptw for downstream bus access.
>> +
>> + "tbu_clk" is required for access to the TBU connected to the
>> + master locally. This clock is optional and not required when
>> + TBU is in the same clock domain as the TCU or when the TBU is
>> + clocked along with the master.
>> +
>> + "cfg_clk" is optional if required to access the TCU's programming
>> + interface, apart from the "tcu_clk".
>> +
>> +- clocks: Phandles for respective clocks described by clock-names.
>> +
>> +- power-domains: Phandles to SMMU's power domain specifier. This is
>> + required even if SMMU belongs to the master's power
>> + domain, as the SMMU will have to be enabled and
>> + accessed before master gets enabled and linked to its
>> + SMMU.
>> +
>> ** Deprecated properties:
>>
>> - mmu-masters (deprecated in favour of the generic "iommus" binding) :
>> @@ -84,6 +106,11 @@ conditions.
>> <0 36 4>,
>> <0 37 4>;
>> #iommu-cells = <1>;
>> + clocks = <&gcc GCC_SMMU_CFG_CLK>,
>> + <&gcc GCC_APSS_TCU_CLK>,
>> + <&gcc GCC_MDP_TBU_CLK>;
>> +
>> + clock-names = "cfg_clk", "tcu_clk", "tbu_clk";
>> };
>>
>> /* device with two stream IDs, 0 and 7 */
>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
>> index f7e11d3..720a1ef 100644
>> --- a/drivers/iommu/arm-smmu.c
>> +++ b/drivers/iommu/arm-smmu.c
>> @@ -341,6 +341,12 @@ struct arm_smmu_master_cfg {
>> #define for_each_cfg_sme(fw, i, idx) \
>> for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
>>
>> +struct mmu500_clk {
>> + struct clk *cfg_clk;
>> + struct clk *tcu_clk;
>> + struct clk *tbu_clk;
>> +};
>> +
>> struct arm_smmu_clks {
>> void *clks;
>> int (*init_clocks)(struct arm_smmu_device *smmu);
>> @@ -455,6 +461,92 @@ static struct arm_smmu_domain *to_smmu_domain(struct iommu_domain *dom)
>> return container_of(dom, struct arm_smmu_domain, domain);
>> }
>>
>> +static int mmu500_enable_clocks(struct arm_smmu_device *smmu)
>> +{
>> + int ret = 0;
>> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
>> +
>> + if (!sclks)
>> + return 0;
>> +
>> + ret = clk_prepare_enable(sclks->cfg_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couldn't enable cfg_clk");
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(sclks->tcu_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couldn't enable tcu_clk");
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + return ret;
>> + }
>> +
>> + ret = clk_prepare_enable(sclks->tbu_clk);
>> + if (ret) {
>> + dev_err(smmu->dev, "Couln't enable tbu_clk");
>> + clk_disable_unprepare(sclks->tcu_clk);
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void mmu500_disable_clocks(struct arm_smmu_device *smmu)
>> +{
>> + struct mmu500_clk *sclks = smmu->smmu_clks.clks;
>> +
>> + if (!sclks) {
>> + clk_disable_unprepare(sclks->tbu_clk);
>> + clk_disable_unprepare(sclks->tcu_clk);
>> + clk_disable_unprepare(sclks->cfg_clk);
>> + }
>> +}
>> +
>> +static int mmu500_init_clocks(struct arm_smmu_device *smmu)
>> +{
>> + struct device *dev = smmu->dev;
>> + struct mmu500_clk *sclks;
>> + int err;
>> +
>> + if (!of_find_property(dev->of_node, "clocks", NULL))
>> + return 0;
>> +
>> + sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
>> + if (!sclks)
>> + return -ENOMEM;
>> +
>> + sclks->cfg_clk = devm_clk_get(dev, "cfg_clk");
>> + if (IS_ERR(sclks->cfg_clk)) {
>> + err = PTR_ERR(sclks->cfg_clk);
>> + /* Ignore all, except -EPROBE_DEFER for optional clocks */
>> + if (err == -EPROBE_DEFER)
>> + return err;
>> + else
>> + sclks->cfg_clk = NULL;
>> + }
>> +
>> + sclks->tcu_clk = devm_clk_get(dev, "tcu_clk");
>> + if (IS_ERR(sclks->tcu_clk)) {
>> + dev_err(dev, "Couldn't get tcu_clk");
>> + return PTR_ERR(sclks->tcu_clk);
>> + }
>> +
>> + sclks->tbu_clk = devm_clk_get(dev, "tbu_clk");
>> + if (IS_ERR(sclks->tbu_clk)) {
>> + err = PTR_ERR(sclks->tbu_clk);
>> + /* Ignore all, ecept -EPROBE_DEFER for optional clocks */
>> + if (err == -EPROBE_DEFER)
>> + return err;
>> + else
>> + sclks->tbu_clk = NULL;
>> + }
>> +
>> + smmu->smmu_clks.clks = sclks;
>> + return 0;
>> +}
>> +
>> static void parse_driver_options(struct arm_smmu_device *smmu)
>> {
>> int i = 0;
>> @@ -1981,7 +2073,8 @@ struct arm_smmu_match_data {
>> NULL, NULL, NULL);
>> ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU,
>> NULL, NULL, NULL);
>> -ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, NULL, NULL, NULL);
>> +ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500, mmu500_init_clocks,
>> + mmu500_enable_clocks, mmu500_disable_clocks);
>> ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
>> NULL, NULL, NULL);
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>
>> _______________________________________________
>> iommu mailing list
>> iommu@lists.linux-foundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH V3 4/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
2017-03-09 15:35 ` Sricharan R
@ 2017-03-09 15:35 ` Sricharan R
-1 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: robin.murphy, will.deacon, joro, iommu, linux-arm-kernel,
linux-arm-msm, m.szyprowski, linux-clk, sboyd, devicetree,
robh+dt, mathieu.poirier, mark.rutland
Cc: sricharan
The smmu device probe/remove and add/remove master device callbacks
gets called when the smmu is not linked to its master, that is without
the context of the master device. So calling runtime apis in those places
separately.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
drivers/iommu/arm-smmu.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f29e28bf..6370421 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1216,11 +1216,16 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
void __iomem *cb_base;
+ unsigned long ret;
int irq;
if (!smmu)
return;
+ ret = pm_runtime_get_sync(smmu->dev);
+ if (IS_ERR_VALUE(ret))
+ dev_warn(smmu->dev, "runtime resume failed");
+
/*
* Disable the context bank and free the page tables before freeing
* it.
@@ -1235,6 +1240,10 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
free_io_pgtable_ops(smmu_domain->pgtbl_ops);
__arm_smmu_free_bitmap(smmu->context_map, cfg->cbndx);
+
+ ret = pm_runtime_put_sync(smmu->dev);
+ if (IS_ERR_VALUE(ret))
+ dev_warn(smmu->dev, "runtime suspend failed");
}
static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
@@ -1663,6 +1672,7 @@ static int arm_smmu_add_device(struct device *dev)
struct arm_smmu_master_cfg *cfg;
struct iommu_fwspec *fwspec = dev->iommu_fwspec;
int i, ret;
+ unsigned long err;
if (using_legacy_binding) {
ret = arm_smmu_register_legacy_master(dev, &smmu);
@@ -1703,12 +1713,20 @@ static int arm_smmu_add_device(struct device *dev)
while (i--)
cfg->smendx[i] = INVALID_SMENDX;
+ err = pm_runtime_get_sync(smmu->dev);
+ if (IS_ERR_VALUE(err))
+ goto out_free;
+
ret = arm_smmu_master_alloc_smes(dev);
if (ret)
goto out_free;
iommu_device_link(&smmu->iommu, dev);
+ err = pm_runtime_put_sync(smmu->dev);
+ if (IS_ERR_VALUE(err))
+ goto out_free;
+
return 0;
out_free:
@@ -1723,7 +1741,7 @@ static void arm_smmu_remove_device(struct device *dev)
struct iommu_fwspec *fwspec = dev->iommu_fwspec;
struct arm_smmu_master_cfg *cfg;
struct arm_smmu_device *smmu;
-
+ unsigned long ret;
if (!fwspec || fwspec->ops != &arm_smmu_ops)
return;
@@ -1731,8 +1749,23 @@ static void arm_smmu_remove_device(struct device *dev)
cfg = fwspec->iommu_priv;
smmu = cfg->smmu;
+ /*
+ * The device link between the master device and
+ * smmu is already purged at this point.
+ * So enable the power to smmu explicitly.
+ */
+
+ ret = pm_runtime_get_sync(smmu->dev);
+ if (IS_ERR_VALUE(ret))
+ dev_warn(smmu->dev, "runtime resume failed");
+
iommu_device_unlink(&smmu->iommu, dev);
arm_smmu_master_free_smes(fwspec);
+
+ ret = pm_runtime_put_sync(smmu->dev);
+ if (IS_ERR_VALUE(ret))
+ dev_warn(smmu->dev, "runtime suspend failed");
+
iommu_group_remove_device(dev);
kfree(fwspec->iommu_priv);
iommu_fwspec_free(dev);
@@ -2316,6 +2349,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
struct arm_smmu_device *smmu;
struct device *dev = &pdev->dev;
int num_irqs, i, err;
+ unsigned long ret;
smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
if (!smmu) {
@@ -2375,7 +2409,12 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
return err;
}
+ platform_set_drvdata(pdev, smmu);
pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;
@@ -2417,9 +2456,11 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
return err;
}
- platform_set_drvdata(pdev, smmu);
arm_smmu_device_reset(smmu);
arm_smmu_test_smr_masks(smmu);
+ ret = pm_runtime_put_sync(dev);
+ if (IS_ERR_VALUE(ret))
+ return err;
/* Oh, for a proper bus abstraction */
if (!iommu_present(&platform_bus_type))
@@ -2449,6 +2490,8 @@ 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);
+ pm_runtime_force_suspend(smmu->dev);
+
return 0;
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH V3 4/5] iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
@ 2017-03-09 15:35 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: linux-arm-kernel
The smmu device probe/remove and add/remove master device callbacks
gets called when the smmu is not linked to its master, that is without
the context of the master device. So calling runtime apis in those places
separately.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
drivers/iommu/arm-smmu.c | 47 +++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index f29e28bf..6370421 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1216,11 +1216,16 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_cfg *cfg = &smmu_domain->cfg;
void __iomem *cb_base;
+ unsigned long ret;
int irq;
if (!smmu)
return;
+ ret = pm_runtime_get_sync(smmu->dev);
+ if (IS_ERR_VALUE(ret))
+ dev_warn(smmu->dev, "runtime resume failed");
+
/*
* Disable the context bank and free the page tables before freeing
* it.
@@ -1235,6 +1240,10 @@ static void arm_smmu_destroy_domain_context(struct iommu_domain *domain)
free_io_pgtable_ops(smmu_domain->pgtbl_ops);
__arm_smmu_free_bitmap(smmu->context_map, cfg->cbndx);
+
+ ret = pm_runtime_put_sync(smmu->dev);
+ if (IS_ERR_VALUE(ret))
+ dev_warn(smmu->dev, "runtime suspend failed");
}
static struct iommu_domain *arm_smmu_domain_alloc(unsigned type)
@@ -1663,6 +1672,7 @@ static int arm_smmu_add_device(struct device *dev)
struct arm_smmu_master_cfg *cfg;
struct iommu_fwspec *fwspec = dev->iommu_fwspec;
int i, ret;
+ unsigned long err;
if (using_legacy_binding) {
ret = arm_smmu_register_legacy_master(dev, &smmu);
@@ -1703,12 +1713,20 @@ static int arm_smmu_add_device(struct device *dev)
while (i--)
cfg->smendx[i] = INVALID_SMENDX;
+ err = pm_runtime_get_sync(smmu->dev);
+ if (IS_ERR_VALUE(err))
+ goto out_free;
+
ret = arm_smmu_master_alloc_smes(dev);
if (ret)
goto out_free;
iommu_device_link(&smmu->iommu, dev);
+ err = pm_runtime_put_sync(smmu->dev);
+ if (IS_ERR_VALUE(err))
+ goto out_free;
+
return 0;
out_free:
@@ -1723,7 +1741,7 @@ static void arm_smmu_remove_device(struct device *dev)
struct iommu_fwspec *fwspec = dev->iommu_fwspec;
struct arm_smmu_master_cfg *cfg;
struct arm_smmu_device *smmu;
-
+ unsigned long ret;
if (!fwspec || fwspec->ops != &arm_smmu_ops)
return;
@@ -1731,8 +1749,23 @@ static void arm_smmu_remove_device(struct device *dev)
cfg = fwspec->iommu_priv;
smmu = cfg->smmu;
+ /*
+ * The device link between the master device and
+ * smmu is already purged at this point.
+ * So enable the power to smmu explicitly.
+ */
+
+ ret = pm_runtime_get_sync(smmu->dev);
+ if (IS_ERR_VALUE(ret))
+ dev_warn(smmu->dev, "runtime resume failed");
+
iommu_device_unlink(&smmu->iommu, dev);
arm_smmu_master_free_smes(fwspec);
+
+ ret = pm_runtime_put_sync(smmu->dev);
+ if (IS_ERR_VALUE(ret))
+ dev_warn(smmu->dev, "runtime suspend failed");
+
iommu_group_remove_device(dev);
kfree(fwspec->iommu_priv);
iommu_fwspec_free(dev);
@@ -2316,6 +2349,7 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
struct arm_smmu_device *smmu;
struct device *dev = &pdev->dev;
int num_irqs, i, err;
+ unsigned long ret;
smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
if (!smmu) {
@@ -2375,7 +2409,12 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
return err;
}
+ platform_set_drvdata(pdev, smmu);
pm_runtime_enable(dev);
+ ret = pm_runtime_get_sync(dev);
+ if (IS_ERR_VALUE(ret))
+ return ret;
+
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;
@@ -2417,9 +2456,11 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
return err;
}
- platform_set_drvdata(pdev, smmu);
arm_smmu_device_reset(smmu);
arm_smmu_test_smr_masks(smmu);
+ ret = pm_runtime_put_sync(dev);
+ if (IS_ERR_VALUE(ret))
+ return err;
/* Oh, for a proper bus abstraction */
if (!iommu_present(&platform_bus_type))
@@ -2449,6 +2490,8 @@ 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);
+ pm_runtime_force_suspend(smmu->dev);
+
return 0;
}
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH V3 5/5] iommu/arm-smmu: Add the device_link between masters and smmu
2017-03-09 15:35 ` Sricharan R
@ 2017-03-09 15:35 ` Sricharan R
-1 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: robin.murphy, will.deacon, joro, iommu, linux-arm-kernel,
linux-arm-msm, m.szyprowski, linux-clk, sboyd, devicetree,
robh+dt, mathieu.poirier, mark.rutland
Cc: sricharan
Finally add the device link between the master device and
smmu, so that the smmu gets runtime enabled/disabled only when the
master needs it. This is done from add_device callback which gets
called once when the master is added to the smmu.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
drivers/iommu/arm-smmu.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 6370421..b06f86c7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1671,6 +1671,7 @@ static int arm_smmu_add_device(struct device *dev)
struct arm_smmu_device *smmu;
struct arm_smmu_master_cfg *cfg;
struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+ struct device_link *link = NULL;
int i, ret;
unsigned long err;
@@ -1727,6 +1728,16 @@ static int arm_smmu_add_device(struct device *dev)
if (IS_ERR_VALUE(err))
goto out_free;
+ /*
+ * Establish the link between smmu and master, so that the
+ * smmu gets runtime enabled/disabled as per the master's
+ * needs.
+ */
+ link = device_link_add(dev, smmu->dev, DL_FLAG_PM_RUNTIME);
+ if (!link)
+ dev_warn(smmu->dev, "Unable to create device link between %s and %s\n",
+ dev_name(smmu->dev), dev_name(dev));
+
return 0;
out_free:
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH V3 5/5] iommu/arm-smmu: Add the device_link between masters and smmu
@ 2017-03-09 15:35 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: linux-arm-kernel
Finally add the device link between the master device and
smmu, so that the smmu gets runtime enabled/disabled only when the
master needs it. This is done from add_device callback which gets
called once when the master is added to the smmu.
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
drivers/iommu/arm-smmu.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 6370421..b06f86c7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1671,6 +1671,7 @@ static int arm_smmu_add_device(struct device *dev)
struct arm_smmu_device *smmu;
struct arm_smmu_master_cfg *cfg;
struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+ struct device_link *link = NULL;
int i, ret;
unsigned long err;
@@ -1727,6 +1728,16 @@ static int arm_smmu_add_device(struct device *dev)
if (IS_ERR_VALUE(err))
goto out_free;
+ /*
+ * Establish the link between smmu and master, so that the
+ * smmu gets runtime enabled/disabled as per the master's
+ * needs.
+ */
+ link = device_link_add(dev, smmu->dev, DL_FLAG_PM_RUNTIME);
+ if (!link)
+ dev_warn(smmu->dev, "Unable to create device link between %s and %s\n",
+ dev_name(smmu->dev), dev_name(dev));
+
return 0;
out_free:
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 47+ messages in thread
[parent not found: <1489073748-3659-1-git-send-email-sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* [PATCH V3 3/5] drivers: arm-smmu: Add clock support for QCOM_SMMUV2
2017-03-09 15:35 ` Sricharan R
(?)
@ 2017-03-09 15:35 ` Sricharan R
-1 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: robin.murphy-5wv7dgnIgG8, will.deacon-5wv7dgnIgG8,
joro-zLv9SwRftAIdnm+yROfE0A,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
linux-clk-u79uwXL29TY76Z2rM5mHXA, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: sricharan-sgV2jX0FEOL9JmXXK+q4OQ
The QCOM_SMMUV2 is an implementation of the arm,smmu-v2 architecture.
The qcom,smmu is instantiated for each of the multimedia cores (for eg)
Venus (video encoder/decoder), mdp (display) etc, and they are connected
to the Multimedia Aggregator Interconnect (MMAGIC). So the access to
any of the MMU's registers, as well as MMU's downstream bus access,
requires the specified MMAGIC clocks to be enabled. So adding a new
binding for the qcom,smmu-v2 and the required mmagic clock bindings for
the same. Also adding the support for enabling the qcom,smmu-v2 clocks in
the driver.
------------- ---------
| VENUS | | MDP |
| | | |
------------- --------
| |
| |
------ ---------
|SMMU | | SMMU |
| | | |
------ --------
| |
| |
-----------------------------------------
| MMAGIC INTERCONNECT (MMSS NOC) |
| |
-----------------------------------------
| |
| ----------------------------------
----- | SYSTEM NOC |
|DDR| | |
----- ---------------------------------
| |
| ------
|<-------------| CPU|
------
Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
.../devicetree/bindings/iommu/arm,smmu.txt | 8 ++
drivers/iommu/arm-smmu.c | 124 +++++++++++++++++++++
2 files changed, 132 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index b369c13..88e02d6 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -17,6 +17,7 @@ conditions.
"arm,mmu-401"
"arm,mmu-500"
"cavium,smmu-v2"
+ "qcom,smmu-v2"
depending on the particular implementation and/or the
version of the architecture implemented.
@@ -74,6 +75,13 @@ conditions.
"cfg_clk" is optional if required to access the TCU's programming
interface, apart from the "tcu_clk".
+ Should have "mmagic_ahb_clk", "mmagic_cfg_ahb_clk",
+ "smmu_core_ahb_clk", "smmu_core_axi_clk",
+ "mmagic_core_axi_clk" for "qcom,smmu-v2"
+
+ "mmagic_core_axi_clk" is required for smmu's access to the
+ downstream bus and rest for the smmu's register group access.
+
- clocks: Phandles for respective clocks described by clock-names.
- power-domains: Phandles to SMMU's power domain specifier. This is
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 720a1ef..f29e28bf 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -309,6 +309,7 @@ enum arm_smmu_implementation {
GENERIC_SMMU,
ARM_MMU500,
CAVIUM_SMMUV2,
+ QCOM_SMMUV2,
};
struct arm_smmu_s2cr {
@@ -341,6 +342,14 @@ struct arm_smmu_master_cfg {
#define for_each_cfg_sme(fw, i, idx) \
for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
+struct qcom_smmu_clk {
+ struct clk *mmagic_ahb_clk;
+ struct clk *mmagic_cfg_ahb_clk;
+ struct clk *smmu_core_ahb_clk;
+ struct clk *smmu_core_axi_clk;
+ struct clk *mmagic_core_axi_clk;
+};
+
struct mmu500_clk {
struct clk *cfg_clk;
struct clk *tcu_clk;
@@ -547,6 +556,117 @@ static int mmu500_init_clocks(struct arm_smmu_device *smmu)
return 0;
}
+static int qcom_smmu_init_clocks(struct arm_smmu_device *smmu)
+{
+ struct device *dev = smmu->dev;
+ struct qcom_smmu_clk *sclks;
+
+ if (!of_find_property(dev->of_node, "clocks", NULL))
+ return 0;
+
+ sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
+ if (!sclks)
+ return -ENOMEM;
+
+ sclks->mmagic_ahb_clk = devm_clk_get(dev, "mmagic_ahb_clk");
+ if (IS_ERR(sclks->mmagic_ahb_clk)) {
+ dev_err(dev, "Couldn't get mmagic_ahb_clk");
+ return PTR_ERR(sclks->mmagic_ahb_clk);
+ }
+
+ sclks->mmagic_cfg_ahb_clk = devm_clk_get(dev, "mmagic_cfg_ahb_clk");
+ if (IS_ERR(sclks->mmagic_cfg_ahb_clk)) {
+ dev_err(dev, "Couldn't get mmagic_cfg_ahb_clk");
+ return PTR_ERR(sclks->mmagic_cfg_ahb_clk);
+ }
+
+ sclks->smmu_core_ahb_clk = devm_clk_get(dev, "smmu_core_ahb_clk");
+ if (IS_ERR(sclks->smmu_core_ahb_clk)) {
+ dev_err(dev, "Couldn't get smmu_core_ahb_clk");
+ return PTR_ERR(sclks->smmu_core_ahb_clk);
+ }
+
+ sclks->smmu_core_axi_clk = devm_clk_get(dev, "smmu_core_axi_clk");
+ if (IS_ERR(sclks->smmu_core_axi_clk)) {
+ dev_err(dev, "Couldn't get smmu_core_axi_clk");
+ return PTR_ERR(sclks->smmu_core_axi_clk);
+ }
+
+ sclks->mmagic_core_axi_clk = devm_clk_get(dev, "mmagic_core_axi_clk");
+ if (IS_ERR(sclks->mmagic_core_axi_clk)) {
+ dev_err(dev, "Couldn't get mmagic_core_axi_clk");
+ return PTR_ERR(sclks->mmagic_core_axi_clk);
+ }
+
+ smmu->smmu_clks.clks = sclks;
+ return 0;
+}
+
+static int qcom_smmu_enable_clocks(struct arm_smmu_device *smmu)
+{
+ int ret = 0;
+ struct qcom_smmu_clk *sclks = smmu->smmu_clks.clks;
+
+ if (!sclks)
+ return 0;
+
+ ret = clk_prepare_enable(sclks->mmagic_ahb_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couldn't enable mmagic_ahb_clk");
+ goto ahb_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->mmagic_cfg_ahb_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable mmagic_cfg_ahb_clk");
+ goto cfg_ahb_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->smmu_core_ahb_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable smmu_core_ahb_clk");
+ goto core_ahb_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->smmu_core_axi_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable smmu_core_axi_clk");
+ goto smmu_core_axi_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->mmagic_core_axi_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable mmagic_core_axi_clk");
+ goto core_axi_clk_fail;
+ }
+
+ return 0;
+
+core_axi_clk_fail:
+ clk_disable_unprepare(sclks->smmu_core_axi_clk);
+smmu_core_axi_clk_fail:
+ clk_disable_unprepare(sclks->smmu_core_ahb_clk);
+core_ahb_clk_fail:
+ clk_disable_unprepare(sclks->mmagic_cfg_ahb_clk);
+cfg_ahb_clk_fail:
+ clk_disable_unprepare(sclks->mmagic_ahb_clk);
+ahb_clk_fail:
+ return ret;
+}
+
+static void qcom_smmu_disable_clocks(struct arm_smmu_device *smmu)
+{
+ struct qcom_smmu_clk *sclks = smmu->smmu_clks.clks;
+
+ if (!sclks) {
+ clk_disable_unprepare(sclks->mmagic_core_axi_clk);
+ clk_disable_unprepare(sclks->smmu_core_axi_clk);
+ clk_disable_unprepare(sclks->smmu_core_ahb_clk);
+ clk_disable_unprepare(sclks->mmagic_cfg_ahb_clk);
+ clk_disable_unprepare(sclks->mmagic_ahb_clk);
+ }
+}
+
static void parse_driver_options(struct arm_smmu_device *smmu)
{
int i = 0;
@@ -2077,6 +2197,9 @@ struct arm_smmu_match_data {
mmu500_enable_clocks, mmu500_disable_clocks);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(qcom_smmuv2, ARM_SMMU_V2, QCOM_SMMUV2,
+ qcom_smmu_init_clocks, qcom_smmu_enable_clocks,
+ qcom_smmu_disable_clocks);
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = &smmu_generic_v1 },
@@ -2085,6 +2208,7 @@ struct arm_smmu_match_data {
{ .compatible = "arm,mmu-401", .data = &arm_mmu401 },
{ .compatible = "arm,mmu-500", .data = &arm_mmu500 },
{ .compatible = "cavium,smmu-v2", .data = &cavium_smmuv2 },
+ { .compatible = "qcom,smmu-v2", .data = &qcom_smmuv2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
--
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
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH V3 3/5] drivers: arm-smmu: Add clock support for QCOM_SMMUV2
@ 2017-03-09 15:35 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: linux-arm-kernel
The QCOM_SMMUV2 is an implementation of the arm,smmu-v2 architecture.
The qcom,smmu is instantiated for each of the multimedia cores (for eg)
Venus (video encoder/decoder), mdp (display) etc, and they are connected
to the Multimedia Aggregator Interconnect (MMAGIC). So the access to
any of the MMU's registers, as well as MMU's downstream bus access,
requires the specified MMAGIC clocks to be enabled. So adding a new
binding for the qcom,smmu-v2 and the required mmagic clock bindings for
the same. Also adding the support for enabling the qcom,smmu-v2 clocks in
the driver.
------------- ---------
| VENUS | | MDP |
| | | |
------------- --------
| |
| |
------ ---------
|SMMU | | SMMU |
| | | |
------ --------
| |
| |
-----------------------------------------
| MMAGIC INTERCONNECT (MMSS NOC) |
| |
-----------------------------------------
| |
| ----------------------------------
----- | SYSTEM NOC |
|DDR| | |
----- ---------------------------------
| |
| ------
|<-------------| CPU|
------
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
.../devicetree/bindings/iommu/arm,smmu.txt | 8 ++
drivers/iommu/arm-smmu.c | 124 +++++++++++++++++++++
2 files changed, 132 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index b369c13..88e02d6 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -17,6 +17,7 @@ conditions.
"arm,mmu-401"
"arm,mmu-500"
"cavium,smmu-v2"
+ "qcom,smmu-v2"
depending on the particular implementation and/or the
version of the architecture implemented.
@@ -74,6 +75,13 @@ conditions.
"cfg_clk" is optional if required to access the TCU's programming
interface, apart from the "tcu_clk".
+ Should have "mmagic_ahb_clk", "mmagic_cfg_ahb_clk",
+ "smmu_core_ahb_clk", "smmu_core_axi_clk",
+ "mmagic_core_axi_clk" for "qcom,smmu-v2"
+
+ "mmagic_core_axi_clk" is required for smmu's access to the
+ downstream bus and rest for the smmu's register group access.
+
- clocks: Phandles for respective clocks described by clock-names.
- power-domains: Phandles to SMMU's power domain specifier. This is
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 720a1ef..f29e28bf 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -309,6 +309,7 @@ enum arm_smmu_implementation {
GENERIC_SMMU,
ARM_MMU500,
CAVIUM_SMMUV2,
+ QCOM_SMMUV2,
};
struct arm_smmu_s2cr {
@@ -341,6 +342,14 @@ struct arm_smmu_master_cfg {
#define for_each_cfg_sme(fw, i, idx) \
for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
+struct qcom_smmu_clk {
+ struct clk *mmagic_ahb_clk;
+ struct clk *mmagic_cfg_ahb_clk;
+ struct clk *smmu_core_ahb_clk;
+ struct clk *smmu_core_axi_clk;
+ struct clk *mmagic_core_axi_clk;
+};
+
struct mmu500_clk {
struct clk *cfg_clk;
struct clk *tcu_clk;
@@ -547,6 +556,117 @@ static int mmu500_init_clocks(struct arm_smmu_device *smmu)
return 0;
}
+static int qcom_smmu_init_clocks(struct arm_smmu_device *smmu)
+{
+ struct device *dev = smmu->dev;
+ struct qcom_smmu_clk *sclks;
+
+ if (!of_find_property(dev->of_node, "clocks", NULL))
+ return 0;
+
+ sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
+ if (!sclks)
+ return -ENOMEM;
+
+ sclks->mmagic_ahb_clk = devm_clk_get(dev, "mmagic_ahb_clk");
+ if (IS_ERR(sclks->mmagic_ahb_clk)) {
+ dev_err(dev, "Couldn't get mmagic_ahb_clk");
+ return PTR_ERR(sclks->mmagic_ahb_clk);
+ }
+
+ sclks->mmagic_cfg_ahb_clk = devm_clk_get(dev, "mmagic_cfg_ahb_clk");
+ if (IS_ERR(sclks->mmagic_cfg_ahb_clk)) {
+ dev_err(dev, "Couldn't get mmagic_cfg_ahb_clk");
+ return PTR_ERR(sclks->mmagic_cfg_ahb_clk);
+ }
+
+ sclks->smmu_core_ahb_clk = devm_clk_get(dev, "smmu_core_ahb_clk");
+ if (IS_ERR(sclks->smmu_core_ahb_clk)) {
+ dev_err(dev, "Couldn't get smmu_core_ahb_clk");
+ return PTR_ERR(sclks->smmu_core_ahb_clk);
+ }
+
+ sclks->smmu_core_axi_clk = devm_clk_get(dev, "smmu_core_axi_clk");
+ if (IS_ERR(sclks->smmu_core_axi_clk)) {
+ dev_err(dev, "Couldn't get smmu_core_axi_clk");
+ return PTR_ERR(sclks->smmu_core_axi_clk);
+ }
+
+ sclks->mmagic_core_axi_clk = devm_clk_get(dev, "mmagic_core_axi_clk");
+ if (IS_ERR(sclks->mmagic_core_axi_clk)) {
+ dev_err(dev, "Couldn't get mmagic_core_axi_clk");
+ return PTR_ERR(sclks->mmagic_core_axi_clk);
+ }
+
+ smmu->smmu_clks.clks = sclks;
+ return 0;
+}
+
+static int qcom_smmu_enable_clocks(struct arm_smmu_device *smmu)
+{
+ int ret = 0;
+ struct qcom_smmu_clk *sclks = smmu->smmu_clks.clks;
+
+ if (!sclks)
+ return 0;
+
+ ret = clk_prepare_enable(sclks->mmagic_ahb_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couldn't enable mmagic_ahb_clk");
+ goto ahb_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->mmagic_cfg_ahb_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable mmagic_cfg_ahb_clk");
+ goto cfg_ahb_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->smmu_core_ahb_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable smmu_core_ahb_clk");
+ goto core_ahb_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->smmu_core_axi_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable smmu_core_axi_clk");
+ goto smmu_core_axi_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->mmagic_core_axi_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable mmagic_core_axi_clk");
+ goto core_axi_clk_fail;
+ }
+
+ return 0;
+
+core_axi_clk_fail:
+ clk_disable_unprepare(sclks->smmu_core_axi_clk);
+smmu_core_axi_clk_fail:
+ clk_disable_unprepare(sclks->smmu_core_ahb_clk);
+core_ahb_clk_fail:
+ clk_disable_unprepare(sclks->mmagic_cfg_ahb_clk);
+cfg_ahb_clk_fail:
+ clk_disable_unprepare(sclks->mmagic_ahb_clk);
+ahb_clk_fail:
+ return ret;
+}
+
+static void qcom_smmu_disable_clocks(struct arm_smmu_device *smmu)
+{
+ struct qcom_smmu_clk *sclks = smmu->smmu_clks.clks;
+
+ if (!sclks) {
+ clk_disable_unprepare(sclks->mmagic_core_axi_clk);
+ clk_disable_unprepare(sclks->smmu_core_axi_clk);
+ clk_disable_unprepare(sclks->smmu_core_ahb_clk);
+ clk_disable_unprepare(sclks->mmagic_cfg_ahb_clk);
+ clk_disable_unprepare(sclks->mmagic_ahb_clk);
+ }
+}
+
static void parse_driver_options(struct arm_smmu_device *smmu)
{
int i = 0;
@@ -2077,6 +2197,9 @@ struct arm_smmu_match_data {
mmu500_enable_clocks, mmu500_disable_clocks);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(qcom_smmuv2, ARM_SMMU_V2, QCOM_SMMUV2,
+ qcom_smmu_init_clocks, qcom_smmu_enable_clocks,
+ qcom_smmu_disable_clocks);
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = &smmu_generic_v1 },
@@ -2085,6 +2208,7 @@ struct arm_smmu_match_data {
{ .compatible = "arm,mmu-401", .data = &arm_mmu401 },
{ .compatible = "arm,mmu-500", .data = &arm_mmu500 },
{ .compatible = "cavium,smmu-v2", .data = &cavium_smmuv2 },
+ { .compatible = "qcom,smmu-v2", .data = &qcom_smmuv2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH V3 3/5] drivers: arm-smmu: Add clock support for QCOM_SMMUV2
@ 2017-03-09 15:35 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-09 15:35 UTC (permalink / raw)
To: robin.murphy, will.deacon, joro, iommu, linux-arm-kernel,
linux-arm-msm, m.szyprowski, linux-clk, sboyd, devicetree,
robh+dt, mathieu.poirier, mark.rutland
Cc: sricharan
The QCOM_SMMUV2 is an implementation of the arm,smmu-v2 architecture.
The qcom,smmu is instantiated for each of the multimedia cores (for eg)
Venus (video encoder/decoder), mdp (display) etc, and they are connected
to the Multimedia Aggregator Interconnect (MMAGIC). So the access to
any of the MMU's registers, as well as MMU's downstream bus access,
requires the specified MMAGIC clocks to be enabled. So adding a new
binding for the qcom,smmu-v2 and the required mmagic clock bindings for
the same. Also adding the support for enabling the qcom,smmu-v2 clocks in
the driver.
------------- ---------
| VENUS | | MDP |
| | | |
------------- --------
| |
| |
------ ---------
|SMMU | | SMMU |
| | | |
------ --------
| |
| |
-----------------------------------------
| MMAGIC INTERCONNECT (MMSS NOC) |
| |
-----------------------------------------
| |
| ----------------------------------
----- | SYSTEM NOC |
|DDR| | |
----- ---------------------------------
| |
| ------
|<-------------| CPU|
------
Signed-off-by: Sricharan R <sricharan@codeaurora.org>
---
.../devicetree/bindings/iommu/arm,smmu.txt | 8 ++
drivers/iommu/arm-smmu.c | 124 +++++++++++++++++++++
2 files changed, 132 insertions(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index b369c13..88e02d6 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -17,6 +17,7 @@ conditions.
"arm,mmu-401"
"arm,mmu-500"
"cavium,smmu-v2"
+ "qcom,smmu-v2"
depending on the particular implementation and/or the
version of the architecture implemented.
@@ -74,6 +75,13 @@ conditions.
"cfg_clk" is optional if required to access the TCU's programming
interface, apart from the "tcu_clk".
+ Should have "mmagic_ahb_clk", "mmagic_cfg_ahb_clk",
+ "smmu_core_ahb_clk", "smmu_core_axi_clk",
+ "mmagic_core_axi_clk" for "qcom,smmu-v2"
+
+ "mmagic_core_axi_clk" is required for smmu's access to the
+ downstream bus and rest for the smmu's register group access.
+
- clocks: Phandles for respective clocks described by clock-names.
- power-domains: Phandles to SMMU's power domain specifier. This is
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 720a1ef..f29e28bf 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -309,6 +309,7 @@ enum arm_smmu_implementation {
GENERIC_SMMU,
ARM_MMU500,
CAVIUM_SMMUV2,
+ QCOM_SMMUV2,
};
struct arm_smmu_s2cr {
@@ -341,6 +342,14 @@ struct arm_smmu_master_cfg {
#define for_each_cfg_sme(fw, i, idx) \
for (i = 0; idx = fwspec_smendx(fw, i), i < fw->num_ids; ++i)
+struct qcom_smmu_clk {
+ struct clk *mmagic_ahb_clk;
+ struct clk *mmagic_cfg_ahb_clk;
+ struct clk *smmu_core_ahb_clk;
+ struct clk *smmu_core_axi_clk;
+ struct clk *mmagic_core_axi_clk;
+};
+
struct mmu500_clk {
struct clk *cfg_clk;
struct clk *tcu_clk;
@@ -547,6 +556,117 @@ static int mmu500_init_clocks(struct arm_smmu_device *smmu)
return 0;
}
+static int qcom_smmu_init_clocks(struct arm_smmu_device *smmu)
+{
+ struct device *dev = smmu->dev;
+ struct qcom_smmu_clk *sclks;
+
+ if (!of_find_property(dev->of_node, "clocks", NULL))
+ return 0;
+
+ sclks = devm_kzalloc(dev, sizeof(*sclks), GFP_KERNEL);
+ if (!sclks)
+ return -ENOMEM;
+
+ sclks->mmagic_ahb_clk = devm_clk_get(dev, "mmagic_ahb_clk");
+ if (IS_ERR(sclks->mmagic_ahb_clk)) {
+ dev_err(dev, "Couldn't get mmagic_ahb_clk");
+ return PTR_ERR(sclks->mmagic_ahb_clk);
+ }
+
+ sclks->mmagic_cfg_ahb_clk = devm_clk_get(dev, "mmagic_cfg_ahb_clk");
+ if (IS_ERR(sclks->mmagic_cfg_ahb_clk)) {
+ dev_err(dev, "Couldn't get mmagic_cfg_ahb_clk");
+ return PTR_ERR(sclks->mmagic_cfg_ahb_clk);
+ }
+
+ sclks->smmu_core_ahb_clk = devm_clk_get(dev, "smmu_core_ahb_clk");
+ if (IS_ERR(sclks->smmu_core_ahb_clk)) {
+ dev_err(dev, "Couldn't get smmu_core_ahb_clk");
+ return PTR_ERR(sclks->smmu_core_ahb_clk);
+ }
+
+ sclks->smmu_core_axi_clk = devm_clk_get(dev, "smmu_core_axi_clk");
+ if (IS_ERR(sclks->smmu_core_axi_clk)) {
+ dev_err(dev, "Couldn't get smmu_core_axi_clk");
+ return PTR_ERR(sclks->smmu_core_axi_clk);
+ }
+
+ sclks->mmagic_core_axi_clk = devm_clk_get(dev, "mmagic_core_axi_clk");
+ if (IS_ERR(sclks->mmagic_core_axi_clk)) {
+ dev_err(dev, "Couldn't get mmagic_core_axi_clk");
+ return PTR_ERR(sclks->mmagic_core_axi_clk);
+ }
+
+ smmu->smmu_clks.clks = sclks;
+ return 0;
+}
+
+static int qcom_smmu_enable_clocks(struct arm_smmu_device *smmu)
+{
+ int ret = 0;
+ struct qcom_smmu_clk *sclks = smmu->smmu_clks.clks;
+
+ if (!sclks)
+ return 0;
+
+ ret = clk_prepare_enable(sclks->mmagic_ahb_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couldn't enable mmagic_ahb_clk");
+ goto ahb_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->mmagic_cfg_ahb_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable mmagic_cfg_ahb_clk");
+ goto cfg_ahb_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->smmu_core_ahb_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable smmu_core_ahb_clk");
+ goto core_ahb_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->smmu_core_axi_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable smmu_core_axi_clk");
+ goto smmu_core_axi_clk_fail;
+ }
+
+ ret = clk_prepare_enable(sclks->mmagic_core_axi_clk);
+ if (ret) {
+ dev_err(smmu->dev, "Couln't enable mmagic_core_axi_clk");
+ goto core_axi_clk_fail;
+ }
+
+ return 0;
+
+core_axi_clk_fail:
+ clk_disable_unprepare(sclks->smmu_core_axi_clk);
+smmu_core_axi_clk_fail:
+ clk_disable_unprepare(sclks->smmu_core_ahb_clk);
+core_ahb_clk_fail:
+ clk_disable_unprepare(sclks->mmagic_cfg_ahb_clk);
+cfg_ahb_clk_fail:
+ clk_disable_unprepare(sclks->mmagic_ahb_clk);
+ahb_clk_fail:
+ return ret;
+}
+
+static void qcom_smmu_disable_clocks(struct arm_smmu_device *smmu)
+{
+ struct qcom_smmu_clk *sclks = smmu->smmu_clks.clks;
+
+ if (!sclks) {
+ clk_disable_unprepare(sclks->mmagic_core_axi_clk);
+ clk_disable_unprepare(sclks->smmu_core_axi_clk);
+ clk_disable_unprepare(sclks->smmu_core_ahb_clk);
+ clk_disable_unprepare(sclks->mmagic_cfg_ahb_clk);
+ clk_disable_unprepare(sclks->mmagic_ahb_clk);
+ }
+}
+
static void parse_driver_options(struct arm_smmu_device *smmu)
{
int i = 0;
@@ -2077,6 +2197,9 @@ struct arm_smmu_match_data {
mmu500_enable_clocks, mmu500_disable_clocks);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2,
NULL, NULL, NULL);
+ARM_SMMU_MATCH_DATA(qcom_smmuv2, ARM_SMMU_V2, QCOM_SMMUV2,
+ qcom_smmu_init_clocks, qcom_smmu_enable_clocks,
+ qcom_smmu_disable_clocks);
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = &smmu_generic_v1 },
@@ -2085,6 +2208,7 @@ struct arm_smmu_match_data {
{ .compatible = "arm,mmu-401", .data = &arm_mmu401 },
{ .compatible = "arm,mmu-500", .data = &arm_mmu500 },
{ .compatible = "cavium,smmu-v2", .data = &cavium_smmuv2 },
+ { .compatible = "qcom,smmu-v2", .data = &qcom_smmuv2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH V3 3/5] drivers: arm-smmu: Add clock support for QCOM_SMMUV2
2017-03-09 15:35 ` Sricharan R
@ 2017-03-16 21:10 ` Rob Herring
-1 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2017-03-16 21:10 UTC (permalink / raw)
To: Sricharan R
Cc: robin.murphy, will.deacon, joro, iommu, linux-arm-kernel,
linux-arm-msm, m.szyprowski, linux-clk, sboyd, devicetree,
mathieu.poirier, mark.rutland
On Thu, Mar 09, 2017 at 09:05:46PM +0530, Sricharan R wrote:
> The QCOM_SMMUV2 is an implementation of the arm,smmu-v2 architecture.
> The qcom,smmu is instantiated for each of the multimedia cores (for eg)
> Venus (video encoder/decoder), mdp (display) etc, and they are connected
> to the Multimedia Aggregator Interconnect (MMAGIC). So the access to
> any of the MMU's registers, as well as MMU's downstream bus access,
> requires the specified MMAGIC clocks to be enabled. So adding a new
> binding for the qcom,smmu-v2 and the required mmagic clock bindings for
> the same. Also adding the support for enabling the qcom,smmu-v2 clocks in
> the driver.
>
> ------------- ---------
> | VENUS | | MDP |
> | | | |
> ------------- --------
> | |
> | |
> ------ ---------
> |SMMU | | SMMU |
> | | | |
> ------ --------
> | |
> | |
> -----------------------------------------
> | MMAGIC INTERCONNECT (MMSS NOC) |
> | |
> -----------------------------------------
> | |
> | ----------------------------------
> ----- | SYSTEM NOC |
> |DDR| | |
> ----- ---------------------------------
> | |
> | ------
> |<-------------| CPU|
> ------
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 8 ++
> drivers/iommu/arm-smmu.c | 124 +++++++++++++++++++++
> 2 files changed, 132 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index b369c13..88e02d6 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -17,6 +17,7 @@ conditions.
> "arm,mmu-401"
> "arm,mmu-500"
> "cavium,smmu-v2"
> + "qcom,smmu-v2"
I know Cavium did it, but I'd prefer to see SoC specific compatibles
here.
>
> depending on the particular implementation and/or the
> version of the architecture implemented.
> @@ -74,6 +75,13 @@ conditions.
> "cfg_clk" is optional if required to access the TCU's programming
> interface, apart from the "tcu_clk".
>
> + Should have "mmagic_ahb_clk", "mmagic_cfg_ahb_clk",
> + "smmu_core_ahb_clk", "smmu_core_axi_clk",
> + "mmagic_core_axi_clk" for "qcom,smmu-v2"
This is instead of the above clocks?
Are these clocks all really part of the SMMU or are the mmagic clocks
working around no proper driver for the mmagic?
> +
> + "mmagic_core_axi_clk" is required for smmu's access to the
> + downstream bus and rest for the smmu's register group access.
> +
> - clocks: Phandles for respective clocks described by clock-names.
>
> - power-domains: Phandles to SMMU's power domain specifier. This is
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH V3 3/5] drivers: arm-smmu: Add clock support for QCOM_SMMUV2
@ 2017-03-16 21:10 ` Rob Herring
0 siblings, 0 replies; 47+ messages in thread
From: Rob Herring @ 2017-03-16 21:10 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 09, 2017 at 09:05:46PM +0530, Sricharan R wrote:
> The QCOM_SMMUV2 is an implementation of the arm,smmu-v2 architecture.
> The qcom,smmu is instantiated for each of the multimedia cores (for eg)
> Venus (video encoder/decoder), mdp (display) etc, and they are connected
> to the Multimedia Aggregator Interconnect (MMAGIC). So the access to
> any of the MMU's registers, as well as MMU's downstream bus access,
> requires the specified MMAGIC clocks to be enabled. So adding a new
> binding for the qcom,smmu-v2 and the required mmagic clock bindings for
> the same. Also adding the support for enabling the qcom,smmu-v2 clocks in
> the driver.
>
> ------------- ---------
> | VENUS | | MDP |
> | | | |
> ------------- --------
> | |
> | |
> ------ ---------
> |SMMU | | SMMU |
> | | | |
> ------ --------
> | |
> | |
> -----------------------------------------
> | MMAGIC INTERCONNECT (MMSS NOC) |
> | |
> -----------------------------------------
> | |
> | ----------------------------------
> ----- | SYSTEM NOC |
> |DDR| | |
> ----- ---------------------------------
> | |
> | ------
> |<-------------| CPU|
> ------
>
> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
> ---
> .../devicetree/bindings/iommu/arm,smmu.txt | 8 ++
> drivers/iommu/arm-smmu.c | 124 +++++++++++++++++++++
> 2 files changed, 132 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index b369c13..88e02d6 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -17,6 +17,7 @@ conditions.
> "arm,mmu-401"
> "arm,mmu-500"
> "cavium,smmu-v2"
> + "qcom,smmu-v2"
I know Cavium did it, but I'd prefer to see SoC specific compatibles
here.
>
> depending on the particular implementation and/or the
> version of the architecture implemented.
> @@ -74,6 +75,13 @@ conditions.
> "cfg_clk" is optional if required to access the TCU's programming
> interface, apart from the "tcu_clk".
>
> + Should have "mmagic_ahb_clk", "mmagic_cfg_ahb_clk",
> + "smmu_core_ahb_clk", "smmu_core_axi_clk",
> + "mmagic_core_axi_clk" for "qcom,smmu-v2"
This is instead of the above clocks?
Are these clocks all really part of the SMMU or are the mmagic clocks
working around no proper driver for the mmagic?
> +
> + "mmagic_core_axi_clk" is required for smmu's access to the
> + downstream bus and rest for the smmu's register group access.
> +
> - clocks: Phandles for respective clocks described by clock-names.
>
> - power-domains: Phandles to SMMU's power domain specifier. This is
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 3/5] drivers: arm-smmu: Add clock support for QCOM_SMMUV2
2017-03-16 21:10 ` Rob Herring
(?)
@ 2017-03-20 14:31 ` Sricharan R
-1 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-20 14:31 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, will.deacon-5wv7dgnIgG8,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Hi Rob,
On 3/17/2017 2:40 AM, Rob Herring wrote:
> On Thu, Mar 09, 2017 at 09:05:46PM +0530, Sricharan R wrote:
>> The QCOM_SMMUV2 is an implementation of the arm,smmu-v2 architecture.
>> The qcom,smmu is instantiated for each of the multimedia cores (for eg)
>> Venus (video encoder/decoder), mdp (display) etc, and they are connected
>> to the Multimedia Aggregator Interconnect (MMAGIC). So the access to
>> any of the MMU's registers, as well as MMU's downstream bus access,
>> requires the specified MMAGIC clocks to be enabled. So adding a new
>> binding for the qcom,smmu-v2 and the required mmagic clock bindings for
>> the same. Also adding the support for enabling the qcom,smmu-v2 clocks in
>> the driver.
>>
>> ------------- ---------
>> | VENUS | | MDP |
>> | | | |
>> ------------- --------
>> | |
>> | |
>> ------ ---------
>> |SMMU | | SMMU |
>> | | | |
>> ------ --------
>> | |
>> | |
>> -----------------------------------------
>> | MMAGIC INTERCONNECT (MMSS NOC) |
>> | |
>> -----------------------------------------
>> | |
>> | ----------------------------------
>> ----- | SYSTEM NOC |
>> |DDR| | |
>> ----- ---------------------------------
>> | |
>> | ------
>> |<-------------| CPU|
>> ------
>>
>> Signed-off-by: Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 8 ++
>> drivers/iommu/arm-smmu.c | 124 +++++++++++++++++++++
>> 2 files changed, 132 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index b369c13..88e02d6 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -17,6 +17,7 @@ conditions.
>> "arm,mmu-401"
>> "arm,mmu-500"
>> "cavium,smmu-v2"
>> + "qcom,smmu-v2"
>
> I know Cavium did it, but I'd prefer to see SoC specific compatibles
> here.
ok, will change it to be soc specific.
>
>>
>> depending on the particular implementation and/or the
>> version of the architecture implemented.
>> @@ -74,6 +75,13 @@ conditions.
>> "cfg_clk" is optional if required to access the TCU's programming
>> interface, apart from the "tcu_clk".
>>
>> + Should have "mmagic_ahb_clk", "mmagic_cfg_ahb_clk",
>> + "smmu_core_ahb_clk", "smmu_core_axi_clk",
>> + "mmagic_core_axi_clk" for "qcom,smmu-v2"
>
> This is instead of the above clocks?
>
These clocks are for 'qcom,smmu-v2'. I should have put
that first, then the clock names.
> Are these clocks all really part of the SMMU or are the mmagic clocks
> working around no proper driver for the mmagic?
>
infact because of the absence of the mmagic driver to handle it.
But i think, i will have to rework this, because handling mmagic
clocks is going to pushed elsewhere, to the the gdscs(powerdomains).
So adding the mmagic clocks should not be required here after that.
Regards,
Sricharan
>> +
>> + "mmagic_core_axi_clk" is required for smmu's access to the
>> + downstream bus and rest for the smmu's register group access.
>> +
>> - clocks: Phandles for respective clocks described by clock-names.
>>
>> - power-domains: Phandles to SMMU's power domain specifier. This is
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH V3 3/5] drivers: arm-smmu: Add clock support for QCOM_SMMUV2
@ 2017-03-20 14:31 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-20 14:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rob,
On 3/17/2017 2:40 AM, Rob Herring wrote:
> On Thu, Mar 09, 2017 at 09:05:46PM +0530, Sricharan R wrote:
>> The QCOM_SMMUV2 is an implementation of the arm,smmu-v2 architecture.
>> The qcom,smmu is instantiated for each of the multimedia cores (for eg)
>> Venus (video encoder/decoder), mdp (display) etc, and they are connected
>> to the Multimedia Aggregator Interconnect (MMAGIC). So the access to
>> any of the MMU's registers, as well as MMU's downstream bus access,
>> requires the specified MMAGIC clocks to be enabled. So adding a new
>> binding for the qcom,smmu-v2 and the required mmagic clock bindings for
>> the same. Also adding the support for enabling the qcom,smmu-v2 clocks in
>> the driver.
>>
>> ------------- ---------
>> | VENUS | | MDP |
>> | | | |
>> ------------- --------
>> | |
>> | |
>> ------ ---------
>> |SMMU | | SMMU |
>> | | | |
>> ------ --------
>> | |
>> | |
>> -----------------------------------------
>> | MMAGIC INTERCONNECT (MMSS NOC) |
>> | |
>> -----------------------------------------
>> | |
>> | ----------------------------------
>> ----- | SYSTEM NOC |
>> |DDR| | |
>> ----- ---------------------------------
>> | |
>> | ------
>> |<-------------| CPU|
>> ------
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 8 ++
>> drivers/iommu/arm-smmu.c | 124 +++++++++++++++++++++
>> 2 files changed, 132 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index b369c13..88e02d6 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -17,6 +17,7 @@ conditions.
>> "arm,mmu-401"
>> "arm,mmu-500"
>> "cavium,smmu-v2"
>> + "qcom,smmu-v2"
>
> I know Cavium did it, but I'd prefer to see SoC specific compatibles
> here.
ok, will change it to be soc specific.
>
>>
>> depending on the particular implementation and/or the
>> version of the architecture implemented.
>> @@ -74,6 +75,13 @@ conditions.
>> "cfg_clk" is optional if required to access the TCU's programming
>> interface, apart from the "tcu_clk".
>>
>> + Should have "mmagic_ahb_clk", "mmagic_cfg_ahb_clk",
>> + "smmu_core_ahb_clk", "smmu_core_axi_clk",
>> + "mmagic_core_axi_clk" for "qcom,smmu-v2"
>
> This is instead of the above clocks?
>
These clocks are for 'qcom,smmu-v2'. I should have put
that first, then the clock names.
> Are these clocks all really part of the SMMU or are the mmagic clocks
> working around no proper driver for the mmagic?
>
infact because of the absence of the mmagic driver to handle it.
But i think, i will have to rework this, because handling mmagic
clocks is going to pushed elsewhere, to the the gdscs(powerdomains).
So adding the mmagic clocks should not be required here after that.
Regards,
Sricharan
>> +
>> + "mmagic_core_axi_clk" is required for smmu's access to the
>> + downstream bus and rest for the smmu's register group access.
>> +
>> - clocks: Phandles for respective clocks described by clock-names.
>>
>> - power-domains: Phandles to SMMU's power domain specifier. This is
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 3/5] drivers: arm-smmu: Add clock support for QCOM_SMMUV2
@ 2017-03-20 14:31 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-03-20 14:31 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland, devicetree, mathieu.poirier, linux-arm-msm, joro,
sboyd, will.deacon, iommu, robin.murphy, linux-clk,
linux-arm-kernel, m.szyprowski
Hi Rob,
On 3/17/2017 2:40 AM, Rob Herring wrote:
> On Thu, Mar 09, 2017 at 09:05:46PM +0530, Sricharan R wrote:
>> The QCOM_SMMUV2 is an implementation of the arm,smmu-v2 architecture.
>> The qcom,smmu is instantiated for each of the multimedia cores (for eg)
>> Venus (video encoder/decoder), mdp (display) etc, and they are connected
>> to the Multimedia Aggregator Interconnect (MMAGIC). So the access to
>> any of the MMU's registers, as well as MMU's downstream bus access,
>> requires the specified MMAGIC clocks to be enabled. So adding a new
>> binding for the qcom,smmu-v2 and the required mmagic clock bindings for
>> the same. Also adding the support for enabling the qcom,smmu-v2 clocks in
>> the driver.
>>
>> ------------- ---------
>> | VENUS | | MDP |
>> | | | |
>> ------------- --------
>> | |
>> | |
>> ------ ---------
>> |SMMU | | SMMU |
>> | | | |
>> ------ --------
>> | |
>> | |
>> -----------------------------------------
>> | MMAGIC INTERCONNECT (MMSS NOC) |
>> | |
>> -----------------------------------------
>> | |
>> | ----------------------------------
>> ----- | SYSTEM NOC |
>> |DDR| | |
>> ----- ---------------------------------
>> | |
>> | ------
>> |<-------------| CPU|
>> ------
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>> .../devicetree/bindings/iommu/arm,smmu.txt | 8 ++
>> drivers/iommu/arm-smmu.c | 124 +++++++++++++++++++++
>> 2 files changed, 132 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> index b369c13..88e02d6 100644
>> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
>> @@ -17,6 +17,7 @@ conditions.
>> "arm,mmu-401"
>> "arm,mmu-500"
>> "cavium,smmu-v2"
>> + "qcom,smmu-v2"
>
> I know Cavium did it, but I'd prefer to see SoC specific compatibles
> here.
ok, will change it to be soc specific.
>
>>
>> depending on the particular implementation and/or the
>> version of the architecture implemented.
>> @@ -74,6 +75,13 @@ conditions.
>> "cfg_clk" is optional if required to access the TCU's programming
>> interface, apart from the "tcu_clk".
>>
>> + Should have "mmagic_ahb_clk", "mmagic_cfg_ahb_clk",
>> + "smmu_core_ahb_clk", "smmu_core_axi_clk",
>> + "mmagic_core_axi_clk" for "qcom,smmu-v2"
>
> This is instead of the above clocks?
>
These clocks are for 'qcom,smmu-v2'. I should have put
that first, then the clock names.
> Are these clocks all really part of the SMMU or are the mmagic clocks
> working around no proper driver for the mmagic?
>
infact because of the absence of the mmagic driver to handle it.
But i think, i will have to rework this, because handling mmagic
clocks is going to pushed elsewhere, to the the gdscs(powerdomains).
So adding the mmagic clocks should not be required here after that.
Regards,
Sricharan
>> +
>> + "mmagic_core_axi_clk" is required for smmu's access to the
>> + downstream bus and rest for the smmu's register group access.
>> +
>> - clocks: Phandles for respective clocks described by clock-names.
>>
>> - power-domains: Phandles to SMMU's power domain specifier. This is
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
2017-03-09 15:35 ` Sricharan R
(?)
@ 2017-03-31 17:54 ` Will Deacon
-1 siblings, 0 replies; 47+ messages in thread
From: Will Deacon @ 2017-03-31 17:54 UTC (permalink / raw)
To: Sricharan R
Cc: robin.murphy-5wv7dgnIgG8, joro-zLv9SwRftAIdnm+yROfE0A,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ,
linux-clk-u79uwXL29TY76Z2rM5mHXA, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8
On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
> This series provides the support for turning on the arm-smmu's
> clocks/power domains using runtime pm. This is done using the
> recently introduced device links patches, which lets the symmu's
> runtime to follow the master's runtime pm, so the smmu remains
> powered only when the masters use it.
Do you have any numbers for the power savings you achieve with this?
How often do we actually manage to stop the SMMU clocks on an SoC with
a handful of masters?
In other words, is this too coarse-grained to be useful, or is it common
that all the devices upstream of the SMMU are suspended?
Thanks,
Will
>
> Took some reference from the exynos runtime patches [2].
> Tested this with MDP, GPU, VENUS devices on apq8096-db820c board.
>
> Previous version of the patchset [1].
>
> [V3]
> * Reworked the patches to keep the clocks init/enabling function
> seperately for each compatible.
>
> * Added clocks bindings for MMU40x/500.
>
> * Added a new compatible for qcom,smmu-v2 implementation and
> the clock bindings for the same.
>
> * Rebased on top of 4.11-rc1
>
> [V2]
> * Split the patches little differently.
>
> * Addressed comments.
>
> * Removed the patch #4 [3] from previous post
> for arm-smmu context save restore. Planning to
> post this separately after reworking/addressing Robin's
> feedback.
>
> * Reversed the sequence to disable clocks than enabling.
> This was required for those cases where the
> clocks are populated in a dependent order from DT.
>
> [1] https://www.spinics.net/lists/linux-arm-msm/msg23870.html
> [2] https://lkml.org/lkml/2016/10/20/70
> [3] https://patchwork.kernel.org/patch/9389717/
>
> Sricharan R (5):
> iommu/arm-smmu: Add pm_runtime/sleep ops
> iommu/arm-smmu: Add support for MMU40x/500 clocks
> drivers: arm-smmu: Add clock support for QCOM_SMMUV2
> iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
> iommu/arm-smmu: Add the device_link between masters and smmu
>
> .../devicetree/bindings/iommu/arm,smmu.txt | 35 +++
> drivers/iommu/arm-smmu.c | 349 ++++++++++++++++++++-
> 2 files changed, 373 insertions(+), 11 deletions(-)
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
--
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
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-03-31 17:54 ` Will Deacon
0 siblings, 0 replies; 47+ messages in thread
From: Will Deacon @ 2017-03-31 17:54 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
> This series provides the support for turning on the arm-smmu's
> clocks/power domains using runtime pm. This is done using the
> recently introduced device links patches, which lets the symmu's
> runtime to follow the master's runtime pm, so the smmu remains
> powered only when the masters use it.
Do you have any numbers for the power savings you achieve with this?
How often do we actually manage to stop the SMMU clocks on an SoC with
a handful of masters?
In other words, is this too coarse-grained to be useful, or is it common
that all the devices upstream of the SMMU are suspended?
Thanks,
Will
>
> Took some reference from the exynos runtime patches [2].
> Tested this with MDP, GPU, VENUS devices on apq8096-db820c board.
>
> Previous version of the patchset [1].
>
> [V3]
> * Reworked the patches to keep the clocks init/enabling function
> seperately for each compatible.
>
> * Added clocks bindings for MMU40x/500.
>
> * Added a new compatible for qcom,smmu-v2 implementation and
> the clock bindings for the same.
>
> * Rebased on top of 4.11-rc1
>
> [V2]
> * Split the patches little differently.
>
> * Addressed comments.
>
> * Removed the patch #4 [3] from previous post
> for arm-smmu context save restore. Planning to
> post this separately after reworking/addressing Robin's
> feedback.
>
> * Reversed the sequence to disable clocks than enabling.
> This was required for those cases where the
> clocks are populated in a dependent order from DT.
>
> [1] https://www.spinics.net/lists/linux-arm-msm/msg23870.html
> [2] https://lkml.org/lkml/2016/10/20/70
> [3] https://patchwork.kernel.org/patch/9389717/
>
> Sricharan R (5):
> iommu/arm-smmu: Add pm_runtime/sleep ops
> iommu/arm-smmu: Add support for MMU40x/500 clocks
> drivers: arm-smmu: Add clock support for QCOM_SMMUV2
> iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
> iommu/arm-smmu: Add the device_link between masters and smmu
>
> .../devicetree/bindings/iommu/arm,smmu.txt | 35 +++
> drivers/iommu/arm-smmu.c | 349 ++++++++++++++++++++-
> 2 files changed, 373 insertions(+), 11 deletions(-)
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-03-31 17:54 ` Will Deacon
0 siblings, 0 replies; 47+ messages in thread
From: Will Deacon @ 2017-03-31 17:54 UTC (permalink / raw)
To: Sricharan R
Cc: robin.murphy, joro, iommu, linux-arm-kernel, linux-arm-msm,
m.szyprowski, linux-clk, sboyd, devicetree, robh+dt,
mathieu.poirier, mark.rutland
On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
> This series provides the support for turning on the arm-smmu's
> clocks/power domains using runtime pm. This is done using the
> recently introduced device links patches, which lets the symmu's
> runtime to follow the master's runtime pm, so the smmu remains
> powered only when the masters use it.
Do you have any numbers for the power savings you achieve with this?
How often do we actually manage to stop the SMMU clocks on an SoC with
a handful of masters?
In other words, is this too coarse-grained to be useful, or is it common
that all the devices upstream of the SMMU are suspended?
Thanks,
Will
>
> Took some reference from the exynos runtime patches [2].
> Tested this with MDP, GPU, VENUS devices on apq8096-db820c board.
>
> Previous version of the patchset [1].
>
> [V3]
> * Reworked the patches to keep the clocks init/enabling function
> seperately for each compatible.
>
> * Added clocks bindings for MMU40x/500.
>
> * Added a new compatible for qcom,smmu-v2 implementation and
> the clock bindings for the same.
>
> * Rebased on top of 4.11-rc1
>
> [V2]
> * Split the patches little differently.
>
> * Addressed comments.
>
> * Removed the patch #4 [3] from previous post
> for arm-smmu context save restore. Planning to
> post this separately after reworking/addressing Robin's
> feedback.
>
> * Reversed the sequence to disable clocks than enabling.
> This was required for those cases where the
> clocks are populated in a dependent order from DT.
>
> [1] https://www.spinics.net/lists/linux-arm-msm/msg23870.html
> [2] https://lkml.org/lkml/2016/10/20/70
> [3] https://patchwork.kernel.org/patch/9389717/
>
> Sricharan R (5):
> iommu/arm-smmu: Add pm_runtime/sleep ops
> iommu/arm-smmu: Add support for MMU40x/500 clocks
> drivers: arm-smmu: Add clock support for QCOM_SMMUV2
> iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
> iommu/arm-smmu: Add the device_link between masters and smmu
>
> .../devicetree/bindings/iommu/arm,smmu.txt | 35 +++
> drivers/iommu/arm-smmu.c | 349 ++++++++++++++++++++-
> 2 files changed, 373 insertions(+), 11 deletions(-)
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>
^ permalink raw reply [flat|nested] 47+ messages in thread
[parent not found: <20170331175457.GD4897-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
2017-03-31 17:54 ` Will Deacon
(?)
@ 2017-04-01 2:58 ` Rob Clark
-1 siblings, 0 replies; 47+ messages in thread
From: Rob Clark @ 2017-04-01 2:58 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Mathieu Poirier,
linux-arm-msm, Stephen Boyd,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
>> This series provides the support for turning on the arm-smmu's
>> clocks/power domains using runtime pm. This is done using the
>> recently introduced device links patches, which lets the symmu's
>> runtime to follow the master's runtime pm, so the smmu remains
>> powered only when the masters use it.
>
> Do you have any numbers for the power savings you achieve with this?
> How often do we actually manage to stop the SMMU clocks on an SoC with
> a handful of masters?
>
> In other words, is this too coarse-grained to be useful, or is it common
> that all the devices upstream of the SMMU are suspended?
well, if you think about a phone/tablet with a command mode panel,
pretty much all devices will be suspended most of the time ;-)
maybe it's a different case with servers.. unfortunately we have to
share the same driver across both..
BR,
-R
> Thanks,
>
> Will
>
>>
>> Took some reference from the exynos runtime patches [2].
>> Tested this with MDP, GPU, VENUS devices on apq8096-db820c board.
>>
>> Previous version of the patchset [1].
>>
>> [V3]
>> * Reworked the patches to keep the clocks init/enabling function
>> seperately for each compatible.
>>
>> * Added clocks bindings for MMU40x/500.
>>
>> * Added a new compatible for qcom,smmu-v2 implementation and
>> the clock bindings for the same.
>>
>> * Rebased on top of 4.11-rc1
>>
>> [V2]
>> * Split the patches little differently.
>>
>> * Addressed comments.
>>
>> * Removed the patch #4 [3] from previous post
>> for arm-smmu context save restore. Planning to
>> post this separately after reworking/addressing Robin's
>> feedback.
>>
>> * Reversed the sequence to disable clocks than enabling.
>> This was required for those cases where the
>> clocks are populated in a dependent order from DT.
>>
>> [1] https://www.spinics.net/lists/linux-arm-msm/msg23870.html
>> [2] https://lkml.org/lkml/2016/10/20/70
>> [3] https://patchwork.kernel.org/patch/9389717/
>>
>> Sricharan R (5):
>> iommu/arm-smmu: Add pm_runtime/sleep ops
>> iommu/arm-smmu: Add support for MMU40x/500 clocks
>> drivers: arm-smmu: Add clock support for QCOM_SMMUV2
>> iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
>> iommu/arm-smmu: Add the device_link between masters and smmu
>>
>> .../devicetree/bindings/iommu/arm,smmu.txt | 35 +++
>> drivers/iommu/arm-smmu.c | 349 ++++++++++++++++++++-
>> 2 files changed, 373 insertions(+), 11 deletions(-)
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-04-01 2:58 ` Rob Clark
0 siblings, 0 replies; 47+ messages in thread
From: Rob Clark @ 2017-04-01 2:58 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
>> This series provides the support for turning on the arm-smmu's
>> clocks/power domains using runtime pm. This is done using the
>> recently introduced device links patches, which lets the symmu's
>> runtime to follow the master's runtime pm, so the smmu remains
>> powered only when the masters use it.
>
> Do you have any numbers for the power savings you achieve with this?
> How often do we actually manage to stop the SMMU clocks on an SoC with
> a handful of masters?
>
> In other words, is this too coarse-grained to be useful, or is it common
> that all the devices upstream of the SMMU are suspended?
well, if you think about a phone/tablet with a command mode panel,
pretty much all devices will be suspended most of the time ;-)
maybe it's a different case with servers.. unfortunately we have to
share the same driver across both..
BR,
-R
> Thanks,
>
> Will
>
>>
>> Took some reference from the exynos runtime patches [2].
>> Tested this with MDP, GPU, VENUS devices on apq8096-db820c board.
>>
>> Previous version of the patchset [1].
>>
>> [V3]
>> * Reworked the patches to keep the clocks init/enabling function
>> seperately for each compatible.
>>
>> * Added clocks bindings for MMU40x/500.
>>
>> * Added a new compatible for qcom,smmu-v2 implementation and
>> the clock bindings for the same.
>>
>> * Rebased on top of 4.11-rc1
>>
>> [V2]
>> * Split the patches little differently.
>>
>> * Addressed comments.
>>
>> * Removed the patch #4 [3] from previous post
>> for arm-smmu context save restore. Planning to
>> post this separately after reworking/addressing Robin's
>> feedback.
>>
>> * Reversed the sequence to disable clocks than enabling.
>> This was required for those cases where the
>> clocks are populated in a dependent order from DT.
>>
>> [1] https://www.spinics.net/lists/linux-arm-msm/msg23870.html
>> [2] https://lkml.org/lkml/2016/10/20/70
>> [3] https://patchwork.kernel.org/patch/9389717/
>>
>> Sricharan R (5):
>> iommu/arm-smmu: Add pm_runtime/sleep ops
>> iommu/arm-smmu: Add support for MMU40x/500 clocks
>> drivers: arm-smmu: Add clock support for QCOM_SMMUV2
>> iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
>> iommu/arm-smmu: Add the device_link between masters and smmu
>>
>> .../devicetree/bindings/iommu/arm,smmu.txt | 35 +++
>> drivers/iommu/arm-smmu.c | 349 ++++++++++++++++++++-
>> 2 files changed, 373 insertions(+), 11 deletions(-)
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>
> _______________________________________________
> iommu mailing list
> iommu at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-04-01 2:58 ` Rob Clark
0 siblings, 0 replies; 47+ messages in thread
From: Rob Clark @ 2017-04-01 2:58 UTC (permalink / raw)
To: Will Deacon
Cc: Sricharan R, Mark Rutland, devicetree, Mathieu Poirier,
linux-arm-msm, Stephen Boyd, iommu, Rob Herring, linux-clk,
linux-arm-kernel
On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
>> This series provides the support for turning on the arm-smmu's
>> clocks/power domains using runtime pm. This is done using the
>> recently introduced device links patches, which lets the symmu's
>> runtime to follow the master's runtime pm, so the smmu remains
>> powered only when the masters use it.
>
> Do you have any numbers for the power savings you achieve with this?
> How often do we actually manage to stop the SMMU clocks on an SoC with
> a handful of masters?
>
> In other words, is this too coarse-grained to be useful, or is it common
> that all the devices upstream of the SMMU are suspended?
well, if you think about a phone/tablet with a command mode panel,
pretty much all devices will be suspended most of the time ;-)
maybe it's a different case with servers.. unfortunately we have to
share the same driver across both..
BR,
-R
> Thanks,
>
> Will
>
>>
>> Took some reference from the exynos runtime patches [2].
>> Tested this with MDP, GPU, VENUS devices on apq8096-db820c board.
>>
>> Previous version of the patchset [1].
>>
>> [V3]
>> * Reworked the patches to keep the clocks init/enabling function
>> seperately for each compatible.
>>
>> * Added clocks bindings for MMU40x/500.
>>
>> * Added a new compatible for qcom,smmu-v2 implementation and
>> the clock bindings for the same.
>>
>> * Rebased on top of 4.11-rc1
>>
>> [V2]
>> * Split the patches little differently.
>>
>> * Addressed comments.
>>
>> * Removed the patch #4 [3] from previous post
>> for arm-smmu context save restore. Planning to
>> post this separately after reworking/addressing Robin's
>> feedback.
>>
>> * Reversed the sequence to disable clocks than enabling.
>> This was required for those cases where the
>> clocks are populated in a dependent order from DT.
>>
>> [1] https://www.spinics.net/lists/linux-arm-msm/msg23870.html
>> [2] https://lkml.org/lkml/2016/10/20/70
>> [3] https://patchwork.kernel.org/patch/9389717/
>>
>> Sricharan R (5):
>> iommu/arm-smmu: Add pm_runtime/sleep ops
>> iommu/arm-smmu: Add support for MMU40x/500 clocks
>> drivers: arm-smmu: Add clock support for QCOM_SMMUV2
>> iommu/arm-smmu: Invoke pm_runtime during probe, add/remove device
>> iommu/arm-smmu: Add the device_link between masters and smmu
>>
>> .../devicetree/bindings/iommu/arm,smmu.txt | 35 +++
>> drivers/iommu/arm-smmu.c | 349 ++++++++++++++++++++-
>> 2 files changed, 373 insertions(+), 11 deletions(-)
>>
>> --
>> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
>>
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
2017-04-01 2:58 ` Rob Clark
@ 2017-04-03 17:23 ` Will Deacon
-1 siblings, 0 replies; 47+ messages in thread
From: Will Deacon @ 2017-04-03 17:23 UTC (permalink / raw)
To: Rob Clark
Cc: Sricharan R, Mark Rutland, devicetree, Mathieu Poirier,
linux-arm-msm, Stephen Boyd, iommu, Rob Herring, linux-clk,
linux-arm-kernel
On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
> On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
> >> This series provides the support for turning on the arm-smmu's
> >> clocks/power domains using runtime pm. This is done using the
> >> recently introduced device links patches, which lets the symmu's
> >> runtime to follow the master's runtime pm, so the smmu remains
> >> powered only when the masters use it.
> >
> > Do you have any numbers for the power savings you achieve with this?
> > How often do we actually manage to stop the SMMU clocks on an SoC with
> > a handful of masters?
> >
> > In other words, is this too coarse-grained to be useful, or is it common
> > that all the devices upstream of the SMMU are suspended?
>
> well, if you think about a phone/tablet with a command mode panel,
> pretty much all devices will be suspended most of the time ;-)
Well, that's really what I was asking about. I assumed that periodic
modem/radio transactions would keep the SMMU clocked, so would like to get a
rough idea of the power savings achieved with this coarse-grained approach.
Will
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-04-03 17:23 ` Will Deacon
0 siblings, 0 replies; 47+ messages in thread
From: Will Deacon @ 2017-04-03 17:23 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
> On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
> > On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
> >> This series provides the support for turning on the arm-smmu's
> >> clocks/power domains using runtime pm. This is done using the
> >> recently introduced device links patches, which lets the symmu's
> >> runtime to follow the master's runtime pm, so the smmu remains
> >> powered only when the masters use it.
> >
> > Do you have any numbers for the power savings you achieve with this?
> > How often do we actually manage to stop the SMMU clocks on an SoC with
> > a handful of masters?
> >
> > In other words, is this too coarse-grained to be useful, or is it common
> > that all the devices upstream of the SMMU are suspended?
>
> well, if you think about a phone/tablet with a command mode panel,
> pretty much all devices will be suspended most of the time ;-)
Well, that's really what I was asking about. I assumed that periodic
modem/radio transactions would keep the SMMU clocked, so would like to get a
rough idea of the power savings achieved with this coarse-grained approach.
Will
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
2017-04-03 17:23 ` Will Deacon
@ 2017-04-04 5:15 ` Sricharan R
-1 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-04-04 5:15 UTC (permalink / raw)
To: Will Deacon, Rob Clark
Cc: Mark Rutland, devicetree, Mathieu Poirier, linux-arm-msm,
Stephen Boyd, iommu, Rob Herring, linux-clk, linux-arm-kernel
Hi Will,
On 4/3/2017 10:53 PM, Will Deacon wrote:
> On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
>> On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
>>> On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
>>>> This series provides the support for turning on the arm-smmu's
>>>> clocks/power domains using runtime pm. This is done using the
>>>> recently introduced device links patches, which lets the symmu's
>>>> runtime to follow the master's runtime pm, so the smmu remains
>>>> powered only when the masters use it.
>>>
>>> Do you have any numbers for the power savings you achieve with this?
>>> How often do we actually manage to stop the SMMU clocks on an SoC with
>>> a handful of masters?
>>>
>>> In other words, is this too coarse-grained to be useful, or is it common
>>> that all the devices upstream of the SMMU are suspended?
>>
>> well, if you think about a phone/tablet with a command mode panel,
>> pretty much all devices will be suspended most of the time ;-)
>
> Well, that's really what I was asking about. I assumed that periodic
> modem/radio transactions would keep the SMMU clocked, so would like to get a
> rough idea of the power savings achieved with this coarse-grained approach.
>
One main reason for introducing this was to enable power for
the iommus separately in those places where the iommu gets
accessed without the context of the master, pm runtime was
done to use the device links feature and also those iommus
had their power-domains to be enabled (during the iommu probe,
faults) (downstream was modelling those power-domains as
'regulators' which was not correct) and have to be clocked
as well.
I was in the process of trying to measure the power difference
that this would achieve. One concern here is, this series depends
on the device link between master and iommu.
So essentially the masters have to be pm runtime adapted fully
to use this. For my testing i was using couple of them (mdp, gpu),
by just enabling pm runtime for them, not full pm runtime though.
But i will come-up with the numbers by instrumenting little more.
The downstream code explicitly turns on the iommu clocks/regulators
in the standalone path (called without master context and after
that, lets the master to control the iommu clocks ( the iommu
clocks are populated in master DT data as well), so ensures iommu
is clocked only when really needed by the master. So number
measured from downstream should also give the power numbers in
another way.
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-04-04 5:15 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-04-04 5:15 UTC (permalink / raw)
To: linux-arm-kernel
Hi Will,
On 4/3/2017 10:53 PM, Will Deacon wrote:
> On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
>> On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
>>> On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
>>>> This series provides the support for turning on the arm-smmu's
>>>> clocks/power domains using runtime pm. This is done using the
>>>> recently introduced device links patches, which lets the symmu's
>>>> runtime to follow the master's runtime pm, so the smmu remains
>>>> powered only when the masters use it.
>>>
>>> Do you have any numbers for the power savings you achieve with this?
>>> How often do we actually manage to stop the SMMU clocks on an SoC with
>>> a handful of masters?
>>>
>>> In other words, is this too coarse-grained to be useful, or is it common
>>> that all the devices upstream of the SMMU are suspended?
>>
>> well, if you think about a phone/tablet with a command mode panel,
>> pretty much all devices will be suspended most of the time ;-)
>
> Well, that's really what I was asking about. I assumed that periodic
> modem/radio transactions would keep the SMMU clocked, so would like to get a
> rough idea of the power savings achieved with this coarse-grained approach.
>
One main reason for introducing this was to enable power for
the iommus separately in those places where the iommu gets
accessed without the context of the master, pm runtime was
done to use the device links feature and also those iommus
had their power-domains to be enabled (during the iommu probe,
faults) (downstream was modelling those power-domains as
'regulators' which was not correct) and have to be clocked
as well.
I was in the process of trying to measure the power difference
that this would achieve. One concern here is, this series depends
on the device link between master and iommu.
So essentially the masters have to be pm runtime adapted fully
to use this. For my testing i was using couple of them (mdp, gpu),
by just enabling pm runtime for them, not full pm runtime though.
But i will come-up with the numbers by instrumenting little more.
The downstream code explicitly turns on the iommu clocks/regulators
in the standalone path (called without master context and after
that, lets the master to control the iommu clocks ( the iommu
clocks are populated in master DT data as well), so ensures iommu
is clocked only when really needed by the master. So number
measured from downstream should also give the power numbers in
another way.
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread
[parent not found: <20170403172307.GI5706-5wv7dgnIgG8@public.gmane.org>]
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
2017-04-03 17:23 ` Will Deacon
(?)
@ 2017-04-04 19:39 ` Stephen Boyd
-1 siblings, 0 replies; 47+ messages in thread
From: Stephen Boyd @ 2017-04-04 19:39 UTC (permalink / raw)
To: Will Deacon
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Mathieu Poirier,
linux-arm-msm, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Rob Herring, linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
On 04/03, Will Deacon wrote:
> On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
> > On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> > > On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
> > >> This series provides the support for turning on the arm-smmu's
> > >> clocks/power domains using runtime pm. This is done using the
> > >> recently introduced device links patches, which lets the symmu's
> > >> runtime to follow the master's runtime pm, so the smmu remains
> > >> powered only when the masters use it.
> > >
> > > Do you have any numbers for the power savings you achieve with this?
> > > How often do we actually manage to stop the SMMU clocks on an SoC with
> > > a handful of masters?
> > >
> > > In other words, is this too coarse-grained to be useful, or is it common
> > > that all the devices upstream of the SMMU are suspended?
> >
> > well, if you think about a phone/tablet with a command mode panel,
> > pretty much all devices will be suspended most of the time ;-)
>
> Well, that's really what I was asking about. I assumed that periodic
> modem/radio transactions would keep the SMMU clocked, so would like to get a
> rough idea of the power savings achieved with this coarse-grained approach.
Sometimes we distribute SMMUs to each IP block in the system and
let each one of those live in their own clock + power domain. In
these cases, the SMMU can be powered down along with the only IP
block that uses it. Furthermore, sometimes we put the IP block
and the SMMU inside the same power domain to really tie the two
together, so we definitely have cases where all devices (device?)
upstream of the SMMU are suspended. And in the case of
multimedia, it could be very often that something like the camera
app isn't open and thus the SMMU dedicated for the camera can be
powered down.
Other times we have two SMMUs in the system where one is
dedicated to GPU and the other is "everything else". Even in
these cases, we can suspend the GPU one when the GPU is inactive
because it's the only consumer. The other SMMU might not be as
fine grained, but I think we still power it down quite often
because the consumers are mostly multimedia devices that aren't
active when the display is off.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-04-04 19:39 ` Stephen Boyd
0 siblings, 0 replies; 47+ messages in thread
From: Stephen Boyd @ 2017-04-04 19:39 UTC (permalink / raw)
To: linux-arm-kernel
On 04/03, Will Deacon wrote:
> On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
> > On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
> > > On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
> > >> This series provides the support for turning on the arm-smmu's
> > >> clocks/power domains using runtime pm. This is done using the
> > >> recently introduced device links patches, which lets the symmu's
> > >> runtime to follow the master's runtime pm, so the smmu remains
> > >> powered only when the masters use it.
> > >
> > > Do you have any numbers for the power savings you achieve with this?
> > > How often do we actually manage to stop the SMMU clocks on an SoC with
> > > a handful of masters?
> > >
> > > In other words, is this too coarse-grained to be useful, or is it common
> > > that all the devices upstream of the SMMU are suspended?
> >
> > well, if you think about a phone/tablet with a command mode panel,
> > pretty much all devices will be suspended most of the time ;-)
>
> Well, that's really what I was asking about. I assumed that periodic
> modem/radio transactions would keep the SMMU clocked, so would like to get a
> rough idea of the power savings achieved with this coarse-grained approach.
Sometimes we distribute SMMUs to each IP block in the system and
let each one of those live in their own clock + power domain. In
these cases, the SMMU can be powered down along with the only IP
block that uses it. Furthermore, sometimes we put the IP block
and the SMMU inside the same power domain to really tie the two
together, so we definitely have cases where all devices (device?)
upstream of the SMMU are suspended. And in the case of
multimedia, it could be very often that something like the camera
app isn't open and thus the SMMU dedicated for the camera can be
powered down.
Other times we have two SMMUs in the system where one is
dedicated to GPU and the other is "everything else". Even in
these cases, we can suspend the GPU one when the GPU is inactive
because it's the only consumer. The other SMMU might not be as
fine grained, but I think we still power it down quite often
because the consumers are mostly multimedia devices that aren't
active when the display is off.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-04-04 19:39 ` Stephen Boyd
0 siblings, 0 replies; 47+ messages in thread
From: Stephen Boyd @ 2017-04-04 19:39 UTC (permalink / raw)
To: Will Deacon
Cc: Rob Clark, Sricharan R, Mark Rutland, devicetree,
Mathieu Poirier, linux-arm-msm, iommu, Rob Herring, linux-clk,
linux-arm-kernel
On 04/03, Will Deacon wrote:
> On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
> > On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
> > > On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
> > >> This series provides the support for turning on the arm-smmu's
> > >> clocks/power domains using runtime pm. This is done using the
> > >> recently introduced device links patches, which lets the symmu's
> > >> runtime to follow the master's runtime pm, so the smmu remains
> > >> powered only when the masters use it.
> > >
> > > Do you have any numbers for the power savings you achieve with this?
> > > How often do we actually manage to stop the SMMU clocks on an SoC with
> > > a handful of masters?
> > >
> > > In other words, is this too coarse-grained to be useful, or is it common
> > > that all the devices upstream of the SMMU are suspended?
> >
> > well, if you think about a phone/tablet with a command mode panel,
> > pretty much all devices will be suspended most of the time ;-)
>
> Well, that's really what I was asking about. I assumed that periodic
> modem/radio transactions would keep the SMMU clocked, so would like to get a
> rough idea of the power savings achieved with this coarse-grained approach.
Sometimes we distribute SMMUs to each IP block in the system and
let each one of those live in their own clock + power domain. In
these cases, the SMMU can be powered down along with the only IP
block that uses it. Furthermore, sometimes we put the IP block
and the SMMU inside the same power domain to really tie the two
together, so we definitely have cases where all devices (device?)
upstream of the SMMU are suspended. And in the case of
multimedia, it could be very often that something like the camera
app isn't open and thus the SMMU dedicated for the camera can be
powered down.
Other times we have two SMMUs in the system where one is
dedicated to GPU and the other is "everything else". Even in
these cases, we can suspend the GPU one when the GPU is inactive
because it's the only consumer. The other SMMU might not be as
fine grained, but I think we still power it down quite often
because the consumers are mostly multimedia devices that aren't
active when the display is off.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
2017-04-04 19:39 ` Stephen Boyd
@ 2017-04-07 18:01 ` Jordan Crouse
-1 siblings, 0 replies; 47+ messages in thread
From: Jordan Crouse @ 2017-04-07 18:01 UTC (permalink / raw)
To: Stephen Boyd
Cc: Will Deacon, Mark Rutland, devicetree, Mathieu Poirier,
linux-arm-msm, iommu, Rob Herring, linux-clk, linux-arm-kernel
On Tue, Apr 04, 2017 at 12:39:14PM -0700, Stephen Boyd wrote:
> On 04/03, Will Deacon wrote:
> > On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
> > > On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
> > > > On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
> > > >> This series provides the support for turning on the arm-smmu's
> > > >> clocks/power domains using runtime pm. This is done using the
> > > >> recently introduced device links patches, which lets the symmu's
> > > >> runtime to follow the master's runtime pm, so the smmu remains
> > > >> powered only when the masters use it.
> > > >
> > > > Do you have any numbers for the power savings you achieve with this?
> > > > How often do we actually manage to stop the SMMU clocks on an SoC with
> > > > a handful of masters?
> > > >
> > > > In other words, is this too coarse-grained to be useful, or is it common
> > > > that all the devices upstream of the SMMU are suspended?
> > >
> > > well, if you think about a phone/tablet with a command mode panel,
> > > pretty much all devices will be suspended most of the time ;-)
> >
> > Well, that's really what I was asking about. I assumed that periodic
> > modem/radio transactions would keep the SMMU clocked, so would like to get a
> > rough idea of the power savings achieved with this coarse-grained approach.
>
> Sometimes we distribute SMMUs to each IP block in the system and
> let each one of those live in their own clock + power domain. In
> these cases, the SMMU can be powered down along with the only IP
> block that uses it. Furthermore, sometimes we put the IP block
> and the SMMU inside the same power domain to really tie the two
> together, so we definitely have cases where all devices (device?)
> upstream of the SMMU are suspended. And in the case of
> multimedia, it could be very often that something like the camera
> app isn't open and thus the SMMU dedicated for the camera can be
> powered down.
>
> Other times we have two SMMUs in the system where one is
> dedicated to GPU and the other is "everything else". Even in
> these cases, we can suspend the GPU one when the GPU is inactive
> because it's the only consumer. The other SMMU might not be as
> fine grained, but I think we still power it down quite often
> because the consumers are mostly multimedia devices that aren't
> active when the display is off.
And just to confuse things even further: with per-instance pagetables we have an
interest in forcing the SMMU clocks *on* because we don't know when the GPU
might try to hit the registers to switch a pagetable and if somebody in the
pipeline is actively trying to do power management at the same time hilarity
will ensue.
The alternative to pm_runtime is the downstream driver that probes the SMMU
clocks from DT and frobs them itself. I think we can agree that is far less
reasonable.
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-04-07 18:01 ` Jordan Crouse
0 siblings, 0 replies; 47+ messages in thread
From: Jordan Crouse @ 2017-04-07 18:01 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Apr 04, 2017 at 12:39:14PM -0700, Stephen Boyd wrote:
> On 04/03, Will Deacon wrote:
> > On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
> > > On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
> > > > On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
> > > >> This series provides the support for turning on the arm-smmu's
> > > >> clocks/power domains using runtime pm. This is done using the
> > > >> recently introduced device links patches, which lets the symmu's
> > > >> runtime to follow the master's runtime pm, so the smmu remains
> > > >> powered only when the masters use it.
> > > >
> > > > Do you have any numbers for the power savings you achieve with this?
> > > > How often do we actually manage to stop the SMMU clocks on an SoC with
> > > > a handful of masters?
> > > >
> > > > In other words, is this too coarse-grained to be useful, or is it common
> > > > that all the devices upstream of the SMMU are suspended?
> > >
> > > well, if you think about a phone/tablet with a command mode panel,
> > > pretty much all devices will be suspended most of the time ;-)
> >
> > Well, that's really what I was asking about. I assumed that periodic
> > modem/radio transactions would keep the SMMU clocked, so would like to get a
> > rough idea of the power savings achieved with this coarse-grained approach.
>
> Sometimes we distribute SMMUs to each IP block in the system and
> let each one of those live in their own clock + power domain. In
> these cases, the SMMU can be powered down along with the only IP
> block that uses it. Furthermore, sometimes we put the IP block
> and the SMMU inside the same power domain to really tie the two
> together, so we definitely have cases where all devices (device?)
> upstream of the SMMU are suspended. And in the case of
> multimedia, it could be very often that something like the camera
> app isn't open and thus the SMMU dedicated for the camera can be
> powered down.
>
> Other times we have two SMMUs in the system where one is
> dedicated to GPU and the other is "everything else". Even in
> these cases, we can suspend the GPU one when the GPU is inactive
> because it's the only consumer. The other SMMU might not be as
> fine grained, but I think we still power it down quite often
> because the consumers are mostly multimedia devices that aren't
> active when the display is off.
And just to confuse things even further: with per-instance pagetables we have an
interest in forcing the SMMU clocks *on* because we don't know when the GPU
might try to hit the registers to switch a pagetable and if somebody in the
pipeline is actively trying to do power management at the same time hilarity
will ensue.
The alternative to pm_runtime is the downstream driver that probes the SMMU
clocks from DT and frobs them itself. I think we can agree that is far less
reasonable.
Jordan
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 47+ messages in thread
[parent not found: <20170407180105.GA22050-9PYrDHPZ2Orvke4nUoYGnHL1okKdlPRT@public.gmane.org>]
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
2017-04-07 18:01 ` Jordan Crouse
(?)
@ 2017-04-10 4:45 ` Sricharan R
-1 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-04-10 4:45 UTC (permalink / raw)
To: Stephen Boyd, Will Deacon, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mathieu Poirier,
linux-arm-msm, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Rob Herring, linux-clk-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Hi Jordon,
On 4/7/2017 11:31 PM, Jordan Crouse wrote:
> On Tue, Apr 04, 2017 at 12:39:14PM -0700, Stephen Boyd wrote:
>> On 04/03, Will Deacon wrote:
>>> On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
>>>> On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
>>>>> On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
>>>>>> This series provides the support for turning on the arm-smmu's
>>>>>> clocks/power domains using runtime pm. This is done using the
>>>>>> recently introduced device links patches, which lets the symmu's
>>>>>> runtime to follow the master's runtime pm, so the smmu remains
>>>>>> powered only when the masters use it.
>>>>>
>>>>> Do you have any numbers for the power savings you achieve with this?
>>>>> How often do we actually manage to stop the SMMU clocks on an SoC with
>>>>> a handful of masters?
>>>>>
>>>>> In other words, is this too coarse-grained to be useful, or is it common
>>>>> that all the devices upstream of the SMMU are suspended?
>>>>
>>>> well, if you think about a phone/tablet with a command mode panel,
>>>> pretty much all devices will be suspended most of the time ;-)
>>>
>>> Well, that's really what I was asking about. I assumed that periodic
>>> modem/radio transactions would keep the SMMU clocked, so would like to get a
>>> rough idea of the power savings achieved with this coarse-grained approach.
>>
>> Sometimes we distribute SMMUs to each IP block in the system and
>> let each one of those live in their own clock + power domain. In
>> these cases, the SMMU can be powered down along with the only IP
>> block that uses it. Furthermore, sometimes we put the IP block
>> and the SMMU inside the same power domain to really tie the two
>> together, so we definitely have cases where all devices (device?)
>> upstream of the SMMU are suspended. And in the case of
>> multimedia, it could be very often that something like the camera
>> app isn't open and thus the SMMU dedicated for the camera can be
>> powered down.
>>
>> Other times we have two SMMUs in the system where one is
>> dedicated to GPU and the other is "everything else". Even in
>> these cases, we can suspend the GPU one when the GPU is inactive
>> because it's the only consumer. The other SMMU might not be as
>> fine grained, but I think we still power it down quite often
>> because the consumers are mostly multimedia devices that aren't
>> active when the display is off.
>
> And just to confuse things even further: with per-instance pagetables we have an
> interest in forcing the SMMU clocks *on* because we don't know when the GPU
> might try to hit the registers to switch a pagetable and if somebody in the
> pipeline is actively trying to do power management at the same time hilarity
> will ensue.
>
Ok, with per-process pagetables which gpu handles by itself, is the gpu driver
not going to keep its own clocks pm_runtime active before handing it over
to the firmware ? which would in this case take care of having the iommu clocks
also enabled because of the device links in the behind.
> The alternative to pm_runtime is the downstream driver that probes the SMMU
> clocks from DT and frobs them itself. I think we can agree that is far less
> reasonable.
The idea here was to keep the iommu clocks only represented inside the IOMMU DT
and handled by that driver. This works fine with the video decoder which is
already fully pm_runtime enabled and works fine with basic gpu testing. Do you
see any issues in testing this with the per-process pagetables ?
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-04-10 4:45 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-04-10 4:45 UTC (permalink / raw)
To: linux-arm-kernel
Hi Jordon,
On 4/7/2017 11:31 PM, Jordan Crouse wrote:
> On Tue, Apr 04, 2017 at 12:39:14PM -0700, Stephen Boyd wrote:
>> On 04/03, Will Deacon wrote:
>>> On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
>>>> On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
>>>>> On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
>>>>>> This series provides the support for turning on the arm-smmu's
>>>>>> clocks/power domains using runtime pm. This is done using the
>>>>>> recently introduced device links patches, which lets the symmu's
>>>>>> runtime to follow the master's runtime pm, so the smmu remains
>>>>>> powered only when the masters use it.
>>>>>
>>>>> Do you have any numbers for the power savings you achieve with this?
>>>>> How often do we actually manage to stop the SMMU clocks on an SoC with
>>>>> a handful of masters?
>>>>>
>>>>> In other words, is this too coarse-grained to be useful, or is it common
>>>>> that all the devices upstream of the SMMU are suspended?
>>>>
>>>> well, if you think about a phone/tablet with a command mode panel,
>>>> pretty much all devices will be suspended most of the time ;-)
>>>
>>> Well, that's really what I was asking about. I assumed that periodic
>>> modem/radio transactions would keep the SMMU clocked, so would like to get a
>>> rough idea of the power savings achieved with this coarse-grained approach.
>>
>> Sometimes we distribute SMMUs to each IP block in the system and
>> let each one of those live in their own clock + power domain. In
>> these cases, the SMMU can be powered down along with the only IP
>> block that uses it. Furthermore, sometimes we put the IP block
>> and the SMMU inside the same power domain to really tie the two
>> together, so we definitely have cases where all devices (device?)
>> upstream of the SMMU are suspended. And in the case of
>> multimedia, it could be very often that something like the camera
>> app isn't open and thus the SMMU dedicated for the camera can be
>> powered down.
>>
>> Other times we have two SMMUs in the system where one is
>> dedicated to GPU and the other is "everything else". Even in
>> these cases, we can suspend the GPU one when the GPU is inactive
>> because it's the only consumer. The other SMMU might not be as
>> fine grained, but I think we still power it down quite often
>> because the consumers are mostly multimedia devices that aren't
>> active when the display is off.
>
> And just to confuse things even further: with per-instance pagetables we have an
> interest in forcing the SMMU clocks *on* because we don't know when the GPU
> might try to hit the registers to switch a pagetable and if somebody in the
> pipeline is actively trying to do power management at the same time hilarity
> will ensue.
>
Ok, with per-process pagetables which gpu handles by itself, is the gpu driver
not going to keep its own clocks pm_runtime active before handing it over
to the firmware ? which would in this case take care of having the iommu clocks
also enabled because of the device links in the behind.
> The alternative to pm_runtime is the downstream driver that probes the SMMU
> clocks from DT and frobs them itself. I think we can agree that is far less
> reasonable.
The idea here was to keep the iommu clocks only represented inside the IOMMU DT
and handled by that driver. This works fine with the video decoder which is
already fully pm_runtime enabled and works fine with basic gpu testing. Do you
see any issues in testing this with the per-process pagetables ?
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH V3 0/5] iommu/arm-smmu: Add runtime pm/sleep support
@ 2017-04-10 4:45 ` Sricharan R
0 siblings, 0 replies; 47+ messages in thread
From: Sricharan R @ 2017-04-10 4:45 UTC (permalink / raw)
To: Stephen Boyd, Will Deacon, Mark Rutland, devicetree,
Mathieu Poirier, linux-arm-msm, iommu, Rob Herring, linux-clk,
linux-arm-kernel
Hi Jordon,
On 4/7/2017 11:31 PM, Jordan Crouse wrote:
> On Tue, Apr 04, 2017 at 12:39:14PM -0700, Stephen Boyd wrote:
>> On 04/03, Will Deacon wrote:
>>> On Fri, Mar 31, 2017 at 10:58:16PM -0400, Rob Clark wrote:
>>>> On Fri, Mar 31, 2017 at 1:54 PM, Will Deacon <will.deacon@arm.com> wrote:
>>>>> On Thu, Mar 09, 2017 at 09:05:43PM +0530, Sricharan R wrote:
>>>>>> This series provides the support for turning on the arm-smmu's
>>>>>> clocks/power domains using runtime pm. This is done using the
>>>>>> recently introduced device links patches, which lets the symmu's
>>>>>> runtime to follow the master's runtime pm, so the smmu remains
>>>>>> powered only when the masters use it.
>>>>>
>>>>> Do you have any numbers for the power savings you achieve with this?
>>>>> How often do we actually manage to stop the SMMU clocks on an SoC with
>>>>> a handful of masters?
>>>>>
>>>>> In other words, is this too coarse-grained to be useful, or is it common
>>>>> that all the devices upstream of the SMMU are suspended?
>>>>
>>>> well, if you think about a phone/tablet with a command mode panel,
>>>> pretty much all devices will be suspended most of the time ;-)
>>>
>>> Well, that's really what I was asking about. I assumed that periodic
>>> modem/radio transactions would keep the SMMU clocked, so would like to get a
>>> rough idea of the power savings achieved with this coarse-grained approach.
>>
>> Sometimes we distribute SMMUs to each IP block in the system and
>> let each one of those live in their own clock + power domain. In
>> these cases, the SMMU can be powered down along with the only IP
>> block that uses it. Furthermore, sometimes we put the IP block
>> and the SMMU inside the same power domain to really tie the two
>> together, so we definitely have cases where all devices (device?)
>> upstream of the SMMU are suspended. And in the case of
>> multimedia, it could be very often that something like the camera
>> app isn't open and thus the SMMU dedicated for the camera can be
>> powered down.
>>
>> Other times we have two SMMUs in the system where one is
>> dedicated to GPU and the other is "everything else". Even in
>> these cases, we can suspend the GPU one when the GPU is inactive
>> because it's the only consumer. The other SMMU might not be as
>> fine grained, but I think we still power it down quite often
>> because the consumers are mostly multimedia devices that aren't
>> active when the display is off.
>
> And just to confuse things even further: with per-instance pagetables we have an
> interest in forcing the SMMU clocks *on* because we don't know when the GPU
> might try to hit the registers to switch a pagetable and if somebody in the
> pipeline is actively trying to do power management at the same time hilarity
> will ensue.
>
Ok, with per-process pagetables which gpu handles by itself, is the gpu driver
not going to keep its own clocks pm_runtime active before handing it over
to the firmware ? which would in this case take care of having the iommu clocks
also enabled because of the device links in the behind.
> The alternative to pm_runtime is the downstream driver that probes the SMMU
> clocks from DT and frobs them itself. I think we can agree that is far less
> reasonable.
The idea here was to keep the iommu clocks only represented inside the IOMMU DT
and handled by that driver. This works fine with the video decoder which is
already fully pm_runtime enabled and works fine with basic gpu testing. Do you
see any issues in testing this with the per-process pagetables ?
Regards,
Sricharan
--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply [flat|nested] 47+ messages in thread