[PATCH 00/13] coresight: Implement device claim protocol

[PATCH 00/13] coresight: Implement device claim protocol

[Suzuki K Poulose]

Coresight architecture defines CLAIM tags for a device to negotiate
control of the components (external agent vs self-hosted). Each device
has a pair of registers (CLAIMSET & CLAIMCLR) for managing the CLAIM
tags. However, the protocol for the CLAIM tags is IMPLEMENTATION DEFINED.
PSCI has recommendations for the use of the CLAIM tags to negotiate
controls for external agent vs self-hosted use, as defined in
ARM DEN 0022D, Section "6.8.1 Debug and Trace save and restore".

This series implements the recommended protocol by PSCI.

There were two options for the implementation.
 1) Have the claim/disclaim operations performed from the coresight
    generic driver - This doesn't work unfortunately for ETM devices
    as the need cross-CPU calls to access the CLAIM registers. Also,
    makes it complex for error recovery and reference counting.

 2) Have the claim/disclaim operations performed from the device
    specific drivers. The disadvantage is that the calls are sprinkled
    in each driver, but this makes the operation much simpler.

This series implements the method (2). The first part of the series
prepares different drivers to handle errors from the lower layer
and clean up the state.

Applies on coresight/next:


Suzuki K Poulose (13):
  coresight: tmc-etr: Refactor for handling errors
  coresight: tmc-etr: Handle errors enabling CATU
  coresight: tmc-etb/etf: Prepare to handle errors enabling
  coresight: etm4x: Add support for handling errors
  coresight: etm3: Add support for handling errors
  coresight: etb10: Handle errors enabling the device
  coresight: dynamic-replicator: Handle multiple connections
  coresight: Add support for CLAIM tag protocol
  coresight: etmx: Claim devices before use
  coresight: funnel: Claim devices before use
  coresight: catu: Claim device before use
  coresight: dynamic-replicator: Claim device for use
  coreisght: tmc: Claim device before use

 drivers/hwtracing/coresight/coresight-catu.c       |  6 ++
 .../coresight/coresight-dynamic-replicator.c       | 79 +++++++++++++-----
 drivers/hwtracing/coresight/coresight-etb10.c      | 18 +++--
 drivers/hwtracing/coresight/coresight-etm3x.c      | 58 ++++++++++----
 drivers/hwtracing/coresight/coresight-etm4x.c      | 52 ++++++++----
 drivers/hwtracing/coresight/coresight-funnel.c     | 26 ++++--
 drivers/hwtracing/coresight/coresight-priv.h       |  7 ++
 drivers/hwtracing/coresight/coresight-tmc-etf.c    | 93 +++++++++++++++-------
 drivers/hwtracing/coresight/coresight-tmc-etr.c    | 80 +++++++++++++------
 drivers/hwtracing/coresight/coresight.c            | 85 ++++++++++++++++++++
 include/linux/coresight.h                          | 20 +++++
 11 files changed, 409 insertions(+), 115 deletions(-)

-- 
2.7.4

[PATCH 01/13] coresight: tmc-etr: Refactor for handling errors

[Suzuki K Poulose]

Refactor the tmc-etr enable operation to make it easier to
handle errors in enabling the hardware. We need to make
sure that the buffer is compatible with the ETR. This
patch re-arranges to make the error handling easier, by
deferring the hardware enablement until all the errors
are checked. This also avoids turning the CATU on/off
during a sysfs read session.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 67 ++++++++++++++++---------
 1 file changed, 43 insertions(+), 24 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 56fea4f..c426935 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -918,21 +918,10 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata)
 		tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset);
 }
 
-static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
-			      struct etr_buf *etr_buf)
+static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
 {
 	u32 axictl, sts;
-
-	/* Callers should provide an appropriate buffer for use */
-	if (WARN_ON(!etr_buf || drvdata->etr_buf))
-		return;
-	drvdata->etr_buf = etr_buf;
-
-	/*
-	 * If this ETR is connected to a CATU, enable it before we turn
-	 * this on
-	 */
-	tmc_etr_enable_catu(drvdata);
+	struct etr_buf *etr_buf = drvdata->etr_buf;
 
 	CS_UNLOCK(drvdata->base);
 
@@ -952,11 +941,8 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
 		axictl |= TMC_AXICTL_ARCACHE_OS;
 	}
 
-	if (etr_buf->mode == ETR_MODE_ETR_SG) {
-		if (WARN_ON(!tmc_etr_has_cap(drvdata, TMC_ETR_SG)))
-			return;
+	if (etr_buf->mode == ETR_MODE_ETR_SG)
 		axictl |= TMC_AXICTL_SCT_GAT_MODE;
-	}
 
 	writel_relaxed(axictl, drvdata->base + TMC_AXICTL);
 	tmc_write_dba(drvdata, etr_buf->hwaddr);
@@ -982,6 +968,32 @@ static void tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
 	CS_LOCK(drvdata->base);
 }
 
+static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
+			     struct etr_buf *etr_buf)
+{
+	/* Callers should provide an appropriate buffer for use */
+	if (WARN_ON(!etr_buf))
+		return -EINVAL;
+
+	if ((etr_buf->mode == ETR_MODE_ETR_SG) &&
+	    WARN_ON(!tmc_etr_has_cap(drvdata, TMC_ETR_SG)))
+		return -EINVAL;
+
+	if (WARN_ON(drvdata->etr_buf))
+		return -EBUSY;
+
+	/* Set the buffer for the session */
+	drvdata->etr_buf = etr_buf;
+	/*
+	 * If this ETR is connected to a CATU, enable it before we turn
+	 * this on.
+	 */
+	tmc_etr_enable_catu(drvdata);
+
+	__tmc_etr_enable_hw(drvdata);
+	return 0;
+}
+
 /*
  * Return the available trace data in the buffer (starts at etr_buf->offset,
  * limited by etr_buf->len) from @pos, with a maximum limit of @len,
@@ -1037,7 +1049,7 @@ static void tmc_etr_sync_sysfs_buf(struct tmc_drvdata *drvdata)
 	}
 }
 
-static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
+static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
 {
 	CS_UNLOCK(drvdata->base);
 
@@ -1053,6 +1065,11 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
 
 	CS_LOCK(drvdata->base);
 
+}
+
+static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
+{
+	__tmc_etr_disable_hw(drvdata);
 	/* Disable CATU device if this ETR is connected to one */
 	tmc_etr_disable_catu(drvdata);
 	/* Reset the ETR buf used by hardware */
@@ -1111,8 +1128,9 @@ static int tmc_enable_etr_sink_sysfs(struct coresight_device *csdev)
 		drvdata->sysfs_buf = new_buf;
 	}
 
-	drvdata->mode = CS_MODE_SYSFS;
-	tmc_etr_enable_hw(drvdata, drvdata->sysfs_buf);
+	ret = tmc_etr_enable_hw(drvdata, drvdata->sysfs_buf);
+	if (!ret)
+		drvdata->mode = CS_MODE_SYSFS;
 out:
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
@@ -1342,8 +1360,9 @@ static int tmc_enable_etr_sink_perf(struct coresight_device *csdev, void *data)
 
 	etr_perf->head = PERF_IDX2OFF(handle->head, etr_perf);
 	drvdata->perf_data = etr_perf;
-	drvdata->mode = CS_MODE_PERF;
-	tmc_etr_enable_hw(drvdata, etr_perf->etr_buf);
+	rc = tmc_etr_enable_hw(drvdata, etr_perf->etr_buf);
+	if (!rc)
+		drvdata->mode = CS_MODE_PERF;
 
 unlock_out:
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
@@ -1425,7 +1444,7 @@ int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
 
 	/* Disable the TMC if we are trying to read from a running session. */
 	if (drvdata->mode == CS_MODE_SYSFS)
-		tmc_etr_disable_hw(drvdata);
+		__tmc_etr_disable_hw(drvdata);
 
 	drvdata->reading = true;
 out:
@@ -1452,7 +1471,7 @@ int tmc_read_unprepare_etr(struct tmc_drvdata *drvdata)
 		 * buffer. Since the tracer is still enabled drvdata::buf can't
 		 * be NULL.
 		 */
