[PATCH v2 0/3] dmaengine: qcom: gpi: Add trace event support

[PATCH v2 0/3] dmaengine: qcom: gpi: Add trace event support

[Praveen Talari]

This series adds tracepoint support to the Qualcomm GPI DMA engine
driver, giving structured runtime visibility into GPI DMA behavior
without requiring invasive debug patches.

Patch 1 adds the qcom_gpi trace events header, covering command
dispatch, IRQ handling, channel/event control state transitions,
completion event processing, ring allocation, and TRE contents.

Patch 2 wires these tracepoints into the driver alongside the
existing dev_dbg() calls.

Patch 3 converts the remaining dev_dbg() call sites in the driver to
use the new tracepoints instead.

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Changes in v2:
- Reorder TP_STRUCT__entry() fields in gpi_ev_process, gpi_alloc_ring,
  and gpi_ring_info to avoid holes: 8/16-bit fields moved to the end,
  and 64-bit/size_t fields moved before the __string() field (treating
  __string() as 4 bytes of metadata). (Steven Rostedt)
- Link to v1: https://patch.msgid.link/20260831-add-trace-support-gpio-v1-0-7b0fd0d0ddb3@oss.qualcomm.com

---
Praveen Talari (3):
      dmaengine: qcom: gpi: trace: Add trace events header for Qualcomm GPI DMA
      dmaengine: qcom: gpi: Add trace event support
      dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints

 drivers/dma/qcom/gpi.c          |  69 ++++-----
 include/trace/events/qcom_gpi.h | 313 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 340 insertions(+), 42 deletions(-)
---
base-commit: e30626823a406725ce29bc75cb8ec467d3e1e326
change-id: 20260831-add-trace-support-gpio-99eb54d42729

Best regards,
--  
Praveen Talari <praveen.talari@oss.qualcomm.com>

[PATCH v2 1/3] dmaengine: qcom: gpi: trace: Add trace events header for Qualcomm GPI DMA

[Praveen Talari]

Add a tracepoint header for the Qualcomm GPI DMA engine driver.

The events defined here cover channel/event command dispatch, IRQ
type handling, channel/event control state transitions, error IRQ
status, completion event processing (including the no-pending-
descriptor and transfer result paths), per-event processing in the
event ring, ring allocation/programming details, already-in-state
checks for pause/resume, and TRE queuing and contents. Together they
give full runtime visibility into GPI DMA behavior in place of the
driver's former dev_dbg() based debug logging.

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 include/trace/events/qcom_gpi.h | 313 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 313 insertions(+)

diff --git a/include/trace/events/qcom_gpi.h b/include/trace/events/qcom_gpi.h
new file mode 100644
index 000000000000..241d76855ac8
--- /dev/null
+++ b/include/trace/events/qcom_gpi.h
@@ -0,0 +1,313 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM qcom_gpi
+
+#if !defined(_TRACE_QCOM_GPI_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_QCOM_GPI_H
+
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(gpi_send_cmd,
+	    TP_PROTO(struct device *dev, u32 chid, u32 cmd, const char *cmd_str),
+	    TP_ARGS(dev, chid, cmd, cmd_str),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __string(cmd_str, cmd_str)
+			     __field(u32, chid)
+			     __field(u32, cmd)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __assign_str(cmd_str);
+			   __entry->chid = chid;
+			   __entry->cmd = cmd;
+	    ),
+
+	    TP_printk("%s: chid=%u cmd=%s(%u)",
+		      __get_str(name), __entry->chid, __get_str(cmd_str),
+		      __entry->cmd)
+);
+
+TRACE_EVENT(gpi_irq_status,
+	    TP_PROTO(struct device *dev, u32 gpii_id, u32 irq_type),
+	    TP_ARGS(dev, gpii_id, irq_type),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, gpii_id)
+			     __field(u32, irq_type)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->gpii_id = gpii_id;
+			   __entry->irq_type = irq_type;
+	    ),
+
+	    TP_printk("%s: gpii=%u irq_type=0x%08x",
+		      __get_str(name), __entry->gpii_id, __entry->irq_type)
+);
+
+TRACE_EVENT(gpi_ch_ctrl_irq,
+	    TP_PROTO(struct device *dev, u32 chid, u32 ch_state),
+	    TP_ARGS(dev, chid, ch_state),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, chid)
+			     __field(u32, ch_state)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->chid = chid;
+			   __entry->ch_state = ch_state;
+	    ),
+
+	    TP_printk("%s: chid=%u ch_state=%u",
+		      __get_str(name), __entry->chid, __entry->ch_state)
+);
+
+TRACE_EVENT(gpi_ev_process,
+	    TP_PROTO(struct device *dev, u32 chid, u32 ev_type, u8 code,
+		     u16 status, u32 length),
+	    TP_ARGS(dev, chid, ev_type, code, status, length),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, chid)
+			     __field(u32, ev_type)
+			     __field(u32, length)
+			     __field(u16, status)
+			     __field(u8, code)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->chid = chid;
+			   __entry->ev_type = ev_type;
+			   __entry->code = code;
+			   __entry->status = status;
+			   __entry->length = length;
+	    ),
+
+	    TP_printk("%s: chid=%u ev_type=0x%02x code=%u status=%u length=%u",
+		      __get_str(name), __entry->chid, __entry->ev_type,
+		      __entry->code, __entry->status, __entry->length)
+);
+
+TRACE_EVENT(gpi_queue_xfer,
+	    TP_PROTO(struct device *dev, u32 chid, u32 num_tre),
+	    TP_ARGS(dev, chid, num_tre),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, chid)
+			     __field(u32, num_tre)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->chid = chid;
+			   __entry->num_tre = num_tre;
+	    ),
+
+	    TP_printk("%s: chid=%u num_tre=%u",
+		      __get_str(name), __entry->chid, __entry->num_tre)
+);
+
+TRACE_EVENT(gpi_gen_err_irq,
+	    TP_PROTO(struct device *dev, u32 gpii_id, u32 irq_stts),
+	    TP_ARGS(dev, gpii_id, irq_stts),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, gpii_id)
+			     __field(u32, irq_stts)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->gpii_id = gpii_id;
+			   __entry->irq_stts = irq_stts;
+	    ),
+
+	    TP_printk("%s: gpii=%u irq_stts=0x%08x",
+		      __get_str(name), __entry->gpii_id, __entry->irq_stts)
+);
+
+TRACE_EVENT(gpi_ev_ctrl_irq,
+	    TP_PROTO(struct device *dev, u32 gpii_id, u32 ev_ch_irq, u32 ev_state),
+	    TP_ARGS(dev, gpii_id, ev_ch_irq, ev_state),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, gpii_id)
+			     __field(u32, ev_ch_irq)
+			     __field(u32, ev_state)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->gpii_id = gpii_id;
+			   __entry->ev_ch_irq = ev_ch_irq;
+			   __entry->ev_state = ev_state;
+	    ),
+
+	    TP_printk("%s: gpii=%u ev_ch_irq=0x%08x ev_state=%u",
+		      __get_str(name), __entry->gpii_id, __entry->ev_ch_irq,
+		      __entry->ev_state)
+);
+
+TRACE_EVENT(gpi_ev_no_desc,
+	    TP_PROTO(struct device *dev, u32 chid, const u32 *ev_dword,
+		     const u32 *tre_dword),
+	    TP_ARGS(dev, chid, ev_dword, tre_dword),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, chid)
+			     __array(u32, ev_dword, 4)
+			     __array(u32, tre_dword, 4)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->chid = chid;
+			   memcpy(__entry->ev_dword, ev_dword, sizeof(__entry->ev_dword));
+			   memcpy(__entry->tre_dword, tre_dword, sizeof(__entry->tre_dword));
+	    ),
+
+	    TP_printk("%s: chid=%u event=%08x:%08x:%08x:%08x pending_tre=%08x:%08x:%08x:%08x",
+		      __get_str(name), __entry->chid,
+		      __entry->ev_dword[0], __entry->ev_dword[1],
+		      __entry->ev_dword[2], __entry->ev_dword[3],
+		      __entry->tre_dword[0], __entry->tre_dword[1],
+		      __entry->tre_dword[2], __entry->tre_dword[3])
+);
+
+TRACE_EVENT(gpi_xfer_result,
+	    TP_PROTO(struct device *dev, u32 chid, int result, u32 residue),
+	    TP_ARGS(dev, chid, result, residue),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, chid)
+			     __field(int, result)
+			     __field(u32, residue)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->chid = chid;
+			   __entry->result = result;
+			   __entry->residue = residue;
+	    ),
+
+	    TP_printk("%s: chid=%u result=%d residue=%u",
+		      __get_str(name), __entry->chid, __entry->result,
+		      __entry->residue)
+);
+
+TRACE_EVENT(gpi_process_event,
+	    TP_PROTO(struct device *dev, u32 chid, u32 type, const u32 *dword),
+	    TP_ARGS(dev, chid, type, dword),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, chid)
+			     __field(u32, type)
+			     __array(u32, dword, 4)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->chid = chid;
+			   __entry->type = type;
+			   memcpy(__entry->dword, dword, sizeof(__entry->dword));
+	    ),
+
+	    TP_printk("%s: chid=%u type=0x%02x %08x:%08x:%08x:%08x",
+		      __get_str(name), __entry->chid, __entry->type,
+		      __entry->dword[0], __entry->dword[1],
+		      __entry->dword[2], __entry->dword[3])
+);
+
+TRACE_EVENT(gpi_alloc_ring,
+	    TP_PROTO(struct device *dev, u32 elements, u32 el_size,
+		     u32 req_len, u64 len, size_t alloc_size),
+	    TP_ARGS(dev, elements, el_size, req_len, len, alloc_size),
+
+	    TP_STRUCT__entry(__field(u64, len)
+			     __field(size_t, alloc_size)
+			     __string(name, dev_name(dev))
+			     __field(u32, elements)
+			     __field(u32, el_size)
+			     __field(u32, req_len)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->elements = elements;
+			   __entry->el_size = el_size;
+			   __entry->req_len = req_len;
+			   __entry->len = len;
+			   __entry->alloc_size = alloc_size;
+	    ),
+
+	    TP_printk("%s: elements=%u el_size=%u req_len=%u len=%llu alloc_size=%zu",
+		      __get_str(name), __entry->elements, __entry->el_size,
+		      __entry->req_len, __entry->len, __entry->alloc_size)
+);
+
+TRACE_EVENT(gpi_ring_info,
+	    TP_PROTO(struct device *dev, dma_addr_t dma_handle, phys_addr_t phys_addr,
+		     u32 len, u32 el_size, u32 elements),
+	    TP_ARGS(dev, dma_handle, phys_addr, len, el_size, elements),
+
+	    TP_STRUCT__entry(__field(u64, dma_handle)
+			     __field(u64, phys_addr)
+			     __string(name, dev_name(dev))
+			     __field(u32, len)
+			     __field(u32, el_size)
+			     __field(u32, elements)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->dma_handle = dma_handle;
+			   __entry->phys_addr = phys_addr;
+			   __entry->len = len;
+			   __entry->el_size = el_size;
+			   __entry->elements = elements;
+	    ),
+
+	    TP_printk("%s: dma_handle=%llx phys_addr=%llx len=%u el_size=%u elements=%u",
+		      __get_str(name), __entry->dma_handle, __entry->phys_addr,
+		      __entry->len, __entry->el_size, __entry->elements)
+);
+
+TRACE_EVENT(gpi_already_state,
+	    TP_PROTO(struct device *dev, u32 gpii_id, u32 pm_state),
+	    TP_ARGS(dev, gpii_id, pm_state),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, gpii_id)
+			     __field(u32, pm_state)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->gpii_id = gpii_id;
+			   __entry->pm_state = pm_state;
+	    ),
+
+	    TP_printk("%s: gpii=%u already in pm_state=%u",
+		      __get_str(name), __entry->gpii_id, __entry->pm_state)
+);
+
+TRACE_EVENT(gpi_tre,
+	    TP_PROTO(struct device *dev, u32 chid, u32 idx, const u32 *dword),
+	    TP_ARGS(dev, chid, idx, dword),
+
+	    TP_STRUCT__entry(__string(name, dev_name(dev))
+			     __field(u32, chid)
+			     __field(u32, idx)
+			     __array(u32, dword, 4)
+	    ),
+
+	    TP_fast_assign(__assign_str(name);
+			   __entry->chid = chid;
+			   __entry->idx = idx;
+			   memcpy(__entry->dword, dword, sizeof(__entry->dword));
+	    ),
+
+	    TP_printk("%s: chid=%u tre[%u]=%08x:%08x:%08x:%08x",
+		      __get_str(name), __entry->chid, __entry->idx,
+		      __entry->dword[0], __entry->dword[1],
+		      __entry->dword[2], __entry->dword[3])
+);
+
+#endif /* _TRACE_QCOM_GPI_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>

