Re: [PATCH] spmi-pmic-arb: add irq tracepoints to the pmic-arb driver

[Steven Rostedt]

On Tue, 26 May 2015 17:39:15 -0600
Ankit Gupta <ankgupta@xxxxxxxxxxxxxx> wrote:

> The spmi-pmic-arb is also an interrupt controller. It gets a
> single aggregate irq and disseminates it to individual
> pmic-peripheral drivers. Each pmic-peripheral has a unique apid
> number, and can have multiple interrupt capable functions.
> The registered apid range shows the lowest and highest apid
> numbers of pmic-peripheral drivers which request irqs. Pid is
> the base address of that peripheral. For performance measurement,
> tracepoints are added at the beginning of the aggregate irq and
> at the end of the individual pmic-peripheral irqs.
> 
> Following is a list showing the new tracepoint events:
> 
> spmi_pmic_arb_aggregate_irq_start: aggregate irq number and registered
> 				   apid range.
> 
> spmi_pmic_arb_apid_irq_end: apid, irq, func_num, sid and pid.
> 
> SPMI Interrupts tracepoints can be enabled like:
> 
> echo 1 >/sys/kernel/debug/tracing/events/spmi-pmic-arb/enable
> 
> and will dump messages that can be viewed in
> /sys/kernel/debug/tracing/trace that look like:
> ... spmi_pmic_arb_aggregate_irq_start: irq=150 registered apid range=(3,189)
> ... spmi_pmic_arb_apid_irq_end: apid=3 irq=1 func_num=0 sid=0 pid=0x8
> 
> Suggested-by: Sagar Dharia <sdharia@xxxxxxxxxxxxxx>
> Signed-off-by: Gilad Avidov <gavidov@xxxxxxxxxxxxxx>
> Signed-off-by: Ankit Gupta <ankgupta@xxxxxxxxxxxxxx>
> ---
>  drivers/spmi/spmi-pmic-arb.c         | 15 ++++++---
>  include/trace/events/spmi-pmic-arb.h | 62 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 5 deletions(-)
>  create mode 100644 include/trace/events/spmi-pmic-arb.h
> 
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 20559ab..342a71d 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -23,6 +23,9 @@
>  #include <linux/slab.h>
>  #include <linux/spmi.h>
>  
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/spmi-pmic-arb.h>
> +
>  /* PMIC Arbiter configuration registers */
>  #define PMIC_ARB_VERSION		0x0000
>  #define PMIC_ARB_INT_EN			0x0004
> @@ -375,16 +378,17 @@ static void periph_interrupt(struct spmi_pmic_arb_dev *pa, u8 apid)
>  	unsigned int irq;
>  	u32 status;
>  	int id;
> +	u16 ppid = pa->apid_to_ppid[apid];
> +	u8 sid = (ppid >> 8) & 0x0F;
> +	u8 pid = ppid & 0xFF;
>  
>  	status = readl_relaxed(pa->intr + SPMI_PIC_IRQ_STATUS(apid));
>  	while (status) {
>  		id = ffs(status) - 1;
>  		status &= ~(1 << id);
> -		irq = irq_find_mapping(pa->domain,
> -				       pa->apid_to_ppid[apid] << 16
> -				     | id << 8
> -				     | apid);
> +		irq = irq_find_mapping(pa->domain, ppid << 16 | id << 8 | apid);
>  		generic_handle_irq(irq);
> +		trace_spmi_pmic_arb_apid_irq_end(apid, irq, id, sid, pid);

It looks like sid is only used for the tracepoint processing. Instead
of doing the work up above "(ppid >> 8) & 0x0F" that would only be used
in the unlikely event that you happen to be tracing, what about moving
the work below into the tracepoint, by passing in ppid, and having:

	__entry->sid = (ppid >> 8) & 0x0F;

That way you would save some CPU cycles when not tracing.

-- Steve

>  	}
>  }
>  
> @@ -399,7 +403,8 @@ static void pmic_arb_chained_irq(unsigned int irq, struct irq_desc *desc)
>  	int i, id;
>  
>  	chained_irq_enter(chip, desc);
> -
> +	trace_spmi_pmic_arb_aggregate_irq_start(irq, pa->min_apid,
> +						pa->max_apid);
>  	for (i = first; i <= last; ++i) {
>  		status = readl_relaxed(intr +
>  				       SPMI_PIC_OWNER_ACC_STATUS(pa->ee, i));
> diff --git a/include/trace/events/spmi-pmic-arb.h b/include/trace/events/spmi-pmic-arb.h
> new file mode 100644
> index 0000000..6c4dbca
> --- /dev/null
> +++ b/include/trace/events/spmi-pmic-arb.h
> @@ -0,0 +1,62 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM spmi-pmic-arb
> +
> +#if !defined(_TRACE_SPMI_PMIC_ARB_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_SPMI_PMIC_ARB_H
> +
> +#include <linux/spmi.h>
> +#include <linux/tracepoint.h>
> +
> +/*
> + * drivers/spmi/spmi-pmic-arb.c
> + */
> +
> +TRACE_EVENT(spmi_pmic_arb_aggregate_irq_start,
> +	TP_PROTO(unsigned int irq, int first, int last),
> +	TP_ARGS(irq, first, last),
> +
> +	TP_STRUCT__entry(
> +		__field		( unsigned int,   irq   )
> +		__field		( int,            first )
> +		__field		( int,            last  )
> +	),
> +
> +	TP_fast_assign(
> +		__entry->irq    = irq;
> +		__entry->first  = first;
> +		__entry->last   = last;
> +	),
> +
> +	TP_printk("irq=%d registered apid range=(%d,%d)",
> +		  (int)__entry->irq, __entry->first, __entry->last)
> +);
> +
> +TRACE_EVENT(spmi_pmic_arb_apid_irq_end,
> +	TP_PROTO(u8 apid, unsigned int irq, int func_num, u8 sid, u8 pid),
> +	TP_ARGS(apid, irq, func_num, sid, pid),
> +
> +	TP_STRUCT__entry(
> +		__field		( u8,           apid     )
> +		__field		( unsigned int, irq      )
> +		__field		( int,          func_num )
> +		__field		( u8,           sid      )
> +		__field		( u8,           pid      )
> +	),
> +
> +	TP_fast_assign(
> +		__entry->apid     = apid;
> +		__entry->irq      = irq;
> +		__entry->func_num = func_num;
> +		__entry->sid      = sid;
> +		__entry->pid      = pid;
> +	),
> +
> +	TP_printk("apid=%d irq=%d func_num=%d sid=%d pid=0x%d",
> +		  (int)__entry->apid, (int)__entry->irq, (int)__entry->func_num,
> +		  (int)__entry->sid, (int)__entry->pid)
> +);
> +
> +#endif /* _TRACE_SPMI_PMIC_ARB_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/