From patchwork Wed Aug 21 19:07:29 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pavel Machek X-Patchwork-Id: 2847879 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.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 253669F271 for ; Wed, 21 Aug 2013 19:08:28 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EC66C20570 for ; Wed, 21 Aug 2013 19:08:26 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B224E2054B for ; Wed, 21 Aug 2013 19:08:23 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1VCDlT-0005xB-FF; Wed, 21 Aug 2013 19:08:19 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VCDlR-0002uw-8j; Wed, 21 Aug 2013 19:08:17 +0000 Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1VCDlF-0002to-NI for linux-arm-kernel@lists.infradead.org; Wed, 21 Aug 2013 19:08:14 +0000 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 512) id 78EC88175D; Wed, 21 Aug 2013 21:07:30 +0200 (CEST) Date: Wed, 21 Aug 2013 21:07:29 +0200 From: Pavel Machek To: Linus Walleij Subject: Re: [PATCHv2 2/2] clocksource: dw_apb_timer_of: Fix read_sched_clock Message-ID: <20130821190729.GA9657@amd.pavel.ucw.cz> References: <1376522956-14960-1-git-send-email-dinguyen@altera.com> <1376522956-14960-2-git-send-email-dinguyen@altera.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130821_150805_980853_69EB4F54 X-CRM114-Status: GOOD ( 31.04 ) X-Spam-Score: -1.9 (-) Cc: Dinh Nguyen , Heiko Stuebner , Arnd Bergmann , Rob Herring , John Stultz , "linux-arm-kernel@lists.infradead.org" , Olof Johansson , Jamie Iles , Thomas Gleixner , Dinh Nguyen , wd@denx.de X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, 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 Wed 2013-08-21 09:04:07, Linus Walleij wrote: > On Thu, Aug 15, 2013 at 1:29 AM, wrote: > > > - sched_io_base = iobase + 0x04; > > + sched_io_base = iobase; > > sched_rate = rate; > > } > > > > static u32 read_sched_clock(void) > > { > > - return __raw_readl(sched_io_base); > > + return ~__raw_readl(sched_io_base + 0x4); > > So what about #define what 0x04 is? > > #define MY_FOO_REGISTER_OFFSET 0x04 > > raw_readl(sched_io_base + MY_FOO_REGISTER_OFFSET); That define is already there, #define APBTMR_N_CURRENT_VALUE 0x04, but it is in .c file, not in header. Actually I believe I had patch that did that, and it was even applied, but it clashed with same other patches, and was dropped. (It is below, for reference). Currently, the code _does not work_, and we have three platforms using it. It would be good to solve this now, and polish it later. Pavel From pavel@denx.de Tue May 7 22:11:26 2013 Date: Tue, 7 May 2013 22:11:26 +0200 From: Pavel Machek To: John Stultz Cc: Arnd Bergmann , tglx@linutronix.de, Jamie Iles , dinguyen@altera.com, wd@denx.de, linux-arm-kernel@lists.infradead.org, olof@lixom.net, kernel list Subject: Re: dw_apb_timer_of.c: remove parts that were picoxcell-specific Message-ID: <20130507201126.GA8169@amd.pavel.ucw.cz> References: <20130426121433.GA16249@amd.pavel.ucw.cz> <201305061545.22587.arnd@arndb.de> <20130506155304.GA6645@amd.pavel.ucw.cz> <201305062324.17080.arnd@arndb.de> <20130507135752.GA3500@amd.pavel.ucw.cz> <51892E5E.8090909@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <51892E5E.8090909@linaro.org> User-Agent: Mutt/1.5.20 (2009-06-14) Status: RO Content-Length: 4987 Lines: 170 Hi! > >>>So I guess it should go through the timekeeping tree...? > >>I would prefer that, but we can also take it through arm-soc, it doesn't > >>really matter. > >I see no response from timekeeping people... so if you could take it, > >that would be great. > > I don't think the original patch ever made it to my inbox. For some reason, I cced johnstul@us.ibm.com. (I guess get_maintainer.pl? But it seems to behave now.) Anyway, patch is below... It would be great if you could apply it... Thanks, Pavel ---- It seems we made a mistake when creating dw_apb_timer_of.c: picoxcell sched_clock had parts that were not related to dw_apb_timer, yet we moved them to dw_apb_timer_of, and tried to use them on socfpga. This results in system where user/system time is not measured properly, as demonstrated by time dd if=/dev/urandom of=/dev/zero bs=100000 count=100 So this patch switches sched_clock to hardware that exists on both platforms, and adds missing of_node_put() in dw_apb_timer_init(). Signed-off-by: Pavel Machek Acked-by: Jamie Iles diff --git a/arch/arm/mach-picoxcell/common.h b/arch/arm/mach-picoxcell/common.h index 481b42a..237fb3b 100644 --- a/arch/arm/mach-picoxcell/common.h +++ b/arch/arm/mach-picoxcell/common.h @@ -12,6 +12,4 @@ #include -extern void dw_apb_timer_init(void); - #endif /* __PICOXCELL_COMMON_H__ */ diff --git a/drivers/clocksource/dw_apb_timer.c b/drivers/clocksource/dw_apb_timer.c index 8c2a35f..460ac99 100644 --- a/drivers/clocksource/dw_apb_timer.c +++ b/drivers/clocksource/dw_apb_timer.c @@ -21,12 +21,6 @@ #define APBT_MIN_PERIOD 4 #define APBT_MIN_DELTA_USEC 200 -#define APBTMR_N_LOAD_COUNT 0x00 -#define APBTMR_N_CURRENT_VALUE 0x04 -#define APBTMR_N_CONTROL 0x08 -#define APBTMR_N_EOI 0x0c -#define APBTMR_N_INT_STATUS 0x10 - #define APBTMRS_INT_STATUS 0xa0 #define APBTMRS_EOI 0xa4 #define APBTMRS_RAW_INT_STATUS 0xa8 diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c index ab09ed3..0fa3104 100644 --- a/drivers/clocksource/dw_apb_timer_of.c +++ b/drivers/clocksource/dw_apb_timer_of.c @@ -57,6 +57,15 @@ static void add_clockevent(struct device_node *event_timer) dw_apb_clockevent_register(ced); } +static void __iomem *sched_io_base; + +/* This is actually same as __apbt_read_clocksource(), but with + different interface */ +static u32 read_sched_clock_sptimer(void) +{ + return ~__raw_readl(sched_io_base + APBTMR_N_CURRENT_VALUE); +} + static void add_clocksource(struct device_node *source_timer) { void __iomem *iobase; @@ -71,41 +80,27 @@ static void add_clocksource(struct device_node *source_timer) dw_apb_clocksource_start(cs); dw_apb_clocksource_register(cs); -} -static void __iomem *sched_io_base; - -static u32 read_sched_clock(void) -{ - return __raw_readl(sched_io_base); + sched_io_base = iobase; + setup_sched_clock(read_sched_clock_sptimer, 32, rate); } -static const struct of_device_id sptimer_ids[] __initconst = { - { .compatible = "picochip,pc3x2-rtc" }, +static const struct of_device_id osctimer_ids[] __initconst = { + { .compatible = "picochip,pc3x2-timer" }, + { .compatible = "snps,dw-apb-timer-osc" }, { .compatible = "snps,dw-apb-timer-sp" }, - { /* Sentinel */ }, + { /* Sentinel */ }, }; -static void init_sched_clock(void) -{ - struct device_node *sched_timer; - u32 rate; - - sched_timer = of_find_matching_node(NULL, sptimer_ids); - if (!sched_timer) - panic("No RTC for sched clock to use"); +/* + You don't have to use dw_apb_timer for scheduler clock, + this should also work fine on arm: - timer_get_base_and_rate(sched_timer, &sched_io_base, &rate); - of_node_put(sched_timer); + twd_local_timer_of_register(); + arch_timer_of_register(); + arch_timer_sched_clock_init(); +*/ - setup_sched_clock(read_sched_clock, 32, rate); -} - -static const struct of_device_id osctimer_ids[] __initconst = { - { .compatible = "picochip,pc3x2-timer" }, - { .compatible = "snps,dw-apb-timer-osc" }, - {}, -}; void __init dw_apb_timer_init(void) { @@ -121,7 +116,6 @@ void __init dw_apb_timer_init(void) panic("No timer for clocksource"); add_clocksource(source_timer); + of_node_put(event_timer); of_node_put(source_timer); - - init_sched_clock(); } diff --git a/include/linux/dw_apb_timer.h b/include/linux/dw_apb_timer.h index dd755ce..a64c987 100644 --- a/include/linux/dw_apb_timer.h +++ b/include/linux/dw_apb_timer.h @@ -17,6 +17,12 @@ #include #include +#define APBTMR_N_LOAD_COUNT 0x00 +#define APBTMR_N_CURRENT_VALUE 0x04 +#define APBTMR_N_CONTROL 0x08 +#define APBTMR_N_EOI 0x0c +#define APBTMR_N_INT_STATUS 0x10 + #define APBTMRS_REG_SIZE 0x14 struct dw_apb_timer {