-		tmc_etr_enable_hw(drvdata, drvdata->sysfs_buf);
+		__tmc_etr_enable_hw(drvdata);
 	} else {
 		/*
 		 * The ETR is not tracing and the buffer was just read.
-- 
2.7.4

[PATCH 02/13] coresight: tmc-etr: Handle errors enabling CATU

[Suzuki K Poulose]

Make sure we honor the errors in CATU device and abort the operation.
While at it, delay setting the etr_buf for the session until we are
sure that we are indeed enabling the ETR.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-etr.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index c426935..daad521 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -751,12 +751,14 @@ tmc_etr_get_catu_device(struct tmc_drvdata *drvdata)
 	return NULL;
 }
 
-static inline void tmc_etr_enable_catu(struct tmc_drvdata *drvdata)
+static inline int tmc_etr_enable_catu(struct tmc_drvdata *drvdata,
+				      struct etr_buf *etr_buf)
 {
 	struct coresight_device *catu = tmc_etr_get_catu_device(drvdata);
 
 	if (catu && helper_ops(catu)->enable)
-		helper_ops(catu)->enable(catu, drvdata->etr_buf);
+		return helper_ops(catu)->enable(catu, etr_buf);
+	return 0;
 }
 
 static inline void tmc_etr_disable_catu(struct tmc_drvdata *drvdata)
@@ -971,6 +973,8 @@ static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
 static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
 			     struct etr_buf *etr_buf)
 {
+	int rc;
+
 	/* Callers should provide an appropriate buffer for use */
 	if (WARN_ON(!etr_buf))
 		return -EINVAL;
@@ -982,16 +986,17 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
 	if (WARN_ON(drvdata->etr_buf))
 		return -EBUSY;
 
-	/* Set the buffer for the session */
-	drvdata->etr_buf = etr_buf;
 	/*
 	 * If this ETR is connected to a CATU, enable it before we turn
 	 * this on.
 	 */
-	tmc_etr_enable_catu(drvdata);
+	rc = tmc_etr_enable_catu(drvdata, etr_buf);
+	if (!rc) {
+		drvdata->etr_buf = etr_buf;
+		__tmc_etr_enable_hw(drvdata);
+	}
 
-	__tmc_etr_enable_hw(drvdata);
-	return 0;
+	return rc;
 }
 
 /*
-- 
2.7.4

[PATCH 03/13] coresight: tmc-etb/etf: Prepare to handle errors enabling

[Suzuki K Poulose]

Prepare to handle errors in enabling the hardware and
report it back to the core driver.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-etf.c | 71 +++++++++++++++----------
 1 file changed, 44 insertions(+), 27 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 4156c95..ceb4b30 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -15,7 +15,7 @@
 static int tmc_set_etf_buffer(struct coresight_device *csdev,
 			      struct perf_output_handle *handle);
 
-static void tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
+static void __tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
 {
 	CS_UNLOCK(drvdata->base);
 
@@ -34,6 +34,12 @@ static void tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
 	CS_LOCK(drvdata->base);
 }
 
+static int tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
+{
+	__tmc_etb_enable_hw(drvdata);
+	return 0;
+}
+
 static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata)
 {
 	char *bufp;
@@ -76,7 +82,7 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
 	CS_LOCK(drvdata->base);
 }
 
-static void tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
+static void __tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
 {
 	CS_UNLOCK(drvdata->base);
 
@@ -92,6 +98,12 @@ static void tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
 	CS_LOCK(drvdata->base);
 }
 
+static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
+{
+	__tmc_etf_enable_hw(drvdata);
+	return 0;
+}
+
 static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
 {
 	CS_UNLOCK(drvdata->base);
@@ -174,8 +186,12 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev)
 		drvdata->buf = buf;
 	}
 
-	drvdata->mode = CS_MODE_SYSFS;
-	tmc_etb_enable_hw(drvdata);
+	ret = tmc_etb_enable_hw(drvdata);
+	if (!ret)
+		drvdata->mode = CS_MODE_SYSFS;
+	else
+		/* Free up the buffer if we failed to enable */
+		used = false;
 out:
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
@@ -194,27 +210,25 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, void *data)
 	struct perf_output_handle *handle = data;
 
 	spin_lock_irqsave(&drvdata->spinlock, flags);
-	if (drvdata->reading) {
+	do {
 		ret = -EINVAL;
-		goto out;
-	}
+		if (drvdata->reading)
+			break;
+		/*
+		 * In Perf mode there can be only one writer per sink.  There
+		 * is also no need to continue if the ETB/ETF is already
+		 * operated from sysFS.
+		 */
+		if (drvdata->mode != CS_MODE_DISABLED)
+			break;
 
-	/*
-	 * In Perf mode there can be only one writer per sink.  There
-	 * is also no need to continue if the ETB/ETR is already operated
-	 * from sysFS.
-	 */
-	if (drvdata->mode != CS_MODE_DISABLED) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	ret = tmc_set_etf_buffer(csdev, handle);
-	if (!ret) {
-		drvdata->mode = CS_MODE_PERF;
-		tmc_etb_enable_hw(drvdata);
-	}
-out:
+		ret = tmc_set_etf_buffer(csdev, handle);
+		if (ret)
+			break;
+		ret  = tmc_etb_enable_hw(drvdata);
+		if (!ret)
+			drvdata->mode = CS_MODE_PERF;
+	} while (0);
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
 	return ret;
@@ -271,6 +285,7 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
 static int tmc_enable_etf_link(struct coresight_device *csdev,
 			       int inport, int outport)
 {
+	int ret;
 	unsigned long flags;
 	struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
@@ -280,11 +295,13 @@ static int tmc_enable_etf_link(struct coresight_device *csdev,
 		return -EBUSY;
 	}
 
-	tmc_etf_enable_hw(drvdata);
-	drvdata->mode = CS_MODE_SYSFS;
+	ret = tmc_etf_enable_hw(drvdata);
+	if (!ret)
+		drvdata->mode = CS_MODE_SYSFS;
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
-	dev_dbg(drvdata->dev, "TMC-ETF enabled\n");
+	if (!ret)
+		dev_dbg(drvdata->dev, "TMC-ETF enabled\n");
 	return 0;
 }
 
@@ -579,7 +596,7 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
 		 * can't be NULL.
 		 */
 		memset(drvdata->buf, 0, drvdata->size);
