diff mbox series

[5/6] dma: tegra: add tracepoints to driver

Message ID 20181031160309.20408-6-ben.dooks@codethink.co.uk
State Superseded
Headers show
Series [1/6] dma: tegra: avoid overflow of byte tracking | expand

Commit Message

Ben Dooks Oct. 31, 2018, 4:03 p.m. UTC 
Add some trace-points to the driver to allow for debuging via the
trace pipe.

Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
Fixes since v1:
- take copy of dmachan name instead of pointer to device
Cc: Ingo Molnar <mingo@redhat.com> (maintainer:TRACING)
Cc: Steven Rostedt <rostedt@goodmis.org> (maintainer:TRACING)
---
 drivers/dma/tegra20-apb-dma.c        |  8 ++++
 include/trace/events/tegra_apb_dma.h | 61 ++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)
 create mode 100644 include/trace/events/tegra_apb_dma.h

Comments

Steven Rostedt Nov. 1, 2018, 1:54 p.m. UTC | #1 
On Wed, 31 Oct 2018 16:03:08 +0000
Ben Dooks <ben.dooks@codethink.co.uk> wrote:


> diff --git a/include/trace/events/tegra_apb_dma.h b/include/trace/events/tegra_apb_dma.h
> new file mode 100644
> index 000000000000..1f55c2c6049d
> --- /dev/null
> +++ b/include/trace/events/tegra_apb_dma.h
> @@ -0,0 +1,61 @@
> +#if !defined(_TRACE_TEGRA_APB_DMA_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_TEGRA_APM_DMA_H
> +
> +#include <linux/tracepoint.h>
> +#include <linux/dmaengine.h>
> +
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM tegra_apb_dma
> +
> +TRACE_EVENT(tegra_dma_tx_status,
> +	TP_PROTO(struct dma_chan *dc, s32 cookie, u32 residue),
> +	TP_ARGS(dc, cookie, residue),
> +	TP_STRUCT__entry(
> +		__string(chan,	16)

Hi Ben,

Was this tested? Because I think __string(chan, 16) would fault, as the
second parameter is meant to be a pointer to the source parameter.

Otherwise, you need to add dev_name(&dc->dev->device) as the second
parameter.

If you are just using fixed string lengths then it's best not to use the
__string() type, as that's for dynamic strings (strings changing in
size for each event) and uses 8 more bytes to store the offset and
length of the string in the event.

If you take a look at the sched_waking or sched_switch trace events in
include/trace/events/sched.h you'll see that it saves the comm string
directly as a character array:

	__array(	char,	comm, 	TASK_COMM_LEN)

For you, you can use:

	__array(char, chan, 16)

> +		__field(__s32,	cookie)
> +		__field(__u32,	residue)
> +	),
> +	TP_fast_assign(
> +		__assign_str(chan, dev_name(&dc->dev->device));

Then to store the array:

		memcpy(__entry->chan, dev_name(&dc->dev->device), 16);

> +		__entry->cookie = cookie;
> +		__entry->residue = residue;
> +	),
> +	TP_printk("channel %s: dma cookie %d, residue %u",

		__entry->chan,

is all that's needed.

Same for the other references to chan.

Unless you meant to store each with a different size, then you need to
replace that "16" with the dev_name(&dc->dev->device).

-- Steve

> +		  __get_str(chan), __entry->cookie, __entry->residue)
> +);
> +
> +TRACE_EVENT(tegra_dma_complete_cb,
> +	TP_PROTO(struct dma_chan *dc, int count, void *ptr),
> +	TP_ARGS(dc, count, ptr),
> +	TP_STRUCT__entry(
> +		__string(chan,	16)
> +		__field(int,	count)
> +		__field(void *,	ptr)
> +		),
> +	TP_fast_assign(
> +		__assign_str(chan, dev_name(&dc->dev->device));
> +		__entry->count = count;
> +		__entry->ptr = ptr;
> +		),
> +	TP_printk("channel %s: done %d, ptr %p",
> +		  __get_str(chan), __entry->count, __entry->ptr)
> +);
> +
> +TRACE_EVENT(tegra_dma_isr,
> +	TP_PROTO(struct dma_chan *dc, int irq),
> +	TP_ARGS(dc, irq),
> +	TP_STRUCT__entry(
> +		__string(chan,	16)
> +		__field(int,	irq)
> +	),
> +	TP_fast_assign(
> +		__assign_str(chan, dev_name(&dc->dev->device));
> +		__entry->irq = irq;
> +	),
> +	TP_printk("%s: irq %d\n",  __get_str(chan), __entry->irq)
> +);
> +
> +#endif /*  _TRACE_TEGRADMA_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
diff mbox series

