Extend logging on TMC start / stop errors
Recent issues with trying to debug TMC timeouts and flush issues shows a general lack of logging and context around the possible errors
Add logging to general wait for stop coresight routines and return values of watched registers.
Update TMC to use this logging.
Changes since v1: Rebase to coresight/next (kernel 6.13-rc2)
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 | 2 + 6 files changed, 86 insertions(+), 25 deletions(-)
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 mike.leach@linaro.org --- drivers/hwtracing/coresight/coresight-core.c | 50 +++++++++++++++----- include/linux/coresight.h | 2 + 2 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index ea38ecf26fcb..feb1a1db355f 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1017,32 +1017,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; + } }
/* @@ -1054,7 +1059,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 055ce5cd5c44..29cb71e82b0b 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -639,6 +639,8 @@ extern int coresight_enable_sysfs(struct coresight_device *csdev); extern void coresight_disable_sysfs(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);
Hi Mike
On 13/12/2024 14:49, Mike Leach wrote:
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.
The patch looks good to me. However, this change doesn't bring much value, other than avoiding a read at the caller side. I guess it is simpler to leave things as is and let the caller read the register value while logging rather than providing another variant.
I am inclined to keep the interface simpler.
Signed-off-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-core.c | 50 +++++++++++++++----- include/linux/coresight.h | 2 + 2 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index ea38ecf26fcb..feb1a1db355f 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1017,32 +1017,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;}
if (!(val & BIT(position)))return 0;
if (!(val & BIT(position))) {rc = 0;goto return_rval;}/* @@ -1054,7 +1059,30 @@ int coresight_timeout(struct csdev_access *csa, u32 offset, udelay(1); }
- return -EAGAIN;
+return_rval:
- *rval = val;
super minor nit: we could be flexible in accepting a "NULL" rval and thus keep the normal variants pass the NULL straight in. i.e,
if (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);
return core_timeout_retval(csa, offset, position, value, NULL);
Suzuki
} EXPORT_SYMBOL_GPL(coresight_timeout); diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 055ce5cd5c44..29cb71e82b0b 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -639,6 +639,8 @@ extern int coresight_enable_sysfs(struct coresight_device *csdev); extern void coresight_disable_sysfs(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);
Hi Suzuki,
On Fri, 13 Dec 2024 at 16:38, Suzuki K Poulose suzuki.poulose@arm.com wrote:
Hi Mike
On 13/12/2024 14:49, Mike Leach wrote:
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.
The patch looks good to me. However, this change doesn't bring much value, other than avoiding a read at the caller side. I guess it is simpler to leave things as is and let the caller read the register value while logging rather than providing another variant.
The reason for returning the register value is that you get the exact read value used at the time of the test - not potentially a different value read later.
I am inclined to keep the interface simpler.
Signed-off-by: Mike Leach mike.leach@linaro.org
drivers/hwtracing/coresight/coresight-core.c | 50 +++++++++++++++----- include/linux/coresight.h | 2 + 2 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-core.c b/drivers/hwtracing/coresight/coresight-core.c index ea38ecf26fcb..feb1a1db355f 100644 --- a/drivers/hwtracing/coresight/coresight-core.c +++ b/drivers/hwtracing/coresight/coresight-core.c @@ -1017,32 +1017,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;} } /*@@ -1054,7 +1059,30 @@ int coresight_timeout(struct csdev_access *csa, u32 offset, udelay(1); }
return -EAGAIN;+return_rval:
*rval = val;super minor nit: we could be flexible in accepting a "NULL" rval and thus keep the normal variants pass the NULL straight in. i.e,
if (rval) *rval = val;
Sure.
Mike
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);return core_timeout_retval(csa, offset, position, value, NULL);Suzuki
} EXPORT_SYMBOL_GPL(coresight_timeout);
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 055ce5cd5c44..29cb71e82b0b 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -639,6 +639,8 @@ extern int coresight_enable_sysfs(struct coresight_device *csdev); extern void coresight_disable_sysfs(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);
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 mike.leach@linaro.org --- .../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 e9876252a789..1a9299adae93 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -34,25 +34,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; @@ -60,12 +71,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 2671926be62a..34513f07c3aa 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -259,7 +259,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);
On 13/12/2024 14:49, Mike Leach wrote:
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 mike.leach@linaro.org
.../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 e9876252a789..1a9299adae93 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -34,25 +34,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);-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; @@ -60,12 +71,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);
"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; }
All of this looks good to me. Do we need to move the MemErr check post the flush ? There is a potential chance of hitting a MemERR on flushing and we could miss reproting those ones ?
Suzuki
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 2671926be62a..34513f07c3aa 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -259,7 +259,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);
Hi Suzuki,
On Mon, 16 Dec 2024 at 09:33, Suzuki K Poulose suzuki.poulose@arm.com wrote:
On 13/12/2024 14:49, Mike Leach wrote:
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 mike.leach@linaro.org
.../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 e9876252a789..1a9299adae93 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-core.c +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c @@ -34,25 +34,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;@@ -60,12 +71,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;All of this looks good to me. Do we need to move the MemErr check post the flush ? There is a potential chance of hitting a MemERR on flushing and we could miss reproting those ones ?
We need the one before - if there is already a Memerr then this will affect the flush operation - on Memerr the formatter will have stopped etc. But no harm in putting another check afterwards.
Mike
Suzuki
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 2671926be62a..34513f07c3aa 100644 --- a/drivers/hwtracing/coresight/coresight-tmc.h +++ b/drivers/hwtracing/coresight/coresight-tmc.h @@ -259,7 +259,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);
Insert additional context around tmc_flush_and_stop() errors.
Signed-off-by: Mike Leach mike.leach@linaro.org --- 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 d4f641cd9de6..62b4b685c1a1 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); @@ -496,7 +500,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 a48bb85d0e7f..122a067d1bb8 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c @@ -1139,7 +1139,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. @@ -1578,7 +1580,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);
-
Mike Leach -
Suzuki K Poulose