[PATCH 0/2] hwtracing: hisi_ptt: Fix reset timeout handling and clean up trace start

[PATCH 0/2] hwtracing: hisi_ptt: Fix reset timeout handling and clean up trace start

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

Patch 1: Propagate the DMA reset timeout error from
  hisi_ptt_wait_dma_reset_done() instead of discarding it. Move
  ctrl->started to the successful path so a failed start does not leave
  the trace marked as active.

Patch 2: Remove the unnecessary 16 MiB memset of trace buffers in
  hisi_ptt_trace_start(). The driver only copies data that hardware has
  written, so the zeroing is not needed.

Sanman Pradhan (2):
  hwtracing: hisi_ptt: Propagate DMA reset timeout in trace_start()
  hwtracing: hisi_ptt: Remove unnecessary trace buffer zeroing in
    trace_start()

 drivers/hwtracing/ptt/hisi_ptt.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

-- 
2.34.1

[PATCH 1/2] hwtracing: hisi_ptt: Propagate DMA reset timeout in trace_start()

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

hisi_ptt_wait_dma_reset_done() discards the return value of
readl_poll_timeout_atomic(). If the DMA engine does not complete its
reset within the timeout, hisi_ptt_trace_start() proceeds to start
tracing regardless.

Return the poll result from hisi_ptt_wait_dma_reset_done() and
propagate it from hisi_ptt_trace_start(). Deassert the reset bit
before returning on timeout, preserving the existing reset cleanup
sequence. Move ctrl->started to the successful path so a failed start
does not leave the trace marked as active.

Fixes: ff0de066b463 ("hwtracing: hisi_ptt: Add trace function support for HiSilicon PCIe Tune and Trace device")
Cc: stable@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 drivers/hwtracing/ptt/hisi_ptt.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
index 94c371c491357..73b93df8504c4 100644
--- a/drivers/hwtracing/ptt/hisi_ptt.c
+++ b/drivers/hwtracing/ptt/hisi_ptt.c
@@ -171,13 +171,13 @@ static bool hisi_ptt_wait_trace_hw_idle(struct hisi_ptt *hisi_ptt)
 					  HISI_PTT_WAIT_TRACE_TIMEOUT_US);
 }
 
-static void hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt)
+static int hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt)
 {
 	u32 val;
 
-	readl_poll_timeout_atomic(hisi_ptt->iobase + HISI_PTT_TRACE_WR_STS,
-				  val, !val, HISI_PTT_RESET_POLL_INTERVAL_US,
-				  HISI_PTT_RESET_TIMEOUT_US);
+	return readl_poll_timeout_atomic(hisi_ptt->iobase + HISI_PTT_TRACE_WR_STS,
+					 val, !val, HISI_PTT_RESET_POLL_INTERVAL_US,
+					 HISI_PTT_RESET_TIMEOUT_US);
 }
 
 static void hisi_ptt_trace_end(struct hisi_ptt *hisi_ptt)
@@ -194,6 +194,7 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
 {
 	struct hisi_ptt_trace_ctrl *ctrl = &hisi_ptt->trace_ctrl;
 	u32 val;
+	int ret;
 	int i;
 
 	/* Check device idle before start trace */
@@ -202,19 +203,21 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
 		return -EBUSY;
 	}
 
-	ctrl->started = true;
-
 	/* Reset the DMA before start tracing */
 	val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
 	val |= HISI_PTT_TRACE_CTRL_RST;
 	writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
 
-	hisi_ptt_wait_dma_reset_done(hisi_ptt);
+	ret = hisi_ptt_wait_dma_reset_done(hisi_ptt);
 
+	/* De-assert reset regardless of whether the wait timed out */
 	val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
 	val &= ~HISI_PTT_TRACE_CTRL_RST;
 	writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
 
+	if (ret)
+		return ret;
+
 	/* Reset the index of current buffer */
 	hisi_ptt->trace_ctrl.buf_index = 0;
 
@@ -234,6 +237,8 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
 	if (!hisi_ptt->trace_ctrl.is_port)
 		val |= HISI_PTT_TRACE_CTRL_FILTER_MODE;
 
+	ctrl->started = true;
+
 	/* Start the Trace */
 	val |= HISI_PTT_TRACE_CTRL_EN;
 	writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
-- 
2.34.1

Re: [PATCH 1/2] hwtracing: hisi_ptt: Propagate DMA reset timeout in trace_start()

[Yicong Yang]