-		tmc_etb_enable_hw(drvdata);
+		__tmc_etb_enable_hw(drvdata);
 	} else {
 		/*
 		 * The ETB/ETF is not tracing and the buffer was just read.
-- 
2.7.4

Re: [PATCH 03/13] coresight: tmc-etb/etf: Prepare to handle errors enabling

[Mathieu Poirier]

On Mon, Aug 06, 2018 at 02:41:45PM +0100, Suzuki K Poulose wrote:
> Prepare to handle errors in enabling the hardware and
> report it back to the core driver.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tmc-etf.c | 71 +++++++++++++++----------
>  1 file changed, 44 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index 4156c95..ceb4b30 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -15,7 +15,7 @@
>  static int tmc_set_etf_buffer(struct coresight_device *csdev,
>  			      struct perf_output_handle *handle);
>  
> -static void tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
> +static void __tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
>  {
>  	CS_UNLOCK(drvdata->base);
>  
> @@ -34,6 +34,12 @@ static void tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
>  	CS_LOCK(drvdata->base);
>  }
>  
> +static int tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
> +{
> +	__tmc_etb_enable_hw(drvdata);
> +	return 0;
> +}
> +
>  static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata)
>  {
>  	char *bufp;
> @@ -76,7 +82,7 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
>  	CS_LOCK(drvdata->base);
>  }
>  
> -static void tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
> +static void __tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
>  {
>  	CS_UNLOCK(drvdata->base);
>  
> @@ -92,6 +98,12 @@ static void tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
>  	CS_LOCK(drvdata->base);
>  }
>  
> +static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
> +{
> +	__tmc_etf_enable_hw(drvdata);
> +	return 0;
> +}
> +
>  static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
>  {
>  	CS_UNLOCK(drvdata->base);
> @@ -174,8 +186,12 @@ static int tmc_enable_etf_sink_sysfs(struct coresight_device *csdev)
>  		drvdata->buf = buf;
>  	}
>  
> -	drvdata->mode = CS_MODE_SYSFS;
> -	tmc_etb_enable_hw(drvdata);
> +	ret = tmc_etb_enable_hw(drvdata);
> +	if (!ret)
> +		drvdata->mode = CS_MODE_SYSFS;
> +	else
> +		/* Free up the buffer if we failed to enable */
> +		used = false;
>  out:
>  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  
> @@ -194,27 +210,25 @@ static int tmc_enable_etf_sink_perf(struct coresight_device *csdev, void *data)
>  	struct perf_output_handle *handle = data;
>  
>  	spin_lock_irqsave(&drvdata->spinlock, flags);
> -	if (drvdata->reading) {
> +	do {
>  		ret = -EINVAL;
> -		goto out;
> -	}
> +		if (drvdata->reading)
> +			break;
> +		/*
> +		 * In Perf mode there can be only one writer per sink.  There
> +		 * is also no need to continue if the ETB/ETF is already
> +		 * operated from sysFS.
> +		 */
> +		if (drvdata->mode != CS_MODE_DISABLED)
> +			break;
>  
> -	/*
> -	 * In Perf mode there can be only one writer per sink.  There
> -	 * is also no need to continue if the ETB/ETR is already operated
> -	 * from sysFS.
> -	 */
> -	if (drvdata->mode != CS_MODE_DISABLED) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -
> -	ret = tmc_set_etf_buffer(csdev, handle);
> -	if (!ret) {
> -		drvdata->mode = CS_MODE_PERF;
> -		tmc_etb_enable_hw(drvdata);
> -	}
> -out:
> +		ret = tmc_set_etf_buffer(csdev, handle);
> +		if (ret)
> +			break;
> +		ret  = tmc_etb_enable_hw(drvdata);
> +		if (!ret)
> +			drvdata->mode = CS_MODE_PERF;
> +	} while (0);
>  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  
>  	return ret;
> @@ -271,6 +285,7 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
>  static int tmc_enable_etf_link(struct coresight_device *csdev,
>  			       int inport, int outport)
>  {
> +	int ret;
>  	unsigned long flags;
>  	struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>  
> @@ -280,11 +295,13 @@ static int tmc_enable_etf_link(struct coresight_device *csdev,
>  		return -EBUSY;
>  	}
>  
> -	tmc_etf_enable_hw(drvdata);
> -	drvdata->mode = CS_MODE_SYSFS;
> +	ret = tmc_etf_enable_hw(drvdata);
> +	if (!ret)
> +		drvdata->mode = CS_MODE_SYSFS;
>  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  
> -	dev_dbg(drvdata->dev, "TMC-ETF enabled\n");
> +	if (!ret)
> +		dev_dbg(drvdata->dev, "TMC-ETF enabled\n");
>  	return 0;

We need to tell the caller if an error as occured:

        return ret;

>  }
>  
> @@ -579,7 +596,7 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata)
>  		 * can't be NULL.
>  		 */
>  		memset(drvdata->buf, 0, drvdata->size);
> -		tmc_etb_enable_hw(drvdata);
> +		__tmc_etb_enable_hw(drvdata);
>  	} else {
>  		/*
>  		 * The ETB/ETF is not tracing and the buffer was just read.
> -- 
> 2.7.4
> 

Re: [PATCH 03/13] coresight: tmc-etb/etf: Prepare to handle errors enabling

[Suzuki K Poulose]

On 15/08/18 20:22, Mathieu Poirier wrote:
> On Mon, Aug 06, 2018 at 02:41:45PM +0100, Suzuki K Poulose wrote:
>> Prepare to handle errors in enabling the hardware and
>> report it back to the core driver.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-tmc-etf.c | 71 +++++++++++++++----------
>>   1 file changed, 44 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> index 4156c95..ceb4b30 100644
>> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c

>> @@ -271,6 +285,7 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
>>   static int tmc_enable_etf_link(struct coresight_device *csdev,
>>   			       int inport, int outport)
>>   {
>> +	int ret;
>>   	unsigned long flags;
>>   	struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>   
>> @@ -280,11 +295,13 @@ static int tmc_enable_etf_link(struct coresight_device *csdev,
>>   		return -EBUSY;
>>   	}
>>   
>> -	tmc_etf_enable_hw(drvdata);
>> -	drvdata->mode = CS_MODE_SYSFS;
>> +	ret = tmc_etf_enable_hw(drvdata);
>> +	if (!ret)
>> +		drvdata->mode = CS_MODE_SYSFS;
>>   	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>>   
>> -	dev_dbg(drvdata->dev, "TMC-ETF enabled\n");
>> +	if (!ret)
>> +		dev_dbg(drvdata->dev, "TMC-ETF enabled\n");
>>   	return 0;
> 
> We need to tell the caller if an error as occured:
> 
>          return ret;
> 

Thanks for catching it, will fix.

Suzuki

[PATCH 04/13] coresight: etm4x: Add support for handling errors

[Suzuki K Poulose]

Add support for handling errors in enabling the component.
The ETM is enabled via cross call to owner CPU. Make
necessary changes to report the error back from the cross
call.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm4x.c | 39 ++++++++++++++++++---------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index c1dcc7c..4f9e6bb 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -77,10 +77,14 @@ static int etm4_trace_id(struct coresight_device *csdev)
 	return drvdata->trcid;
 }
 
-static void etm4_enable_hw(void *info)
+struct etm4_enable_arg {
+	struct etmv4_drvdata *drvdata;
+	int rc;
+};
+
+static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
 {
 	int i;
-	struct etmv4_drvdata *drvdata = info;
 	struct etmv4_config *config = &drvdata->config;
 
 	CS_UNLOCK(drvdata->base);
@@ -177,6 +181,16 @@ static void etm4_enable_hw(void *info)
 	CS_LOCK(drvdata->base);
 
 	dev_dbg(drvdata->dev, "cpu: %d enable smp call done\n", drvdata->cpu);
+	return 0;
+}
+
+static void etm4_enable_hw_smp_call(void *info)
+{
+	struct etm4_enable_arg *arg = info;
+
+	if (WARN_ON(!arg))
+		return;
+	arg->rc = etm4_enable_hw(arg->drvdata);
 }
 
 static int etm4_parse_event_config(struct etmv4_drvdata *drvdata,
@@ -242,7 +256,7 @@ static int etm4_enable_perf(struct coresight_device *csdev,
 	if (ret)
 		goto out;
 	/* And enable it */
-	etm4_enable_hw(drvdata);
+	ret = etm4_enable_hw(drvdata);
 
 out:
 	return ret;
@@ -251,6 +265,7 @@ static int etm4_enable_perf(struct coresight_device *csdev,
 static int etm4_enable_sysfs(struct coresight_device *csdev)
 {
 	struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+	struct etm4_enable_arg arg = { 0 };
 	int ret;
 
 	spin_lock(&drvdata->spinlock);
@@ -259,19 +274,17 @@ static int etm4_enable_sysfs(struct coresight_device *csdev)
 	 * Executing etm4_enable_hw on the cpu whose ETM is being enabled
 	 * ensures that register writes occur when cpu is powered.
 	 */
+	arg.drvdata = drvdata;
 	ret = smp_call_function_single(drvdata->cpu,
-				       etm4_enable_hw, drvdata, 1);
-	if (ret)
-		goto err;
-
-	drvdata->sticky_enable = true;
+				       etm4_enable_hw_smp_call, &arg, 1);
+	if (!ret)
+		ret = arg.rc;
+	if (!ret)
+		drvdata->sticky_enable = true;
 	spin_unlock(&drvdata->spinlock);
 
-	dev_dbg(drvdata->dev, "ETM tracing enabled\n");
-	return 0;
-
-err:
-	spin_unlock(&drvdata->spinlock);
+	if (!ret)
+		dev_dbg(drvdata->dev, "ETM tracing enabled\n");
 	return ret;
 }
 
-- 
2.7.4

[PATCH 05/13] coresight: etm3: Add support for handling errors

[Suzuki K Poulose]

Add support for reporting errors back from the SMP cross
function call for enabling ETM.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm3x.c | 42 ++++++++++++++++++---------
 1 file changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
index 9ce8fba..771691c 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x.c
@@ -355,11 +355,10 @@ static int etm_parse_event_config(struct etm_drvdata *drvdata,
 	return 0;
 }
 
-static void etm_enable_hw(void *info)
+static int etm_enable_hw(struct etm_drvdata *drvdata)
 {
 	int i;
 	u32 etmcr;
-	struct etm_drvdata *drvdata = info;
 	struct etm_config *config = &drvdata->config;
 
 	CS_UNLOCK(drvdata->base);
@@ -421,6 +420,21 @@ static void etm_enable_hw(void *info)
 	CS_LOCK(drvdata->base);
 
 	dev_dbg(drvdata->dev, "cpu: %d enable smp call done\n", drvdata->cpu);
+	return 0;
+}
+
+struct etm_enable_arg {
+	struct etm_drvdata *drvdata;
+	int rc;
+}
+
+static void etm_enable_hw_smp_call(void *info)
+{
+	struct etm_enable_arg *arg = info;
+
+	if (WARN_ON(!arg))
+		return;
+	arg->rc = etm_enable_hw(arg->drvdata);
 }
 
 static int etm_cpu_id(struct coresight_device *csdev)
@@ -475,14 +489,13 @@ static int etm_enable_perf(struct coresight_device *csdev,
 	/* Configure the tracer based on the session's specifics */
 	etm_parse_event_config(drvdata, event);
 	/* And enable it */
-	etm_enable_hw(drvdata);
-
-	return 0;
+	return etm_enable_hw(drvdata);
 }
 
 static int etm_enable_sysfs(struct coresight_device *csdev)
 {
 	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+	struct etm_enable_arg arg = { 0 };
 	int ret;
 
 	spin_lock(&drvdata->spinlock);
@@ -492,20 +505,21 @@ static int etm_enable_sysfs(struct coresight_device *csdev)
 	 * hw configuration will take place on the local CPU during bring up.
 	 */
 	if (cpu_online(drvdata->cpu)) {
+		arg.drvdata = drvdata;
 		ret = smp_call_function_single(drvdata->cpu,
-					       etm_enable_hw, drvdata, 1);
-		if (ret)
-			goto err;
+					       etm_enable_hw_smp_call, &arg, 1);
+		if (!ret)
+			ret = arg.rc;
+		if (!ret)
+			drvdata->sticky_enable = true;
+	} else {
+		ret = -ENODEV;
 	}
 
-	drvdata->sticky_enable = true;
 	spin_unlock(&drvdata->spinlock);
 
-	dev_dbg(drvdata->dev, "ETM tracing enabled\n");
-	return 0;
-
-err:
-	spin_unlock(&drvdata->spinlock);
+	if (!ret)
+		dev_dbg(drvdata->dev, "ETM tracing enabled\n");
 	return ret;
 }
 
-- 
2.7.4

Re: [PATCH 05/13] coresight: etm3: Add support for handling errors

[Mathieu Poirier]

On Mon, Aug 06, 2018 at 02:41:47PM +0100, Suzuki K Poulose wrote:
> Add support for reporting errors back from the SMP cross
> function call for enabling ETM.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm3x.c | 42 ++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
> index 9ce8fba..771691c 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
> @@ -355,11 +355,10 @@ static int etm_parse_event_config(struct etm_drvdata *drvdata,
>  	return 0;
>  }
>  
> -static void etm_enable_hw(void *info)
> +static int etm_enable_hw(struct etm_drvdata *drvdata)
>  {
>  	int i;
>  	u32 etmcr;
> -	struct etm_drvdata *drvdata = info;
>  	struct etm_config *config = &drvdata->config;
>  
>  	CS_UNLOCK(drvdata->base);
> @@ -421,6 +420,21 @@ static void etm_enable_hw(void *info)
>  	CS_LOCK(drvdata->base);
>  
>  	dev_dbg(drvdata->dev, "cpu: %d enable smp call done\n", drvdata->cpu);
> +	return 0;
> +}
> +
> +struct etm_enable_arg {
> +	struct etm_drvdata *drvdata;
> +	int rc;
> +}

This doesn't compile a ';' is missing.

> +
> +static void etm_enable_hw_smp_call(void *info)
> +{
> +	struct etm_enable_arg *arg = info;
> +
> +	if (WARN_ON(!arg))
> +		return;
> +	arg->rc = etm_enable_hw(arg->drvdata);
>  }
>  
>  static int etm_cpu_id(struct coresight_device *csdev)
> @@ -475,14 +489,13 @@ static int etm_enable_perf(struct coresight_device *csdev,
>  	/* Configure the tracer based on the session's specifics */
>  	etm_parse_event_config(drvdata, event);
>  	/* And enable it */
> -	etm_enable_hw(drvdata);
> -
> -	return 0;
> +	return etm_enable_hw(drvdata);
>  }
>  
>  static int etm_enable_sysfs(struct coresight_device *csdev)
>  {
>  	struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +	struct etm_enable_arg arg = { 0 };
>  	int ret;
>  
>  	spin_lock(&drvdata->spinlock);
> @@ -492,20 +505,21 @@ static int etm_enable_sysfs(struct coresight_device *csdev)
>  	 * hw configuration will take place on the local CPU during bring up.
>  	 */
>  	if (cpu_online(drvdata->cpu)) {
> +		arg.drvdata = drvdata;
>  		ret = smp_call_function_single(drvdata->cpu,
> -					       etm_enable_hw, drvdata, 1);
> -		if (ret)
> -			goto err;
> +					       etm_enable_hw_smp_call, &arg, 1);
> +		if (!ret)
> +			ret = arg.rc;
> +		if (!ret)
> +			drvdata->sticky_enable = true;
> +	} else {
> +		ret = -ENODEV;
>  	}
>  
> -	drvdata->sticky_enable = true;
>  	spin_unlock(&drvdata->spinlock);
>  
> -	dev_dbg(drvdata->dev, "ETM tracing enabled\n");
> -	return 0;
> -
> -err:
> -	spin_unlock(&drvdata->spinlock);
> +	if (!ret)
> +		dev_dbg(drvdata->dev, "ETM tracing enabled\n");
>  	return ret;
>  }
>  
> -- 
> 2.7.4
> 

