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

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