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

[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