Re: [PATCH] coresight: tmc: fix failure to disable/enable ETF after reading

Hi,

On Wed, 23 Apr 2025 at 11:37, Jie Gan jie.gan@oss.qualcomm.com wrote:

From: Mao Jinlong quic_jinlmao@quicinc.com

From: Mao Jinlong quic_jinlmao@quicinc.com

What are these extra email addresses for?

ETF may fail to re-enable after reading, and driver->reading will not be set to false, this will cause failure to enable/disable to ETF. This change set driver->reading to false even if re-enabling fail.

Fixes: 669c4614236a7 ("coresight: tmc: Don't enable TMC when it's not ready.")

This SHA and message appear not be present in any upstream / coresight branch.

Co-developed-by: Yuanfang Zhang quic_yuanfang@quicinc.com Signed-off-by: Yuanfang Zhang quic_yuanfang@quicinc.com Signed-off-by: Mao Jinlong quic_jinlmao@quicinc.com Signed-off-by: Jie Gan jie.gan@oss.qualcomm.com

---

drivers/hwtracing/coresight/coresight-tmc-etf.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c index d858740001c2..8c9f14e36bc2 100644 --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c @@ -87,6 +87,12 @@ static void __tmc_etb_disable_hw(struct tmc_drvdata *drvdata) { CS_UNLOCK(drvdata->base);

•   /* Check if the etf already disabled*/
  

•   if (!(readl_relaxed(drvdata->base + TMC_CTL) & TMC_CTL_CAPT_EN)) {
  

•           CS_LOCK(drvdata->base);
  

•           return;
  

•   }
  

What does this have to do with the stated function of the patch - this is unnecessary. Under what scenario can this function be called with the ETB previously disabled?

    tmc_flush_and_stop(drvdata);
    /*
     * When operating in sysFS mode the content of the buffer needs to be

@@ -747,7 +753,6 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata) char *buf = NULL; enum tmc_mode mode; unsigned long flags;

•   int rc = 0;
  
    /* config types are set a boot time and never change */
    if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETB &&
  

@@ -773,11 +778,7 @@ int tmc_read_unprepare_etb(struct tmc_drvdata *drvdata) * can't be NULL. */ memset(drvdata->buf, 0, drvdata->size);

•           rc = __tmc_etb_enable_hw(drvdata);
  

•           if (rc) {
  

•                   raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
  

•                   return rc;
  

•           }
  

•           __tmc_etb_enable_hw(drvdata);
  

Dropping a valid error check is not acceptable. If a TMC cannot be re-enabled, then that is a hardware error that needs noting and dealing with.

Regards

Mike

    } else {
            /*
             * The ETB/ETF is not tracing and the buffer was just read.

-- 2.34.1