Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755024AbbBBTln (ORCPT ); Mon, 2 Feb 2015 14:41:43 -0500 Received: from smtp.codeaurora.org ([198.145.11.231]:39636 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754576AbbBBTlm (ORCPT ); Mon, 2 Feb 2015 14:41:42 -0500 Message-ID: <54CFD2F4.3090800@codeaurora.org> Date: Mon, 02 Feb 2015 11:41:40 -0800 From: Stephen Boyd User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Steven Rostedt CC: Mike Turquette , linux-kernel@vger.kernel.org Subject: Re: [PATCH] clk: Add tracepoints for hardware operations References: <1422663371-31056-1-git-send-email-sboyd@codeaurora.org> <20150202110033.6462d4f4@gandalf.local.home> In-Reply-To: <20150202110033.6462d4f4@gandalf.local.home> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2615 Lines: 78 On 02/02/15 08:00, Steven Rostedt wrote: > On Fri, 30 Jan 2015 16:16:11 -0800 > Stephen Boyd wrote: > >> It's useful to have tracepoints around operations that change the >> hardware state so that we can debug clock hardware performance >> and operations. Four basic types of events are supported: on/off >> events for enable, disable, prepare, unprepare that only record >> an event and a clock name, rate changing events for >> clk_set_{min_,max_}rate{_range}(), phase changing events for >> clk_set_phase() and parent changing events for clk_set_parent(). >> >> Cc: Steven Rostedt > I don't see anything wrong with the implementation of the tracepoints. > Now whether or not they are useful is up to the clk maintainer to > decide. > >> Signed-off-by: Stephen Boyd >> --- >> drivers/clk/clk.c | 32 ++++++++ >> include/trace/events/clk.h | 198 +++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 230 insertions(+) >> create mode 100644 include/trace/events/clk.h >> > > >> unlock_out: >> @@ -861,9 +868,12 @@ static void clk_core_unprepare(struct clk_core *clk) >> >> WARN_ON(clk->enable_count > 0); >> >> + trace_clk_unprepare(clk); >> + >> if (clk->ops->unprepare) >> clk->ops->unprepare(clk->hw); >> >> + trace_clk_unprepare_complete(clk); >> clk_core_unprepare(clk->parent); > I guess you do not care about the clk_core_unprepare time. Function trace will handle that? > >> } >> >> @@ -901,6 +911,8 @@ static int clk_core_prepare(struct clk_core *clk) >> if (ret) >> return ret; >> >> + trace_clk_prepare(clk); >> + >> if (clk->ops->prepare) { >> ret = clk->ops->prepare(clk->hw); >> if (ret) { >> @@ -908,6 +920,8 @@ static int clk_core_prepare(struct clk_core *clk) >> return ret; >> } >> } >> + >> + trace_clk_prepare_complete(clk); > I'm curious to why you do not put the tracepoint within the if > statement, and only show the tracepoints if the clock prepare is > actually called. Also, if you exit out with that return, will you tools > be OK with seeing the clk_prepare but not the clk_prepare_complete? > Ah good point. I'll rework it so we always get the tracepoint around the clk op. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/