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

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

[Ankit Gupta]

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@codeaurora.org>
Signed-off-by: Gilad Avidov <gavidov@codeaurora.org>
Signed-off-by: Ankit Gupta <ankgupta@codeaurora.org>
---
 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);
 	}
 }
 
@@ -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>
-- 
1.8.5.2

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

[Stephen Boyd]

On 05/26/2015 04:39 PM, Ankit Gupta 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@codeaurora.org>
> Signed-off-by: Gilad Avidov <gavidov@codeaurora.org>
> Signed-off-by: Ankit Gupta <ankgupta@codeaurora.org>
> ---

How is this any better than irq tracepoints that we already have for 
generic irqs?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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@codeaurora.org> 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@codeaurora.org>
> Signed-off-by: Gilad Avidov <gavidov@codeaurora.org>
> Signed-off-by: Ankit Gupta <ankgupta@codeaurora.org>
> ---
>  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>

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

[Ankit Gupta]

> On 05/26/2015 04:39 PM, Ankit Gupta 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@codeaurora.org>
>> Signed-off-by: Gilad Avidov <gavidov@codeaurora.org>
>> Signed-off-by: Ankit Gupta <ankgupta@codeaurora.org>
>> ---
>
> How is this any better than irq tracepoints that we already have for
> generic irqs?
>
It is better than generic irq tracepoints because it provides bus specific
information (sid and address(pid) of slave write), driver specific
information (apid (pmic-peripheral) and func_num) and statistics (apid
range).
Recall that *slave* read/write cannot be traced by the spmi framework ftrace.
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>

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

[Ankit Gupta]

> On Tue, 26 May 2015 17:39:15 -0600
> Ankit Gupta <ankgupta@codeaurora.org> 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@codeaurora.org>
>> Signed-off-by: Gilad Avidov <gavidov@codeaurora.org>
>> Signed-off-by: Ankit Gupta <ankgupta@codeaurora.org>
>> ---
>>  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
Will do it.
>
>>  	}
>>  }
>>
>> @@ -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-arm-msm"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

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

[Stephen Boyd]

On 05/27, Ankit Gupta wrote:
> >
> > How is this any better than irq tracepoints that we already have for
> > generic irqs?
> >
> It is better than generic irq tracepoints because it provides bus specific
> information (sid and address(pid) of slave write), driver specific
> information (apid (pmic-peripheral) and func_num) and statistics (apid
> range).
> Recall that *slave* read/write cannot be traced by the spmi framework ftrace.
> 

Don't we already get all this information based on how we map
interrupts to devices in DT? It feels to me that the same
argument here could be applied to all the random gpio expanders
and chained interrupt controllers that we support in the kernel.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Gilad Avidov]

On Wed, 27 May 2015 13:06:29 -0700
Stephen Boyd <sboyd@codeaurora.org> wrote:

> On 05/27, Ankit Gupta wrote:
> > >
> > > How is this any better than irq tracepoints that we already have
> > > for generic irqs?
> > >
> > It is better than generic irq tracepoints because it provides bus
> > specific information (sid and address(pid) of slave write), driver
> > specific information (apid (pmic-peripheral) and func_num) and
> > statistics (apid range).
> > Recall that *slave* read/write cannot be traced by the spmi
> > framework ftrace.
> > 
> 
> Don't we already get all this information based on how we map
> interrupts to devices in DT? It feels to me that the same
> argument here could be applied to all the random gpio expanders
> and chained interrupt controllers that we support in the kernel.
> 

We don't.
We could get the same information if we had the irq-domain and hw-irq
32bit value. While these values are available from /proc/interrupt,
they are not available from the irq event tracing.

This patch traces spmi slave-originated events. If we had one such
slave we could cross information from both sources to filter the trace
events. However, we have hundreds of transaction-capable spmi slaves.

Thanks,
Gilad

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

[Stephen Boyd]

On 05/28/2015 04:02 PM, Gilad Avidov wrote:
> On Wed, 27 May 2015 13:06:29 -0700
> Stephen Boyd <sboyd@codeaurora.org> wrote:
>
>> On 05/27, Ankit Gupta wrote:
>>>> How is this any better than irq tracepoints that we already have
>>>> for generic irqs?
>>>>
>>> It is better than generic irq tracepoints because it provides bus
>>> specific information (sid and address(pid) of slave write), driver
>>> specific information (apid (pmic-peripheral) and func_num) and
>>> statistics (apid range).
>>> Recall that *slave* read/write cannot be traced by the spmi
>>> framework ftrace.
>>>
>> Don't we already get all this information based on how we map
>> interrupts to devices in DT? It feels to me that the same
>> argument here could be applied to all the random gpio expanders
>> and chained interrupt controllers that we support in the kernel.
>>
> We don't.
> We could get the same information if we had the irq-domain and hw-irq
> 32bit value. While these values are available from /proc/interrupt,
> they are not available from the irq event tracing.

Hm.. maybe we should add those trace events then into the irq domain 
layer? Or even extend the irq tracepoints to include the hw-irq and 
irq-domain name?

>
> This patch traces spmi slave-originated events. If we had one such
> slave we could cross information from both sources to filter the trace
> events. However, we have hundreds of transaction-capable spmi slaves.
>

Sorry I'm not following this point. If it was about tracing 
slave-originated events wouldn't it be more than irqs then?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Gilad Avidov]

On Thu, 28 May 2015 18:15:59 -0700
Stephen Boyd <sboyd@codeaurora.org> wrote:

> On 05/28/2015 04:02 PM, Gilad Avidov wrote:
> > On Wed, 27 May 2015 13:06:29 -0700
> > Stephen Boyd <sboyd@codeaurora.org> wrote:
> >
> >> On 05/27, Ankit Gupta wrote:
> >>>> How is this any better than irq tracepoints that we already have
> >>>> for generic irqs?
> >>>>
> >>> It is better than generic irq tracepoints because it provides bus
> >>> specific information (sid and address(pid) of slave write), driver
> >>> specific information (apid (pmic-peripheral) and func_num) and
> >>> statistics (apid range).
> >>> Recall that *slave* read/write cannot be traced by the spmi
> >>> framework ftrace.
> >>>
> >> Don't we already get all this information based on how we map
> >> interrupts to devices in DT? It feels to me that the same
> >> argument here could be applied to all the random gpio expanders
> >> and chained interrupt controllers that we support in the kernel.
> >>
> > We don't.
> > We could get the same information if we had the irq-domain and
> > hw-irq 32bit value. While these values are available
> > from /proc/interrupt, they are not available from the irq event
> > tracing.
> 
> Hm.. maybe we should add those trace events then into the irq domain 
> layer? Or even extend the irq tracepoints to include the hw-irq and 
> irq-domain name?
> 
I agree, adding hw-irq and irq-domain name to irq-tracing is a better
idea.

> >
> > This patch traces spmi slave-originated events. If we had one such
> > slave we could cross information from both sources to filter the
> > trace events. However, we have hundreds of transaction-capable spmi
> > slaves.
> >
> 
> Sorry I'm not following this point. If it was about tracing 
> slave-originated events wouldn't it be more than irqs then?
> 
Slave originated events are converted to irqs by the spmi controller.

Thanks,
Gilad