* [PATCH v3 0/2] dmaengine: qcom: gpi: Add trace event support
@ 2026-09-25 20:51 Praveen Talari
2026-09-25 20:51 ` [PATCH v3 1/2] " Praveen Talari
2026-09-25 20:51 ` [PATCH v3 2/2] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints Praveen Talari
0 siblings, 2 replies; 7+ messages in thread
From: Praveen Talari @ 2026-09-25 20:51 UTC (permalink / raw)
To: konrad.dybcio, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Vinod Koul, Frank Li
Cc: chandana.chiluveru, mukesh.savaliya, linux-kernel,
linux-trace-kernel, linux-arm-msm, dmaengine, 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 (private to the driver,
in drivers/dma/qcom/gpi_trace.h) and wires the tracepoints into the
driver alongside the existing dev_dbg() calls.
Patch 2 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 v3:
- Move the qcom_gpi trace events header from
include/trace/events/qcom_gpi.h into drivers/dma/qcom/gpi_trace.h,
since it has no users outside the GPI DMA driver, and squash it
into the tracepoint-wiring patch.
- Drop the redundant per-function breakdown paragraph from the
dev_dbg()-to-tracepoint conversion patch; the first paragraph
already describes the change.
- Link to v2: https://patch.msgid.link/20260909-add-trace-support-gpio-v2-0-01f81739bb88@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 (2):
dmaengine: qcom: gpi: Add trace event support
dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints
drivers/dma/qcom/gpi.c | 69 ++++------
drivers/dma/qcom/gpi_trace.h | 317 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 344 insertions(+), 42 deletions(-)
---
base-commit: 4c253ac4b29b8c6cc6fdef8f92d4facde62e63b9
change-id: 20260831-add-trace-support-gpio-99eb54d42729
Best regards,
--
Praveen Talari <praveen.talari@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v3 1/2] dmaengine: qcom: gpi: Add trace event support 2026-09-25 20:51 [PATCH v3 0/2] dmaengine: qcom: gpi: Add trace event support Praveen Talari @ 2026-09-25 20:51 ` Praveen Talari 2026-09-25 21:02 ` sashiko-bot 2026-10-13 21:44 ` Frank Li 2026-09-25 20:51 ` [PATCH v3 2/2] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints Praveen Talari 1 sibling, 2 replies; 7+ messages in thread From: Praveen Talari @ 2026-09-25 20:51 UTC (permalink / raw) To: konrad.dybcio, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Vinod Koul, Frank Li Cc: chandana.chiluveru, mukesh.savaliya, linux-kernel, linux-trace-kernel, linux-arm-msm, dmaengine, Praveen Talari Add tracepoint support to the Qualcomm GPI DMA engine driver, giving structured runtime visibility into GPI DMA behavior without requiring invasive debug patches. The events 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. Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com> --- drivers/dma/qcom/gpi.c | 11 ++ drivers/dma/qcom/gpi_trace.h | 317 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 328 insertions(+) diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c index c41bfac65ddf..fafe6ac848d9 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 "gpi_trace.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); diff --git a/drivers/dma/qcom/gpi_trace.h b/drivers/dma/qcom/gpi_trace.h new file mode 100644 index 000000000000..f16d156833b9 --- /dev/null +++ b/drivers/dma/qcom/gpi_trace.h @@ -0,0 +1,317 @@ +/* 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 */ +#undef TRACE_INCLUDE_PATH +#undef TRACE_INCLUDE_FILE +#define TRACE_INCLUDE_PATH ../../drivers/dma/qcom +#define TRACE_INCLUDE_FILE gpi_trace +#include <trace/define_trace.h> -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] dmaengine: qcom: gpi: Add trace event support 2026-09-25 20:51 ` [PATCH v3 1/2] " Praveen Talari @ 2026-09-25 21:02 ` sashiko-bot 2026-10-13 21:44 ` Frank Li 1 sibling, 0 replies; 7+ messages in thread From: sashiko-bot @ 2026-09-25 21:02 UTC (permalink / raw) To: Praveen Talari; +Cc: Frank.Li, dmaengine, linux-trace-kernel, vkoul Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dmaengine: gpi: dropped trace events for coalesced interrupts -- commit 0bb8d8f2455ccff160b2666b22bf4854db9eb970 Author: Praveen Talari <praveen.talari@oss.qualcomm.com> dmaengine: qcom: gpi: Add trace event support This commit adds tracepoint support to the Qualcomm GPI DMA engine driver for structured runtime visibility into GPI DMA behavior. The events cover channel command dispatch, IRQ type handling, and state transitions among other details. > diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c > index c41bfac65ddf3..fafe6ac848d94 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); [Severity: Medium] Will this miss tracing coalesced interrupts? Because trace_gpi_irq_status() is placed strictly before the do { ... } while (type); loop in gpi_handle_irq(), when new interrupts arrive during the handler execution and the loop re-reads the type register, the tracepoint is not called again for the newly read type. Should the tracepoint be moved inside the loop, or placed iteratively after every register read? > > do { > /* global gpii error */ -- Sashiko AI review · https://sashiko.dev/#/patchset/20260926-add-trace-support-gpio-v3-0-8572bf7456b2@oss.qualcomm.com?part=1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] dmaengine: qcom: gpi: Add trace event support 2026-09-25 20:51 ` [PATCH v3 1/2] " Praveen Talari 2026-09-25 21:02 ` sashiko-bot @ 2026-10-13 21:44 ` Frank Li 1 sibling, 0 replies; 7+ messages in thread From: Frank Li @ 2026-10-13 21:44 UTC (permalink / raw) To: Praveen Talari Cc: konrad.dybcio, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Vinod Koul, Frank Li, chandana.chiluveru, mukesh.savaliya, linux-kernel, linux-trace-kernel, linux-arm-msm, dmaengine On Sat, Sep 26, 2026 at 02:21:35AM +0530, Praveen Talari wrote: > Add tracepoint support to the Qualcomm GPI DMA engine driver, giving > structured runtime visibility into GPI DMA behavior without requiring > invasive debug patches. > > The events 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. > > Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com> > --- Reviewed-by: Frank Li <Frank.Li@nxp.com> > drivers/dma/qcom/gpi.c | 11 ++ > drivers/dma/qcom/gpi_trace.h | 317 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 328 insertions(+) > > diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c > index c41bfac65ddf..fafe6ac848d9 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 "gpi_trace.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); > diff --git a/drivers/dma/qcom/gpi_trace.h b/drivers/dma/qcom/gpi_trace.h > new file mode 100644 > index 000000000000..f16d156833b9 > --- /dev/null > +++ b/drivers/dma/qcom/gpi_trace.h > @@ -0,0 +1,317 @@ > +/* 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 */ > +#undef TRACE_INCLUDE_PATH > +#undef TRACE_INCLUDE_FILE > +#define TRACE_INCLUDE_PATH ../../drivers/dma/qcom > +#define TRACE_INCLUDE_FILE gpi_trace > +#include <trace/define_trace.h> > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 2/2] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints 2026-09-25 20:51 [PATCH v3 0/2] dmaengine: qcom: gpi: Add trace event support Praveen Talari 2026-09-25 20:51 ` [PATCH v3 1/2] " Praveen Talari @ 2026-09-25 20:51 ` Praveen Talari 2026-09-25 21:02 ` sashiko-bot 1 sibling, 1 reply; 7+ messages in thread From: Praveen Talari @ 2026-09-25 20:51 UTC (permalink / raw) To: konrad.dybcio, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Vinod Koul, Frank Li Cc: chandana.chiluveru, mukesh.savaliya, linux-kernel, linux-trace-kernel, linux-arm-msm, dmaengine, 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. 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 fafe6ac848d9..6aff2a0d5416 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints 2026-09-25 20:51 ` [PATCH v3 2/2] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints Praveen Talari @ 2026-09-25 21:02 ` sashiko-bot 2026-10-13 21:48 ` Frank Li 0 siblings, 1 reply; 7+ messages in thread From: sashiko-bot @ 2026-09-25 21:02 UTC (permalink / raw) To: Praveen Talari; +Cc: dmaengine, linux-trace-kernel, vkoul, Frank.Li Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] dmaengine: qcom: gpi: unused variable gpi_ev_state_str causes compiler warning -- commit 16ff3240f7873902688743d98570cd5eebf13594 Author: Praveen Talari <praveen.talari@oss.qualcomm.com> dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints This commit replaces dev_dbg() based debug logging in the GPI DMA driver with tracepoints, providing structured runtime visibility. > diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c > index fafe6ac848d94..6aff2a0d54161 100644 > --- a/drivers/dma/qcom/gpi.c > +++ b/drivers/dma/qcom/gpi.c [ ... ] > @@ -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); [Severity: Low] Is it possible that this change in gpi_handle_irq() within drivers/dma/qcom/gpi.c leaves the gpi_ev_state_str static array and TO_GPI_EV_STATE_STR macro as orphaned dead code? Since trace_gpi_ev_ctrl_irq() only takes the integer value, the final consumer of the macro is removed. Could this regression trigger a -Wunused-variable compiler warning for the unreferenced static array when compiling the driver with standard kernel warning flags enabled? For reference, the array is defined earlier in the file as: static const char *const gpi_ev_state_str[MAX_EV_STATES] = { [EV_STATE_NOT_ALLOCATED] = "NOT ALLOCATED", [EV_STATE_ALLOCATED] = "ALLOCATED", }; #define TO_GPI_EV_STATE_STR(_state) (((_state) >= MAX_EV_STATES) ? \ "INVALID" : gpi_ev_state_str[(_state)]) -- Sashiko AI review · https://sashiko.dev/#/patchset/20260926-add-trace-support-gpio-v3-0-8572bf7456b2@oss.qualcomm.com?part=2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints 2026-09-25 21:02 ` sashiko-bot @ 2026-10-13 21:48 ` Frank Li 0 siblings, 0 replies; 7+ messages in thread From: Frank Li @ 2026-10-13 21:48 UTC (permalink / raw) To: sashiko-reviews Cc: Praveen Talari, dmaengine, linux-trace-kernel, vkoul, Frank.Li On Fri, Sep 25, 2026 at 09:02:44PM +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Low] dmaengine: qcom: gpi: unused variable gpi_ev_state_str causes compiler warning > -- > > commit 16ff3240f7873902688743d98570cd5eebf13594 > Author: Praveen Talari <praveen.talari@oss.qualcomm.com> > > dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints > > This commit replaces dev_dbg() based debug logging in the GPI DMA driver > with tracepoints, providing structured runtime visibility. > > > diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c > > index fafe6ac848d94..6aff2a0d54161 100644 > > --- a/drivers/dma/qcom/gpi.c > > +++ b/drivers/dma/qcom/gpi.c > [ ... ] > > @@ -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); > > [Severity: Low] > Is it possible that this change in gpi_handle_irq() within > drivers/dma/qcom/gpi.c leaves the gpi_ev_state_str static array and > TO_GPI_EV_STATE_STR macro as orphaned dead code? > > Since trace_gpi_ev_ctrl_irq() only takes the integer value, the final > consumer of the macro is removed. Could this regression trigger a > -Wunused-variable compiler warning for the unreferenced static array > when compiling the driver with standard kernel warning flags enabled? > > For reference, the array is defined earlier in the file as: > > static const char *const gpi_ev_state_str[MAX_EV_STATES] = { > [EV_STATE_NOT_ALLOCATED] = "NOT ALLOCATED", > [EV_STATE_ALLOCATED] = "ALLOCATED", > }; Please Remove dead code also. Frank > > #define TO_GPI_EV_STATE_STR(_state) (((_state) >= MAX_EV_STATES) ? \ > "INVALID" : gpi_ev_state_str[(_state)]) > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20260926-add-trace-support-gpio-v3-0-8572bf7456b2@oss.qualcomm.com?part=2 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-13 21:48 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-25 20:51 [PATCH v3 0/2] dmaengine: qcom: gpi: Add trace event support Praveen Talari 2026-09-25 20:51 ` [PATCH v3 1/2] " Praveen Talari 2026-09-25 21:02 ` sashiko-bot 2026-10-13 21:44 ` Frank Li 2026-09-25 20:51 ` [PATCH v3 2/2] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints Praveen Talari 2026-09-25 21:02 ` sashiko-bot 2026-10-13 21:48 ` Frank Li
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.