-- 
2.34.1

Re: [PATCH v2 1/3] dmaengine: qcom: gpi: trace: Add trace events header for Qualcomm GPI DMA

[Frank Li]

On Wed, Sep 09, 2026 at 10:39:41AM +0530, Praveen Talari wrote:
> Add a tracepoint header for the Qualcomm GPI DMA engine driver.
>
> The events defined here cover channel/event command dispatch, IRQ
> type handling, channel/event control state transitions, error IRQ
> status, completion event processing (including the no-pending-
> descriptor and transfer result paths), per-event processing in the
> event ring, ring allocation/programming details, already-in-state
> checks for pause/resume, and TRE queuing and contents. Together they
> give full runtime visibility into GPI DMA behavior in place of the
> driver's former dev_dbg() based debug logging.
>
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
>  include/trace/events/qcom_gpi.h | 313 ++++++++++++++++++++++++++++++++++++++++

Any user use it outside drivers/dma? if no, move it in drivers/dma

and please squash to patch 2, with user together.

Frank

>  1 file changed, 313 insertions(+)
>
> diff --git a/include/trace/events/qcom_gpi.h b/include/trace/events/qcom_gpi.h
> new file mode 100644
> index 000000000000..241d76855ac8
> --- /dev/null
> +++ b/include/trace/events/qcom_gpi.h
> @@ -0,0 +1,313 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM qcom_gpi
> +
> +#if !defined(_TRACE_QCOM_GPI_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_QCOM_GPI_H
> +
> +#include <linux/tracepoint.h>
> +
> +TRACE_EVENT(gpi_send_cmd,
> +	    TP_PROTO(struct device *dev, u32 chid, u32 cmd, const char *cmd_str),
> +	    TP_ARGS(dev, chid, cmd, cmd_str),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __string(cmd_str, cmd_str)
> +			     __field(u32, chid)
> +			     __field(u32, cmd)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __assign_str(cmd_str);
> +			   __entry->chid = chid;
> +			   __entry->cmd = cmd;
> +	    ),
> +
> +	    TP_printk("%s: chid=%u cmd=%s(%u)",
> +		      __get_str(name), __entry->chid, __get_str(cmd_str),
> +		      __entry->cmd)
> +);
> +
> +TRACE_EVENT(gpi_irq_status,
> +	    TP_PROTO(struct device *dev, u32 gpii_id, u32 irq_type),
> +	    TP_ARGS(dev, gpii_id, irq_type),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, gpii_id)
> +			     __field(u32, irq_type)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->gpii_id = gpii_id;
> +			   __entry->irq_type = irq_type;
> +	    ),
> +
> +	    TP_printk("%s: gpii=%u irq_type=0x%08x",
> +		      __get_str(name), __entry->gpii_id, __entry->irq_type)
> +);
> +
> +TRACE_EVENT(gpi_ch_ctrl_irq,
> +	    TP_PROTO(struct device *dev, u32 chid, u32 ch_state),
> +	    TP_ARGS(dev, chid, ch_state),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, chid)
> +			     __field(u32, ch_state)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->chid = chid;
> +			   __entry->ch_state = ch_state;
> +	    ),
> +
> +	    TP_printk("%s: chid=%u ch_state=%u",
> +		      __get_str(name), __entry->chid, __entry->ch_state)
> +);
> +
> +TRACE_EVENT(gpi_ev_process,
> +	    TP_PROTO(struct device *dev, u32 chid, u32 ev_type, u8 code,
> +		     u16 status, u32 length),
> +	    TP_ARGS(dev, chid, ev_type, code, status, length),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, chid)
> +			     __field(u32, ev_type)
> +			     __field(u32, length)
> +			     __field(u16, status)
> +			     __field(u8, code)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->chid = chid;
> +			   __entry->ev_type = ev_type;
> +			   __entry->code = code;
> +			   __entry->status = status;
> +			   __entry->length = length;
> +	    ),
> +
> +	    TP_printk("%s: chid=%u ev_type=0x%02x code=%u status=%u length=%u",
> +		      __get_str(name), __entry->chid, __entry->ev_type,
> +		      __entry->code, __entry->status, __entry->length)
> +);
> +
> +TRACE_EVENT(gpi_queue_xfer,
> +	    TP_PROTO(struct device *dev, u32 chid, u32 num_tre),
> +	    TP_ARGS(dev, chid, num_tre),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, chid)
> +			     __field(u32, num_tre)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->chid = chid;
> +			   __entry->num_tre = num_tre;
> +	    ),
> +
> +	    TP_printk("%s: chid=%u num_tre=%u",
> +		      __get_str(name), __entry->chid, __entry->num_tre)
> +);
> +
> +TRACE_EVENT(gpi_gen_err_irq,
> +	    TP_PROTO(struct device *dev, u32 gpii_id, u32 irq_stts),
> +	    TP_ARGS(dev, gpii_id, irq_stts),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, gpii_id)
> +			     __field(u32, irq_stts)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->gpii_id = gpii_id;
> +			   __entry->irq_stts = irq_stts;
> +	    ),
> +
> +	    TP_printk("%s: gpii=%u irq_stts=0x%08x",
> +		      __get_str(name), __entry->gpii_id, __entry->irq_stts)
> +);
> +
> +TRACE_EVENT(gpi_ev_ctrl_irq,
> +	    TP_PROTO(struct device *dev, u32 gpii_id, u32 ev_ch_irq, u32 ev_state),
> +	    TP_ARGS(dev, gpii_id, ev_ch_irq, ev_state),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, gpii_id)
> +			     __field(u32, ev_ch_irq)
> +			     __field(u32, ev_state)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->gpii_id = gpii_id;
> +			   __entry->ev_ch_irq = ev_ch_irq;
> +			   __entry->ev_state = ev_state;
> +	    ),
> +
> +	    TP_printk("%s: gpii=%u ev_ch_irq=0x%08x ev_state=%u",
> +		      __get_str(name), __entry->gpii_id, __entry->ev_ch_irq,
> +		      __entry->ev_state)
> +);
> +
> +TRACE_EVENT(gpi_ev_no_desc,
> +	    TP_PROTO(struct device *dev, u32 chid, const u32 *ev_dword,
> +		     const u32 *tre_dword),
> +	    TP_ARGS(dev, chid, ev_dword, tre_dword),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, chid)
> +			     __array(u32, ev_dword, 4)
> +			     __array(u32, tre_dword, 4)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->chid = chid;
> +			   memcpy(__entry->ev_dword, ev_dword, sizeof(__entry->ev_dword));
> +			   memcpy(__entry->tre_dword, tre_dword, sizeof(__entry->tre_dword));
> +	    ),
> +
> +	    TP_printk("%s: chid=%u event=%08x:%08x:%08x:%08x pending_tre=%08x:%08x:%08x:%08x",
> +		      __get_str(name), __entry->chid,
> +		      __entry->ev_dword[0], __entry->ev_dword[1],
> +		      __entry->ev_dword[2], __entry->ev_dword[3],
> +		      __entry->tre_dword[0], __entry->tre_dword[1],
> +		      __entry->tre_dword[2], __entry->tre_dword[3])
> +);
> +
> +TRACE_EVENT(gpi_xfer_result,
> +	    TP_PROTO(struct device *dev, u32 chid, int result, u32 residue),
> +	    TP_ARGS(dev, chid, result, residue),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, chid)
> +			     __field(int, result)
> +			     __field(u32, residue)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->chid = chid;
> +			   __entry->result = result;
> +			   __entry->residue = residue;
> +	    ),
> +
> +	    TP_printk("%s: chid=%u result=%d residue=%u",
> +		      __get_str(name), __entry->chid, __entry->result,
> +		      __entry->residue)
> +);
> +
> +TRACE_EVENT(gpi_process_event,
> +	    TP_PROTO(struct device *dev, u32 chid, u32 type, const u32 *dword),
> +	    TP_ARGS(dev, chid, type, dword),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, chid)
> +			     __field(u32, type)
> +			     __array(u32, dword, 4)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->chid = chid;
> +			   __entry->type = type;
> +			   memcpy(__entry->dword, dword, sizeof(__entry->dword));
> +	    ),
> +
> +	    TP_printk("%s: chid=%u type=0x%02x %08x:%08x:%08x:%08x",
> +		      __get_str(name), __entry->chid, __entry->type,
> +		      __entry->dword[0], __entry->dword[1],
> +		      __entry->dword[2], __entry->dword[3])
> +);
> +
> +TRACE_EVENT(gpi_alloc_ring,
> +	    TP_PROTO(struct device *dev, u32 elements, u32 el_size,
> +		     u32 req_len, u64 len, size_t alloc_size),
> +	    TP_ARGS(dev, elements, el_size, req_len, len, alloc_size),
> +
> +	    TP_STRUCT__entry(__field(u64, len)
> +			     __field(size_t, alloc_size)
> +			     __string(name, dev_name(dev))
> +			     __field(u32, elements)
> +			     __field(u32, el_size)
> +			     __field(u32, req_len)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->elements = elements;
> +			   __entry->el_size = el_size;
> +			   __entry->req_len = req_len;
> +			   __entry->len = len;
> +			   __entry->alloc_size = alloc_size;
> +	    ),
> +
> +	    TP_printk("%s: elements=%u el_size=%u req_len=%u len=%llu alloc_size=%zu",
> +		      __get_str(name), __entry->elements, __entry->el_size,
> +		      __entry->req_len, __entry->len, __entry->alloc_size)
> +);
> +
> +TRACE_EVENT(gpi_ring_info,
> +	    TP_PROTO(struct device *dev, dma_addr_t dma_handle, phys_addr_t phys_addr,
> +		     u32 len, u32 el_size, u32 elements),
> +	    TP_ARGS(dev, dma_handle, phys_addr, len, el_size, elements),
> +
> +	    TP_STRUCT__entry(__field(u64, dma_handle)
> +			     __field(u64, phys_addr)
> +			     __string(name, dev_name(dev))
> +			     __field(u32, len)
> +			     __field(u32, el_size)
> +			     __field(u32, elements)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->dma_handle = dma_handle;
> +			   __entry->phys_addr = phys_addr;
> +			   __entry->len = len;
> +			   __entry->el_size = el_size;
> +			   __entry->elements = elements;
> +	    ),
> +
> +	    TP_printk("%s: dma_handle=%llx phys_addr=%llx len=%u el_size=%u elements=%u",
> +		      __get_str(name), __entry->dma_handle, __entry->phys_addr,
> +		      __entry->len, __entry->el_size, __entry->elements)
> +);
> +
> +TRACE_EVENT(gpi_already_state,
> +	    TP_PROTO(struct device *dev, u32 gpii_id, u32 pm_state),
> +	    TP_ARGS(dev, gpii_id, pm_state),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, gpii_id)
> +			     __field(u32, pm_state)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->gpii_id = gpii_id;
> +			   __entry->pm_state = pm_state;
> +	    ),
> +
> +	    TP_printk("%s: gpii=%u already in pm_state=%u",
> +		      __get_str(name), __entry->gpii_id, __entry->pm_state)
> +);
> +
> +TRACE_EVENT(gpi_tre,
> +	    TP_PROTO(struct device *dev, u32 chid, u32 idx, const u32 *dword),
> +	    TP_ARGS(dev, chid, idx, dword),
> +
> +	    TP_STRUCT__entry(__string(name, dev_name(dev))
> +			     __field(u32, chid)
> +			     __field(u32, idx)
> +			     __array(u32, dword, 4)
> +	    ),
> +
> +	    TP_fast_assign(__assign_str(name);
> +			   __entry->chid = chid;
> +			   __entry->idx = idx;
> +			   memcpy(__entry->dword, dword, sizeof(__entry->dword));
> +	    ),
> +
> +	    TP_printk("%s: chid=%u tre[%u]=%08x:%08x:%08x:%08x",
> +		      __get_str(name), __entry->chid, __entry->idx,
> +		      __entry->dword[0], __entry->dword[1],
> +		      __entry->dword[2], __entry->dword[3])
> +);
> +
> +#endif /* _TRACE_QCOM_GPI_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
>
> --
> 2.34.1
>

