From patchwork Tue Jul 19 13:46:40 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marc Zyngier X-Patchwork-Id: 9237491 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CB9B16075D for ; Tue, 19 Jul 2016 13:47:51 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B9CE620246 for ; Tue, 19 Jul 2016 13:47:51 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AE624271FD; Tue, 19 Jul 2016 13:47:51 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=unavailable version=3.3.1 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.wl.linuxfoundation.org (Postfix) with ESMTPS id 3A1FF20246 for ; Tue, 19 Jul 2016 13:47:51 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bPVNF-0006qH-Kn; Tue, 19 Jul 2016 13:47:49 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bPVMX-0005rO-Nq; Tue, 19 Jul 2016 13:47:10 +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 68A163A1; Tue, 19 Jul 2016 06:47:54 -0700 (PDT) Received: from [10.1.207.16] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id BBCBE3F25F; Tue, 19 Jul 2016 06:46:41 -0700 (PDT) Subject: Re: [PATCH v2] drivers/perf: arm-pmu: Handle per-interrupt affinity mask To: Geert Uytterhoeven , Catalin Marinas , Will Deacon References: <1467815627-4061-1-git-send-email-marc.zyngier@arm.com> From: Marc Zyngier X-Enigmail-Draft-Status: N1110 Organization: ARM Ltd Message-ID: <578E2F40.1000309@arm.com> Date: Tue, 19 Jul 2016 14:46:40 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.7.0 MIME-Version: 1.0 In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160719_064705_984805_402904E0 X-CRM114-Status: GOOD ( 23.30 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: huangtao@rock-chips.com, "devicetree@vger.kernel.org" , Heiko Stuebner , Brian Norris , "linux-kernel@vger.kernel.org" , Linux-Renesas , "open list:ARM/Rockchip SoC..." , Rob Herring , David Wu , cf@rock-chips.com, Mark Rutland , Xu Jianqun , "linux-arm-kernel@lists.infradead.org" , Caesar Wang Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Geert, On 19/07/16 14:25, Geert Uytterhoeven wrote: > Hi Marc, Catalin, Will, > > On Wed, Jul 6, 2016 at 4:33 PM, Marc Zyngier wrote: >> On a big-little system, PMUs can be wired to CPUs using per CPU >> interrups (PPI). In this case, it is important to make sure that >> the enable/disable do happen on the right set of CPUs. >> >> So instead of relying on the interrupt-affinity property, we can >> use the actual percpu affinity that DT exposes as part of the >> interrupt specifier. The DT binding is also updated to reflect >> the fact that the interrupt-affinity property shouldn't be used >> in that case. >> >> Signed-off-by: Marc Zyngier >> --- >> * From v1: >> - propagate the error if irq_get_percpu_devid_partition fails > > This patch, which is commit 19a469a58720ea96 in arm64/for-next/core, broke > the PMU on r8a7740/armadillo800eva: > > -hw perfevents: enabled with armv7_cortex_a9 PMU driver, 7 > counters available > +hw perfevents: /pmu: failed to probe PMU! > +hw perfevents: /pmu: failed to register PMU devices! > +armv7-pmu: probe of pmu failed with error -22 > > This is a single-core Cortex A9. > >> Documentation/devicetree/bindings/arm/pmu.txt | 4 +++- >> drivers/perf/arm_pmu.c | 27 ++++++++++++++++++++++----- >> 2 files changed, 25 insertions(+), 6 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/arm/pmu.txt b/Documentation/devicetree/bindings/arm/pmu.txt >> index 74d5417..61c8b46 100644 >> --- a/Documentation/devicetree/bindings/arm/pmu.txt >> +++ b/Documentation/devicetree/bindings/arm/pmu.txt >> @@ -39,7 +39,9 @@ Optional properties: >> When using a PPI, specifies a list of phandles to CPU >> nodes corresponding to the set of CPUs which have >> a PMU of this type signalling the PPI listed in the >> - interrupts property. >> + interrupts property, unless this is already specified >> + by the PPI interrupt specifier itself (in which case >> + the interrupt-affinity property shouldn't be present). >> >> This property should be present when there is more than >> a single SPI. > > On a single core, there's only a single SPI, hence there's no need for an > "interrupt-affinity" property. > >> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c >> index 140436a..8e4d7f5 100644 >> --- a/drivers/perf/arm_pmu.c >> +++ b/drivers/perf/arm_pmu.c >> @@ -961,9 +964,23 @@ static int of_pmu_irq_cfg(struct arm_pmu *pmu) >> i++; >> } while (1); >> >> - /* If we didn't manage to parse anything, claim to support all CPUs */ >> - if (cpumask_weight(&pmu->supported_cpus) == 0) >> - cpumask_setall(&pmu->supported_cpus); >> + /* If we didn't manage to parse anything, try the interrupt affinity */ >> + if (cpumask_weight(&pmu->supported_cpus) == 0) { >> + if (!using_spi) { > > However, using_spi is never set to true in the absence of that property, > causing the wrong branch to be taken... > >> + /* If using PPIs, check the affinity of the partition */ >> + int ret, irq; >> + >> + irq = platform_get_irq(pdev, 0); >> + ret = irq_get_percpu_devid_partition(irq, &pmu->supported_cpus); > > ... and ret to become -22 here. Thanks for the thorough analysis. Could you please give the following patchlet a go: and let me know if that helps? Thanks, M. diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c index 2513365..9275e08 100644 --- a/drivers/perf/arm_pmu.c +++ b/drivers/perf/arm_pmu.c @@ -958,11 +958,12 @@ static int of_pmu_irq_cfg(struct arm_pmu *pmu) /* If we didn't manage to parse anything, try the interrupt affinity */ if (cpumask_weight(&pmu->supported_cpus) == 0) { - if (!using_spi) { + int irq = platform_get_irq(pdev, 0); + + if (irq_is_percpu(irq)) { /* If using PPIs, check the affinity of the partition */ - int ret, irq; + int ret; - irq = platform_get_irq(pdev, 0); ret = irq_get_percpu_devid_partition(irq, &pmu->supported_cpus); if (ret) { kfree(irqs);