Re: [PATCH] coresight: tmc-etr: Don't enable ETR when it's not ready

[James Clark]

On 09/01/2023 23:43, Yabin Cui wrote:
> Otherwise, it may cause error in AXI bus and result in a kernel panic.

Hi Yabin,

Thanks for the fix. Do you have a reproducer for this or some more info?
For example is it a regression or has it always been there? And on which
platform.

Thanks
James

> 
> Signed-off-by: Yabin Cui <yabinc@xxxxxxxxxx>
> ---
>  .../hwtracing/coresight/coresight-tmc-core.c   |  4 +++-
>  .../hwtracing/coresight/coresight-tmc-etr.c    | 18 +++++++++++++++---
>  drivers/hwtracing/coresight/coresight-tmc.h    |  2 +-
>  3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-core.c b/drivers/hwtracing/coresight/coresight-tmc-core.c
> index 07abf28ad725..c106d142e632 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-core.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-core.c
> @@ -31,7 +31,7 @@ DEFINE_CORESIGHT_DEVLIST(etb_devs, "tmc_etb");
>  DEFINE_CORESIGHT_DEVLIST(etf_devs, "tmc_etf");
>  DEFINE_CORESIGHT_DEVLIST(etr_devs, "tmc_etr");
>  
> -void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata)
> +int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata)
>  {
>  	struct coresight_device *csdev = drvdata->csdev;
>  	struct csdev_access *csa = &csdev->access;
> @@ -40,7 +40,9 @@ void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata)
>  	if (coresight_timeout(csa, TMC_STS, TMC_STS_TMCREADY_BIT, 1)) {
>  		dev_err(&csdev->dev,
>  			"timeout while waiting for TMC to be Ready\n");
> +		return -EBUSY;
>  	}
> +	return 0;
>  }
>  
>  void tmc_flush_and_stop(struct tmc_drvdata *drvdata)
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 867ad8bb9b0c..2da99dd41ed6 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -983,15 +983,21 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata)
>  	etr_buf->ops->sync(etr_buf, rrp, rwp);
>  }
>  
> -static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
> +static int __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
>  {
>  	u32 axictl, sts;
>  	struct etr_buf *etr_buf = drvdata->etr_buf;
> +	int rc = 0;
>  
>  	CS_UNLOCK(drvdata->base);
>  
>  	/* Wait for TMCSReady bit to be set */
> -	tmc_wait_for_tmcready(drvdata);
> +	rc = tmc_wait_for_tmcready(drvdata);
> +	if (rc) {
> +		dev_err(&drvdata->csdev->dev, "not ready ETR isn't enabled\n");
> +		CS_LOCK(drvdata->base);
> +		return rc;
> +	}
>  
>  	writel_relaxed(etr_buf->size / 4, drvdata->base + TMC_RSZ);
>  	writel_relaxed(TMC_MODE_CIRCULAR_BUFFER, drvdata->base + TMC_MODE);
> @@ -1032,6 +1038,7 @@ static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
>  	tmc_enable_hw(drvdata);
>  
>  	CS_LOCK(drvdata->base);
> +	return rc;
>  }
>  
>  static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
> @@ -1060,7 +1067,12 @@ static int tmc_etr_enable_hw(struct tmc_drvdata *drvdata,
>  	rc = coresight_claim_device(drvdata->csdev);
>  	if (!rc) {
>  		drvdata->etr_buf = etr_buf;
> -		__tmc_etr_enable_hw(drvdata);
> +		rc = __tmc_etr_enable_hw(drvdata);
> +		if (rc) {
> +			drvdata->etr_buf = NULL;
> +			coresight_disclaim_device(drvdata->csdev);
> +			tmc_etr_disable_catu(drvdata);
> +		}
>  	}
>  
>  	return rc;
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
> index 66959557cf39..01c0382a29c0 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.h
> +++ b/drivers/hwtracing/coresight/coresight-tmc.h
> @@ -255,7 +255,7 @@ struct tmc_sg_table {
>  };
>  
>  /* Generic functions */
> -void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata);
> +int tmc_wait_for_tmcready(struct tmc_drvdata *drvdata);
>  void tmc_flush_and_stop(struct tmc_drvdata *drvdata);
>  void tmc_enable_hw(struct tmc_drvdata *drvdata);
>  void tmc_disable_hw(struct tmc_drvdata *drvdata);