Re: [PATCH 05/13] coresight: etm3: Add support for handling errors

[Suzuki K Poulose]

On 15/08/18 20:34, Mathieu Poirier wrote:
> On Mon, Aug 06, 2018 at 02:41:47PM +0100, Suzuki K Poulose wrote:
>> Add support for reporting errors back from the SMP cross
>> function call for enabling ETM.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-etm3x.c | 42 ++++++++++++++++++---------
>>   1 file changed, 28 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
>> index 9ce8fba..771691c 100644
>> --- a/drivers/hwtracing/coresight/coresight-etm3x.c
>> +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
>> @@ -355,11 +355,10 @@ static int etm_parse_event_config(struct etm_drvdata *drvdata,
>>   	return 0;
>>   }
>>   
>> -static void etm_enable_hw(void *info)
>> +static int etm_enable_hw(struct etm_drvdata *drvdata)
>>   {
>>   	int i;
>>   	u32 etmcr;
>> -	struct etm_drvdata *drvdata = info;
>>   	struct etm_config *config = &drvdata->config;
>>   
>>   	CS_UNLOCK(drvdata->base);
>> @@ -421,6 +420,21 @@ static void etm_enable_hw(void *info)
>>   	CS_LOCK(drvdata->base);
>>   
>>   	dev_dbg(drvdata->dev, "cpu: %d enable smp call done\n", drvdata->cpu);
>> +	return 0;
>> +}
>> +
>> +struct etm_enable_arg {
>> +	struct etm_drvdata *drvdata;
>> +	int rc;
>> +}
> 
> This doesn't compile a ';' is missing.

Oops. Sorry about that. I will give the series a spin on arm32.

Suzuki

[PATCH 06/13] coresight: etb10: Handle errors enabling the device

[Suzuki K Poulose]

Prepare the etb10 driver to return errors in enabling
the device.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-etb10.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
index 9fd77fd..37d2c88 100644
--- a/drivers/hwtracing/coresight/coresight-etb10.c
+++ b/drivers/hwtracing/coresight/coresight-etb10.c
@@ -107,7 +107,7 @@ static unsigned int etb_get_buffer_depth(struct etb_drvdata *drvdata)
 	return depth;
 }
 
-static void etb_enable_hw(struct etb_drvdata *drvdata)
+static void __etb_enable_hw(struct etb_drvdata *drvdata)
 {
 	int i;
 	u32 depth;
@@ -135,6 +135,12 @@ static void etb_enable_hw(struct etb_drvdata *drvdata)
 	CS_LOCK(drvdata->base);
 }
 
+static int etb_enable_hw(struct etb_drvdata *drvdata)
+{
+	__etb_enable_hw(drvdata);
+	return 0;
+}
+
 static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
 {
 	int ret = 0;
@@ -150,7 +156,7 @@ static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
 	if (mode == CS_MODE_PERF) {
 		ret = etb_set_buffer(csdev, (struct perf_output_handle *)data);
 		if (ret)
-			goto out;
+			return ret;
 	}
 
 	val = local_cmpxchg(&drvdata->mode,
@@ -172,12 +178,14 @@ static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
 		goto out;
 
 	spin_lock_irqsave(&drvdata->spinlock, flags);
-	etb_enable_hw(drvdata);
+	ret = etb_enable_hw(drvdata);
 	spin_unlock_irqrestore(&drvdata->spinlock, flags);
 
-out:
-	if (!ret)
+	if (ret)
+		local_cmpxchg(&drvdata->mode, mode, CS_MODE_DISABLED);
+	else
 		dev_dbg(drvdata->dev, "ETB enabled\n");
+
 	return ret;
 }
 
-- 
2.7.4

Re: [PATCH 06/13] coresight: etb10: Handle errors enabling the device

[Mathieu Poirier]

Hi Suzuki,

On Mon, Aug 06, 2018 at 02:41:48PM +0100, Suzuki K Poulose wrote:
> Prepare the etb10 driver to return errors in enabling
> the device.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etb10.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index 9fd77fd..37d2c88 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -107,7 +107,7 @@ static unsigned int etb_get_buffer_depth(struct etb_drvdata *drvdata)
>  	return depth;
>  }
>  
> -static void etb_enable_hw(struct etb_drvdata *drvdata)
> +static void __etb_enable_hw(struct etb_drvdata *drvdata)
>  {
>  	int i;
>  	u32 depth;
> @@ -135,6 +135,12 @@ static void etb_enable_hw(struct etb_drvdata *drvdata)
>  	CS_LOCK(drvdata->base);
>  }
>  
> +static int etb_enable_hw(struct etb_drvdata *drvdata)
> +{
> +	__etb_enable_hw(drvdata);
> +	return 0;
> +}
> +
>  static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
>  {
>  	int ret = 0;
> @@ -150,7 +156,7 @@ static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
>  	if (mode == CS_MODE_PERF) {
>  		ret = etb_set_buffer(csdev, (struct perf_output_handle *)data);
>  		if (ret)
> -			goto out;
> +			return ret;
>  	}
>  
>  	val = local_cmpxchg(&drvdata->mode,
> @@ -172,12 +178,14 @@ static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
>  		goto out;
>  
>  	spin_lock_irqsave(&drvdata->spinlock, flags);
> -	etb_enable_hw(drvdata);
> +	ret = etb_enable_hw(drvdata);
>  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  
> -out:
> -	if (!ret)
> +	if (ret)
> +		local_cmpxchg(&drvdata->mode, mode, CS_MODE_DISABLED);
> +	else

I also have to do hackish things with my work on
CPU-wide trace scenarios because drvdata->mode is of type local_t.  Instead of
living with it I'll send out a patch later today that moves it to a u32 like ETF
and ETR.

Please look at it and if you're happy, add it to this patchset and do your 
modifications on top of it.

Thanks,
Mathieu

>  		dev_dbg(drvdata->dev, "ETB enabled\n");
> +
>  	return ret;
>  }
>  
> -- 
> 2.7.4
> 

Re: [PATCH 06/13] coresight: etb10: Handle errors enabling the device

[Mathieu Poirier]

On Mon, Aug 06, 2018 at 02:41:48PM +0100, Suzuki K Poulose wrote:
> Prepare the etb10 driver to return errors in enabling
> the device.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etb10.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index 9fd77fd..37d2c88 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -107,7 +107,7 @@ static unsigned int etb_get_buffer_depth(struct etb_drvdata *drvdata)
>  	return depth;
>  }
>  
> -static void etb_enable_hw(struct etb_drvdata *drvdata)
> +static void __etb_enable_hw(struct etb_drvdata *drvdata)
>  {
>  	int i;
>  	u32 depth;
> @@ -135,6 +135,12 @@ static void etb_enable_hw(struct etb_drvdata *drvdata)
>  	CS_LOCK(drvdata->base);
>  }
>  
> +static int etb_enable_hw(struct etb_drvdata *drvdata)
> +{
> +	__etb_enable_hw(drvdata);
> +	return 0;
> +}
> +
>  static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
>  {
>  	int ret = 0;
> @@ -150,7 +156,7 @@ static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
>  	if (mode == CS_MODE_PERF) {
>  		ret = etb_set_buffer(csdev, (struct perf_output_handle *)data);
>  		if (ret)
> -			goto out;
> +			return ret;
>  	}
>  
>  	val = local_cmpxchg(&drvdata->mode,
> @@ -172,12 +178,14 @@ static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
>  		goto out;

Leaving this here doesn't compile when 'out' is removed below.

>  
>  	spin_lock_irqsave(&drvdata->spinlock, flags);
> -	etb_enable_hw(drvdata);
> +	ret = etb_enable_hw(drvdata);
>  	spin_unlock_irqrestore(&drvdata->spinlock, flags);
>  
> -out:
> -	if (!ret)
> +	if (ret)
> +		local_cmpxchg(&drvdata->mode, mode, CS_MODE_DISABLED);
> +	else
>  		dev_dbg(drvdata->dev, "ETB enabled\n");
> +
>  	return ret;
>  }
>  
> -- 
> 2.7.4
> 

Re: [PATCH 06/13] coresight: etb10: Handle errors enabling the device

[Suzuki K Poulose]

On 15/08/18 20:38, Mathieu Poirier wrote:
> On Mon, Aug 06, 2018 at 02:41:48PM +0100, Suzuki K Poulose wrote:
>> Prepare the etb10 driver to return errors in enabling
>> the device.
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-etb10.c | 18 +++++++++++++-----
>>   1 file changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
>> index 9fd77fd..37d2c88 100644
>> --- a/drivers/hwtracing/coresight/coresight-etb10.c
>> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
>> @@ -107,7 +107,7 @@ static unsigned int etb_get_buffer_depth(struct etb_drvdata *drvdata)
>>   	return depth;
>>   }
>>   
>> -static void etb_enable_hw(struct etb_drvdata *drvdata)
>> +static void __etb_enable_hw(struct etb_drvdata *drvdata)
>>   {
>>   	int i;
>>   	u32 depth;
>> @@ -135,6 +135,12 @@ static void etb_enable_hw(struct etb_drvdata *drvdata)
>>   	CS_LOCK(drvdata->base);
>>   }
>>   
>> +static int etb_enable_hw(struct etb_drvdata *drvdata)
>> +{
>> +	__etb_enable_hw(drvdata);
>> +	return 0;
>> +}
>> +
>>   static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
>>   {
>>   	int ret = 0;
>> @@ -150,7 +156,7 @@ static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
>>   	if (mode == CS_MODE_PERF) {
>>   		ret = etb_set_buffer(csdev, (struct perf_output_handle *)data);
>>   		if (ret)
>> -			goto out;
>> +			return ret;
>>   	}
>>   
>>   	val = local_cmpxchg(&drvdata->mode,
>> @@ -172,12 +178,14 @@ static int etb_enable(struct coresight_device *csdev, u32 mode, void *data)
>>   		goto out;
> 
> Leaving this here doesn't compile when 'out' is removed below.

I will fix it for arm32. Sorry about that.

Suzuki

[PATCH 07/13] coresight: dynamic-replicator: Handle multiple connections

[Suzuki K Poulose]

When a replicator port is enabled, we block the traffic
on the other port and route all traffic to the new enabled
port. If there are two active trace sessions each targeting
the two different paths from the replicator, the second session
will disable the first session and route all the data to the
second path.
                    ETR
                 /
e.g, replicator
                 \
                    ETB

If CPU0 is operated in sysfs mode to ETR and CPU1 is operated
in perf mode to ETB, depending on the order in which the
replicator is enabled one device is blocked.

Ideally we need trace-id for the session to make the
right choice. That implies we need a trace-id allocation
logic for the coresight subsystem and use that to route
the traffic. The short term solution is to only manage
the "target port" and leave the other port untouched.
That leaves both the paths unaffected, except that some
unwanted traffic may be pushed to the paths (if the Trace-IDs
are not far enough), which is still fine and can be filtered
out while processing rather than silently blocking the data.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 .../coresight/coresight-dynamic-replicator.c       | 66 ++++++++++++++++------
 1 file changed, 48 insertions(+), 18 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
index ebb8043..97f4673 100644
--- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
@@ -34,26 +34,42 @@ struct replicator_state {
 	struct coresight_device	*csdev;
 };
 
+/*
+ * replicator_reset : Reset the replicator configuration to sane values.
+ */
+static void replicator_reset(struct replicator_state *drvdata)
+{
+	CS_UNLOCK(drvdata->base);
+
+	writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER0);
+	writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER1);
+
+	CS_LOCK(drvdata->base);
+}
+
 static int replicator_enable(struct coresight_device *csdev, int inport,
 			      int outport)
 {
+	u32 reg;
 	struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	CS_UNLOCK(drvdata->base);
-
-	/*
-	 * Ensure that the other port is disabled
-	 * 0x00 - passing through the replicator unimpeded
-	 * 0xff - disable (or impede) the flow of ATB data
-	 */
-	if (outport == 0) {
-		writel_relaxed(0x00, drvdata->base + REPLICATOR_IDFILTER0);
-		writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER1);
-	} else {
-		writel_relaxed(0x00, drvdata->base + REPLICATOR_IDFILTER1);
-		writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER0);
+	switch (outport) {
+	case 0:
+		reg = REPLICATOR_IDFILTER0;
+		break;
+	case 1:
+		reg = REPLICATOR_IDFILTER1;
+		break;
+	default:
+		WARN_ON(1);
+		return -EINVAL;
 	}
 
