From patchwork Fri Jan 29 18:30:51 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Boyd X-Patchwork-Id: 8166441 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 5525D9FBE9 for ; Fri, 29 Jan 2016 18:32:43 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7326C20374 for ; Fri, 29 Jan 2016 18:32:42 +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 90E5C20361 for ; Fri, 29 Jan 2016 18:32:40 +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 1aPDpG-0005NF-9y; Fri, 29 Jan 2016 18:31:18 +0000 Received: from smtp.codeaurora.org ([198.145.29.96]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aPDpB-0005CB-Fo for linux-arm-kernel@lists.infradead.org; Fri, 29 Jan 2016 18:31:14 +0000 Received: from smtp.codeaurora.org (localhost [127.0.0.1]) by smtp.codeaurora.org (Postfix) with ESMTP id B173D60499; Fri, 29 Jan 2016 18:30:52 +0000 (UTC) Received: by smtp.codeaurora.org (Postfix, from userid 1000) id A50EA60498; Fri, 29 Jan 2016 18:30:52 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 Received: from localhost (i-global254.qualcomm.com [199.106.103.254]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) (Authenticated sender: sboyd@smtp.codeaurora.org) by smtp.codeaurora.org (Postfix) with ESMTPSA id E624B60314; Fri, 29 Jan 2016 18:30:51 +0000 (UTC) Date: Fri, 29 Jan 2016 10:30:51 -0800 From: Stephen Boyd To: Robin Murphy Subject: Re: [PATCH] clocksource/arm_arch_timer: Enable and verify MMIO access Message-ID: <20160129183051.GO12841@codeaurora.org> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Virus-Scanned: ClamAV using ClamSMTP X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160129_103113_597125_4041AC65 X-CRM114-Status: GOOD ( 26.34 ) X-Spam-Score: -1.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: mark.rutland@arm.com, tglx@linutronix.de, daniel.lezcano@linaro.org, linux-kernel@vger.kernel.org, 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-Virus-Scanned: ClamAV using ClamSMTP On 01/29, Robin Murphy wrote: > So far, we have been blindly assuming that if a memory-mapped timer > frame exists, then we have access to it. Whilst it's the firmware's > job to give us non-secure access to frames in the first place, we > should not rely on it always being generous enough to also configure > CNTACR if it's not even using those frames itself. Hm, that first sentence is sort of misleading. We've been blindly assuming that the firmware has configured CNTACR to have the correct bits set for virtual/physical access. We've always relied on status = "disabled" to figure out if we can access an entire frame or not. > > Explicitly enable feature-level access per-frame, and verify that the > access we want is really implemented before trying to make use of it. > > Signed-off-by: Robin Murphy > --- > drivers/clocksource/arm_arch_timer.c | 39 ++++++++++++++++++++++++++++-------- > 1 file changed, 31 insertions(+), 8 deletions(-) > > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c > index c64d543..c88485d 100644 > --- a/drivers/clocksource/arm_arch_timer.c > +++ b/drivers/clocksource/arm_arch_timer.c > @@ -765,20 +772,34 @@ static void __init arch_timer_mem_init(struct device_node *np) > */ > for_each_available_child_of_node(np, frame) { > int n; > + u32 cntacr; > > if (of_property_read_u32(frame, "frame-number", &n)) { > pr_err("arch_timer: Missing frame-number\n"); > - of_node_put(best_frame); > of_node_put(frame); > - return; > + goto out; > } > > - if (cnttidr & CNTTIDR_VIRT(n)) { > + /* Try enabling everything, and see what sticks */ > + cntacr = CNTACR_RFRQ | CNTACR_RWPT | CNTACR_RPCT | > + CNTACR_RWVT | CNTACR_RVOFF | CNTACR_RVCT; > + writel_relaxed(cntacr, cntctlbase + CNTACR(n)); > + cntacr = readl_relaxed(cntctlbase + CNTACR(n)); > + > + if (~cntacr & CNTACR_RFRQ) > + continue; Do we need this check? If we can't read the frequency we fall back to looking for the DT property, so it shouldn't matter if we can't read the hardware there. > + > + if ((cnttidr & CNTTIDR_VIRT(n)) && > + !(~cntacr & (CNTACR_RWVT | CNTACR_RVCT))) { > of_node_put(best_frame); > best_frame = frame; > arch_timer_mem_use_virtual = true; > break; > } > + > + if (~cntacr & (CNTACR_RWPT | CNTACR_RPCT)) > + continue; > + > of_node_put(best_frame); > best_frame = of_node_get(frame); > } Otherwise the patch looks fine and passes some light testing on qcom devices. BTW, I'd like to add this patch on top so that we get some info in /proc/iomem about which frame region is in use. ---8<--- From: Stephen Boyd Subject: [PATCH] clocksource/arm_arch_timer: Map frame with of_io_request_and_map() Let's use the of_io_request_and_map() API so that the frame region is protected and shows up in /proc/iomem. Signed-off-by: Stephen Boyd --- drivers/clocksource/arm_arch_timer.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index c88485d489bf..59a08fd4f76a 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -804,7 +804,8 @@ static void __init arch_timer_mem_init(struct device_node *np) best_frame = of_node_get(frame); } - base = arch_counter_base = of_iomap(best_frame, 0); + base = arch_counter_base = of_io_request_and_map(best_frame, 0, + "arch_mem_timer"); if (!base) { pr_err("arch_timer: Can't map frame's registers\n"); goto out;