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: 10+ 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:23 ` sashiko-bot
2026-09-09 5:09 ` [PATCH v2 3/3] dmaengine: qcom: gpi: Convert dev_dbg() calls to tracepoints Praveen Talari
2026-09-09 5:22 ` sashiko-bot
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 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.