Re: [PATCH v2 2/3] coresight: tmc: Update error logging in tmc common functions

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 ?

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);