[PATCH v2 2/3] dmaengine: qcom: gpi: Add trace event support

[Praveen Talari]

Wire up the qcom_gpi tracepoints into the GPI DMA engine driver to
provide runtime visibility into driver behavior without requiring
invasive debug patches.

Tracepoints are added at the same points as their existing dev_dbg()
counterparts: channel/event command dispatch in gpi_send_cmd(),
top-level interrupt type in gpi_handle_irq(), channel state
transitions in gpi_process_ch_ctrl_irq(), completion event handling
in gpi_process_xfer_compl_event() and gpi_process_imed_data_event(),
and TRE queuing in gpi_issue_pending().

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 drivers/dma/qcom/gpi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index a5055a6273af..b09354a73b46 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -17,6 +17,9 @@
 #include "../dmaengine.h"
 #include "../virt-dma.h"
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/qcom_gpi.h>
+
 #define TRE_TYPE_DMA		0x10
 #define TRE_TYPE_IMMEDIATE_DMA	0x11
 #define TRE_TYPE_GO		0x20
@@ -683,6 +686,7 @@ static int gpi_send_cmd(struct gpii *gpii, struct gchan *gchan,
 
 	dev_dbg(gpii->gpi_dev->dev,
 		"sending cmd: %s:%u\n", TO_GPI_CMD_STR(gpi_cmd), chid);
+	trace_gpi_send_cmd(gpii->gpi_dev->dev, chid, gpi_cmd, TO_GPI_CMD_STR(gpi_cmd));
 
 	/* send opcode and wait for completion */
 	reinit_completion(&gpii->cmd_completion);
@@ -773,6 +777,7 @@ static void gpi_process_ch_ctrl_irq(struct gpii *gpii)
 		if (gpii->gpi_cmd == GPI_CH_CMD_DE_ALLOC)
 			state = DEFAULT_CH_STATE;
 		gchan->ch_state = state;
+		trace_gpi_ch_ctrl_irq(gpii->gpi_dev->dev, chid, gchan->ch_state);
 
 		/*
 		 * Triggering complete all if ch_state is not a stop in process.
@@ -841,6 +846,7 @@ static irqreturn_t gpi_handle_irq(int irq, void *data)
 
 	offset = GPII_n_CNTXT_TYPE_IRQ_OFFS(gpii->gpii_id);
 	type = gpi_read_reg(gpii, gpii->regs + offset);
+	trace_gpi_irq_status(gpii->gpi_dev->dev, gpii_id, type);
 
 	do {
 		/* global gpii error */
@@ -968,6 +974,8 @@ static void gpi_process_imed_data_event(struct gchan *gchan,
 	smp_wmb();
 
 	chid = imed_event->chid;
+	trace_gpi_ev_process(gpii->gpi_dev->dev, chid, IMMEDIATE_DATA_EV_TYPE,
+			     imed_event->code, imed_event->status, imed_event->length);
 	if (imed_event->code == MSM_GPI_TCE_EOT && gpii->ieob_set) {
 		if (chid == GPI_RX_CHAN)
 			goto gpi_free_desc;
@@ -1043,6 +1051,8 @@ static void gpi_process_xfer_compl_event(struct gchan *gchan,
 	smp_wmb();
 
 	chid = compl_event->chid;
+	trace_gpi_ev_process(gpii->gpi_dev->dev, chid, XFER_COMPLETE_EV_TYPE,
+			     compl_event->code, compl_event->status, compl_event->length);
 	if (compl_event->code == MSM_GPI_TCE_EOT && gpii->ieob_set) {
 		if (chid == GPI_RX_CHAN)
 			goto gpi_free_desc;
@@ -1887,6 +1897,7 @@ static void gpi_issue_pending(struct dma_chan *chan)
 	}
 
 	gpi_desc = to_gpi_desc(vd);
+	trace_gpi_queue_xfer(gpii->gpi_dev->dev, gchan->chid, gpi_desc->num_tre);
 	for (i = 0; i < gpi_desc->num_tre; i++) {
 		tre = &gpi_desc->tre[i];
 		gpi_queue_xfer(gpii, gchan, tre, &wp);

-- 
2.34.1

Re: [PATCH v2 2/3] dmaengine: qcom: gpi: Add trace event support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [Medium] The `gpi_irq_status` tracepoint is only called before the interrupt processing loop, silently omitting back-to-back interrupts.

Pre-existing issues:
- [High] Tasklet Use-After-Free due to incorrect teardown order in gpi_free_chan_resources.
- [High] Incorrect channel cleanup in gpi_ch_init error path.
- [High] Leak of transfer ring DMA memory when gpi_ch_init fails.
- [High] Missing dma_wmb() barrier before writing to the channel doorbell register, which may cause the hardware to read stale descriptors.
- [High] Missing dma_rmb() barrier before reading event ring descriptors, risking processing of stale event data.
--

commit 4cf86dc6d78f8720092d0dce56c8bcfee50f39a4
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>

    dmaengine: qcom: gpi: Add trace event support
    
    Wire up the qcom_gpi tracepoints into the GPI DMA engine driver to
    provide runtime visibility into driver behavior without requiring
    invasive debug patches.

> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index a5055a6273af6..b09354a73b46e 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c

[ ... ]

> @@ -841,6 +846,7 @@ static irqreturn_t gpi_handle_irq(int irq, void *data)
>  
>  	offset = GPII_n_CNTXT_TYPE_IRQ_OFFS(gpii->gpii_id);
>  	type = gpi_read_reg(gpii, gpii->regs + offset);
> +	trace_gpi_irq_status(gpii->gpi_dev->dev, gpii_id, type);
>  
>  	do {
>  		/* global gpii error */

[Severity: Medium]
Will this tracepoint miss subsequent back-to-back interrupts?

In drivers/dma/qcom/gpi.c:gpi_handle_irq(), the hardware can trigger
back-to-back interrupts. The function reads the type register before the
loop, traces it, and then processes the interrupt. It reads the type
register again at the end of the loop and finds another interrupt, but
continues the loop without invoking trace_gpi_irq_status() for the newly
read type.

[ ... ]

> @@ -1887,6 +1897,7 @@ static void gpi_issue_pending(struct dma_chan *chan)
>  	}
>  
>  	gpi_desc = to_gpi_desc(vd);
> +	trace_gpi_queue_xfer(gpii->gpi_dev->dev, gchan->chid, gpi_desc->num_tre);
>  	for (i = 0; i < gpi_desc->num_tre; i++) {
>  		tre = &gpi_desc->tre[i];
>  		gpi_queue_xfer(gpii, gchan, tre, &wp);

[Severity: High]
This is a pre-existing issue, but is there a missing dma_wmb() barrier
before writing to the channel doorbell register?

After the loop populates descriptors in
drivers/dma/qcom/gpi.c:gpi_issue_pending(), the doorbell is rung:

	gpi_desc->db = ch_ring->wp;
	gpi_write_ch_db(gchan, &gchan->ch_ring, gpi_desc->db);

Since gpi_write_ch_db() uses writel_relaxed(), without a preceding
dma_wmb(), could the CPU reorder the relaxed MMIO doorbell write before
the main memory TRE writes become globally visible, causing the hardware
to process uninitialized or stale descriptors?

[Severity: High]
This isn't a bug introduced by this patch, but does
gpi_free_chan_resources() in drivers/dma/qcom/gpi.c have a race
condition that could lead to a use-after-free?

	tasklet_kill(&gpii->ev_task);

	/* send command to de allocate event ring */
	if (cur_state == ACTIVE_STATE)
		gpi_send_cmd(gpii, NULL, GPI_EV_CMD_DEALLOC);

	gpi_free_ring(&gpii->ev_ring, gpii);

	/* disable interrupts */
	if (cur_state == ACTIVE_STATE)
		gpi_disable_interrupts(gpii);

The tasklet is killed and the ring is freed before the interrupt is
disabled. If a delayed or spurious hardware interrupt occurs in this
window, the interrupt handler will read pm_state as PREPARE_TERMINATE,
and could schedule the tasklet again. The tasklet would then run and
access the freed ev_ring.

[Severity: High]
This is a pre-existing issue, but does gpi_ch_init() in
drivers/dma/qcom/gpi.c have an incorrect channel cleanup in its
error path?

error_start_chan:
	for (i = i - 1; i >= 0; i--) {
		gpi_stop_chan(&gpii->gchan[i]);
		gpi_send_cmd(gpii, gchan, GPI_CH_CMD_RESET);
	}
	i = 2;
error_alloc_chan:
	for (i = i - 1; i >= 0; i--)
		gpi_reset_chan(gchan, GPI_CH_CMD_DE_ALLOC);

If initialization fails, it jumps to the error path. The loop iterates
over i, but it erroneously passes the single argument gchan to
gpi_stop_chan(), gpi_send_cmd(), and gpi_reset_chan() instead of
&gpii->gchan[i]. Could this repeatedly reset the same channel while
abandoning the other channel in a partially initialized state?

[Severity: High]
This isn't a bug introduced by this patch, but does
gpi_alloc_chan_resources() in drivers/dma/qcom/gpi.c leak the transfer
ring DMA memory when gpi_ch_init() fails?

	ret = gpi_alloc_ring(&gchan->ch_ring, CHAN_TRES,
			     sizeof(struct gpi_tre), gpii);
	if (ret)
		goto xfer_alloc_err;

	ret = gpi_ch_init(gchan);

	mutex_unlock(&gpii->ctrl_lock);

	return ret;

When gpi_ch_init() returns an error, the function exits without calling
gpi_free_ring() to release the locally allocated gchan->ch_ring.

[Severity: High]
This is a pre-existing issue, but is there a missing dma_rmb() barrier
before reading event ring descriptors in
drivers/dma/qcom/gpi.c:gpi_process_events()?

	cntxt_rp = gpi_read_reg(gpii, gpii->ev_ring_rp_lsb_reg);
	rp = to_virtual(ev_ring, cntxt_rp);

	do {
		while (rp != ev_ring->rp) {
			gpi_event = ev_ring->rp;

The CPU reads the hardware progress register via gpi_read_reg() which
uses readl_relaxed(). It then immediately reads the corresponding DMA
memory at ev_ring->rp. Without an intervening dma_rmb(), could the CPU
speculatively fetch the descriptors from memory before the progress
pointer read completes, reading stale events?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-add-trace-support-gpio-v2-0-01f81739bb88@oss.qualcomm.com?part=2

[PATCH v2 3/3] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints

[Praveen Talari]

Replace the remaining dev_dbg() based debug logging in the GPI DMA
driver with the qcom_gpi tracepoints, providing structured runtime
visibility into GPI DMA behavior without requiring invasive debug
patches. dev_err() calls are left untouched.

Tracepoints are added at the same points as the dev_dbg() calls they
replace: channel/event command dispatch in gpi_send_cmd(), error and
top-level interrupt status in gpi_process_gen_err_irq() and
gpi_handle_irq(), event/channel control state transitions in the IRQ
handler and gpi_process_ch_ctrl_irq(), completion event handling
(including the no-pending-descriptor and transfer result paths) in
gpi_process_imed_data_event(), gpi_process_xfer_compl_event() and
gpi_process_events(), ring allocation details in gpi_alloc_ring(),
already-in-state checks in gpi_pause()/gpi_resume(), TRE contents in
gpi_create_i2c_tre()/gpi_create_spi_tre(), and TRE queuing in
gpi_issue_pending().

Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
 drivers/dma/qcom/gpi.c | 58 ++++++++++++++------------------------------------
 1 file changed, 16 insertions(+), 42 deletions(-)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index b09354a73b46..8e7d25a46147 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -684,8 +684,6 @@ static int gpi_send_cmd(struct gpii *gpii, struct gchan *gchan,
 	if (IS_CHAN_CMD(gpi_cmd))
 		chid = gchan->chid;
 
-	dev_dbg(gpii->gpi_dev->dev,
-		"sending cmd: %s:%u\n", TO_GPI_CMD_STR(gpi_cmd), chid);
 	trace_gpi_send_cmd(gpii->gpi_dev->dev, chid, gpi_cmd, TO_GPI_CMD_STR(gpi_cmd));
 
 	/* send opcode and wait for completion */
@@ -797,7 +795,7 @@ static void gpi_process_gen_err_irq(struct gpii *gpii)
 	u32 irq_stts = gpi_read_reg(gpii, gpii->regs + offset);
 
 	/* clear the status */
-	dev_dbg(gpii->gpi_dev->dev, "irq_stts:0x%x\n", irq_stts);
+	trace_gpi_gen_err_irq(gpii->gpi_dev->dev, gpii_id, irq_stts);
 
 	/* Clear the register */
 	offset = GPII_n_CNTXT_GPII_IRQ_CLR_OFFS(gpii_id);
@@ -866,8 +864,6 @@ static irqreturn_t gpi_handle_irq(int irq, void *data)
 			u32 ev_state;
 			u32 ev_ch_irq;
 
-			dev_dbg(gpii->gpi_dev->dev,
-				"processing EV CTRL interrupt\n");
 			offset = GPII_n_CNTXT_SRC_EV_CH_IRQ_OFFS(gpii_id);
 			ev_ch_irq = gpi_read_reg(gpii, gpii->regs + offset);
 
@@ -887,15 +883,14 @@ static irqreturn_t gpi_handle_irq(int irq, void *data)
 				ev_state = DEFAULT_EV_CH_STATE;
 
 			gpii->ev_state = ev_state;
-			dev_dbg(gpii->gpi_dev->dev, "setting EV state to %s\n",
-				TO_GPI_EV_STATE_STR(gpii->ev_state));
+			trace_gpi_ev_ctrl_irq(gpii->gpi_dev->dev, gpii_id, ev_ch_irq,
+					      gpii->ev_state);
 			complete_all(&gpii->cmd_completion);
 			type &= ~(GPII_n_CNTXT_TYPE_IRQ_MSK_EV_CTRL);
 		}
 
 		/* channel control irq */
 		if (type & GPII_n_CNTXT_TYPE_IRQ_MSK_CH_CTRL) {
-			dev_dbg(gpii->gpi_dev->dev, "process CH CTRL interrupts\n");
 			gpi_process_ch_ctrl_irq(gpii);
 			type &= ~(GPII_n_CNTXT_TYPE_IRQ_MSK_CH_CTRL);
 		}
@@ -945,17 +940,10 @@ static void gpi_process_imed_data_event(struct gchan *gchan,
 		struct gpi_tre *gpi_tre;
 
 		spin_unlock_irqrestore(&gchan->vc.lock, flags);
-		dev_dbg(gpii->gpi_dev->dev, "event without a pending descriptor!\n");
 		gpi_ere = (struct gpi_ere *)imed_event;
-		dev_dbg(gpii->gpi_dev->dev,
-			"Event: %08x %08x %08x %08x\n",
-			gpi_ere->dword[0], gpi_ere->dword[1],
-			gpi_ere->dword[2], gpi_ere->dword[3]);
 		gpi_tre = tre;
-		dev_dbg(gpii->gpi_dev->dev,
-			"Pending TRE: %08x %08x %08x %08x\n",
-			gpi_tre->dword[0], gpi_tre->dword[1],
-			gpi_tre->dword[2], gpi_tre->dword[3]);
+		trace_gpi_ev_no_desc(gpii->gpi_dev->dev, imed_event->chid,
+				     gpi_ere->dword, gpi_tre->dword);
 		return;
 	}
 	gpi_desc = to_gpi_desc(vd);
@@ -1064,11 +1052,10 @@ static void gpi_process_xfer_compl_event(struct gchan *gchan,
 		dev_err(gpii->gpi_dev->dev, "Error in Transaction\n");
 		result.result = DMA_TRANS_ABORTED;
 	} else {
-		dev_dbg(gpii->gpi_dev->dev, "Transaction Success\n");
 		result.result = DMA_TRANS_NOERROR;
 	}
 	result.residue = gpi_desc->len - compl_event->length;
-	dev_dbg(gpii->gpi_dev->dev, "Residue %d\n", result.residue);
+	trace_gpi_xfer_result(gpii->gpi_dev->dev, chid, result.result, result.residue);
 
 	dma_cookie_complete(&vd->tx);
 	dmaengine_desc_get_callback_invoke(&vd->tx, &result);
@@ -1100,11 +1087,8 @@ static void gpi_process_events(struct gpii *gpii)
 			chid = gpi_event->xfer_compl_event.chid;
 			type = gpi_event->xfer_compl_event.type;
 
-			dev_dbg(gpii->gpi_dev->dev,
-				"Event: CHID:%u, type:%x %08x %08x %08x %08x\n",
-				chid, type, gpi_event->gpi_ere.dword[0],
-				gpi_event->gpi_ere.dword[1], gpi_event->gpi_ere.dword[2],
-				gpi_event->gpi_ere.dword[3]);
+			trace_gpi_process_event(gpii->gpi_dev->dev, chid, type,
+						gpi_event->gpi_ere.dword);
 
 			switch (type) {
 			case XFER_COMPLETE_EV_TYPE:
@@ -1113,7 +1097,6 @@ static void gpi_process_events(struct gpii *gpii)
 							     &gpi_event->xfer_compl_event);
 				break;
 			case STALE_EV_TYPE:
-				dev_dbg(gpii->gpi_dev->dev, "stale event, not processing\n");
 				break;
 			case IMMEDIATE_DATA_EV_TYPE:
 				gchan = &gpii->gchan[chid];
@@ -1121,11 +1104,8 @@ static void gpi_process_events(struct gpii *gpii)
 							    &gpi_event->immediate_data_event);
 				break;
 			case QUP_NOTIF_EV_TYPE:
-				dev_dbg(gpii->gpi_dev->dev, "QUP_NOTIF_EV_TYPE\n");
 				break;
 			default:
-				dev_dbg(gpii->gpi_dev->dev,
-					"not supported event type:0x%x\n", type);
 			}
 			gpi_ring_recycle_ev_element(ev_ring);
 		}
@@ -1409,10 +1389,8 @@ static int gpi_alloc_ring(struct gpi_ring *ring, u32 elements,
 		bit++;
 	len = 1 << bit;
 	ring->alloc_size = (len + (len - 1));
-	dev_dbg(gpii->gpi_dev->dev,
-		"#el:%u el_size:%u len:%u actual_len:%llu alloc_size:%zu\n",
-		  elements, el_size, (elements * el_size), len,
-		  ring->alloc_size);
+	trace_gpi_alloc_ring(gpii->gpi_dev->dev, elements, el_size,
+			     (elements * el_size), len, ring->alloc_size);
 
 	ring->pre_aligned = dma_alloc_coherent(gpii->gpi_dev->dev,
 					       ring->alloc_size,
@@ -1437,10 +1415,8 @@ static int gpi_alloc_ring(struct gpi_ring *ring, u32 elements,
 	/* update to other cores */
 	smp_wmb();
 
-	dev_dbg(gpii->gpi_dev->dev,
-		"phy_pre:%pad phy_alig:%pa len:%u el_size:%u elements:%u\n",
-		&ring->dma_handle, &ring->phys_addr, ring->len,
-		ring->el_size, ring->elements);
+	trace_gpi_ring_info(gpii->gpi_dev->dev, ring->dma_handle, ring->phys_addr,
+			    ring->len, ring->el_size, ring->elements);
 
 	return 0;
 }
@@ -1542,7 +1518,7 @@ static int gpi_pause(struct dma_chan *chan)
 	 * client needs to call pause only once
 	 */
 	if (gpii->pm_state == PAUSE_STATE) {
-		dev_dbg(gpii->gpi_dev->dev, "channel is already paused\n");
+		trace_gpi_already_state(gpii->gpi_dev->dev, gpii->gpii_id, gpii->pm_state);
 		mutex_unlock(&gpii->ctrl_lock);
 		return 0;
 	}
@@ -1578,7 +1554,7 @@ static int gpi_resume(struct dma_chan *chan)
 
 	mutex_lock(&gpii->ctrl_lock);
 	if (gpii->pm_state == ACTIVE_STATE) {
-		dev_dbg(gpii->gpi_dev->dev, "channel is already active\n");
+		trace_gpi_already_state(gpii->gpi_dev->dev, gpii->gpii_id, gpii->pm_state);
 		mutex_unlock(&gpii->ctrl_lock);
 		return 0;
 	}
@@ -1703,8 +1679,7 @@ static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
 	}
 
 	for (i = 0; i < tre_idx; i++)
-		dev_dbg(dev, "TRE:%d %x:%x:%x:%x\n", i, desc->tre[i].dword[0],
-			desc->tre[i].dword[1], desc->tre[i].dword[2], desc->tre[i].dword[3]);
+		trace_gpi_tre(dev, chan->chid, i, desc->tre[i].dword);
 
 	return tre_idx;
 }
@@ -1797,8 +1772,7 @@ static int gpi_create_spi_tre(struct gchan *chan, struct gpi_desc *desc,
 					 TRE_FLAGS_IEOT);
 
 	for (i = 0; i < tre_idx; i++)
-		dev_dbg(dev, "TRE:%d %x:%x:%x:%x\n", i, desc->tre[i].dword[0],
-			desc->tre[i].dword[1], desc->tre[i].dword[2], desc->tre[i].dword[3]);
+		trace_gpi_tre(dev, chan->chid, i, desc->tre[i].dword);
 
 	return tre_idx;
 }

-- 
2.34.1

Re: [PATCH v2 3/3] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:

Pre-existing issues:
- [High] Unvalidated hardware event 'chid' used as an array index.
- [High] Unvalidated hardware event 'tre_index' leads to out-of-bounds memory read.
- [High] The driver drops gchan->vc.lock while a DMA descriptor remains on the virt-dma issued list, leading to a race with channel termination and a use-after-free.
- [High] Error path in gpi_resume() leaves the hardware IRQ enabled while the software state remains paused, leading to unbalanced IRQ enables upon retry.
- [High] The driver does not validate the physical address (`ptr`) returned in a DMA completion event, allowing untrusted input to corrupt ring buffer pointers.
- [High] Missing dma_rmb() before reading the DMA event ring buffer.
- [High] Missing memory barrier before ringing the event doorbell in gpi_process_events().
- [High] Missing dma_wmb() before writing to the channel doorbell in gpi_issue_pending().
--

commit 1bf9fce9a4cdb676de685f3966a35631e5323808
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>

dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints

Replace the remaining dev_dbg() based debug logging in the GPI DMA
driver with the qcom_gpi tracepoints, providing structured runtime
visibility into GPI DMA behavior without requiring invasive debug
patches. dev_err() calls are left untouched.

> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index b09354a73b46e..8e7d25a461475 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
> @@ -945,17 +940,10 @@ static void gpi_process_imed_data_event(struct gchan *gchan,
>  		struct gpi_tre *gpi_tre;
>  
>  		spin_unlock_irqrestore(&gchan->vc.lock, flags);
> -		dev_dbg(gpii->gpi_dev->dev, "event without a pending descriptor!\n");
>  		gpi_ere = (struct gpi_ere *)imed_event;
> -		dev_dbg(gpii->gpi_dev->dev,
> -			"Event: %08x %08x %08x %08x\n",
> -			gpi_ere->dword[0], gpi_ere->dword[1],
> -			gpi_ere->dword[2], gpi_ere->dword[3]);
>  		gpi_tre = tre;
> -		dev_dbg(gpii->gpi_dev->dev,
> -			"Pending TRE: %08x %08x %08x %08x\n",
> -			gpi_tre->dword[0], gpi_tre->dword[1],
> -			gpi_tre->dword[2], gpi_tre->dword[3]);
> +		trace_gpi_ev_no_desc(gpii->gpi_dev->dev, imed_event->chid,
> +				     gpi_ere->dword, gpi_tre->dword);
>  		return;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does this code lead to an
out-of-bounds memory read?

drivers/dma/qcom/gpi.c:gpi_process_imed_data_event() {
    ...
    void *tre = ch_ring->base + (ch_ring->el_size * imed_event->tre_index);
    ...
    gpi_tre = tre;
    trace_gpi_ev_no_desc(gpii->gpi_dev->dev, imed_event->chid,
                         gpi_ere->dword, gpi_tre->dword);
}

The tre_index is read from imed_event->tre_index without a bounds check against
ch_ring->elements. If it exceeds the ring size, will the tracepoint dereference
out-of-bounds memory?

[ ... ]

> @@ -1064,11 +1052,10 @@ static void gpi_process_xfer_compl_event(struct gchan *gchan,
>  		dev_err(gpii->gpi_dev->dev, "Error in Transaction\n");
>  		result.result = DMA_TRANS_ABORTED;
>  	} else {
> -		dev_dbg(gpii->gpi_dev->dev, "Transaction Success\n");
>  		result.result = DMA_TRANS_NOERROR;
>  	}
>  	result.residue = gpi_desc->len - compl_event->length;
> -	dev_dbg(gpii->gpi_dev->dev, "Residue %d\n", result.residue);
> +	trace_gpi_xfer_result(gpii->gpi_dev->dev, chid, result.result, result.residue);
>  
>  	dma_cookie_complete(&vd->tx);
>  	dmaengine_desc_get_callback_invoke(&vd->tx, &result);

[Severity: High]
This is a pre-existing issue, but does this driver validate the physical address
returned in a DMA completion event?

drivers/dma/qcom/gpi.c:gpi_process_xfer_compl_event() {
    ...
    void *ev_rp = to_virtual(ch_ring, compl_event->ptr);
    ...
}

The compl_event->ptr is a physical address provided by the hardware DMA event,
and to_virtual() converts it without bounds checking. If the hardware yields an
invalid ptr outside the physical bounds of the ring buffer, can this corrupt the
ring pointer tracking by assigning a bogus virtual pointer to ch_ring->rp?

[Severity: High]
This isn't a bug introduced by this patch, but can this code race with channel
termination and cause a use-after-free?

drivers/dma/qcom/gpi.c:gpi_process_xfer_compl_event() {
    ...
gpi_free_desc:
    spin_lock_irqsave(&gchan->vc.lock, flags);
    list_del(&vd->node);
    spin_unlock_irqrestore(&gchan->vc.lock, flags);
    kfree(gpi_desc);
}

The interrupt handler drops gchan->vc.lock earlier after getting the descriptor
via vchan_next_desc(), without removing it from the desc_issued list.

If a concurrent vchan_terminate_all() acquires the lock, removes, and frees the
descriptor, the IRQ handler will then resume, modify the freed descriptor
(dma_cookie_complete), and call list_del(&vd->node) on freed memory.

This also applies to gpi_process_imed_data_event() where the same pattern exists.

[ ... ]

> @@ -1100,11 +1087,8 @@ static void gpi_process_events(struct gpii *gpii)
>  			chid = gpi_event->xfer_compl_event.chid;
>  			type = gpi_event->xfer_compl_event.type;
>  
> -			dev_dbg(gpii->gpi_dev->dev,
> -				"Event: CHID:%u, type:%x %08x %08x %08x %08x\n",
> -				chid, type, gpi_event->gpi_ere.dword[0],
> -				gpi_event->gpi_ere.dword[1], gpi_event->gpi_ere.dword[2],
> -				gpi_event->gpi_ere.dword[3]);
> +			trace_gpi_process_event(gpii->gpi_dev->dev, chid, type,
> +						gpi_event->gpi_ere.dword);
>  
>  			switch (type) {
>  			case XFER_COMPLETE_EV_TYPE:
>  				gchan = &gpii->gchan[chid];

[Severity: High]
This isn't a bug introduced by this patch, but is a dma_rmb() missing before
reading the DMA event ring buffer?

drivers/dma/qcom/gpi.c:gpi_process_events() {
    cntxt_rp = gpi_read_reg(gpii, gpii->ev_ring_rp_lsb_reg);
    rp = to_virtual(ev_ring, cntxt_rp);

    do {
        while (rp != ev_ring->rp) {
            gpi_event = ev_ring->rp;
    ...
}

Because gpi_read_reg() wraps readl_relaxed(), it provides no ordering guarantees
against normal memory reads. Can the CPU speculatively fetch from the DMA-mapped
event ring before the register read completes and confirms there is new data?

[Severity: High]
This is a pre-existing issue, but does this code use an unvalidated hardware
event chid as an array index?

drivers/dma/qcom/gpi.c:gpi_process_events() {
    ...
    chid = gpi_event->xfer_compl_event.chid;
    type = gpi_event->xfer_compl_event.type;

    trace_gpi_process_event(gpii->gpi_dev->dev, chid, type,
                            gpi_event->gpi_ere.dword);

    switch (type) {
    case XFER_COMPLETE_EV_TYPE:
        gchan = &gpii->gchan[chid];
    ...
}

If the hardware is compromised or fails and sends a chid value greater than or
equal to MAX_CHANNELS_PER_GPII, indexing gpii->gchan[chid] will read out of
bounds. Can this corrupt adjacent memory or crash the kernel?

[ ... ]

> @@ -1121,11 +1104,8 @@ static void gpi_process_events(struct gpii *gpii)
>  							    &gpi_event->immediate_data_event);
>  				break;
>  			case QUP_NOTIF_EV_TYPE:
> -				dev_dbg(gpii->gpi_dev->dev, "QUP_NOTIF_EV_TYPE\n");
>  				break;
>  			default:
> -				dev_dbg(gpii->gpi_dev->dev,
> -					"not supported event type:0x%x\n", type);
>  			}
>  			gpi_ring_recycle_ev_element(ev_ring);
>  		}

[Severity: High]
This is a pre-existing issue, but is a memory barrier missing before ringing
the event doorbell?

drivers/dma/qcom/gpi.c:gpi_process_events() {
    ...
            gpi_ring_recycle_ev_element(ev_ring);
        }
        gpi_write_ev_db(gpii, ev_ring, ev_ring->wp);
    ...
}

The gpi_write_ev_db() wraps writel_relaxed(). The smp_wmb() inside
gpi_ring_recycle_ev_element() only orders normal memory writes, not prior memory
reads against subsequent MMIO writes. Does this mean the hardware might
overwrite the DMA event ring descriptor before the CPU actually finishes reading
it? A full mb() or rmb() is typically required before the doorbell.

[ ... ]

> @@ -1578,7 +1554,7 @@ static int gpi_resume(struct dma_chan *chan)
>  
>  	mutex_lock(&gpii->ctrl_lock);
>  	if (gpii->pm_state == ACTIVE_STATE) {
> -		dev_dbg(gpii->gpi_dev->dev, "channel is already active\n");
> +		trace_gpi_already_state(gpii->gpi_dev->dev, gpii->gpii_id, gpii->pm_state);
>  		mutex_unlock(&gpii->ctrl_lock);
>  		return 0;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does this error path leave the
hardware IRQ enabled while the software state remains paused?

drivers/dma/qcom/gpi.c:gpi_resume() {
    ...
    enable_irq(gpii->irq);

    for (i = 0; i < MAX_CHANNELS_PER_GPII; i++) {
        ret = gpi_send_cmd(gpii, &gpii->gchan[i], GPI_CH_CMD_START);
        if (ret) {
            dev_err(gpii->gpi_dev->dev, "Error starting chan, ret:%d\n", ret);
            mutex_unlock(&gpii->ctrl_lock);
            return ret;
        }
    ...
}

If gpi_send_cmd() fails, it returns without disabling the IRQ, and pm_state
remains PAUSE_STATE. On the next resume attempt, will it bypass the active state
check and call enable_irq() a second time, resulting in an unbalanced IRQ enable
and a kernel warning?

[Severity: High]
This is a pre-existing issue, but is a dma_wmb() missing before writing to the
channel doorbell?

drivers/dma/qcom/gpi.c:gpi_issue_pending() {
    for (i = 0; i < gpi_desc->num_tre; i++) {
        tre = &gpi_desc->tre[i];
        gpi_queue_xfer(gpii, gchan, tre, &wp);
    }

    gpi_desc->db = ch_ring->wp;
    gpi_write_ch_db(gchan, &gchan->ch_ring, gpi_desc->db);
}

Descriptors are written to the DMA ring using memcpy() in gpi_queue_xfer(), and
then the doorbell is rung via gpi_write_ch_db(), which wraps writel_relaxed().
Since writel_relaxed() does not order against prior normal memory writes, can
the doorbell write reach the device before the memcpy() is visible in RAM?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-add-trace-support-gpio-v2-0-01f81739bb88@oss.qualcomm.com?part=3

Re: [PATCH v2 3/3] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints

[Frank Li]

On Wed, Sep 09, 2026 at 10:39:43AM +0530, Praveen Talari wrote:
> Replace the remaining dev_dbg() based debug logging in the GPI DMA
> driver with the qcom_gpi tracepoints, providing structured runtime
> visibility into GPI DMA behavior without requiring invasive debug
> patches. dev_err() calls are left untouched.
>
> Tracepoints are added at the same points as the dev_dbg() calls they
> replace: channel/event command dispatch in gpi_send_cmd(), error and
> top-level interrupt status in gpi_process_gen_err_irq() and
> gpi_handle_irq(), event/channel control state transitions in the IRQ
> handler and gpi_process_ch_ctrl_irq(), completion event handling
> (including the no-pending-descriptor and transfer result paths) in
> gpi_process_imed_data_event(), gpi_process_xfer_compl_event() and
> gpi_process_events(), ring allocation details in gpi_alloc_ring(),
> already-in-state checks in gpi_pause()/gpi_resume(), TRE contents in
> gpi_create_i2c_tre()/gpi_create_spi_tre(), and TRE queuing in
> gpi_issue_pending().

needn't this paragraph. The first paragraph is clear enough.

Frank
>
> Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
> ---
>  drivers/dma/qcom/gpi.c | 58 ++++++++++++++------------------------------------
>  1 file changed, 16 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index b09354a73b46..8e7d25a46147 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
> @@ -684,8 +684,6 @@ static int gpi_send_cmd(struct gpii *gpii, struct gchan *gchan,
>  	if (IS_CHAN_CMD(gpi_cmd))
>  		chid = gchan->chid;
>
> -	dev_dbg(gpii->gpi_dev->dev,
> -		"sending cmd: %s:%u\n", TO_GPI_CMD_STR(gpi_cmd), chid);
>  	trace_gpi_send_cmd(gpii->gpi_dev->dev, chid, gpi_cmd, TO_GPI_CMD_STR(gpi_cmd));
>
>  	/* send opcode and wait for completion */
> @@ -797,7 +795,7 @@ static void gpi_process_gen_err_irq(struct gpii *gpii)
>  	u32 irq_stts = gpi_read_reg(gpii, gpii->regs + offset);
>
>  	/* clear the status */
> -	dev_dbg(gpii->gpi_dev->dev, "irq_stts:0x%x\n", irq_stts);
> +	trace_gpi_gen_err_irq(gpii->gpi_dev->dev, gpii_id, irq_stts);
>
>  	/* Clear the register */
>  	offset = GPII_n_CNTXT_GPII_IRQ_CLR_OFFS(gpii_id);
> @@ -866,8 +864,6 @@ static irqreturn_t gpi_handle_irq(int irq, void *data)
>  			u32 ev_state;
>  			u32 ev_ch_irq;
>
> -			dev_dbg(gpii->gpi_dev->dev,
> -				"processing EV CTRL interrupt\n");
>  			offset = GPII_n_CNTXT_SRC_EV_CH_IRQ_OFFS(gpii_id);
>  			ev_ch_irq = gpi_read_reg(gpii, gpii->regs + offset);
>
> @@ -887,15 +883,14 @@ static irqreturn_t gpi_handle_irq(int irq, void *data)
>  				ev_state = DEFAULT_EV_CH_STATE;
>
>  			gpii->ev_state = ev_state;
> -			dev_dbg(gpii->gpi_dev->dev, "setting EV state to %s\n",
> -				TO_GPI_EV_STATE_STR(gpii->ev_state));
> +			trace_gpi_ev_ctrl_irq(gpii->gpi_dev->dev, gpii_id, ev_ch_irq,
> +					      gpii->ev_state);
>  			complete_all(&gpii->cmd_completion);
>  			type &= ~(GPII_n_CNTXT_TYPE_IRQ_MSK_EV_CTRL);
>  		}
>
>  		/* channel control irq */
>  		if (type & GPII_n_CNTXT_TYPE_IRQ_MSK_CH_CTRL) {
> -			dev_dbg(gpii->gpi_dev->dev, "process CH CTRL interrupts\n");
>  			gpi_process_ch_ctrl_irq(gpii);
>  			type &= ~(GPII_n_CNTXT_TYPE_IRQ_MSK_CH_CTRL);
>  		}
> @@ -945,17 +940,10 @@ static void gpi_process_imed_data_event(struct gchan *gchan,
>  		struct gpi_tre *gpi_tre;
>
>  		spin_unlock_irqrestore(&gchan->vc.lock, flags);
> -		dev_dbg(gpii->gpi_dev->dev, "event without a pending descriptor!\n");
>  		gpi_ere = (struct gpi_ere *)imed_event;
> -		dev_dbg(gpii->gpi_dev->dev,
> -			"Event: %08x %08x %08x %08x\n",
> -			gpi_ere->dword[0], gpi_ere->dword[1],
> -			gpi_ere->dword[2], gpi_ere->dword[3]);
>  		gpi_tre = tre;
> -		dev_dbg(gpii->gpi_dev->dev,
> -			"Pending TRE: %08x %08x %08x %08x\n",
> -			gpi_tre->dword[0], gpi_tre->dword[1],
> -			gpi_tre->dword[2], gpi_tre->dword[3]);
> +		trace_gpi_ev_no_desc(gpii->gpi_dev->dev, imed_event->chid,
> +				     gpi_ere->dword, gpi_tre->dword);
>  		return;
>  	}
>  	gpi_desc = to_gpi_desc(vd);
> @@ -1064,11 +1052,10 @@ static void gpi_process_xfer_compl_event(struct gchan *gchan,
>  		dev_err(gpii->gpi_dev->dev, "Error in Transaction\n");
>  		result.result = DMA_TRANS_ABORTED;
>  	} else {
> -		dev_dbg(gpii->gpi_dev->dev, "Transaction Success\n");
>  		result.result = DMA_TRANS_NOERROR;
>  	}
>  	result.residue = gpi_desc->len - compl_event->length;
> -	dev_dbg(gpii->gpi_dev->dev, "Residue %d\n", result.residue);
> +	trace_gpi_xfer_result(gpii->gpi_dev->dev, chid, result.result, result.residue);
>
>  	dma_cookie_complete(&vd->tx);
>  	dmaengine_desc_get_callback_invoke(&vd->tx, &result);
> @@ -1100,11 +1087,8 @@ static void gpi_process_events(struct gpii *gpii)
>  			chid = gpi_event->xfer_compl_event.chid;
>  			type = gpi_event->xfer_compl_event.type;
>
> -			dev_dbg(gpii->gpi_dev->dev,
> -				"Event: CHID:%u, type:%x %08x %08x %08x %08x\n",
> -				chid, type, gpi_event->gpi_ere.dword[0],
> -				gpi_event->gpi_ere.dword[1], gpi_event->gpi_ere.dword[2],
> -				gpi_event->gpi_ere.dword[3]);
> +			trace_gpi_process_event(gpii->gpi_dev->dev, chid, type,
> +						gpi_event->gpi_ere.dword);
>
>  			switch (type) {
>  			case XFER_COMPLETE_EV_TYPE:
> @@ -1113,7 +1097,6 @@ static void gpi_process_events(struct gpii *gpii)
>  							     &gpi_event->xfer_compl_event);
>  				break;
>  			case STALE_EV_TYPE:
> -				dev_dbg(gpii->gpi_dev->dev, "stale event, not processing\n");
>  				break;
>  			case IMMEDIATE_DATA_EV_TYPE:
>  				gchan = &gpii->gchan[chid];
> @@ -1121,11 +1104,8 @@ static void gpi_process_events(struct gpii *gpii)
>  							    &gpi_event->immediate_data_event);
>  				break;
>  			case QUP_NOTIF_EV_TYPE:
> -				dev_dbg(gpii->gpi_dev->dev, "QUP_NOTIF_EV_TYPE\n");
>  				break;
>  			default:
> -				dev_dbg(gpii->gpi_dev->dev,
> -					"not supported event type:0x%x\n", type);
>  			}
>  			gpi_ring_recycle_ev_element(ev_ring);
>  		}
> @@ -1409,10 +1389,8 @@ static int gpi_alloc_ring(struct gpi_ring *ring, u32 elements,
>  		bit++;
>  	len = 1 << bit;
>  	ring->alloc_size = (len + (len - 1));
> -	dev_dbg(gpii->gpi_dev->dev,
> -		"#el:%u el_size:%u len:%u actual_len:%llu alloc_size:%zu\n",
> -		  elements, el_size, (elements * el_size), len,
> -		  ring->alloc_size);
> +	trace_gpi_alloc_ring(gpii->gpi_dev->dev, elements, el_size,
> +			     (elements * el_size), len, ring->alloc_size);
>
>  	ring->pre_aligned = dma_alloc_coherent(gpii->gpi_dev->dev,
>  					       ring->alloc_size,
> @@ -1437,10 +1415,8 @@ static int gpi_alloc_ring(struct gpi_ring *ring, u32 elements,
>  	/* update to other cores */
>  	smp_wmb();
>
> -	dev_dbg(gpii->gpi_dev->dev,
> -		"phy_pre:%pad phy_alig:%pa len:%u el_size:%u elements:%u\n",
> -		&ring->dma_handle, &ring->phys_addr, ring->len,
> -		ring->el_size, ring->elements);
> +	trace_gpi_ring_info(gpii->gpi_dev->dev, ring->dma_handle, ring->phys_addr,
> +			    ring->len, ring->el_size, ring->elements);
>
>  	return 0;
>  }
> @@ -1542,7 +1518,7 @@ static int gpi_pause(struct dma_chan *chan)
>  	 * client needs to call pause only once
>  	 */
>  	if (gpii->pm_state == PAUSE_STATE) {
> -		dev_dbg(gpii->gpi_dev->dev, "channel is already paused\n");
> +		trace_gpi_already_state(gpii->gpi_dev->dev, gpii->gpii_id, gpii->pm_state);
>  		mutex_unlock(&gpii->ctrl_lock);
>  		return 0;
>  	}
> @@ -1578,7 +1554,7 @@ static int gpi_resume(struct dma_chan *chan)
>
>  	mutex_lock(&gpii->ctrl_lock);
>  	if (gpii->pm_state == ACTIVE_STATE) {
> -		dev_dbg(gpii->gpi_dev->dev, "channel is already active\n");
> +		trace_gpi_already_state(gpii->gpi_dev->dev, gpii->gpii_id, gpii->pm_state);
>  		mutex_unlock(&gpii->ctrl_lock);
>  		return 0;
>  	}
> @@ -1703,8 +1679,7 @@ static int gpi_create_i2c_tre(struct gchan *chan, struct gpi_desc *desc,
>  	}
>
>  	for (i = 0; i < tre_idx; i++)
> -		dev_dbg(dev, "TRE:%d %x:%x:%x:%x\n", i, desc->tre[i].dword[0],
> -			desc->tre[i].dword[1], desc->tre[i].dword[2], desc->tre[i].dword[3]);
> +		trace_gpi_tre(dev, chan->chid, i, desc->tre[i].dword);
>
>  	return tre_idx;
>  }
> @@ -1797,8 +1772,7 @@ static int gpi_create_spi_tre(struct gchan *chan, struct gpi_desc *desc,
>  					 TRE_FLAGS_IEOT);
>
>  	for (i = 0; i < tre_idx; i++)
> -		dev_dbg(dev, "TRE:%d %x:%x:%x:%x\n", i, desc->tre[i].dword[0],
> -			desc->tre[i].dword[1], desc->tre[i].dword[2], desc->tre[i].dword[3]);
> +		trace_gpi_tre(dev, chan->chid, i, desc->tre[i].dword);
>
>  	return tre_idx;
>  }
>
> --
> 2.34.1
>