From patchwork Thu Oct 15 15:41:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Rutland X-Patchwork-Id: 7407421 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id CEF289F1D5 for ; Thu, 15 Oct 2015 15:48:44 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DC31120839 for ; Thu, 15 Oct 2015 15:48:43 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F049720825 for ; Thu, 15 Oct 2015 15:48:42 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Zmkl2-0003Iz-Qo; Thu, 15 Oct 2015 15:47:56 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1ZmkfB-0005KS-H5 for linux-arm-kernel@lists.infradead.org; Thu, 15 Oct 2015 15:41:55 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id AACD23C; Thu, 15 Oct 2015 08:41:28 -0700 (PDT) Received: from leverpostej (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D600F3F49B; Thu, 15 Oct 2015 08:41:29 -0700 (PDT) Date: Thu, 15 Oct 2015 16:41:26 +0100 From: Mark Rutland To: Will Deacon Subject: Re: [PATCH v5 2/2] arm: perf: Add event descriptions Message-ID: <20151015154126.GL8825@leverpostej> References: <20151007182735.GA18706@dreric01-gentoo.localdomain> <20151009101338.GH26278@arm.com> <20151009165330.GA22415@dreric01-gentoo.localdomain> <20151012143025.GG16124@arm.com> <20151012181037.GA16013@dreric01-gentoo.localdomain> <20151013153644.GC18121@dreric01-gentoo.localdomain> <20151015132111.GD29301@arm.com> <20151015151506.GC18965@dreric01-gentoo.localdomain> <20151015152915.GH29301@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20151015152915.GH29301@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20151015_084153_740038_C05A6ADA X-CRM114-Status: GOOD ( 25.81 ) X-Spam-Score: -6.9 (------) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Russell King , Peter Zijlstra , Pawel Moll , "linux-kernel@vger.kernel.org" , Arnaldo Carvalho de Melo , Ingo Molnar , Wade Cherry , Drew Richardson , "linux-arm-kernel@lists.infradead.org" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Oct 15, 2015 at 04:29:15PM +0100, Will Deacon wrote: > On Thu, Oct 15, 2015 at 08:15:06AM -0700, Drew Richardson wrote: > > On Thu, Oct 15, 2015 at 02:21:12PM +0100, Will Deacon wrote: > > > On Tue, Oct 13, 2015 at 08:36:45AM -0700, Drew Richardson wrote: > > > > Add additional information about the ARM architected hardware events > > > > to make counters self describing. This makes the hardware PMUs easier > > > > to use as perf list contains possible events instead of users having > > > > to refer to documentation like the ARM TRMs. > > > > > > > > Signed-off-by: Drew Richardson > > > > --- > > > > arch/arm/kernel/perf_event_v7.c | 121 ++++++++++++++++++++++++++++++++++++++++ > > > > drivers/perf/arm_pmu.c | 1 + > > > > 2 files changed, 122 insertions(+) > > > > > > [...] > > > > > > > diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c > > > > index 2365a32a595e..e933d2dd71c0 100644 > > > > --- a/drivers/perf/arm_pmu.c > > > > +++ b/drivers/perf/arm_pmu.c > > > > @@ -548,6 +548,7 @@ static void armpmu_init(struct arm_pmu *armpmu) > > > > .stop = armpmu_stop, > > > > .read = armpmu_read, > > > > .filter_match = armpmu_filter_match, > > > > + .attr_groups = armpmu->pmu.attr_groups, > > > > > > I don't understand this hunk. What's it doing? > > > > I'm not 100% clear either on what it's doing. But without this line > > the attr_groups don't get passed on and I don't see them on my TC2. I > > debugged the issue down to this but it may not be the proper way to > > solve the problem. > > Oh yuck, it's because we call armpmu_init after cpu_pmu_init and the former > uses struct initialisation and ends up zeroing anything set previously. > > We should probably tidy all this up: > > * Remove armpmu_register and call perf_pmu_register directly from > arm_pmu_device_probe instead > > * Call armpmu_init immediately prior to arm_cpu_init Assuming you mean cpu_pmu_init, how about the below? Thanks, Mark. ---->8---- From f8a99a406f3dd7a272a6dee4a551436fea851f38 Mon Sep 17 00:00:00 2001 From: Mark Rutland Date: Thu, 15 Oct 2015 16:32:17 +0100 Subject: [PATCH] drivers/perf: kill armpmu_register Nothing outside of drivers/perf/arm_pmu.c should call armpmu_register any more, so it no longer needs to be in include/linux/perf/arm_pmu.h. Additionally, by folding it in to arm_pmu_device_probe we can allow drivers to override struct pmu fields without getting blatted by the armpmu code. This patch folds armpmu_register into arm_pmu_device_probe. The logging to the console is moved to after the PMU is successfully registered with the core perf code. Signed-off-by: Mark Rutland Suggested-by: Will Deacon Cc: Drew Richardson Cc: Pawel Moll --- drivers/perf/arm_pmu.c | 14 +++++--------- include/linux/perf/arm_pmu.h | 2 -- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c index f346960..15463cb 100644 --- a/drivers/perf/arm_pmu.c +++ b/drivers/perf/arm_pmu.c @@ -583,14 +583,6 @@ static void armpmu_init(struct arm_pmu *armpmu) }; } -int armpmu_register(struct arm_pmu *armpmu, int type) -{ - armpmu_init(armpmu); - pr_info("enabled with %s PMU driver, %d counters available\n", - armpmu->name, armpmu->num_events); - return perf_pmu_register(&armpmu->pmu, armpmu->name, type); -} - /* Set at runtime when we know what CPU type we are. */ static struct arm_pmu *__oprofile_cpu_pmu; @@ -934,14 +926,18 @@ int arm_pmu_device_probe(struct platform_device *pdev, goto out_free; } + armpmu_init(pmu); ret = cpu_pmu_init(pmu); if (ret) goto out_free; - ret = armpmu_register(pmu, -1); + ret = perf_pmu_register(&pmu->pmu, pmu->name, -1); if (ret) goto out_destroy; + pr_info("enabled with %s PMU driver, %d counters available\n", + pmu->name, pmu->num_events); + return 0; out_destroy: diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h index bfa673b..83b5e34 100644 --- a/include/linux/perf/arm_pmu.h +++ b/include/linux/perf/arm_pmu.h @@ -111,8 +111,6 @@ struct arm_pmu { #define to_arm_pmu(p) (container_of(p, struct arm_pmu, pmu)) -int armpmu_register(struct arm_pmu *armpmu, int type); - u64 armpmu_event_update(struct perf_event *event); int armpmu_event_set_period(struct perf_event *event);