From: Praveen Talari <praveen.talari@oss.qualcomm.com>
To: Frank Li <Frank.li@oss.nxp.com>
Cc: konrad.dybcio@oss.qualcomm.com,
Steven Rostedt <rostedt@goodmis.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Vinod Koul <vkoul@kernel.org>, Frank Li <Frank.Li@kernel.org>,
chandana.chiluveru@oss.qualcomm.com,
mukesh.savaliya@oss.qualcomm.com, linux-kernel@vger.kernel.org,
linux-trace-kernel@vger.kernel.org,
linux-arm-msm@vger.kernel.org, dmaengine@vger.kernel.org
Subject: Re: [PATCH v2 3/3] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints
Date: Fri, 25 Sep 2026 19:40:46 +0530 [thread overview]
Message-ID: <b2befdae-09b6-4ffe-aa67-05d6172f4823@oss.qualcomm.com> (raw)
In-Reply-To: <aqHIiBMVmuBKx6lM@SMW015318>
Hi
On 10-09-2026 02:28, Frank Li wrote:
> 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.
Sure, will update in next patch.
Thanks,
Praveen
>
> 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
>>
prev parent reply other threads:[~2026-09-25 14:10 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-09 5:09 [PATCH v2 0/3] dmaengine: qcom: gpi: Add trace event support Praveen Talari
2026-09-09 5:09 ` [PATCH v2 1/3] dmaengine: qcom: gpi: trace: Add trace events header for Qualcomm GPI DMA Praveen Talari
2026-09-09 20:56 ` Frank Li
2026-09-25 14:10 ` Praveen Talari
2026-09-09 5:09 ` [PATCH v2 2/3] dmaengine: qcom: gpi: Add trace event support Praveen Talari
2026-09-09 5:09 ` [PATCH v2 3/3] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints Praveen Talari
2026-09-09 20:58 ` Frank Li
2026-09-25 14:10 ` Praveen Talari [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b2befdae-09b6-4ffe-aa67-05d6172f4823@oss.qualcomm.com \
--to=praveen.talari@oss.qualcomm.com \
--cc=Frank.Li@kernel.org \
--cc=Frank.li@oss.nxp.com \
--cc=chandana.chiluveru@oss.qualcomm.com \
--cc=dmaengine@vger.kernel.org \
--cc=konrad.dybcio@oss.qualcomm.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=mukesh.savaliya@oss.qualcomm.com \
--cc=rostedt@goodmis.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®