On Tue, 23 Jan 2018 10:56:55 -0700 Lina Iyer <ilina@xxxxxxxxxxxxxx> wrote: > From: Archana Sathyakumar <asathyak@xxxxxxxxxxxxxx> Hi Lina and Archana, > > Log key PDC pin configuration in FTRACE. > > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx> > Signed-off-by: Archana Sathyakumar <asathyak@xxxxxxxxxxxxxx> > Signed-off-by: Lina Iyer <ilina@xxxxxxxxxxxxxx> > --- > drivers/irqchip/qcom-pdc.c | 6 ++++++ > include/trace/events/pdc.h | 50 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 56 insertions(+) > create mode 100644 include/trace/events/pdc.h > > diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c > index 57f1348bd81c..9b626e9f3a29 100644 > --- a/drivers/irqchip/qcom-pdc.c > +++ b/drivers/irqchip/qcom-pdc.c > @@ -26,6 +26,8 @@ > #include <linux/platform_device.h> > #include <linux/slab.h> > #include <linux/types.h> > +#define CREATE_TRACE_POINTS > +#include "trace/events/pdc.h" > > #include "qcom-pdc.h" > > @@ -78,6 +80,7 @@ static inline void pdc_enable_intr(struct irq_data *d, bool on) > break; > udelay(5); > } while (1); > + trace_irq_pin_config("enable", (u32)pin_out, (u32)d->hwirq, 0, on); I'm concerned with that string you are passing in. > spin_unlock_irqrestore(&pdc_lock, flags); > WARN_ON(r_enable != enable); > } > @@ -161,6 +164,9 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type) > udelay(5); > } while (1); > > + trace_irq_pin_config("type_config", (u32)pin_out, (u32)d->hwirq, This one too. > + pdc_type, 0); > + > /* > * If type is edge triggered, forward that as Rising edge as PDC > * takes care of converting falling edge to rising edge signal > diff --git a/include/trace/events/pdc.h b/include/trace/events/pdc.h > new file mode 100644 > index 000000000000..bec54c414880 > --- /dev/null > +++ b/include/trace/events/pdc.h > @@ -0,0 +1,50 @@ > +/* Copyright (c) 2017-2018, The Linux Foundation. All rights reserved. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 and > + * only version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#undef TRACE_SYSTEM > +#define TRACE_SYSTEM pdc > + > +#if !defined(_TRACE_PDC_) || defined(TRACE_HEADER_MULTI_READ) > +#define _TRACE_PDC_H_ > + > +#include <linux/tracepoint.h> > + > +TRACE_EVENT(irq_pin_config, > + > + TP_PROTO(char *func, u32 pin, u32 hwirq, u32 type, u32 enable), > + > + TP_ARGS(func, pin, hwirq, type, enable), > + > + TP_STRUCT__entry( > + __field(char *, func) > + __field(u32, pin) > + __field(u32, hwirq) > + __field(u32, type) > + __field(u32, enable) > + ), > + > + TP_fast_assign( > + __entry->pin = pin; > + __entry->func = func; > + __entry->hwirq = hwirq; > + __entry->type = type; > + __entry->enable = enable; > + ), > + > + TP_printk("%s hwirq:%u pin:%u type:%u enable:%u", %s is dereferencing a pointer, which works fine when reading the ASCII translated trace files (/sys/kernel/debug/tracing/trace and friends). But it breaks when we use trace-cmd or perf. Because they record the binary data and they do not have access to memory inside the kernel. What I would recommend is in pdc.h have: #define PDC_ENTRY 1 #define PDC_TYPE_CONFIG 2 Then have the tracepoints do instead of: trace_irq_pin_config("enable", ...); trace_irq_pin_config("type_config", ...); Have them do: trace_irq_pin_config(PDC_ENTRY, ...); trace_irq_pin_config(PDC_TYPE_CONFIG, ...); And then have: TP_PROTO(u32 func, ...) TP_STRUCT__entry( __field(u32, func) [...] TP_fast_assign( __entry->func = func; [...] TP_printk("%s hwirq...", print_symbolic(__entry->func, { PDC_ENTRY, "entry" }, { PDC_TYPE_CONFIG, "type_config" }), [...] And you will get the same effect, with trace-cmd and perf still working as expected. -- Steve > + __entry->func, __entry->pin, __entry->hwirq, __entry->type, > + __entry->enable) > +); > + > +#endif > +#define TRACE_INCLUDE_FILE pdc > +#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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html