On 2026/4/9 09:07, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
>
> hisi_ptt_wait_dma_reset_done() discards the return value of
> readl_poll_timeout_atomic(). If the DMA engine does not complete its
> reset within the timeout, hisi_ptt_trace_start() proceeds to start
> tracing regardless.
>
> Return the poll result from hisi_ptt_wait_dma_reset_done() and
> propagate it from hisi_ptt_trace_start(). Deassert the reset bit
> before returning on timeout, preserving the existing reset cleanup
> sequence. Move ctrl->started to the successful path so a failed start
> does not leave the trace marked as active.
>
> Fixes: ff0de066b463 ("hwtracing: hisi_ptt: Add trace function support for HiSilicon PCIe Tune and Trace device")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>
> ---
>  drivers/hwtracing/ptt/hisi_ptt.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
> index 94c371c491357..73b93df8504c4 100644
> --- a/drivers/hwtracing/ptt/hisi_ptt.c
> +++ b/drivers/hwtracing/ptt/hisi_ptt.c
> @@ -171,13 +171,13 @@ static bool hisi_ptt_wait_trace_hw_idle(struct hisi_ptt *hisi_ptt)
>  					  HISI_PTT_WAIT_TRACE_TIMEOUT_US);
>  }
>  
> -static void hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt)
> +static int hisi_ptt_wait_dma_reset_done(struct hisi_ptt *hisi_ptt)

the other status wait functions in this driver return a boolean, it's better to keep consistence.

>  {
>  	u32 val;
>  
> -	readl_poll_timeout_atomic(hisi_ptt->iobase + HISI_PTT_TRACE_WR_STS,
> -				  val, !val, HISI_PTT_RESET_POLL_INTERVAL_US,
> -				  HISI_PTT_RESET_TIMEOUT_US);
> +	return readl_poll_timeout_atomic(hisi_ptt->iobase + HISI_PTT_TRACE_WR_STS,
> +					 val, !val, HISI_PTT_RESET_POLL_INTERVAL_US,
> +					 HISI_PTT_RESET_TIMEOUT_US);
>  }
>  
>  static void hisi_ptt_trace_end(struct hisi_ptt *hisi_ptt)
> @@ -194,6 +194,7 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
>  {
>  	struct hisi_ptt_trace_ctrl *ctrl = &hisi_ptt->trace_ctrl;
>  	u32 val;
> +	int ret;
>  	int i;
>  
>  	/* Check device idle before start trace */
> @@ -202,19 +203,21 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
>  		return -EBUSY;
>  	}
>  
> -	ctrl->started = true;
> -
>  	/* Reset the DMA before start tracing */
>  	val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>  	val |= HISI_PTT_TRACE_CTRL_RST;
>  	writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>  
> -	hisi_ptt_wait_dma_reset_done(hisi_ptt);
> +	ret = hisi_ptt_wait_dma_reset_done(hisi_ptt);
>  
> +	/* De-assert reset regardless of whether the wait timed out */
>  	val = readl(hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>  	val &= ~HISI_PTT_TRACE_CTRL_RST;
>  	writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);
>  
> +	if (ret)
> +		return ret;
> +

could add some error log here for better debug. otherwise looks good to me.

the timeout wasn't checked since the hardware reset will be finished in the limited time normally,
which is less than the HISI_PTT_RESET_TIMEOUT_US. It'll be better to add this check in case
there's something wrong with the device.

Thanks.

>  	/* Reset the index of current buffer */
>  	hisi_ptt->trace_ctrl.buf_index = 0;
>  
> @@ -234,6 +237,8 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
>  	if (!hisi_ptt->trace_ctrl.is_port)
>  		val |= HISI_PTT_TRACE_CTRL_FILTER_MODE;
>  
> +	ctrl->started = true;
> +
>  	/* Start the Trace */
>  	val |= HISI_PTT_TRACE_CTRL_EN;
>  	writel(val, hisi_ptt->iobase + HISI_PTT_TRACE_CTRL);

Re: [PATCH 1/2] hwtracing: hisi_ptt: Propagate DMA reset timeout in trace_start()

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

On 2026/4/14, Yicong Yang wrote:
> the other status wait functions in this driver return a boolean, it's better to keep consistence.

Good point. Will switch hisi_ptt_wait_dma_reset_done() to return bool,
matching hisi_ptt_wait_trace_hw_idle() and hisi_ptt_wait_tuning_finish().

> could add some error log here for better debug. otherwise looks good to me.

Will add a pci_err() on the timeout path.