+	CS_UNLOCK(drvdata->base);
+
+
+	/* Ensure that the outport is enabled. */
+	writel_relaxed(0x00, drvdata->base + reg);
 	CS_LOCK(drvdata->base);
 
 	dev_dbg(drvdata->dev, "REPLICATOR enabled\n");
@@ -63,15 +79,25 @@ static int replicator_enable(struct coresight_device *csdev, int inport,
 static void replicator_disable(struct coresight_device *csdev, int inport,
 				int outport)
 {
+	u32 reg;
 	struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent);
 
+	switch (outport) {
+	case 0:
+		reg = REPLICATOR_IDFILTER0;
+		break;
+	case 1:
+		reg = REPLICATOR_IDFILTER1;
+		break;
+	default:
+		WARN_ON(1);
+		return;
+	}
+
 	CS_UNLOCK(drvdata->base);
 
 	/* disable the flow of ATB data through port */
-	if (outport == 0)
-		writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER0);
-	else
-		writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER1);
+	writel_relaxed(0xff, drvdata->base + reg);
 
 	CS_LOCK(drvdata->base);
 
@@ -156,7 +182,11 @@ static int replicator_probe(struct amba_device *adev, const struct amba_id *id)
 	desc.groups = replicator_groups;
 	drvdata->csdev = coresight_register(&desc);
 
-	return PTR_ERR_OR_ZERO(drvdata->csdev);
+	if (!IS_ERR(drvdata->csdev)) {
+		replicator_reset(drvdata);
+		return 0;
+	}
+	return PTR_ERR(drvdata->csdev);
 }
 
 #ifdef CONFIG_PM
-- 
2.7.4

[PATCH 08/13] coresight: Add support for CLAIM tag protocol

[Suzuki K Poulose]

Add support for the CLAIM tag protocol for negotiating the
device ownership with other agents trying to use the coresight
component (internal vs. external). The Coresight architecture
specifies CLAIM tags (managed via CLAIMSET CLAIMCLR registers)
to negotiate the ownership of the device. PSCI recommends the
reservation of the bits in CLAIM tags for self-hosted and external
debug use. This patch implements the protocol for claiming
the devices before they are actually used.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-priv.h |  7 +++
 drivers/hwtracing/coresight/coresight.c      | 85 ++++++++++++++++++++++++++++
 include/linux/coresight.h                    | 20 +++++++
 3 files changed, 112 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
index c11da55..579f349 100644
--- a/drivers/hwtracing/coresight/coresight-priv.h
+++ b/drivers/hwtracing/coresight/coresight-priv.h
@@ -25,6 +25,13 @@
 #define CORESIGHT_DEVID		0xfc8
 #define CORESIGHT_DEVTYPE	0xfcc
 
+
+/*
+ * Coresight device CLAIM protocol.
+ * See PSCI - ARM DEN 0022D, Section: 6.8.1 Debug and Trace save and restore.
+ */
+#define CORESIGHT_CLAIM_SELF_HOSTED	BIT(1)
+
 #define TIMEOUT_US		100
 #define BMVAL(val, lsb, msb)	((val & GENMASK(msb, lsb)) >> lsb)
 
diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
index e73ca6a..8f06866 100644
--- a/drivers/hwtracing/coresight/coresight.c
+++ b/drivers/hwtracing/coresight/coresight.c
@@ -128,6 +128,91 @@ static int coresight_find_link_outport(struct coresight_device *csdev,
 	return -ENODEV;
 }
 
