[PATCH 0/9] coresight: Add PM runtime awareness

[PATCH 0/9] coresight: Add PM runtime awareness

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

This patchset is using the runtime PM API and the generic power
domain sub-system to prevent coresight power domains from being
switched off while trace scenarios are still being executed.

This is supplemented with the creation of two new generic power
domains for the big and LITTLE clusters on the vexpress-tc2 platform.
By adding coresigth tracers to the power domain they belong to and
using the new generic power domain logic in the spc driver, clusters
are kept powered for as long as coresight operations are ongoing.

Mathieu Poirier (9):
  coresight-etm3x: Adding runtime PM awareness
  coresight-etb: Adding runtime PM awareness
  coresight-funnel: Adding runtime PM awareness
  coresight-tmc: Adding runtime PM awareness
  coresight-tpiu: Adding runtime PM awareness
  coresight-etm3x: Fixing suspend/wake modes
  ARM: vexpress/TC2: Add generic power domain awareness to scp driver
  coresight: Adding DT generic power domain support
  coresight: Documenting reference to generic PD bindings

 .../devicetree/bindings/arm/coresight.txt          |   4 +
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts         |  10 ++
 arch/arm/mach-vexpress/Kconfig                     |   1 +
 arch/arm/mach-vexpress/spc.c                       | 124 ++++++++++++++++++++-
 drivers/coresight/coresight-etb10.c                |  33 ++++--
 drivers/coresight/coresight-etm.h                  |   4 +-
 drivers/coresight/coresight-etm3x.c                |  69 ++++++++----
 drivers/coresight/coresight-funnel.c               |   9 +-
 drivers/coresight/coresight-tmc.c                  |   8 +-
 drivers/coresight/coresight-tpiu.c                 |   7 +-
 10 files changed, 229 insertions(+), 40 deletions(-)

-- 
1.9.1

[PATCH 1/9] coresight-etm3x: Adding runtime PM awareness

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

Using the runtime API whenever HW access is required.  As
such and by associating a coresight component to a power
domain in the device tree, faults associated to accessing
unpowered devices are mitigated.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/coresight/coresight-etm3x.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/coresight/coresight-etm3x.c b/drivers/coresight/coresight-etm3x.c
index d9e3ed6aa857..7df9ffc51501 100644
--- a/drivers/coresight/coresight-etm3x.c
+++ b/drivers/coresight/coresight-etm3x.c
@@ -30,6 +30,7 @@
 #include <linux/amba/bus.h>
 #include <linux/seq_file.h>
 #include <linux/uaccess.h>
+#include <linux/pm_runtime.h>
 #include <asm/sections.h>
 
 #include "coresight-etm.h"
@@ -148,7 +149,8 @@ static void etm_clr_pwrup(struct etm_drvdata *drvdata)
  * method where we have to account for CP14 configurations.
 
  * Return: 0 as soon as the bit has taken the desired state or -EAGAIN if
- * TIMEOUT_US has elapsed, which ever happens first.
+ * TIMEOUT_US has elapsed, which ever happens first.  The power is assumed to be
+ * switched on by the caller.
  */
 
 static int coresight_timeout_etm(struct etm_drvdata *drvdata, u32 offset,
@@ -181,7 +183,6 @@ static int coresight_timeout_etm(struct etm_drvdata *drvdata, u32 offset,
 	return -EAGAIN;
 }
 
-
 static void etm_set_prog(struct etm_drvdata *drvdata)
 {
 	u32 etmcr;
@@ -253,6 +254,7 @@ static void etm_enable_hw(void *info)
 	u32 etmcr;
 	struct etm_drvdata *drvdata = info;
 
+	pm_runtime_get_sync(drvdata->dev);
 	CS_UNLOCK(drvdata->base);
 
 	/* Turn engine on */
@@ -320,6 +322,7 @@ static int etm_trace_id_simple(struct etm_drvdata *drvdata)
 	if (!drvdata->enable)
 		return drvdata->traceid;
 
+	/* Assuming caller has switched on power */
 	return (etm_readl(drvdata, ETMTRACEIDR) & ETM_TRACEID_MASK);
 }
 
@@ -335,6 +338,7 @@ static int etm_trace_id(struct coresight_device *csdev)
 	if (clk_prepare_enable(drvdata->clk))
 		goto out;
 
+	pm_runtime_get_sync(drvdata->dev);
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 
 	CS_UNLOCK(drvdata->base);
@@ -342,6 +346,7 @@ static int etm_trace_id(struct coresight_device *csdev)
 	CS_LOCK(drvdata->base);
 
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
+	pm_runtime_put_sync(drvdata->dev);
 	clk_disable_unprepare(drvdata->clk);
 out:
 	return trace_id;
@@ -403,6 +408,7 @@ static void etm_disable_hw(void *info)
 
 	etm_set_pwrdwn(drvdata);
 	CS_LOCK(drvdata->base);
+	pm_runtime_put_sync(drvdata->dev);
 
 	dev_dbg(drvdata->dev, "cpu: %d disable smp call done\n", drvdata->cpu);
 }