Patch

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 3fa3a1ac4f57..22114c9a6e98 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -38,6 +38,9 @@ 
 
 #include "dmaengine.h"
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/tegra_apb_dma.h>
+
 #define TEGRA_APBDMA_GENERAL			0x0
 #define TEGRA_APBDMA_GENERAL_ENABLE		BIT(31)
 
@@ -672,6 +675,8 @@  static void tegra_dma_tasklet(unsigned long data)
 		dmaengine_desc_get_callback(&dma_desc->txd, &cb);
 		cb_count = dma_desc->cb_count;
 		dma_desc->cb_count = 0;
+		trace_tegra_dma_complete_cb(&tdc->dma_chan, cb_count,
+					    cb.callback);
 		spin_unlock_irqrestore(&tdc->lock, flags);
 		while (cb_count--)
 			dmaengine_desc_callback_invoke(&cb, NULL);
@@ -688,6 +693,7 @@  static irqreturn_t tegra_dma_isr(int irq, void *dev_id)
 
 	spin_lock_irqsave(&tdc->lock, flags);
 
+	trace_tegra_dma_isr(&tdc->dma_chan, irq);
 	status = tdc_read(tdc, TEGRA_APBDMA_CHAN_STATUS);
 	if (status & TEGRA_APBDMA_STATUS_ISE_EOC) {
 		tdc_write(tdc, TEGRA_APBDMA_CHAN_STATUS, status);
@@ -931,6 +937,8 @@  static enum dma_status tegra_dma_tx_status(struct dma_chan *dc,
 		dma_set_residue(txstate, residual);
 	}
 
+	trace_tegra_dma_tx_status(&tdc->dma_chan, cookie,
+				  txstate ? txstate->residue : -1);
 	spin_unlock_irqrestore(&tdc->lock, flags);
 	return ret;
 }
diff --git a/include/trace/events/tegra_apb_dma.h b/include/trace/events/tegra_apb_dma.h
new file mode 100644
index 000000000000..1f55c2c6049d
--- /dev/null
+++ b/include/trace/events/tegra_apb_dma.h
@@ -0,0 +1,61 @@ 
+#if !defined(_TRACE_TEGRA_APB_DMA_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_TEGRA_APM_DMA_H
+
+#include <linux/tracepoint.h>
+#include <linux/dmaengine.h>
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM tegra_apb_dma
+
+TRACE_EVENT(tegra_dma_tx_status,
+	TP_PROTO(struct dma_chan *dc, s32 cookie, u32 residue),
+	TP_ARGS(dc, cookie, residue),
+	TP_STRUCT__entry(
+		__string(chan,	16)
+		__field(__s32,	cookie)
+		__field(__u32,	residue)
+	),
+	TP_fast_assign(
+		__assign_str(chan, dev_name(&dc->dev->device));
+		__entry->cookie = cookie;
+		__entry->residue = residue;
+	),
+	TP_printk("channel %s: dma cookie %d, residue %u",
+		  __get_str(chan), __entry->cookie, __entry->residue)
+);
+
+TRACE_EVENT(tegra_dma_complete_cb,
+	TP_PROTO(struct dma_chan *dc, int count, void *ptr),
+	TP_ARGS(dc, count, ptr),
+	TP_STRUCT__entry(
+		__string(chan,	16)
+		__field(int,	count)
+		__field(void *,	ptr)
+		),
+	TP_fast_assign(
+		__assign_str(chan, dev_name(&dc->dev->device));
+		__entry->count = count;
+		__entry->ptr = ptr;
+		),
+	TP_printk("channel %s: done %d, ptr %p",
+		  __get_str(chan), __entry->count, __entry->ptr)
+);
+
+TRACE_EVENT(tegra_dma_isr,
+	TP_PROTO(struct dma_chan *dc, int irq),
+	TP_ARGS(dc, irq),
+	TP_STRUCT__entry(
+		__string(chan,	16)
+		__field(int,	irq)
+	),
+	TP_fast_assign(
+		__assign_str(chan, dev_name(&dc->dev->device));
+		__entry->irq = irq;
+	),
+	TP_printk("%s: irq %d\n",  __get_str(chan), __entry->irq)
+);
+
+#endif /*  _TRACE_TEGRADMA_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>