> the timeout wasn't checked since the hardware reset will be finished in the limited time normally,
> which is less than the HISI_PTT_RESET_TIMEOUT_US. It'll be better to add this check in case
> there's something wrong with the device.

Agreed. v2 will address both comments.

Thank you, for the review.

Thank you.

Regards,
Sanman Pradhan

[PATCH 2/2] hwtracing: hisi_ptt: Remove unnecessary trace buffer zeroing in trace_start()

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

hisi_ptt_trace_start() clears all four trace buffers before enabling
tracing.

This is unnecessary. On trace stop, hisi_ptt_update_aux() copies only
the number of bytes reported in HISI_PTT_TRACE_WR_STS. On buffer-full
interrupts, it copies a full completed buffer. In both cases the driver
only consumes data written by hardware.

Remove the buffer clearing from the trace start path.

Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
 drivers/hwtracing/ptt/hisi_ptt.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
index 73b93df8504c4..cbfbd1f146ba3 100644
--- a/drivers/hwtracing/ptt/hisi_ptt.c
+++ b/drivers/hwtracing/ptt/hisi_ptt.c
@@ -195,7 +195,6 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
 	struct hisi_ptt_trace_ctrl *ctrl = &hisi_ptt->trace_ctrl;
 	u32 val;
 	int ret;
-	int i;
 
 	/* Check device idle before start trace */
 	if (!hisi_ptt_wait_trace_hw_idle(hisi_ptt)) {
@@ -221,10 +220,6 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
 	/* Reset the index of current buffer */
 	hisi_ptt->trace_ctrl.buf_index = 0;
 
-	/* Zero the trace buffers */
-	for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++)
-		memset(ctrl->trace_buf[i].addr, 0, HISI_PTT_TRACE_BUF_SIZE);
-
 	/* Clear the interrupt status */
 	writel(HISI_PTT_TRACE_INT_STAT_MASK, hisi_ptt->iobase + HISI_PTT_TRACE_INT_STAT);
 	writel(0, hisi_ptt->iobase + HISI_PTT_TRACE_INT_MASK);
-- 
2.34.1

Re: [PATCH 2/2] hwtracing: hisi_ptt: Remove unnecessary trace buffer zeroing in trace_start()

[Yicong Yang]

On 2026/4/9 09:07, Pradhan, Sanman wrote:
> From: Sanman Pradhan <psanman@juniper.net>
>
> hisi_ptt_trace_start() clears all four trace buffers before enabling
> tracing.
>
> This is unnecessary. On trace stop, hisi_ptt_update_aux() copies only
> the number of bytes reported in HISI_PTT_TRACE_WR_STS. On buffer-full
> interrupts, it copies a full completed buffer. In both cases the driver
> only consumes data written by hardware.
>
> Remove the buffer clearing from the trace start path.
>
> Signed-off-by: Sanman Pradhan <psanman@juniper.net>

Reviewed-by: Yicong Yang <yangyccccc@gmail.com>

> ---
>  drivers/hwtracing/ptt/hisi_ptt.c | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/drivers/hwtracing/ptt/hisi_ptt.c b/drivers/hwtracing/ptt/hisi_ptt.c
> index 73b93df8504c4..cbfbd1f146ba3 100644
> --- a/drivers/hwtracing/ptt/hisi_ptt.c
> +++ b/drivers/hwtracing/ptt/hisi_ptt.c
> @@ -195,7 +195,6 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
>  	struct hisi_ptt_trace_ctrl *ctrl = &hisi_ptt->trace_ctrl;
>  	u32 val;
>  	int ret;
> -	int i;
>  
>  	/* Check device idle before start trace */
>  	if (!hisi_ptt_wait_trace_hw_idle(hisi_ptt)) {
> @@ -221,10 +220,6 @@ static int hisi_ptt_trace_start(struct hisi_ptt *hisi_ptt)
>  	/* Reset the index of current buffer */
>  	hisi_ptt->trace_ctrl.buf_index = 0;
>  
> -	/* Zero the trace buffers */
> -	for (i = 0; i < HISI_PTT_TRACE_BUF_CNT; i++)
> -		memset(ctrl->trace_buf[i].addr, 0, HISI_PTT_TRACE_BUF_SIZE);
> -
>  	/* Clear the interrupt status */
>  	writel(HISI_PTT_TRACE_INT_STAT_MASK, hisi_ptt->iobase + HISI_PTT_TRACE_INT_STAT);
>  	writel(0, hisi_ptt->iobase + HISI_PTT_TRACE_INT_MASK);