@@ -488,6 +494,7 @@ static ssize_t etmsr_show(struct device *dev,
 	if (ret)
 		return ret;
 
+	pm_runtime_get_sync(dev->parent);
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 	CS_UNLOCK(drvdata->base);
 
@@ -495,6 +502,7 @@ static ssize_t etmsr_show(struct device *dev,
 
 	CS_LOCK(drvdata->base);
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
+	pm_runtime_put_sync(dev->parent);
 	clk_disable_unprepare(drvdata->clk);
 
 	return sprintf(buf, "%#lx\n", val);
@@ -1330,6 +1338,7 @@ static ssize_t seq_curr_state_show(struct device *dev,
 	if (ret)
 		return ret;
 
+	pm_runtime_get_sync(dev->parent);
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 
 	CS_UNLOCK(drvdata->base);
@@ -1337,6 +1346,7 @@ static ssize_t seq_curr_state_show(struct device *dev,
 	CS_LOCK(drvdata->base);
 
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
+	pm_runtime_put_sync(dev->parent);
 	clk_disable_unprepare(drvdata->clk);
 out:
 	return sprintf(buf, "%#lx\n", val);
@@ -1525,6 +1535,7 @@ static ssize_t status_show(struct device *dev,
 	if (ret)
 		return ret;
 
+	pm_runtime_get_sync(dev->parent);
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 
 	CS_UNLOCK(drvdata->base);
@@ -1550,6 +1561,7 @@ static ssize_t status_show(struct device *dev,
 	CS_LOCK(drvdata->base);
 
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
+	pm_runtime_put_sync(dev->parent);
 	clk_disable_unprepare(drvdata->clk);
 
 	return ret;
@@ -1572,6 +1584,7 @@ static ssize_t traceid_show(struct device *dev,
 	if (ret)
 		return ret;
 
+	pm_runtime_get_sync(dev->parent);
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 	CS_UNLOCK(drvdata->base);
 
@@ -1579,6 +1592,7 @@ static ssize_t traceid_show(struct device *dev,
 
 	CS_LOCK(drvdata->base);
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
+	pm_runtime_put_sync(dev->parent);
 	clk_disable_unprepare(drvdata->clk);
 out:
 	return sprintf(buf, "%#lx\n", val);
@@ -1860,6 +1874,9 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
 	if (boot_enable) {
 		coresight_enable(drvdata->csdev);
 		drvdata->boot_enable = true;
+	} else {
+		pm_runtime_set_suspended(dev);
+		pm_runtime_put_noidle(dev);
 	}
 
 	return 0;
-- 
1.9.1

[PATCH 2/9] coresight-etb: Adding runtime PM awareness

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

Using the runtime API whenever HW access is required.  As
such and by associating a coresight component to a power
domain in the device tree, faults associated to accessing
unpowered devices are mitigated.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/coresight/coresight-etb10.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/drivers/coresight/coresight-etb10.c b/drivers/coresight/coresight-etb10.c
index c922d4aded8a..71fd6cdba21c 100644
--- a/drivers/coresight/coresight-etb10.c
+++ b/drivers/coresight/coresight-etb10.c
@@ -26,6 +26,7 @@
 #include <linux/seq_file.h>
 #include <linux/coresight.h>
 #include <linux/amba/bus.h>
+#include <linux/pm_runtime.h>
 
 #include "coresight-priv.h"
 
@@ -92,17 +93,11 @@ struct etb_drvdata {
 
 static unsigned int etb_get_buffer_depth(struct etb_drvdata *drvdata)
 {
-	int ret;
 	u32 depth = 0;
 
-	ret = clk_prepare_enable(drvdata->clk);
-	if (ret)
-		return ret;
-
 	/* RO registers don't need locking */
 	depth = readl_relaxed(drvdata->base + ETB_RAM_DEPTH_REG);
 
-	clk_disable_unprepare(drvdata->clk);
 	return depth;
 }
 
@@ -144,6 +139,7 @@ static int etb_enable(struct coresight_device *csdev)
 	if (ret)
 		return ret;
 
+	pm_runtime_get_sync(drvdata->dev);
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 	etb_enable_hw(drvdata);
 	drvdata->enable = true;
@@ -250,8 +246,9 @@ static void etb_disable(struct coresight_device *csdev)
 	etb_disable_hw(drvdata);
 	etb_dump_hw(drvdata);
 	drvdata->enable = false;
-	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
+	spin_unlock_irqrestore(&drvdata->spinlock, flags);
+	pm_runtime_put_sync(drvdata->dev);
 	clk_disable_unprepare(drvdata->clk);
 
 	dev_info(drvdata->dev, "ETB disabled\n");
@@ -266,10 +263,16 @@ static const struct coresight_ops etb_cs_ops = {
 	.sink_ops	= &etb_sink_ops,
 };
 
-static void etb_dump(struct etb_drvdata *drvdata)
+static int etb_dump(struct etb_drvdata *drvdata)
 {
+	int ret;
 	unsigned long flags;
 
+	ret = clk_prepare_enable(drvdata->clk);
+	if (ret)
+		return ret;
+
+	pm_runtime_get_sync(drvdata->dev);
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 	if (drvdata->enable) {
 		etb_disable_hw(drvdata);
@@ -277,8 +280,11 @@ static void etb_dump(struct etb_drvdata *drvdata)
 		etb_enable_hw(drvdata);
 	}
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
+	pm_runtime_put_sync(drvdata->dev);
+	clk_disable_unprepare(drvdata->clk);
 
 	dev_info(drvdata->dev, "ETB dumped\n");
+	return 0;
 }
 
 static int etb_open(struct inode *inode, struct file *file)
@@ -296,11 +302,14 @@ static int etb_open(struct inode *inode, struct file *file)
 static ssize_t etb_read(struct file *file, char __user *data,
 				size_t len, loff_t *ppos)
 {
+	int ret;
 	u32 depth;
 	struct etb_drvdata *drvdata = container_of(file->private_data,
 						   struct etb_drvdata, miscdev);
 
-	etb_dump(drvdata);
+	ret = etb_dump(drvdata);
+	if (ret)
+		return ret;
 
 	depth = drvdata->buffer_depth;
 	if (*ppos + len > depth * 4)
@@ -349,6 +358,7 @@ static ssize_t status_show(struct device *dev,
 	if (ret)
 		goto out;
 
+	pm_runtime_get_sync(dev);
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 	CS_UNLOCK(drvdata->base);
 
@@ -363,7 +373,7 @@ static ssize_t status_show(struct device *dev,
 
 	CS_LOCK(drvdata->base);
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
-
+	pm_runtime_put_sync(dev);
 	clk_disable_unprepare(drvdata->clk);
 
 	return sprintf(buf,
@@ -486,6 +496,9 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
 	if (ret)
 		goto err_misc_register;
 
+	pm_runtime_set_suspended(dev);
+	pm_runtime_put_noidle(dev);
+
 	dev_info(dev, "ETB initialized\n");
 	return 0;
 
-- 
1.9.1

[PATCH 3/9] coresight-funnel: Adding runtime PM awareness

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

Using the runtime API whenever HW access is required.  As
such and by associating a coresight component to a power
domain in the device tree, faults associated to accessing
unpowered devices are mitigated.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/coresight/coresight-funnel.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/coresight/coresight-funnel.c b/drivers/coresight/coresight-funnel.c
index 2108edffe1f4..b655ecfb23c6 100644
--- a/drivers/coresight/coresight-funnel.c
+++ b/drivers/coresight/coresight-funnel.c
@@ -21,6 +21,7 @@
 #include <linux/clk.h>
 #include <linux/coresight.h>
 #include <linux/amba/bus.h>
+#include <linux/pm_runtime.h>
 
 #include "coresight-priv.h"
 
@@ -73,6 +74,7 @@ static int funnel_enable(struct coresight_device *csdev, int inport,
 	if (ret)
 		return ret;
 
+	pm_runtime_get_sync(drvdata->dev);
 	funnel_enable_hw(drvdata, inport);
 
 	dev_info(drvdata->dev, "FUNNEL inport %d enabled\n", inport);
@@ -98,7 +100,7 @@ static void funnel_disable(struct coresight_device *csdev, int inport,
 	struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
 	funnel_disable_hw(drvdata, inport);
-
+	pm_runtime_put_sync(drvdata->dev);
 	clk_disable_unprepare(drvdata->clk);
 
 	dev_info(drvdata->dev, "FUNNEL inport %d disabled\n", inport);
@@ -161,8 +163,10 @@ static ssize_t funnel_ctrl_show(struct device *dev,
 	if (ret)
 		return ret;
 
+	pm_runtime_get_sync(drvdata->dev);
 	val = get_funnel_ctrl_hw(drvdata);
 	clk_disable_unprepare(drvdata->clk);
+	pm_runtime_put_sync(drvdata->dev);
 
 	return sprintf(buf, "%#x\n", val);
 }
@@ -222,6 +226,9 @@ static int funnel_probe(struct amba_device *adev, const struct amba_id *id)
 	if (IS_ERR(drvdata->csdev))
 		return PTR_ERR(drvdata->csdev);
 
+	pm_runtime_set_suspended(dev);
+	pm_runtime_put_noidle(dev);
+
 	dev_info(dev, "FUNNEL initialized\n");
 	return 0;
 }
-- 
1.9.1

[PATCH 4/9] coresight-tmc: Adding runtime PM awareness

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

Using the runtime API whenever HW access is required.  As
such and by associating a coresight component to a power
domain in the device tree, faults associated to accessing
unpowered devices are mitigated.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/coresight/coresight-tmc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/coresight/coresight-tmc.c b/drivers/coresight/coresight-tmc.c
index ce2c293f1707..274907717a33 100644
--- a/drivers/coresight/coresight-tmc.c
+++ b/drivers/coresight/coresight-tmc.c
@@ -27,6 +27,7 @@
 #include <linux/of.h>
 #include <linux/coresight.h>
 #include <linux/amba/bus.h>
+#include <linux/pm_runtime.h>
 
 #include "coresight-priv.h"
 
@@ -249,9 +250,11 @@ static int tmc_enable(struct tmc_drvdata *drvdata, enum tmc_mode mode)
 	if (ret)
 		return ret;
 
+	pm_runtime_get_sync(drvdata->dev);
 	spin_lock_irqsave(&drvdata->spinlock, flags);
 	if (drvdata->reading) {
 		spin_unlock_irqrestore(&drvdata->spinlock, flags);
+		pm_runtime_put_sync(drvdata->dev);
 		clk_disable_unprepare(drvdata->clk);
 		return -EBUSY;
 	}
@@ -385,7 +388,7 @@ static void tmc_disable(struct tmc_drvdata *drvdata, enum tmc_mode mode)
 out:
 	drvdata->enable = false;
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
-
+	pm_runtime_put_sync(drvdata->dev);
 	clk_disable_unprepare(drvdata->clk);
 
 	dev_info(drvdata->dev, "TMC disabled\n");
@@ -717,6 +720,9 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
 	if (ret)
 		goto err_misc_register;
 
+	pm_runtime_set_suspended(dev);
+	pm_runtime_put_noidle(dev);
+
 	dev_info(dev, "TMC initialized\n");
 	return 0;
 
-- 
1.9.1

[PATCH 5/9] coresight-tpiu: Adding runtime PM awareness

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

Using the runtime API whenever HW access is required.  As
such and by associating a coresight component to a power
domain in the device tree, faults associated to accessing
unpowered devices are mitigated.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/coresight/coresight-tpiu.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/coresight/coresight-tpiu.c b/drivers/coresight/coresight-tpiu.c
index ae101082791a..199244700c99 100644
--- a/drivers/coresight/coresight-tpiu.c
+++ b/drivers/coresight/coresight-tpiu.c
@@ -20,6 +20,7 @@
 #include <linux/clk.h>
 #include <linux/coresight.h>
 #include <linux/amba/bus.h>
+#include <linux/pm_runtime.h>
 
 #include "coresight-priv.h"
 
@@ -78,6 +79,7 @@ static int tpiu_enable(struct coresight_device *csdev)
 	if (ret)
 		return ret;
 
+	pm_runtime_get_sync(drvdata->dev);
 	tpiu_enable_hw(drvdata);
 
 	dev_info(drvdata->dev, "TPIU enabled\n");
@@ -101,7 +103,7 @@ static void tpiu_disable(struct coresight_device *csdev)
 	struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
 	tpiu_disable_hw(drvdata);
-
+	pm_runtime_put_sync(drvdata->dev);
 	clk_disable_unprepare(drvdata->clk);
 
 	dev_info(drvdata->dev, "TPIU disabled\n");
@@ -171,6 +173,9 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id)
 	if (IS_ERR(drvdata->csdev))
 		return PTR_ERR(drvdata->csdev);
 
+	pm_runtime_set_suspended(dev);
+	pm_runtime_put_noidle(dev);
+
 	dev_info(dev, "TPIU initialized\n");
 	return 0;
 }
-- 
1.9.1

[PATCH 6/9] coresight-etm3x: Fixing suspend/wake modes

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

This patch makes sure power is managed properly during power state
transitions.  Checks are introduced to make sure HW access functions
aren't called uselessly to avoid wasting power and accessing
blocks that may be powered down.

Lastly the CPU_ONLINE state has been removed from the cpu callback
as it wasn't useful anymore.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 drivers/coresight/coresight-etm.h   |  4 ++--
 drivers/coresight/coresight-etm3x.c | 48 ++++++++++++++++++++-----------------
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/coresight/coresight-etm.h b/drivers/coresight/coresight-etm.h
index 501c5fac8a45..a0e30ca2e32d 100644
--- a/drivers/coresight/coresight-etm.h
+++ b/drivers/coresight/coresight-etm.h
@@ -148,7 +148,7 @@
  * @arch:	ETM/PTM version number.
  * @use_cpu14:	true if management registers need to be accessed via CP14.
  * @enable:	is this ETM/PTM currently tracing.
- * @sticky_enable: true if ETM base configuration has been done.
+ * @suspended:	is the CPU associated to the ETM/PTM suspended?
  * @boot_enable:true if we should start tracing at boot time.
  * @os_unlock:	true if access to management registers is allowed.
  * @nr_addr_cmp:Number of pairs of address comparators as found in ETMCCR.
@@ -200,7 +200,7 @@ struct etm_drvdata {
 	u8				arch;
 	bool				use_cp14;
 	bool				enable;
-	bool				sticky_enable;
+	bool				suspended;
 	bool				boot_enable;
 	bool				os_unlock;
 	u8				nr_addr_cmp;
diff --git a/drivers/coresight/coresight-etm3x.c b/drivers/coresight/coresight-etm3x.c
index 7df9ffc51501..e09c3ef76d16 100644
--- a/drivers/coresight/coresight-etm3x.c
+++ b/drivers/coresight/coresight-etm3x.c
@@ -254,6 +254,12 @@ static void etm_enable_hw(void *info)
 	u32 etmcr;
 	struct etm_drvdata *drvdata = info;
 
+	if (clk_prepare_enable(drvdata->clk)) {
+		dev_dbg(drvdata->dev, "cpu: %d can't enable hw\n",
+			drvdata->cpu);
+		return;
+	}
+
 	pm_runtime_get_sync(drvdata->dev);
 	CS_UNLOCK(drvdata->base);
 
@@ -314,6 +320,8 @@ static void etm_enable_hw(void *info)
 	etm_clr_prog(drvdata);
 	CS_LOCK(drvdata->base);
 
+	drvdata->suspended = false;
+
 	dev_dbg(drvdata->dev, "cpu: %d enable smp call done\n", drvdata->cpu);
 }
 
@@ -355,14 +363,12 @@ out:
 static int etm_enable(struct coresight_device *csdev)
 {
 	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
-	int ret;
-
-	ret = clk_prepare_enable(drvdata->clk);
-	if (ret)
-		goto err_clk;
+	int ret = 0;
 
 	spin_lock(&drvdata->spinlock);
 
+	if (drvdata->enable)
+		goto out;
 	/*
 	 * Configure the ETM only if the CPU is online.  If it isn't online
 	 * hw configuration will take place when 'CPU_STARTING' is received
@@ -372,20 +378,17 @@ static int etm_enable(struct coresight_device *csdev)
 		ret = smp_call_function_single(drvdata->cpu,
 					       etm_enable_hw, drvdata, 1);
 		if (ret)
-			goto err;
+			goto out;
 	}
 
 	drvdata->enable = true;
-	drvdata->sticky_enable = true;
-
 	spin_unlock(&drvdata->spinlock);
 
 	dev_info(drvdata->dev, "ETM tracing enabled\n");
 	return 0;
-err:
+out:
 	spin_unlock(&drvdata->spinlock);
 	clk_disable_unprepare(drvdata->clk);
-err_clk:
 	return ret;
 }
 
@@ -397,6 +400,9 @@ static void etm_disable_hw(void *info)
 	CS_UNLOCK(drvdata->base);
 	etm_set_prog(drvdata);
 
+	if (!drvdata->enable)
+		return;
+
 	/* Program trace enable to low by using always false event */
 	etm_writel(drvdata, ETM_HARD_WIRE_RES_A | ETM_EVENT_NOT_A, ETMTEEVR);
 
@@ -409,6 +415,9 @@ static void etm_disable_hw(void *info)
 	etm_set_pwrdwn(drvdata);
 	CS_LOCK(drvdata->base);
 	pm_runtime_put_sync(drvdata->dev);
+	clk_disable_unprepare(drvdata->clk);
+
+	drvdata->suspended = true;
 
 	dev_dbg(drvdata->dev, "cpu: %d disable smp call done\n", drvdata->cpu);
 }
@@ -426,18 +435,20 @@ static void etm_disable(struct coresight_device *csdev)
 	get_online_cpus();
 	spin_lock(&drvdata->spinlock);
 
+	if (!drvdata->enable)
+		goto out;
 	/*
 	 * Executing etm_disable_hw on the cpu whose ETM is being disabled
 	 * ensures that register writes occur when cpu is powered.
 	 */
-	smp_call_function_single(drvdata->cpu, etm_disable_hw, drvdata, 1);
+	if (!drvdata->suspended)
+		smp_call_function_single(drvdata->cpu,
+					 etm_disable_hw, drvdata, 1);
 	drvdata->enable = false;
-
+out:
 	spin_unlock(&drvdata->spinlock);
 	put_online_cpus();
 
-	clk_disable_unprepare(drvdata->clk);
-
 	dev_info(drvdata->dev, "ETM tracing disabled\n");
 }
 
@@ -1674,14 +1685,7 @@ static int etm_cpu_callback(struct notifier_block *nfb, unsigned long action,
 			etm_enable_hw(etmdrvdata[cpu]);
 		spin_unlock(&etmdrvdata[cpu]->spinlock);
 		break;
-
-	case CPU_ONLINE:
-		if (etmdrvdata[cpu]->boot_enable &&
-		    !etmdrvdata[cpu]->sticky_enable)
-			coresight_enable(etmdrvdata[cpu]->csdev);
-		break;
-
-	case CPU_DYING:
+	case CPU_DEAD:
 		spin_lock(&etmdrvdata[cpu]->spinlock);
 		if (etmdrvdata[cpu]->enable)
 			etm_disable_hw(etmdrvdata[cpu]);
-- 
1.9.1

[PATCH 7/9] ARM: vexpress/TC2: Add generic power domain awareness to scp driver

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

This patch makes use of the generic power domain API to model the
power domains associated to the A7 and A15 clusters.  From there
any IP block (like coresight tracers) that are part of those domains
can register with the runtime pm core to guarantee that other
components won't inadvertently switch off the power while they are
being used.

The logic associated to the generic power domains doesn't take
care of swithing off the clusters - it simply monitors when other
components need the clusters to remain powered.  Proceeding this
way costs just a little more power but the system is already in
diagnostic mode (coresight), making the argument irrelevant.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 arch/arm/mach-vexpress/Kconfig |   1 +
 arch/arm/mach-vexpress/spc.c   | 124 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 124 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-vexpress/Kconfig b/arch/arm/mach-vexpress/Kconfig
index d6b16d9a7838..f90809bbdaa7 100644
--- a/arch/arm/mach-vexpress/Kconfig
+++ b/arch/arm/mach-vexpress/Kconfig
@@ -21,6 +21,7 @@ menuconfig ARCH_VEXPRESS
 	select VEXPRESS_CONFIG
 	select VEXPRESS_SYSCFG
 	select MFD_VEXPRESS_SYSREG
+	select PM_GENERIC_DOMAINS if PM
 	help
 	  This option enables support for systems using Cortex processor based
 	  ARM core and logic (FPGA) tiles on the Versatile Express motherboard,
diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
index f61158c6ce71..4ff1009f2d16 100644
--- a/arch/arm/mach-vexpress/spc.c
+++ b/arch/arm/mach-vexpress/spc.c
@@ -28,6 +28,8 @@
 #include <linux/pm_opp.h>
 #include <linux/slab.h>
 #include <linux/semaphore.h>
+#include <linux/of_platform.h>
+#include <linux/pm_domain.h>
 
 #include <asm/cacheflush.h>
 
@@ -92,6 +94,9 @@
 #define STAT_ERR(type)		((1 << 1) << (type << 2))
 #define RESPONSE_MASK(type)	(STAT_COMPLETE(type) | STAT_ERR(type))
 
+static atomic_t gpd_A7_cluster_on = ATOMIC_INIT(0);
+static atomic_t gpd_A15_cluster_on = ATOMIC_INIT(0);
+
 struct ve_spc_opp {
 	unsigned long freq;
 	unsigned long u_volt;
@@ -209,12 +214,19 @@ void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr)
  */
 void ve_spc_powerdown(u32 cluster, bool enable)
 {
+	bool is_a15;
 	u32 pwdrn_reg;
 
 	if (cluster >= MAX_CLUSTERS)
 		return;
 
-	pwdrn_reg = cluster_is_a15(cluster) ? A15_PWRDN_EN : A7_PWRDN_EN;
+	is_a15 = cluster_is_a15(cluster);
+	if (is_a15 && atomic_read(&gpd_A15_cluster_on))
+		return;
+	else if (!is_a15 && atomic_read(&gpd_A7_cluster_on))
+		return;
+
+	pwdrn_reg = is_a15 ? A15_PWRDN_EN : A7_PWRDN_EN;
 	writel_relaxed(enable, info->baseaddr + pwdrn_reg);
 }
 
@@ -447,6 +459,116 @@ static int ve_init_opp_table(struct device *cpu_dev)
 	return ret;
 }
 
+#ifdef CONFIG_PM_GENERIC_DOMAINS_OF
+static int scp_power_on_pd_A7(struct generic_pm_domain *domain)
+{
+	/*
+	 * Simply tell mcpm we need power.  Nothing else needs to be done
+	 * as CPUs in the cluster are already powered when we reach this point.
+	 */
+	atomic_set(&gpd_A7_cluster_on, 1);
+	return 0;
+}
+
+static int scp_power_off_pd_A7(struct generic_pm_domain *domain)
+{
+	atomic_set(&gpd_A7_cluster_on, 0);
+	return 0;
+}
+
+static int scp_power_on_pd_A15(struct generic_pm_domain *domain)
+{
+	/*
+	 * Simply tell mcpm we need power.  Nothing else needs to be done
+	 * as CPUs in the cluster are already powered when we reach this point.
+	 */
+	atomic_set(&gpd_A15_cluster_on, 1);
+	return 0;
+}
+
+static int scp_power_off_pd_A15(struct generic_pm_domain *domain)
+{
+	atomic_set(&gpd_A15_cluster_on, 0);
+	return 0;
+}
+
+static int (*const scp_power_fct[MAX_CLUSTERS * 2])
+		(struct generic_pm_domain *domain) = {
+		scp_power_on_pd_A15, scp_power_off_pd_A15,
+		scp_power_on_pd_A7, scp_power_off_pd_A7};
+
+static void scp_init_pd(struct generic_pm_domain *pd,
+			    struct device_node *np,
+			    struct platform_device *pdev, int cluster)
+{
+	char name[50];
+	int index = cluster * 2;
+
+	snprintf(name, sizeof(name), "%s-%d", np->name, cluster);
+
+	pd->name = kstrdup(name, GFP_KERNEL);
+	pd->power_on = scp_power_fct[index];
+	pd->power_off =  scp_power_fct[index + 1];
+	platform_set_drvdata(pdev, pd);
+
+	pm_genpd_init(pd, NULL, true);
+	pm_genpd_poweron(pd);
+}
+
+static __init int ve_spc_gpd_init(void)
+{
+	struct genpd_onecell_data *data;
+	struct generic_pm_domain *pd, **cluster_pds;
+	struct platform_device *pdev;
+	struct device *dev;
+	struct device_node *np;
+	int count;
+
+	np = of_find_compatible_node(NULL, NULL,
+				     "arm,vexpress-power-controller");
+	if (!np)
+		return -EINVAL;
+
+	cluster_pds = kzalloc(sizeof(struct generic_pm_domain *) * MAX_CLUSTERS,
+			      GFP_KERNEL);
+	if (!cluster_pds)
+		goto err_cluster_kzalloc;
+
+	data = kzalloc(sizeof(struct genpd_onecell_data), GFP_KERNEL);
+	if (!data)
+		goto err_data;
+
+	pdev = of_find_device_by_node(np);
+	dev = &pdev->dev;
+
+	for (count = 0; count < MAX_CLUSTERS; count++) {
+		pd = kzalloc(sizeof(struct generic_pm_domain), GFP_KERNEL);
+		if (!pd)
+			goto err_pd_kzalloc;
+		scp_init_pd(pd, np, pdev, count);
+		cluster_pds[count] = pd;
+	}
+
+	data->num_domains = count;
+	data->domains = cluster_pds;
+
+	of_genpd_add_provider_onecell(np, data);
+	return 0;
+
+err_pd_kzalloc:
+	while (--count >= 0)
+		kfree(cluster_pds[count]);
+
+err_cluster_kzalloc:
+	of_node_put(np);
+err_data:
+	kfree(cluster_pds);
+	return -ENOMEM;
+
+}
+arch_initcall(ve_spc_gpd_init);
+#endif
+
 int __init ve_spc_init(void __iomem *baseaddr, u32 a15_clusid, int irq)
 {
 	int ret;
-- 
1.9.1

Re: [PATCH 7/9] ARM: vexpress/TC2: Add generic power domain awareness to scp driver

[Lorenzo Pieralisi]

Hi Mathieu,

On Tue, Jan 06, 2015 at 04:37:11PM +0000, mathieu.poirier@linaro.org wrote:

[...]

> diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
> index f61158c6ce71..4ff1009f2d16 100644
> --- a/arch/arm/mach-vexpress/spc.c
> +++ b/arch/arm/mach-vexpress/spc.c
> @@ -28,6 +28,8 @@
>  #include <linux/pm_opp.h>
>  #include <linux/slab.h>
>  #include <linux/semaphore.h>
> +#include <linux/of_platform.h>
> +#include <linux/pm_domain.h>
>  
>  #include <asm/cacheflush.h>
>  
> @@ -92,6 +94,9 @@
>  #define STAT_ERR(type)		((1 << 1) << (type << 2))
>  #define RESPONSE_MASK(type)	(STAT_COMPLETE(type) | STAT_ERR(type))
>  
> +static atomic_t gpd_A7_cluster_on = ATOMIC_INIT(0);
> +static atomic_t gpd_A15_cluster_on = ATOMIC_INIT(0);
> +
>  struct ve_spc_opp {
>  	unsigned long freq;
>  	unsigned long u_volt;
> @@ -209,12 +214,19 @@ void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr)
>   */
>  void ve_spc_powerdown(u32 cluster, bool enable)
>  {
> +	bool is_a15;
>  	u32 pwdrn_reg;
>  
>  	if (cluster >= MAX_CLUSTERS)
>  		return;
>  
> -	pwdrn_reg = cluster_is_a15(cluster) ? A15_PWRDN_EN : A7_PWRDN_EN;
> +	is_a15 = cluster_is_a15(cluster);
> +	if (is_a15 && atomic_read(&gpd_A15_cluster_on))
> +		return;
> +	else if (!is_a15 && atomic_read(&gpd_A7_cluster_on))
> +		return;
> +
> +	pwdrn_reg = is_a15 ? A15_PWRDN_EN : A7_PWRDN_EN;
>  	writel_relaxed(enable, info->baseaddr + pwdrn_reg);

I do not like this, for multiple reasons.

(1) It might not do what you want. I am not sure that nuking the power
    down request is safe. I have to check the power controller behaviour
    when a core is going through power down sequence but the PWRDN_EN
    bit is not set. You might end up with cores that are just being
    reset on pending IRQ (defeating your purpose) or stuck forever.
(2) It must be done by attaching the power domains to CPUidle states.
    Idle states are automagically disabled when the domain is turned on, it
    is cleaner, and more robust than what you do here. On the downside,
    we have to work together to add power domain information to DT idle
    states specifications/bindings.

    (see pm_genpd_attach_cpuidle() drivers/base/power/domain.c)

(3) I do not like the is_a15 comparisons, I think this can be done with
    cluster indexing the atomic variable, but since you are removing
    this code ;-) it does not really matter.

>  }
>  
> @@ -447,6 +459,116 @@ static int ve_init_opp_table(struct device *cpu_dev)
>  	return ret;
>  }
>  
> +#ifdef CONFIG_PM_GENERIC_DOMAINS_OF
> +static int scp_power_on_pd_A7(struct generic_pm_domain *domain)
> +{
> +	/*
> +	 * Simply tell mcpm we need power.  Nothing else needs to be done
> +	 * as CPUs in the cluster are already powered when we reach this point.
> +	 */
> +	atomic_set(&gpd_A7_cluster_on, 1);
> +	return 0;
> +}
> +
> +static int scp_power_off_pd_A7(struct generic_pm_domain *domain)
> +{
> +	atomic_set(&gpd_A7_cluster_on, 0);
> +	return 0;
> +}
> +
> +static int scp_power_on_pd_A15(struct generic_pm_domain *domain)
> +{
> +	/*
> +	 * Simply tell mcpm we need power.  Nothing else needs to be done
> +	 * as CPUs in the cluster are already powered when we reach this point.
> +	 */
> +	atomic_set(&gpd_A15_cluster_on, 1);
> +	return 0;
> +}
> +
> +static int scp_power_off_pd_A15(struct generic_pm_domain *domain)
> +{
> +	atomic_set(&gpd_A15_cluster_on, 0);
> +	return 0;
> +}
> +
> +static int (*const scp_power_fct[MAX_CLUSTERS * 2])
> +		(struct generic_pm_domain *domain) = {
> +		scp_power_on_pd_A15, scp_power_off_pd_A15,
> +		scp_power_on_pd_A7, scp_power_off_pd_A7};

I think you can remove the functions above and the corresponding atomic
variables, see my comment above.

> +static void scp_init_pd(struct generic_pm_domain *pd,
> +			    struct device_node *np,
> +			    struct platform_device *pdev, int cluster)
> +{
> +	char name[50];
> +	int index = cluster * 2;
> +
> +	snprintf(name, sizeof(name), "%s-%d", np->name, cluster);
> +
> +	pd->name = kstrdup(name, GFP_KERNEL);
> +	pd->power_on = scp_power_fct[index];
> +	pd->power_off =  scp_power_fct[index + 1];
> +	platform_set_drvdata(pdev, pd);
> +
> +	pm_genpd_init(pd, NULL, true);
> +	pm_genpd_poweron(pd);
> +}
> +
> +static __init int ve_spc_gpd_init(void)
> +{
> +	struct genpd_onecell_data *data;
> +	struct generic_pm_domain *pd, **cluster_pds;
> +	struct platform_device *pdev;
> +	struct device *dev;
> +	struct device_node *np;
> +	int count;
> +
> +	np = of_find_compatible_node(NULL, NULL,
> +				     "arm,vexpress-power-controller");

See the bindings discussions, there is not such a thing as
vexpress-power-controller.

> +	if (!np)
> +		return -EINVAL;
> +
> +	cluster_pds = kzalloc(sizeof(struct generic_pm_domain *) * MAX_CLUSTERS,
> +			      GFP_KERNEL);
> +	if (!cluster_pds)
> +		goto err_cluster_kzalloc;

You are freeing a pointer that failed to be allocated.

> +
> +	data = kzalloc(sizeof(struct genpd_onecell_data), GFP_KERNEL);
> +	if (!data)
> +		goto err_data;

Mmm...is that the label you want to jump to ? it fails to put the OF
node.

> +
> +	pdev = of_find_device_by_node(np);
> +	dev = &pdev->dev;
> +
> +	for (count = 0; count < MAX_CLUSTERS; count++) {
> +		pd = kzalloc(sizeof(struct generic_pm_domain), GFP_KERNEL);
> +		if (!pd)
> +			goto err_pd_kzalloc;

This label does not free the data pointer. I think you should rework
the error exit paths.

> +		scp_init_pd(pd, np, pdev, count);
> +		cluster_pds[count] = pd;
> +	}
> +
> +	data->num_domains = count;
> +	data->domains = cluster_pds;
> +
> +	of_genpd_add_provider_onecell(np, data);
> +	return 0;
> +
> +err_pd_kzalloc:
> +	while (--count >= 0)
> +		kfree(cluster_pds[count]);
> +
> +err_cluster_kzalloc:
> +	of_node_put(np);
> +err_data:
> +	kfree(cluster_pds);
> +	return -ENOMEM;
> +
> +}
> +arch_initcall(ve_spc_gpd_init);

It should not be an arch initcall, call it from spc_init, and make it an
empty static inline if !CONFIG_PM_GENERIC_DOMAINS_OF, eg:

static inline int ve_spc_gpd_init(void)
{
	return -ENOTSUPP;
}

Lorenzo

> +#endif
> +
>  int __init ve_spc_init(void __iomem *baseaddr, u32 a15_clusid, int irq)
>  {
>  	int ret;
> -- 
> 1.9.1
> 
> 

Re: [PATCH 7/9] ARM: vexpress/TC2: Add generic power domain awareness to scp driver

[Mathieu Poirier]

On 7 January 2015 at 04:39, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> Hi Mathieu,
>
> On Tue, Jan 06, 2015 at 04:37:11PM +0000, mathieu.poirier@linaro.org wrote:
>
> [...]
>
>> diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
>> index f61158c6ce71..4ff1009f2d16 100644
>> --- a/arch/arm/mach-vexpress/spc.c
>> +++ b/arch/arm/mach-vexpress/spc.c
>> @@ -28,6 +28,8 @@
>>  #include <linux/pm_opp.h>
>>  #include <linux/slab.h>
>>  #include <linux/semaphore.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/pm_domain.h>
>>
>>  #include <asm/cacheflush.h>
>>
>> @@ -92,6 +94,9 @@
>>  #define STAT_ERR(type)               ((1 << 1) << (type << 2))
>>  #define RESPONSE_MASK(type)  (STAT_COMPLETE(type) | STAT_ERR(type))
>>
>> +static atomic_t gpd_A7_cluster_on = ATOMIC_INIT(0);
>> +static atomic_t gpd_A15_cluster_on = ATOMIC_INIT(0);
>> +
>>  struct ve_spc_opp {
>>       unsigned long freq;
>>       unsigned long u_volt;
>> @@ -209,12 +214,19 @@ void ve_spc_set_resume_addr(u32 cluster, u32 cpu, u32 addr)
>>   */
>>  void ve_spc_powerdown(u32 cluster, bool enable)
>>  {
>> +     bool is_a15;
>>       u32 pwdrn_reg;
>>
>>       if (cluster >= MAX_CLUSTERS)
>>               return;
>>
>> -     pwdrn_reg = cluster_is_a15(cluster) ? A15_PWRDN_EN : A7_PWRDN_EN;
>> +     is_a15 = cluster_is_a15(cluster);
>> +     if (is_a15 && atomic_read(&gpd_A15_cluster_on))
>> +             return;
>> +     else if (!is_a15 && atomic_read(&gpd_A7_cluster_on))
>> +             return;
>> +
>> +     pwdrn_reg = is_a15 ? A15_PWRDN_EN : A7_PWRDN_EN;
>>       writel_relaxed(enable, info->baseaddr + pwdrn_reg);
>
> I do not like this, for multiple reasons.
>
> (1) It might not do what you want. I am not sure that nuking the power
>     down request is safe. I have to check the power controller behaviour
>     when a core is going through power down sequence but the PWRDN_EN
>     bit is not set. You might end up with cores that are just being
>     reset on pending IRQ (defeating your purpose) or stuck forever.

I understand your concerns.

> (2) It must be done by attaching the power domains to CPUidle states.
>     Idle states are automagically disabled when the domain is turned on, it
>     is cleaner, and more robust than what you do here.

I like that idea.

> On the downside,
>     we have to work together to add power domain information to DT idle
>     states specifications/bindings.
>
>     (see pm_genpd_attach_cpuidle() drivers/base/power/domain.c)

I definitely will.

>
> (3) I do not like the is_a15 comparisons, I think this can be done with
>     cluster indexing the atomic variable, but since you are removing
>     this code ;-) it does not really matter.

Agreed.

>
>>  }
>>
>> @@ -447,6 +459,116 @@ static int ve_init_opp_table(struct device *cpu_dev)
>>       return ret;
>>  }
>>
>> +#ifdef CONFIG_PM_GENERIC_DOMAINS_OF
>> +static int scp_power_on_pd_A7(struct generic_pm_domain *domain)
>> +{
>> +     /*
>> +      * Simply tell mcpm we need power.  Nothing else needs to be done
>> +      * as CPUs in the cluster are already powered when we reach this point.
>> +      */
>> +     atomic_set(&gpd_A7_cluster_on, 1);
>> +     return 0;
>> +}
>> +
>> +static int scp_power_off_pd_A7(struct generic_pm_domain *domain)
>> +{
>> +     atomic_set(&gpd_A7_cluster_on, 0);
>> +     return 0;
>> +}
>> +
>> +static int scp_power_on_pd_A15(struct generic_pm_domain *domain)
>> +{
>> +     /*
>> +      * Simply tell mcpm we need power.  Nothing else needs to be done
>> +      * as CPUs in the cluster are already powered when we reach this point.
>> +      */
>> +     atomic_set(&gpd_A15_cluster_on, 1);
>> +     return 0;
>> +}
>> +
>> +static int scp_power_off_pd_A15(struct generic_pm_domain *domain)
>> +{
>> +     atomic_set(&gpd_A15_cluster_on, 0);
>> +     return 0;
>> +}
>> +
>> +static int (*const scp_power_fct[MAX_CLUSTERS * 2])
>> +             (struct generic_pm_domain *domain) = {
>> +             scp_power_on_pd_A15, scp_power_off_pd_A15,
>> +             scp_power_on_pd_A7, scp_power_off_pd_A7};
>
> I think you can remove the functions above and the corresponding atomic
> variables, see my comment above.
>
>> +static void scp_init_pd(struct generic_pm_domain *pd,
>> +                         struct device_node *np,
>> +                         struct platform_device *pdev, int cluster)
>> +{
>> +     char name[50];
>> +     int index = cluster * 2;
>> +
>> +     snprintf(name, sizeof(name), "%s-%d", np->name, cluster);
>> +
>> +     pd->name = kstrdup(name, GFP_KERNEL);
>> +     pd->power_on = scp_power_fct[index];
>> +     pd->power_off =  scp_power_fct[index + 1];
>> +     platform_set_drvdata(pdev, pd);
>> +
>> +     pm_genpd_init(pd, NULL, true);
>> +     pm_genpd_poweron(pd);
>> +}
>> +
>> +static __init int ve_spc_gpd_init(void)
>> +{
>> +     struct genpd_onecell_data *data;
>> +     struct generic_pm_domain *pd, **cluster_pds;
>> +     struct platform_device *pdev;
>> +     struct device *dev;
>> +     struct device_node *np;
>> +     int count;
>> +
>> +     np = of_find_compatible_node(NULL, NULL,
>> +                                  "arm,vexpress-power-controller");
>
> See the bindings discussions, there is not such a thing as
> vexpress-power-controller.

I looked at other examples that prefixed the "power-controller" part
with something specific.  In thinking further on it
"arm,power-controller,v2p-ca15_a7" is likely a better choice.

>
>> +     if (!np)
>> +             return -EINVAL;
>> +
>> +     cluster_pds = kzalloc(sizeof(struct generic_pm_domain *) * MAX_CLUSTERS,
>> +                           GFP_KERNEL);
>> +     if (!cluster_pds)
>> +             goto err_cluster_kzalloc;
>
> You are freeing a pointer that failed to be allocated.
>
>> +
>> +     data = kzalloc(sizeof(struct genpd_onecell_data), GFP_KERNEL);
>> +     if (!data)
>> +             goto err_data;
>
> Mmm...is that the label you want to jump to ? it fails to put the OF
> node.
>
>> +
>> +     pdev = of_find_device_by_node(np);
>> +     dev = &pdev->dev;
>> +
>> +     for (count = 0; count < MAX_CLUSTERS; count++) {
>> +             pd = kzalloc(sizeof(struct generic_pm_domain), GFP_KERNEL);
>> +             if (!pd)
>> +                     goto err_pd_kzalloc;
>
> This label does not free the data pointer. I think you should rework
> the error exit paths.
>
>> +             scp_init_pd(pd, np, pdev, count);
>> +             cluster_pds[count] = pd;
>> +     }
>> +
>> +     data->num_domains = count;
>> +     data->domains = cluster_pds;
>> +
>> +     of_genpd_add_provider_onecell(np, data);
>> +     return 0;
>> +
>> +err_pd_kzalloc:
>> +     while (--count >= 0)
>> +             kfree(cluster_pds[count]);
>> +
>> +err_cluster_kzalloc:
>> +     of_node_put(np);
>> +err_data:
>> +     kfree(cluster_pds);
>> +     return -ENOMEM;
>> +
>> +}
>> +arch_initcall(ve_spc_gpd_init);
>
> It should not be an arch initcall, call it from spc_init, and make it an
> empty static inline if !CONFIG_PM_GENERIC_DOMAINS_OF, eg:

That was my first intention but calling @platform_set_drvdata(); on
the node returned by @of_find_compatible_node() crashed the system.  I
will investigate further.

>
> static inline int ve_spc_gpd_init(void)
> {
>         return -ENOTSUPP;
> }
>
> Lorenzo
>
>> +#endif
>> +
>>  int __init ve_spc_init(void __iomem *baseaddr, u32 a15_clusid, int irq)
>>  {
>>       int ret;
>> --
>> 1.9.1
>>
>>

[PATCH 8/9] coresight: Adding DT generic power domain support

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

ETM and PTM tracers need to hookup to the generic power domain
API in order to make sure their power domain doesn't get switched
off by other system components.

On the vexpress a single entity (the SPC) is responsible for power
management of the A7 and A15 clusters domain.  As such using a single
power domain representation with power-domain-cells to represent the
domains, as required by the generic power domain binding.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
index 33920df03640..8f6fc26b59fb 100644
--- a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
+++ b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
@@ -358,6 +358,11 @@
 		};
 	};
 
+	A7_A15_cluster_pd: A7-A15-cluster-pd {
+		compatible = "arm,vexpress-power-controller";
+		#power-domain-cells = <1>;
+	};
+
 	etb@0,20010000 {
 		compatible = "arm,coresight-etb10", "arm,primecell";
 		reg = <0 0x20010000 0 0x1000>;
@@ -494,6 +499,7 @@
 		cpu = <&cpu0>;
 		clocks = <&oscclk6a>;
 		clock-names = "apb_pclk";
+		power-domains = <&A7_A15_cluster_pd 0>;
 		port {
 			ptm0_out_port: endpoint {
 				remote-endpoint = <&funnel_in_port0>;
@@ -508,6 +514,7 @@
 		cpu = <&cpu1>;
 		clocks = <&oscclk6a>;
 		clock-names = "apb_pclk";
+		power-domains = <&A7_A15_cluster_pd 0>;
 		port {
 			ptm1_out_port: endpoint {
 				remote-endpoint = <&funnel_in_port1>;
@@ -522,6 +529,7 @@
 		cpu = <&cpu2>;
 		clocks = <&oscclk6a>;
 		clock-names = "apb_pclk";
+		power-domains = <&A7_A15_cluster_pd 1>;
 		port {
 			etm0_out_port: endpoint {
 				remote-endpoint = <&funnel_in_port2>;
@@ -536,6 +544,7 @@
 		cpu = <&cpu3>;
 		clocks = <&oscclk6a>;
 		clock-names = "apb_pclk";
+		power-domains = <&A7_A15_cluster_pd 1>;
 		port {
 			etm1_out_port: endpoint {
 				remote-endpoint = <&funnel_in_port4>;
@@ -550,6 +559,7 @@
 		cpu = <&cpu4>;
 		clocks = <&oscclk6a>;
 		clock-names = "apb_pclk";
+		power-domains = <&A7_A15_cluster_pd 1>;
 		port {
 			etm2_out_port: endpoint {
 				remote-endpoint = <&funnel_in_port5>;
-- 
1.9.1

[PATCH 9/9] coresight: Documenting reference to generic PD bindings

[mathieu.poirier]

From: Mathieu Poirier <mathieu.poirier@linaro.org>

Each coresight block can be part of a power domain.  Using the
generic power domain subsystems to manage power to individual
domains guarantes that coresight operations won't be interrupted
by other components.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
 Documentation/devicetree/bindings/arm/coresight.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/coresight.txt b/Documentation/devicetree/bindings/arm/coresight.txt
index d790f49066f3..27f96f0d36ef 100644
--- a/Documentation/devicetree/bindings/arm/coresight.txt
+++ b/Documentation/devicetree/bindings/arm/coresight.txt
@@ -50,6 +50,10 @@ its hardware characteristcs.
 	* cpu: the cpu phandle this ETM/PTM is affined to. When omitted the
 	  source is considered to belong to CPU0.
 
+	* power-domains: a handle to the generic power domain node this
+	  coresight block is affined to.  When omitted the component is
+	  assumed to always be powered.
+
 * Optional property for TMC:
 
 	* arm,buffer-size: size of contiguous buffer space for TMC ETR
-- 
1.9.1