Recent issues with trying to debug TMC timeouts and flush issues shows
a general lack of logging and context around the possible errors
Patchset addresses that.
Signed-off-by: Mike Leach <[email protected]>
Mike Leach (3):
coresight: Update timeout functions to allow return of test register
value
coresight: tmc: Update error logging in tmc common functions
coresight: etf: etr: Update logging around flush_and_stop() errors
drivers/hwtracing/coresight/coresight-core.c | 50 +++++++++++++++----
.../hwtracing/coresight/coresight-tmc-core.c | 37 +++++++++++---
.../hwtracing/coresight/coresight-tmc-etf.c | 12 +++--
.../hwtracing/coresight/coresight-tmc-etr.c | 8 ++-
drivers/hwtracing/coresight/coresight-tmc.h | 2 +-
include/linux/coresight.h | 10 +++-
6 files changed, 93 insertions(+), 26 deletions(-)
--
2.17.1
Current coresight_timeout function spins on a bit on a test register,
till bit value achieved or timeout hit.
Add another function to return the full value of the register being
tested.
Signed-off-by: Mike Leach <[email protected]>
---
drivers/hwtracing/coresight/coresight-core.c | 50 +++++++++++++++-----
include/linux/coresight.h | 10 +++-
2 files changed, 48 insertions(+), 12 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c
index d3bf82c0de1d..c4db111ab32b 100644
--- a/drivers/hwtracing/coresight/coresight-core.c
+++ b/drivers/hwtracing/coresight/coresight-core.c
@@ -1456,32 +1456,37 @@ static void coresight_remove_conns(struct coresight_device *csdev)
}
/**
- * coresight_timeout - loop until a bit has changed to a specific register
- * state.
+ * coresight_timeout_retval - loop until a bit has changed to a specific register
+ * state. Return final register value
* @csa: coresight device access for the device
* @offset: Offset of the register from the base of the device.
* @position: the position of the bit of interest.
* @value: the value the bit should have.
+ * @rval: the last read value of the register being tested.
*
* Return: 0 as soon as the bit has taken the desired state or -EAGAIN if
* TIMEOUT_US has elapsed, which ever happens first.
*/
-int coresight_timeout(struct csdev_access *csa, u32 offset,
- int position, int value)
+int coresight_timeout_retval(struct csdev_access *csa, u32 offset,
+ int position, int value, u32 *rval)
{
- int i;
- u32 val;
+ int i, rc = -EAGAIN;
+ u32 val = 0;
for (i = TIMEOUT_US; i > 0; i--) {
val = csdev_access_read32(csa, offset);
/* waiting on the bit to go from 0 to 1 */
if (value) {
- if (val & BIT(position))
- return 0;
+ if (val & BIT(position)) {
+ rc = 0;
+ goto return_rval;
+ }
/* waiting on the bit to go from 1 to 0 */
} else {
- if (!(val & BIT(position)))
- return 0;
+ if (!(val & BIT(position))) {
+ rc = 0;
+ goto return_rval;
+ }
}
/*
@@ -1493,7 +1498,30 @@ int coresight_timeout(struct csdev_access *csa, u32 offset,
udelay(1);
}
- return -EAGAIN;
+return_rval:
+ *rval = val;
+
+ return rc;
+}
+EXPORT_SYMBOL_GPL(coresight_timeout_retval);
+
+/**
+ * coresight_timeout - loop until a bit has changed to a specific register
+ * state
+ * @csa: coresight device access for the device
+ * @offset: Offset of the register from the base of the device.
+ * @position: the position of the bit of interest.
+ * @value: the value the bit should have.
+ *
+ * Return: 0 as soon as the bit has taken the desired state or -EAGAIN if
+ * TIMEOUT_US has elapsed, which ever happens first.
+ */
+int coresight_timeout(struct csdev_access *csa, u32 offset,
+ int position, int value)
+{
+ u32 rval = 0;
+
+ return coresight_timeout_retval(csa, offset, position, value, &rval);
}
EXPORT_SYMBOL_GPL(coresight_timeout);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h
index f19a47b9bb5a..6b6b45ef6971 100644
--- a/include/linux/coresight.h
+++ b/include/linux/coresight.h
@@ -500,7 +500,8 @@ extern int coresight_enable(struct coresight_device *csdev);
extern void coresight_disable(struct coresight_device *csdev);
extern int coresight_timeout(struct csdev_access *csa, u32 offset,
int position, int value);
-
+extern int coresight_timeout_retval(struct csdev_access *csa, u32 offset,
+ int position, int value, u32 *rval);
extern int coresight_claim_device(struct coresight_device *csdev);
extern int coresight_claim_device_unlocked(struct coresight_device *csdev);
@@ -536,6 +537,13 @@ static inline int coresight_timeout(struct csdev_access *csa, u32 offset,
return 1;
}
+static inline int coresight_timeout_retval(struct csdev_access *csa, u32 offset,
+ int position, int value, u32 *rval)
+{
+ *rval = 0;
+ return 1;
+}
+
static inline int coresight_claim_device_unlocked(struct coresight_device *csdev)
{
return -EINVAL;
--
2.17.1
Enhance the error logging in the tmc_wait_for_tmcready() and
tmc_flush_and_stop() to print key tmc register values on error
conditions to improve hardware debug information.
Signed-off-by: Mike Leach <[email protected]>
---
.../hwtracing/coresight/coresight-tmc-core.c | 37 +++++++++++++++----
drivers/hwtracing/coresight/coresight-tmc.h | 2 +-
2 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
index c106d142e632..f048f450843d 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-core.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
@@ -31,25 +31,36 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb");
DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf");
DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr");
+#define TMC_WAIT_READY_FMT_STR "timeout while waiting for TMC to be Ready [STS=0x%04x]\n"
+
int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata)
{
struct coresight_device *csdev = drvdata->csdev;
struct csdev_access *csa = &csdev->access;
+ u32 tmc_sts = 0;
/* Ensure formatter, unformatter and hardware fifo are empty */
- if (coresight_timeout(csa, TMC_STS, TMC_STS_TMCREADY_BIT, 1)) {
- dev_err(&csdev->dev,
- "timeout while waiting for TMC to be Ready\n");
+ if (coresight_timeout_retval(csa, TMC_STS, TMC_STS_TMCREADY_BIT, 1,
+ &tmc_sts)) {
+ dev_err(&csdev->dev, TMC_WAIT_READY_FMT_STR, tmc_sts);
return -EBUSY;
}
return 0;
}
-void tmc_flush_and_stop(struct tmc_drvdata *drvdata)
+int tmc_flush_and_stop(struct tmc_drvdata *drvdata)
{
struct coresight_device *csdev = drvdata->csdev;
struct csdev_access *csa = &csdev->access;
- u32 ffcr;
+ u32 ffcr, ffsr, tmc_sts;
+ int rc = 0;
+
+ /* note any MemErr present when stopping TMC */
+ tmc_sts = readl_relaxed(drvdata->base + TMC_STS);
+ if (tmc_sts & TMC_STS_MEMERR)
+ dev_err(&csdev->dev,
+ "MemErr detected before Manual Flush; STS[0x%02x]\n",
+ tmc_sts);
ffcr = readl_relaxed(drvdata->base + TMC_FFCR);
ffcr |= TMC_FFCR_STOP_ON_FLUSH;
@@ -57,12 +68,22 @@ void tmc_flush_and_stop(struct tmc_drvdata *drvdata)
ffcr |= BIT(TMC_FFCR_FLUSHMAN_BIT);
writel_relaxed(ffcr, drvdata->base + TMC_FFCR);
/* Ensure flush completes */
- if (coresight_timeout(csa, TMC_FFCR, TMC_FFCR_FLUSHMAN_BIT, 0)) {
+ if (coresight_timeout_retval(csa, TMC_FFCR, TMC_FFCR_FLUSHMAN_BIT, 0,
+ &ffcr)) {
+ ffsr = readl_relaxed(drvdata->base + TMC_FFSR);
dev_err(&csdev->dev,
- "timeout while waiting for completion of Manual Flush\n");
+ "timeout while waiting for completion of Manual Flush\n");
+ dev_err(&csdev->dev,
+ "regs: FFCR[0x%02x] FFSR[0x%02x] STS[0x%02x]\n",
+ ffcr, ffsr, tmc_sts);
+ rc = -EBUSY;
}
- tmc_wait_for_tmcready(drvdata);
+ if (tmc_wait_for_tmcready(drvdata)) {
+ dev_err(&csdev->dev, "TMC ready error after Manual flush\n");
+ rc = -EBUSY;
+ }
+ return rc;
}
void tmc_enable_hw(struct tmc_drvdata *drvdata)
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 01c0382a29c0..314f8244787f 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -256,7 +256,7 @@ struct tmc_sg_table {
/* Generic functions */
int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata);
-void tmc_flush_and_stop(struct tmc_drvdata *drvdata);
+int tmc_flush_and_stop(struct tmc_drvdata *drvdata);
void tmc_enable_hw(struct tmc_drvdata *drvdata);
void tmc_disable_hw(struct tmc_drvdata *drvdata);
u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata);
--
2.17.1
Insert additional context around tmc_flush_and_stop() errors.
Signed-off-by: Mike Leach <[email protected]>
---
drivers/hwtracing/coresight/coresight-tmc-etf.c | 12 +++++++++---
drivers/hwtracing/coresight/coresight-tmc-etr.c | 8 ++++++--
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 0ab1f73c2d06..c8a4d4eff64f 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -84,7 +84,9 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
{
CS_UNLOCK(drvdata->base);
- tmc_flush_and_stop(drvdata);
+ if (tmc_flush_and_stop(drvdata))
+ dev_err(&drvdata->csdev->dev,
+ "Flush and stop error disabling ETB\n");
/*
* When operating in sysFS mode the content of the buffer needs to be
* read before the TMC is disabled.
@@ -146,7 +148,9 @@ static void tmc_etf_disable_hw(struct tmc_drvdata *drvdata)
CS_UNLOCK(drvdata->base);
- tmc_flush_and_stop(drvdata);
+ if (tmc_flush_and_stop(drvdata))
+ dev_err(&drvdata->csdev->dev,
+ "Flush and stop error disabling ETF\n");
tmc_disable_hw(drvdata);
coresight_disclaim_device_unlocked(csdev);
CS_LOCK(drvdata->base);
@@ -492,7 +496,9 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev,
CS_UNLOCK(drvdata->base);
- tmc_flush_and_stop(drvdata);
+ if (tmc_flush_and_stop(drvdata))
+ dev_err(&drvdata->csdev->dev,
+ "Flush and stop error updating perf buffer\n");
read_ptr = tmc_read_rrp(drvdata);
write_ptr = tmc_read_rwp(drvdata);
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 918d461fcf4a..ceae6a093612 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -1145,7 +1145,9 @@ static void __tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
{
CS_UNLOCK(drvdata->base);
- tmc_flush_and_stop(drvdata);
+ if (tmc_flush_and_stop(drvdata))
+ dev_err(&drvdata->csdev->dev,
+ "Flush and stop error disabling ETR\n");
/*
* When operating in sysFS mode the content of the buffer needs to be
* read before the TMC is disabled.
@@ -1548,7 +1550,9 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
CS_UNLOCK(drvdata->base);
- tmc_flush_and_stop(drvdata);
+ if (tmc_flush_and_stop(drvdata))
+ dev_err(&csdev->dev,
+ "Flush and Stop error updating perf buffer\n");
tmc_sync_etr_buf(drvdata);
CS_LOCK(drvdata->base);
--
2.17.1
On 10/02/2023 15:10, Mike Leach wrote:
> Recent issues with trying to debug TMC timeouts and flush issues shows
> a general lack of logging and context around the possible errors
>
> Patchset addresses that.
>
> Signed-off-by: Mike Leach <[email protected]>
>
> Mike Leach (3):
> coresight: Update timeout functions to allow return of test register
> value
> coresight: tmc: Update error logging in tmc common functions
> coresight: etf: etr: Update logging around flush_and_stop() errors
>
> drivers/hwtracing/coresight/coresight-core.c | 50 +++++++++++++++----
> .../hwtracing/coresight/coresight-tmc-core.c | 37 +++++++++++---
> .../hwtracing/coresight/coresight-tmc-etf.c | 12 +++--
> .../hwtracing/coresight/coresight-tmc-etr.c | 8 ++-
> drivers/hwtracing/coresight/coresight-tmc.h | 2 +-
> include/linux/coresight.h | 10 +++-
> 6 files changed, 93 insertions(+), 26 deletions(-)
>
Looks good to me.
Reviewed-by: James Clark <[email protected]>
At some point I might go and add some TRACE_EVENT(coresight_...)s around
things like enabling and disabling devices. It might help to see what
led up to an error rather than just the error itself.