+static inline u32 coresight_read_claim_tags(void __iomem *base)
+{
+	return readl_relaxed(base + CORESIGHT_CLAIMCLR);
+}
+
+static inline bool coresight_is_claimed_self_hosted(void __iomem *base)
+{
+	return coresight_read_claim_tags(base) == CORESIGHT_CLAIM_SELF_HOSTED;
+}
+
+static inline bool coresight_is_claimed_any(void __iomem *base)
+{
+	return coresight_read_claim_tags(base) != 0;
+}
+
+static inline void coresight_set_claim_tags(void __iomem *base)
+{
+	writel_relaxed(CORESIGHT_CLAIM_SELF_HOSTED, base + CORESIGHT_CLAIMSET);
+	isb();
+}
+
+static inline void coresight_clear_claim_tags(void __iomem *base)
+{
+	writel_relaxed(CORESIGHT_CLAIM_SELF_HOSTED, base + CORESIGHT_CLAIMCLR);
+	isb();
+}
+
+/*
+ * coresight_claim_device_unlocked : Claim the device for self-hosted usage
+ * to prevent an external tool from touching this device. As per PSCI
+ * standards, section "Preserving the execution context" => "Debug and Trace
+ * save and Restore", DBGCLAIM[1] is reserved for Self-hosted debug/trace and
+ * DBGCLAIM[0] is reserved for external tools.
+ *
+ * Called with CS_UNLOCKed for the component.
+ * Returns : 0 on success
+ */
+int coresight_claim_device_unlocked(void __iomem *base)
+{
+	if (coresight_is_claimed_any(base))
+		return -EBUSY;
+
+	coresight_set_claim_tags(base);
+	if (coresight_is_claimed_self_hosted(base))
+		return 0;
+	/* There was a race setting the tags, clean up and fail */
+	coresight_clear_claim_tags(base);
+	return -EBUSY;
+}
+
+int coresight_claim_device(void __iomem *base)
+{
+	int rc;
+
+	CS_UNLOCK(base);
+	rc = coresight_claim_device_unlocked(base);
+	CS_LOCK(base);
+
+	return rc;
+}
+
+/*
+ * coresight_disclaim_device_unlocked : Clear the claim tags for the device.
+ * Called with CS_UNLOCKed for the component.
+ */
+void coresight_disclaim_device_unlocked(void __iomem *base)
+{
+
+	if (coresight_is_claimed_self_hosted(base))
+		coresight_clear_claim_tags(base);
+	else
+		/*
+		 * Either we or the external agent doesn't follow
+		 * the protocol.
+		 */
+		WARN_ON_ONCE(1);
+}
+
+void coresight_disclaim_device(void __iomem *base)
+{
+	CS_UNLOCK(base);
+	coresight_disclaim_device_unlocked(base);
+	CS_LOCK(base);
+}
+
 static int coresight_enable_sink(struct coresight_device *csdev,
 				 u32 mode, void *data)
 {
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index 5353582..16151a2 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -257,6 +257,13 @@ extern int coresight_enable(struct coresight_device *csdev);
 extern void coresight_disable(struct coresight_device *csdev);
 extern int coresight_timeout(void __iomem *addr, u32 offset,
 			     int position, int value);
+
+extern int coresight_claim_device(void __iomem *base);
+extern int coresight_claim_device_unlocked(void __iomem *base);
+
+extern void coresight_disclaim_device(void __iomem *base);
+extern void coresight_disclaim_device_unlocked(void __iomem *base);
+
 #else
 static inline struct coresight_device *
 coresight_register(struct coresight_desc *desc) { return NULL; }
@@ -266,6 +273,19 @@ coresight_enable(struct coresight_device *csdev) { return -ENOSYS; }
 static inline void coresight_disable(struct coresight_device *csdev) {}
 static inline int coresight_timeout(void __iomem *addr, u32 offset,
 				     int position, int value) { return 1; }
+static inline int coresight_claim_device_unlocked(void __iomem *base)
+{
+	return 0;
+}
+
+static inline int coresight_claim_device(void __iomem *base)
+{
+	return 0;
+}
+
+static inline void coresight_disclaim_device(void __iomem *base) {}
+static inline void coresight_disclaim_device_unlocked(void __iomem *base) {}
+
 #endif
 
 #ifdef CONFIG_OF
-- 
2.7.4

Re: [PATCH 08/13] coresight: Add support for CLAIM tag protocol

[Mathieu Poirier]

On Mon, Aug 06, 2018 at 02:41:50PM +0100, Suzuki K Poulose wrote:
> Add support for the CLAIM tag protocol for negotiating the
> device ownership with other agents trying to use the coresight
> component (internal vs. external). The Coresight architecture
> specifies CLAIM tags (managed via CLAIMSET CLAIMCLR registers)
> to negotiate the ownership of the device. PSCI recommends the
> reservation of the bits in CLAIM tags for self-hosted and external
> debug use. This patch implements the protocol for claiming
> the devices before they are actually used.

I think the first paragraph of the cover letter (minus the reference to the
documentation since you've included it below) would be perfect instead of the
above.

> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-priv.h |  7 +++
>  drivers/hwtracing/coresight/coresight.c      | 85 ++++++++++++++++++++++++++++
>  include/linux/coresight.h                    | 20 +++++++
>  3 files changed, 112 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index c11da55..579f349 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -25,6 +25,13 @@
>  #define CORESIGHT_DEVID		0xfc8
>  #define CORESIGHT_DEVTYPE	0xfcc
>  
> +
> +/*
> + * Coresight device CLAIM protocol.
> + * See PSCI - ARM DEN 0022D, Section: 6.8.1 Debug and Trace save and restore.
> + */
> +#define CORESIGHT_CLAIM_SELF_HOSTED	BIT(1)
> +
>  #define TIMEOUT_US		100
>  #define BMVAL(val, lsb, msb)	((val & GENMASK(msb, lsb)) >> lsb)
>  
> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> index e73ca6a..8f06866 100644
> --- a/drivers/hwtracing/coresight/coresight.c
> +++ b/drivers/hwtracing/coresight/coresight.c
> @@ -128,6 +128,91 @@ static int coresight_find_link_outport(struct coresight_device *csdev,
>  	return -ENODEV;
>  }
>  
> +static inline u32 coresight_read_claim_tags(void __iomem *base)
> +{
> +	return readl_relaxed(base + CORESIGHT_CLAIMCLR);
> +}
> +
> +static inline bool coresight_is_claimed_self_hosted(void __iomem *base)
> +{
> +	return coresight_read_claim_tags(base) == CORESIGHT_CLAIM_SELF_HOSTED;
> +}
> +
> +static inline bool coresight_is_claimed_any(void __iomem *base)
> +{
> +	return coresight_read_claim_tags(base) != 0;
> +}
> +
> +static inline void coresight_set_claim_tags(void __iomem *base)
> +{
> +	writel_relaxed(CORESIGHT_CLAIM_SELF_HOSTED, base + CORESIGHT_CLAIMSET);
> +	isb();
> +}
> +
> +static inline void coresight_clear_claim_tags(void __iomem *base)
> +{
> +	writel_relaxed(CORESIGHT_CLAIM_SELF_HOSTED, base + CORESIGHT_CLAIMCLR);
> +	isb();
> +}
> +
> +/*
> + * coresight_claim_device_unlocked : Claim the device for self-hosted usage
> + * to prevent an external tool from touching this device. As per PSCI
> + * standards, section "Preserving the execution context" => "Debug and Trace
> + * save and Restore", DBGCLAIM[1] is reserved for Self-hosted debug/trace and
> + * DBGCLAIM[0] is reserved for external tools.
> + *
> + * Called with CS_UNLOCKed for the component.
> + * Returns : 0 on success
> + */
> +int coresight_claim_device_unlocked(void __iomem *base)
> +{
> +	if (coresight_is_claimed_any(base))
> +		return -EBUSY;
> +
> +	coresight_set_claim_tags(base);
> +	if (coresight_is_claimed_self_hosted(base))
> +		return 0;
> +	/* There was a race setting the tags, clean up and fail */
> +	coresight_clear_claim_tags(base);
> +	return -EBUSY;
> +}
> +
> +int coresight_claim_device(void __iomem *base)
> +{
> +	int rc;
> +
> +	CS_UNLOCK(base);
> +	rc = coresight_claim_device_unlocked(base);
> +	CS_LOCK(base);
> +
> +	return rc;
> +}
> +
> +/*
> + * coresight_disclaim_device_unlocked : Clear the claim tags for the device.
> + * Called with CS_UNLOCKed for the component.
> + */
> +void coresight_disclaim_device_unlocked(void __iomem *base)
> +{
> +
> +	if (coresight_is_claimed_self_hosted(base))
> +		coresight_clear_claim_tags(base);
> +	else
> +		/*
> +		 * Either we or the external agent doesn't follow
> +		 * the protocol.
> +		 */
> +		WARN_ON_ONCE(1);

When writing "... or the external agent doesn't follow the protocol", I deduce
that an external agent would have trampled our claim tag.  I think this needs
to be said explicitly in the comment.

> +}
> +
> +void coresight_disclaim_device(void __iomem *base)
> +{
> +	CS_UNLOCK(base);
> +	coresight_disclaim_device_unlocked(base);
> +	CS_LOCK(base);
> +}
> +
>  static int coresight_enable_sink(struct coresight_device *csdev,
>  				 u32 mode, void *data)
>  {
> diff --git a/include/linux/coresight.h b/include/linux/coresight.h
> index 5353582..16151a2 100644
> --- a/include/linux/coresight.h
> +++ b/include/linux/coresight.h
> @@ -257,6 +257,13 @@ extern int coresight_enable(struct coresight_device *csdev);
>  extern void coresight_disable(struct coresight_device *csdev);
>  extern int coresight_timeout(void __iomem *addr, u32 offset,
>  			     int position, int value);
> +
> +extern int coresight_claim_device(void __iomem *base);
> +extern int coresight_claim_device_unlocked(void __iomem *base);
> +
> +extern void coresight_disclaim_device(void __iomem *base);
> +extern void coresight_disclaim_device_unlocked(void __iomem *base);
> +
>  #else
>  static inline struct coresight_device *
>  coresight_register(struct coresight_desc *desc) { return NULL; }
> @@ -266,6 +273,19 @@ coresight_enable(struct coresight_device *csdev) { return -ENOSYS; }
>  static inline void coresight_disable(struct coresight_device *csdev) {}
>  static inline int coresight_timeout(void __iomem *addr, u32 offset,
>  				     int position, int value) { return 1; }
> +static inline int coresight_claim_device_unlocked(void __iomem *base)
> +{
> +	return 0;
> +}
> +
> +static inline int coresight_claim_device(void __iomem *base)
> +{
> +	return 0;
> +}

Returning 0 would give a caller the impression the operation has succeeded when
in fact it didn't.  I think we should return an error code here.

> +
> +static inline void coresight_disclaim_device(void __iomem *base) {}
> +static inline void coresight_disclaim_device_unlocked(void __iomem *base) {}
> +
>  #endif
>  
>  #ifdef CONFIG_OF
> -- 
> 2.7.4
> 

Re: [PATCH 08/13] coresight: Add support for CLAIM tag protocol

[Suzuki K Poulose]

On 15/08/18 00:20, Mathieu Poirier wrote:
> On Mon, Aug 06, 2018 at 02:41:50PM +0100, Suzuki K Poulose wrote:
>> Add support for the CLAIM tag protocol for negotiating the
>> device ownership with other agents trying to use the coresight
>> component (internal vs. external). The Coresight architecture
>> specifies CLAIM tags (managed via CLAIMSET CLAIMCLR registers)
>> to negotiate the ownership of the device. PSCI recommends the
>> reservation of the bits in CLAIM tags for self-hosted and external
>> debug use. This patch implements the protocol for claiming
>> the devices before they are actually used.
> 
> I think the first paragraph of the cover letter (minus the reference to the
> documentation since you've included it below) would be perfect instead of the
> above.
> 
>>
>> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> ---
>>   drivers/hwtracing/coresight/coresight-priv.h |  7 +++
>>   drivers/hwtracing/coresight/coresight.c      | 85 ++++++++++++++++++++++++++++
>>   include/linux/coresight.h                    | 20 +++++++
>>   3 files changed, 112 insertions(+)

>> +void coresight_disclaim_device_unlocked(void __iomem *base)
>> +{
>> +
>> +	if (coresight_is_claimed_self_hosted(base))
>> +		coresight_clear_claim_tags(base);
>> +	else
>> +		/*
>> +		 * Either we or the external agent doesn't follow
>> +		 * the protocol.
>> +		 */
>> +		WARN_ON_ONCE(1);
> 
> When writing "... or the external agent doesn't follow the protocol", I deduce
> that an external agent would have trampled our claim tag.  I think this needs
> to be said explicitly in the comment.
> 

>> +static inline int coresight_claim_device_unlocked(void __iomem *base)
>> +{
>> +	return 0;
>> +}
>> +
>> +static inline int coresight_claim_device(void __iomem *base)
>> +{
>> +	return 0;
>> +}
> 
> Returning 0 would give a caller the impression the operation has succeeded when
> in fact it didn't.  I think we should return an error code here.


Agreed on all points, will respin.

Suzuki

[PATCH 09/13] coresight: etmx: Claim devices before use

[Suzuki K Poulose]

Use the CLAIM tags to grab the device for self-hosted usage.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-etm3x.c | 18 +++++++++++++++---
 drivers/hwtracing/coresight/coresight-etm4x.c | 15 ++++++++++++---
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
index 771691c..b810bbc 100644
--- a/drivers/hwtracing/coresight/coresight-etm3x.c
+++ b/drivers/hwtracing/coresight/coresight-etm3x.c
@@ -357,7 +357,7 @@ static int etm_parse_event_config(struct etm_drvdata *drvdata,
 
 static int etm_enable_hw(struct etm_drvdata *drvdata)
 {
-	int i;
+	int i, rc;
 	u32 etmcr;
 	struct etm_config *config = &drvdata->config;
 
@@ -369,6 +369,9 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
 	etm_set_pwrup(drvdata);
 	/* Make sure all registers are accessible */
 	etm_os_unlock(drvdata);
+	rc = coresight_claim_device_unlocked(drvdata->base);
+	if (rc)
+		goto done;
 
 	etm_set_prog(drvdata);
 
@@ -417,10 +420,15 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
 	etm_writel(drvdata, 0x0, ETMVMIDCVR);
 
 	etm_clr_prog(drvdata);
+
+done:
+	if (rc)
+		etm_set_pwrdwn(drvdata);
 	CS_LOCK(drvdata->base);
 
-	dev_dbg(drvdata->dev, "cpu: %d enable smp call done\n", drvdata->cpu);
-	return 0;
+	dev_dbg(drvdata->dev, "cpu: %d enable smp call done: %d\n",
+		drvdata->cpu, rc);
+	return rc;
 }
 
 struct etm_enable_arg {
@@ -561,6 +569,8 @@ static void etm_disable_hw(void *info)
 	struct etm_config *config = &drvdata->config;
 
 	CS_UNLOCK(drvdata->base);
+
+
 	etm_set_prog(drvdata);
 
 	/* Read back sequencer and counters for post trace analysis */
@@ -569,6 +579,8 @@ static void etm_disable_hw(void *info)
 	for (i = 0; i < drvdata->nr_cntr; i++)
 		config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i));
 
+	coresight_disclaim_device_unlocked(drvdata->base);
+
 	etm_set_pwrdwn(drvdata);
 	CS_LOCK(drvdata->base);
 
diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
index 4f9e6bb..20d1015 100644
--- a/drivers/hwtracing/coresight/coresight-etm4x.c
+++ b/drivers/hwtracing/coresight/coresight-etm4x.c
@@ -84,13 +84,17 @@ struct etm4_enable_arg {
 
 static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
 {
-	int i;
+	int i, rc;
 	struct etmv4_config *config = &drvdata->config;
 
 	CS_UNLOCK(drvdata->base);
 
 	etm4_os_unlock(drvdata);
 
+	rc = coresight_claim_device_unlocked(drvdata->base);
+	if (rc)
+		goto done;
+
 	/* Disable the trace unit before programming trace registers */
 	writel_relaxed(0, drvdata->base + TRCPRGCTLR);
 
@@ -178,10 +182,12 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
 		dev_err(drvdata->dev,
 			"timeout while waiting for Idle Trace Status\n");
 
+done:
 	CS_LOCK(drvdata->base);
 
-	dev_dbg(drvdata->dev, "cpu: %d enable smp call done\n", drvdata->cpu);
-	return 0;
+	dev_dbg(drvdata->dev, "cpu: %d enable smp call done: %d\n",
+		drvdata->cpu, rc);
+	return rc;
 }
 
 static void etm4_enable_hw_smp_call(void *info)
@@ -326,6 +332,7 @@ static void etm4_disable_hw(void *info)
 
 	CS_UNLOCK(drvdata->base);
 
+
 	/* power can be removed from the trace unit now */
 	control = readl_relaxed(drvdata->base + TRCPDCR);
 	control &= ~TRCPDCR_PU;
@@ -341,6 +348,8 @@ static void etm4_disable_hw(void *info)
 	isb();
 	writel_relaxed(control, drvdata->base + TRCPRGCTLR);
 
+	coresight_disclaim_device_unlocked(drvdata->base);
+
 	CS_LOCK(drvdata->base);
 
 	dev_dbg(drvdata->dev, "cpu: %d disable smp call done\n", drvdata->cpu);
-- 
2.7.4

Re: [PATCH 09/13] coresight: etmx: Claim devices before use

[Mathieu Poirier]

On Mon, Aug 06, 2018 at 02:41:51PM +0100, Suzuki K Poulose wrote:
> Use the CLAIM tags to grab the device for self-hosted usage.
> 
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
>  drivers/hwtracing/coresight/coresight-etm3x.c | 18 +++++++++++++++---
>  drivers/hwtracing/coresight/coresight-etm4x.c | 15 ++++++++++++---
>  2 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
> index 771691c..b810bbc 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
> @@ -357,7 +357,7 @@ static int etm_parse_event_config(struct etm_drvdata *drvdata,
>  
>  static int etm_enable_hw(struct etm_drvdata *drvdata)
>  {
> -	int i;
> +	int i, rc;
>  	u32 etmcr;
>  	struct etm_config *config = &drvdata->config;
>  
> @@ -369,6 +369,9 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
>  	etm_set_pwrup(drvdata);
>  	/* Make sure all registers are accessible */
>  	etm_os_unlock(drvdata);
> +	rc = coresight_claim_device_unlocked(drvdata->base);
> +	if (rc)
> +		goto done;
>  
>  	etm_set_prog(drvdata);
>  
> @@ -417,10 +420,15 @@ static int etm_enable_hw(struct etm_drvdata *drvdata)
>  	etm_writel(drvdata, 0x0, ETMVMIDCVR);
>  
>  	etm_clr_prog(drvdata);
> +
> +done:
> +	if (rc)
> +		etm_set_pwrdwn(drvdata);
>  	CS_LOCK(drvdata->base);
>  
> -	dev_dbg(drvdata->dev, "cpu: %d enable smp call done\n", drvdata->cpu);
> -	return 0;
> +	dev_dbg(drvdata->dev, "cpu: %d enable smp call done: %d\n",
> +		drvdata->cpu, rc);
> +	return rc;
>  }
>  
>  struct etm_enable_arg {
> @@ -561,6 +569,8 @@ static void etm_disable_hw(void *info)
>  	struct etm_config *config = &drvdata->config;
>  
>  	CS_UNLOCK(drvdata->base);
> +
> +

This is extra, please remove.

>  	etm_set_prog(drvdata);
>  
>  	/* Read back sequencer and counters for post trace analysis */
> @@ -569,6 +579,8 @@ static void etm_disable_hw(void *info)
>  	for (i = 0; i < drvdata->nr_cntr; i++)
>  		config->cntr_val[i] = etm_readl(drvdata, ETMCNTVRn(i));
>  
> +	coresight_disclaim_device_unlocked(drvdata->base);
> +
>  	etm_set_pwrdwn(drvdata);
>  	CS_LOCK(drvdata->base);
>  
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> index 4f9e6bb..20d1015 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> @@ -84,13 +84,17 @@ struct etm4_enable_arg {
>  
>  static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>  {
> -	int i;
> +	int i, rc;
>  	struct etmv4_config *config = &drvdata->config;
>  
>  	CS_UNLOCK(drvdata->base);
>  
>  	etm4_os_unlock(drvdata);
>  
> +	rc = coresight_claim_device_unlocked(drvdata->base);
> +	if (rc)
> +		goto done;
> +
>  	/* Disable the trace unit before programming trace registers */
>  	writel_relaxed(0, drvdata->base + TRCPRGCTLR);
>  
> @@ -178,10 +182,12 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata)
>  		dev_err(drvdata->dev,
>  			"timeout while waiting for Idle Trace Status\n");
>  
> +done:
>  	CS_LOCK(drvdata->base);
>  
> -	dev_dbg(drvdata->dev, "cpu: %d enable smp call done\n", drvdata->cpu);
> -	return 0;
> +	dev_dbg(drvdata->dev, "cpu: %d enable smp call done: %d\n",
> +		drvdata->cpu, rc);
> +	return rc;
>  }
>  
>  static void etm4_enable_hw_smp_call(void *info)
> @@ -326,6 +332,7 @@ static void etm4_disable_hw(void *info)
>  
>  	CS_UNLOCK(drvdata->base);
>  
> +

Same here.

>  	/* power can be removed from the trace unit now */
>  	control = readl_relaxed(drvdata->base + TRCPDCR);
>  	control &= ~TRCPDCR_PU;
> @@ -341,6 +348,8 @@ static void etm4_disable_hw(void *info)
>  	isb();
>  	writel_relaxed(control, drvdata->base + TRCPRGCTLR);
>  
> +	coresight_disclaim_device_unlocked(drvdata->base);
> +
>  	CS_LOCK(drvdata->base);
>  
>  	dev_dbg(drvdata->dev, "cpu: %d disable smp call done\n", drvdata->cpu);
> -- 
> 2.7.4
> 

[PATCH 10/13] coresight: funnel: Claim devices before use

[Suzuki K Poulose]

Use the CLAIM protocol to grab the ownership of the component.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-funnel.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
index ee7a30b..9279251 100644
--- a/drivers/hwtracing/coresight/coresight-funnel.c
+++ b/drivers/hwtracing/coresight/coresight-funnel.c
@@ -25,6 +25,7 @@
 #define FUNNEL_HOLDTIME_MASK	0xf00
 #define FUNNEL_HOLDTIME_SHFT	0x8
 #define FUNNEL_HOLDTIME		(0x7 << FUNNEL_HOLDTIME_SHFT)
+#define FUNNEL_ENSx_MASK	0xff
 
 /**
  * struct funnel_drvdata - specifics associated to a funnel component
@@ -42,31 +43,42 @@ struct funnel_drvdata {
 	unsigned long		priority;
 };
 
-static void funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
+static int funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
 {
 	u32 functl;
+	int rc = 0;
 
 	CS_UNLOCK(drvdata->base);
 
 	functl = readl_relaxed(drvdata->base + FUNNEL_FUNCTL);
+	/* Claim the device only when we enable the first slave */
+	if (!(functl & FUNNEL_ENSx_MASK)) {
+		rc = coresight_claim_device_unlocked(drvdata->base);
+		if (rc)
+			goto done;
+	}
+
 	functl &= ~FUNNEL_HOLDTIME_MASK;
 	functl |= FUNNEL_HOLDTIME;
 	functl |= (1 << port);
 	writel_relaxed(functl, drvdata->base + FUNNEL_FUNCTL);
 	writel_relaxed(drvdata->priority, drvdata->base + FUNNEL_PRICTL);
-
+done:
 	CS_LOCK(drvdata->base);
+	return rc;
 }
 
 static int funnel_enable(struct coresight_device *csdev, int inport,
 			 int outport)
 {
+	int rc;
 	struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
 
-	funnel_enable_hw(drvdata, inport);
+	rc = funnel_enable_hw(drvdata, inport);
 
-	dev_dbg(drvdata->dev, "FUNNEL inport %d enabled\n", inport);
-	return 0;
+	if (!rc)
+		dev_dbg(drvdata->dev, "FUNNEL inport %d enabled\n", inport);
+	return rc;
 }
 
 static void funnel_disable_hw(struct funnel_drvdata *drvdata, int inport)
@@ -79,6 +91,10 @@ static void funnel_disable_hw(struct funnel_drvdata *drvdata, int inport)
 	functl &= ~(1 << inport);
 	writel_relaxed(functl, drvdata->base + FUNNEL_FUNCTL);
 
+	/* Disclaim the device if none of the slaves are now active */
+	if (!(functl & FUNNEL_ENSx_MASK))
+		coresight_disclaim_device_unlocked(drvdata->base);
+
 	CS_LOCK(drvdata->base);
 }
 
-- 
2.7.4

[PATCH 11/13] coresight: catu: Claim device before use

[Suzuki K Poulose]

Use the CLAIM protocol to grab the ownership of the component when
in use.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-catu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-catu.c b/drivers/hwtracing/coresight/coresight-catu.c
index ff94e58..170fbb6 100644
--- a/drivers/hwtracing/coresight/coresight-catu.c
+++ b/drivers/hwtracing/coresight/coresight-catu.c
@@ -406,6 +406,7 @@ static inline int catu_wait_for_ready(struct catu_drvdata *drvdata)
 
 static int catu_enable_hw(struct catu_drvdata *drvdata, void *data)
 {
+	int rc;
 	u32 control, mode;
 	struct etr_buf *etr_buf = data;
 
@@ -418,6 +419,10 @@ static int catu_enable_hw(struct catu_drvdata *drvdata, void *data)
 		return -EBUSY;
 	}
 
+	rc = coresight_claim_device_unlocked(drvdata->base);
+	if (rc)
+		return rc;
+
 	control |= BIT(CATU_CONTROL_ENABLE);
 
 	if (etr_buf && etr_buf->mode == ETR_MODE_CATU) {
@@ -459,6 +464,7 @@ static int catu_disable_hw(struct catu_drvdata *drvdata)
 	int rc = 0;
 
 	catu_write_control(drvdata, 0);
+	coresight_disclaim_device_unlocked(drvdata->base);
 	if (catu_wait_for_ready(drvdata)) {
 		dev_info(drvdata->dev, "Timeout while waiting for READY\n");
 		rc = -EAGAIN;
-- 
2.7.4

[PATCH 12/13] coresight: dynamic-replicator: Claim device for use

[Suzuki K Poulose]

Use CLAIM protocol to make sure the device is available for use.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 .../coresight/coresight-dynamic-replicator.c       | 23 +++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
index 97f4673..299667b 100644
--- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
+++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
@@ -41,8 +41,11 @@ static void replicator_reset(struct replicator_state *drvdata)
 {
 	CS_UNLOCK(drvdata->base);
 
-	writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER0);
-	writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER1);
+	if (!coresight_claim_device_unlocked(drvdata->base)) {
+		writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER0);
+		writel_relaxed(0xff, drvdata->base + REPLICATOR_IDFILTER1);
+		coresight_disclaim_device_unlocked(drvdata->base);
+	}
 
 	CS_LOCK(drvdata->base);
 }
@@ -50,6 +53,7 @@ static void replicator_reset(struct replicator_state *drvdata)
 static int replicator_enable(struct coresight_device *csdev, int inport,
 			      int outport)
 {
+	int rc = 0;
 	u32 reg;
 	struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent);
 
@@ -67,13 +71,19 @@ static int replicator_enable(struct coresight_device *csdev, int inport,
 
 	CS_UNLOCK(drvdata->base);
 
+	if ((readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0) == 0xff) &&
+	    (readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1) == 0xff))
+		rc = coresight_claim_device_unlocked(drvdata->base);
 
 	/* Ensure that the outport is enabled. */
-	writel_relaxed(0x00, drvdata->base + reg);
+	if (!rc) {
+		writel_relaxed(0x00, drvdata->base + reg);
+		dev_dbg(drvdata->dev, "REPLICATOR enabled\n");
+	}
+
 	CS_LOCK(drvdata->base);
 
-	dev_dbg(drvdata->dev, "REPLICATOR enabled\n");
-	return 0;
+	return rc;
 }
 
 static void replicator_disable(struct coresight_device *csdev, int inport,
@@ -99,6 +109,9 @@ static void replicator_disable(struct coresight_device *csdev, int inport,
 	/* disable the flow of ATB data through port */
 	writel_relaxed(0xff, drvdata->base + reg);
 
+	if ((readl_relaxed(drvdata->base + REPLICATOR_IDFILTER0) == 0xff) &&
+	    (readl_relaxed(drvdata->base + REPLICATOR_IDFILTER1) == 0xff))
+		coresight_disclaim_device_unlocked(drvdata->base);
 	CS_LOCK(drvdata->base);
 
 	dev_dbg(drvdata->dev, "REPLICATOR disabled\n");
-- 
2.7.4

[PATCH 13/13] coreisght: tmc: Claim device before use

[Suzuki K Poulose]

Use CLAIM tags to make sure the device is available for use.

Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 drivers/hwtracing/coresight/coresight-tmc-etf.c | 22 +++++++++++++++++++---
 drivers/hwtracing/coresight/coresight-tmc-etr.c |  4 ++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index ceb4b30..59d862d6e 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -36,6 +36,11 @@ static void __tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
 
 static int tmc_etb_enable_hw(struct tmc_drvdata *drvdata)
 {
+	int rc = coresight_claim_device(drvdata->base);
+
+	if (rc)
+		return rc;
+
 	__tmc_etb_enable_hw(drvdata);
 	return 0;
 }
@@ -66,7 +71,7 @@ static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata)
 	return;
 }
 
-static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
+static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
 {
 	CS_UNLOCK(drvdata->base);
 
@@ -82,6 +87,12 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
 	CS_LOCK(drvdata->base);
 }
 
+static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
+{
+	coresight_disclaim_device(drvdata);
+	__tmc_etb_disable_hw(drvdata);
+}
+
 static void __tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
 {
 	CS_UNLOCK(drvdata->base);
@@ -100,6 +111,11 @@ static void __tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
 
 static int tmc_etf_enable_hw(struct tmc_drvdata *drvdata)
 {
+	int rc = coresight_claim_device(drvdata->base);
+
+	if (rc)
+		return rc;
+
 	__tmc_etf_enable_hw(drvdata);
 	return 0;
 }
@@ -110,7 +126,7 @@ static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
 
 	tmc_flush_and_stop(drvdata);
 	tmc_disable_hw(drvdata);
-
+	coresight_disclaim_device_unlocked(drvdata->base);
 	CS_LOCK(drvdata->base);
 }
 
@@ -556,7 +572,7 @@ int tmc_read_prepare_etb(struct tmc_drvdata *drvdata)
 
 	/* Disable the TMC if need be */
 	if (drvdata->mode == CS_MODE_SYSFS)
-		tmc_etb_disable_hw(drvdata);
+		__tmc_etb_disable_hw(drvdata);
 
 	drvdata->reading = true;
 out:
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index daad521..f684283 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -991,6 +991,9 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
 	 * this on.
 	 */
 	rc = tmc_etr_enable_catu(drvdata, etr_buf);
+	if (rc)
+		return rc;
+	rc = coresight_claim_device(drvdata->base);
 	if (!rc) {
 		drvdata->etr_buf = etr_buf;
 		__tmc_etr_enable_hw(drvdata);
@@ -1077,6 +1080,7 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
 	__tmc_etr_disable_hw(drvdata);
 	/* Disable CATU device if this ETR is connected to one */
 	tmc_etr_disable_catu(drvdata);
+	coresight_disclaim_device(drvdata->base);
 	/* Reset the ETR buf used by hardware */
 	drvdata->etr_buf = NULL;
 }
-- 
2.7.4