From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lina Iyer Subject: Re: [PATCH RFC 3/4] drivers: irqchip: pdc: log PDC info in FTRACE Date: Thu, 25 Jan 2018 15:45:39 +0000 Message-ID: <20180125154539.GB24587@codeaurora.org> References: <20180123175656.11942-1-ilina@codeaurora.org> <20180123175656.11942-4-ilina@codeaurora.org> <20180123131326.0f4121e7@gandalf.local.home> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:38930 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751065AbeAYPpm (ORCPT ); Thu, 25 Jan 2018 10:45:42 -0500 Content-Disposition: inline In-Reply-To: <20180123131326.0f4121e7@gandalf.local.home> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Steven Rostedt Cc: tglx@linutronix.de, jason@lakedaemon.net, marc.zyngier@arm.com, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, sboyd@codeaurora.org, rnayak@codeaurora.org, asathyak@codeaurora.org On Tue, Jan 23 2018 at 18:13 +0000, Steven Rostedt wrote: >On Tue, 23 Jan 2018 10:56:55 -0700 >Lina Iyer wrote: > >> From: Archana Sathyakumar > >Hi Lina and Archana, > >> >> Log key PDC pin configuration in FTRACE. >> >> Cc: Steven Rostedt >> Signed-off-by: Archana Sathyakumar >> Signed-off-by: Lina Iyer >> --- >> 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 >> #include >> #include >> +#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 >> + >> +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. > Thanks Steve. I will take care of it in the next spin. -- Lina >-- Steve > > >> + __entry->func, __entry->pin, __entry->hwirq, __entry->type, >> + __entry->enable) >> +); >> + >> +#endif >> +#define TRACE_INCLUDE_FILE pdc >> +#include >