From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932282AbbKMNwi (ORCPT ); Fri, 13 Nov 2015 08:52:38 -0500 Received: from mout.kundenserver.de ([212.227.17.13]:57406 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751981AbbKMNwe (ORCPT ); Fri, 13 Nov 2015 08:52:34 -0500 From: Arnd Bergmann To: Jisheng Zhang Cc: daniel.lezcano@linaro.org, tglx@linutronix.de, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2] clocksource/drivers/dw_apb_timer: Use {readl|writel}_relaxed Date: Fri, 13 Nov 2015 14:51:57 +0100 Message-ID: <5804676.futR40KtNg@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20151113205727.24354052@xhacker> References: <1447417883-7881-1-git-send-email-jszhang@marvell.com> <3713227.4t0cg6qNF9@wuerfel> <20151113205727.24354052@xhacker> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:sI/0WRy3iDQYzJ5AJKQhjG13kZ1bYejBt0LSA2794NP/LxANnOC IgJyHNppEs15/b/fP4VZ0/Uuqe4BqfCxCuKPbKJ6GkqbotS9JLjdLyvPmflb+c13oe5T4Fz N5RSX959ODuPtvSkpsSvGuoujcMOrkPxIgQmItSQ08cW2SJRSuAUNYfDWEJeD46BgJ817Fs VCiHosOC88Xw00+mdulTw== X-UI-Out-Filterresults: notjunk:1;V01:K0:kgTWdEuSTYs=:gIu9Z1trcCwDuzEFAQS6hY dS7X3zKBnS5QguFvZaNi8OytCUBRduaEH5uWD/NyAKWs8FActKH7Kv+XvhSRfDnGxOHQoLsRo NjjeKTmiDHLuYQf/Wk60wmiFlOE9Iy1IxbAh3XYBl13YAZmEcJA+Ofj+fl7Yn7//g7OmfO6u1 wBRNVfivrldrX3/NvsGDUrAboWvjsiD5AXO/whFa+NnbZUHVWLUfdzKAUzHOYLOAcwx62nNDS VG9nfcEHw8PZ3e64jdGBk9pEhuPTFGk6y5NUgpLW7VtCPzxM8WuNBZcmTHwXX7IbY7+wYeXCr o7MduCyhQg2vh8zQcR7Dr0kOln2zqgBhHbllLnXGKaz80xAAvmT6Ds1Th4g7dD4IUBNXnrnHe 4QmI4zAHlV+NR5L2O5L3v049LFmU9FQGWXs9fcx5tHG7T1hkcGWtJ0y+LC6Mo209LGOh+jp2B tBYMwYscxE3MNJTkPOU9bEw+2zY5LFBXNjaIdRWs0UEGNfTfZ6+qwKBfoygDidGfWfZ2C6wpt RYVaa9YvFOkvBopcKIMN0/Mh1cTxBXQFM6pTH6jAWseTLhp+M5jpBMXoEhGfO67miadrXlPfT MW/gMriNnrnjd/OzBsNZiI1mE0Kjl3A8ur+ZogEToWgZmpHBmEH7AkvuIhXG+drZAApmj9nX/ MXhaCf0rEC/6MFkvLAzyhqZy0y0ws/S/0C7fXVy6L45yrT+E/NxKAgH7heltHrborx+NxgfIo qcwOh0awNl3GGztl Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday 13 November 2015 20:57:27 Jisheng Zhang wrote: > > > that the conversion is correct. You could introduce apbt_readl_releaxed() > > etc functions and call them from __apbt_read_clocksource() > > and apbt_next_event(). > > I'm not sure whether such changes would make the code a bit complex. From > another side, it's safe to always use relaxed version in this driver, so > is it better to switch to relaxed version no matter the code path benefit from > it or not? We've had problems in the past when people blindly converted whole drivers, so I try to discourage that in general, if only to get people to pay more attention when copying from one driver to another. > PS: for the global timer related patch, I just hack code a bit to make it works > as clockevent on my platform, and I still try to think about a test case to > measure the improvement, cyclictest? Any idea is appreciated. Measuring performance of timers is tricky by definition, but you could try to sample the amount of time you spend setting up timers like this diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c index a2cb6fae9295..da88347718ae 100644 --- a/drivers/clocksource/arm_global_timer.c +++ b/drivers/clocksource/arm_global_timer.c @@ -95,6 +95,8 @@ static u64 gt_counter_read(void) static void gt_compare_set(unsigned long delta, int periodic) { u64 counter = gt_counter_read(); + static u64 total_time; + static u32 count; unsigned long ctrl; counter += delta; @@ -110,6 +112,12 @@ static void gt_compare_set(unsigned long delta, int periodic) ctrl |= GT_CONTROL_COMP_ENABLE | GT_CONTROL_IRQ_ENABLE; writel(ctrl, gt_base + GT_CONTROL); + + total_time += gt_counter_read() - counter; + count++; + + if (((count - 1) & 0xfff) == 0xfff) + printk(KERN_INFO "gt_compare_set time %lld\n", total_time / (count >> 12)); } static int gt_clockevent_shutdown(struct clock_event_device *evt) Arnd