Hi Steven, On 22.11.19 19:07, Steven Rostedt wrote: > On Fri, 22 Nov 2019 17:27:12 +0200 > Georgi Djakov <georgi.djakov@xxxxxxxxxx> wrote: >> index 28f2ab0824d5..725029ae7a2c 100644 >> --- a/drivers/interconnect/Makefile >> +++ b/drivers/interconnect/Makefile >> @@ -1,5 +1,6 @@ >> # SPDX-License-Identifier: GPL-2.0 >> >> +CFLAGS_core.o := -I$(src) >> icc-core-objs := core.o >> >> obj-$(CONFIG_INTERCONNECT) += icc-core.o >> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c >> index 1ddad8ef3cf0..b218a2609f6b 100644 >> --- a/drivers/interconnect/core.c >> +++ b/drivers/interconnect/core.c >> @@ -19,6 +19,9 @@ >> #include <linux/of.h> >> #include <linux/overflow.h> >> >> +#define CREATE_TRACE_POINTS >> +#include "trace.h" >> + > > You may want to move this below the include of internal.h, as you don't > want CREATE_TRACE_POINTS defined when including any other header, or it > can cause issues if that header has some tracepoint header inside it. > > It may not be the case now, but could cause for headaches in the > future, if other headers get included in internal.h. > >> #include "internal.h" >> >> static DEFINE_IDR(icc_idr); >> @@ -435,6 +438,8 @@ int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw) >> >> /* aggregate requests for this node */ >> aggregate_requests(node); >> + >> + trace_icc_set_bw(path, node, i, avg_bw, peak_bw); >> } >> >> ret = apply_constraints(path); >> @@ -453,6 +458,8 @@ int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw) >> >> mutex_unlock(&icc_lock); >> >> + trace_icc_set_bw_end(path, ret); >> + >> return ret; >> } >> EXPORT_SYMBOL_GPL(icc_set_bw); >> diff --git a/drivers/interconnect/trace.h b/drivers/interconnect/trace.h >> new file mode 100644 >> index 000000000000..d2421bf7b389 >> --- /dev/null >> +++ b/drivers/interconnect/trace.h >> @@ -0,0 +1,90 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * Interconnect framework tracepoints >> + * Copyright (c) 2019, Linaro Ltd. >> + * Author: Georgi Djakov <georgi.djakov@xxxxxxxxxx> >> + */ >> + >> +#undef TRACE_SYSTEM >> +#define TRACE_SYSTEM interconnect >> + >> +#if !defined(_TRACE_INTERCONNECT_H) || defined(TRACE_HEADER_MULTI_READ) >> +#define _TRACE_INTERCONNECT_H >> + >> +#include <linux/interconnect.h> >> +#include <linux/tracepoint.h> >> + >> +#include "internal.h" > > And you include it here too, perhaps it is best not to have it here, > and then just have it before trace.h is called? Sure! Thanks a lot for reviewing! BR, Georgi > > The rest looks good. Besides the comments above: > > Reviewed-by: Steven Rostedt (VMware) <rostedt@xxxxxxxxxxx> > > -- Steve
