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 <[email protected]> > --- > 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 >
