From patchwork Wed May 21 18:34:12 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chirantan Ekbote X-Patchwork-Id: 4218401 Return-Path: X-Original-To: patchwork-linux-samsung-soc@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id F2084BEEAB for ; Wed, 21 May 2014 18:34:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 901CB2026C for ; Wed, 21 May 2014 18:34:17 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 438882021B for ; Wed, 21 May 2014 18:34:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751616AbaEUSeO (ORCPT ); Wed, 21 May 2014 14:34:14 -0400 Received: from mail-ob0-f175.google.com ([209.85.214.175]:61635 "EHLO mail-ob0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751137AbaEUSeN (ORCPT ); Wed, 21 May 2014 14:34:13 -0400 Received: by mail-ob0-f175.google.com with SMTP id wo20so2581116obc.34 for ; Wed, 21 May 2014 11:34:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=faE4+J+KKW+FeDzUQZ/NEmO5iqjBDdAIYGJeb0/AOCw=; b=QRkQe/8W2gsc5k+D1Uzr1hQZ5/ryFn3nfVYujsA+riJ2iG0XpaPv2es1/VWVrYBAF7 4qz2DqMyoooT8t8lslTA74UfsNnbYJDJWC+wA8/5+p3N0pq3Ektclco08dzxpmYHlrxQ H0uk82qeOJmgBYxHYDjXelxOW5PBT8FFYseQnYDJK09vLkuygLDJxhKtXnJEyvBrB0bU G/NqdrJq18PpVKnC0aYnIL6C0xYw6ZPdl4AFmCa7mXQmlPUPxdGblWJnKy3+lQRG5J6l G063mqTL5WnfvxpWgufj+pMHRlgfqA/EaLV8KCxdCE3vI5fo+Zph1w+sliF4x1x+dVAe ei4g== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:sender:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; bh=faE4+J+KKW+FeDzUQZ/NEmO5iqjBDdAIYGJeb0/AOCw=; b=JUsBpZmJkNZ6S8Kebp78bDK33u0SOO+0KmYboOlHkDETOL5DleKyLX4d0soZSfXNXn jgxj1EpWt1RfBTvT6wlaNVIiE8sUuYHJAIKlyQzjqCwE5A2Hkm1W7mF0r5H3ojpbH7Xf 0L0KXzCxXauOGr6F2l40Y1rMdx+uV8Lx7XyzM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:sender:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=faE4+J+KKW+FeDzUQZ/NEmO5iqjBDdAIYGJeb0/AOCw=; b=WBYkVXGcWx9WaYDhP5awA1GnOSHkuQTAK8k4mOiHCNU73XMD53yTcG2dSY7Meiuby3 skal4y27n4bLRse5fBxptkKM/++pPMzsvtI3cNYpz6o4GZCKIfvXwmZu/+MMF8bXL5Lj Ch2ZgzPCfzQbJ8ws1K0Rke6uKa87Fe6qZyT0JAGjORu1VEa/VvRHRz1ON7B4rxMoMtrT MCWYpNCHlUNonB9Wv8nHr+sx7+1fZVQN0WTN3whrbNsMGU7dTLUA5QoAcl90jT765ScY Fkcn5GaGSosG/Y+weTPj6/urq2LXTr8fVJ1fUytuNlWRFd66k5LhRQ1mwrytwm4sDM71 D6Aw== X-Gm-Message-State: ALoCoQnxoMMfxIuWpaVzWdDjU9Hmf1lesfOMzuW8ehCzKUwlRimKIULN/rUPWzv54zUGJSe3e/AK MIME-Version: 1.0 X-Received: by 10.60.69.71 with SMTP id c7mr5482882oeu.82.1400697252833; Wed, 21 May 2014 11:34:12 -0700 (PDT) Received: by 10.182.123.231 with HTTP; Wed, 21 May 2014 11:34:12 -0700 (PDT) In-Reply-To: <033a01cf74f2$d025ce80$70716b80$@samsung.com> References: <1400188079-21832-1-git-send-email-chirantan@chromium.org> <53752E25.9060604@gmail.com> <53753443.8010303@gmail.com> <53753C17.1090002@gmail.com> <53754CE2.3000905@gmail.com> <033a01cf74f2$d025ce80$70716b80$@samsung.com> Date: Wed, 21 May 2014 11:34:12 -0700 X-Google-Sender-Auth: 0y8xO0uNFMPxWQYhHSmPFi0vyi0 Message-ID: Subject: Re: [PATCH] arm: dts: exynos5: Remove multi core timer From: Chirantan Ekbote To: Kukjin Kim Cc: Sonny Rao , Doug Anderson , Tomasz Figa , David Riley , Russell King , Olof Johansson , "linux-arm-kernel@lists.infradead.org" , linux-samsung-soc Sender: linux-samsung-soc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-samsung-soc@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham 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, May 21, 2014 at 5:47 AM, Kukjin Kim wrote: > Chirantan Ekbote wrote: >> >> >>> Anyway, I'm by no means opposed to switching to arch timers. They >> >>> provide a well designed, generic interface and drivers shared by >> >>> multiple platforms, which means more code sharing and possibly more eyes >> >>> looking at the code, which is always good. However if they don't support >> >>> low power states correctly, we can't just remove MCT. >> >> >> >> I think low power states aren't in mainline (right?). >> >> >> >> One solution that might work could be to leave the device tree entry >> >> alone but change the MCT init code to simply act as a no-op if it sees >> >> an arch timer is in the device tree and enabled. Then when/if someone >> >> got the low power states enabled we could just change source code >> >> rather than dts files. >> >> >> Doug and I were talking about this and we think we may have a way to >> have the mct and arch timers co-exist. The main issue is that the mct >> (and therefore arch timer) gets cleared once during boot and every >> time we do a suspend / resume. This happens in >> exynos4_mct_frc_start() but it's not immediately clear to us why the >> counter needs to be reset at all. If we remove the lines that clear >> the counter then there is no longer an issue with having both the mct >> and the arch timers on at the same time. >> > Yeah, actually we don't need to reset the count value after suspend/resume. > So, how about following? I think, it should be fine to you. > > diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c > index 8d64200..d24db6f 100644 > --- a/drivers/clocksource/exynos_mct.c > +++ b/drivers/clocksource/exynos_mct.c > @@ -157,12 +157,15 @@ static void exynos4_mct_frc_start(u32 hi, u32 lo) > { > u32 reg; > > - exynos4_mct_write(lo, EXYNOS4_MCT_G_CNT_L); > - exynos4_mct_write(hi, EXYNOS4_MCT_G_CNT_U); > - > reg = __raw_readl(reg_base + EXYNOS4_MCT_G_TCON); > - reg |= MCT_G_TCON_START; > - exynos4_mct_write(reg, EXYNOS4_MCT_G_TCON); > + > + if (!(reg & MCT_G_TCON_START)) { > + exynos4_mct_write(lo, EXYNOS4_MCT_G_CNT_L); > + exynos4_mct_write(hi, EXYNOS4_MCT_G_CNT_U); > + > + reg |= MCT_G_TCON_START; > + exynos4_mct_write(reg, EXYNOS4_MCT_G_TCON); > + } > } > So if I understand correctly, we still need to reset the counter during boot? This is still a problem because there would be a big jump in time since the sched_clock code would think that the timer had wrapped around. Additionally, any clock events that were scheduled before the reset would be delayed until the cycle counter caught back up to the value it had previously. This manifests itself in our tree as an extra long jiffy when the xor code is measuring the software checksum speed and it delays the entire boot process. There is a hacky solution to this, which is to ensure that the mct is always initialized before the arch timer. This should be possible by reordering the entries in the device tree. We would also need to change the clocksource rating for one of the two (either increase the arch timer rating or decrease the mct rating) to ensure that the kernel still picked the arch timer as its clocksource. > >> Alternately, if there is some code that depends on the mct being reset >> we could store an offset instead of clearing the counter and then >> subtract that offset every time something reads it. Doug has a patch >> that does this at >> https://chromium-review.googlesource.com/#/c/200298/. Effectively the >> visible behavior will not change. Would either of these options work? >> > Hmm...I cannot open the webpage :( > I've attached the patch to this email. Chirantan From 468ab95fb57f1b72da838aa46346635a48a94af4 Mon Sep 17 00:00:00 2001 From: Doug Anderson Date: Fri, 16 May 2014 14:12:13 -0700 Subject: [PATCH] clocksource: mct: Don't clear the mct On exynos5 SoCs, the mct and the arch timers appear to share the same root timer. ...so clearing the mct actually ends up clearing up the arch timer and that confuses the heck out of the arch timer code. Let's allow the arch timer and mct driver to coexist by just not ever clearing the timer. Just in case someone cares, we'll still pretend that we cleared it by keeping track of our own offset. BUG=chrome-os-partner:13805 TEST=Arch timer no longers goes all screwy Change-Id: I2c4bd3cb0fede67c36a2734b2972763f079f2872 Signed-off-by: Doug Anderson --- drivers/clocksource/exynos_mct.c | 42 +++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c index b067219..32b8008 100644 --- a/drivers/clocksource/exynos_mct.c +++ b/drivers/clocksource/exynos_mct.c @@ -93,6 +93,7 @@ static unsigned long clk_rate; static unsigned int mct_int_type; static int mct_irqs[MCT_NR_IRQS]; static int nr_mct_irqs; +static cycle_t exynos_mct_offset; static const char *irq_names[MCT_NR_LOCAL_IRQS] = { "mct_tick0_irq", @@ -176,24 +177,13 @@ static void exynos4_mct_write(unsigned int value, unsigned long offset) } /* Clocksource handling */ -static void exynos4_mct_frc_start(u32 hi, u32 lo) -{ - u32 reg; - - exynos4_mct_write(lo, EXYNOS4_MCT_G_CNT_L); - exynos4_mct_write(hi, EXYNOS4_MCT_G_CNT_U); - - reg = __raw_readl(reg_base + EXYNOS4_MCT_G_TCON); - reg |= MCT_G_TCON_START; - exynos4_mct_write(reg, EXYNOS4_MCT_G_TCON); -} - static notrace u32 exynos4_read_sched_clock(void) { - return __raw_readl(reg_base + EXYNOS4_MCT_G_CNT_L); + return __raw_readl(reg_base + EXYNOS4_MCT_G_CNT_L) - + (exynos_mct_offset & 0xffffffff); } -static cycle_t exynos4_frc_read(struct clocksource *cs) +static cycle_t _exynos4_frc_read(void) { u32 lo, hi; static u32 __suspend_volatile_bss hi2; @@ -207,9 +197,25 @@ static cycle_t exynos4_frc_read(struct clocksource *cs) return ((cycle_t)hi << 32) | lo; } +static cycle_t exynos4_frc_read(struct clocksource *cs) +{ + return _exynos4_frc_read() - exynos_mct_offset; +} + +static void exynos4_mct_frc_start(void) +{ + u32 reg; + + exynos_mct_offset = _exynos4_frc_read(); + + reg = __raw_readl(reg_base + EXYNOS4_MCT_G_TCON); + reg |= MCT_G_TCON_START; + exynos4_mct_write(reg, EXYNOS4_MCT_G_TCON); +} + static void exynos4_frc_resume(void) { - exynos4_mct_frc_start(0, 0); + exynos4_mct_frc_start(); } struct clocksource mct_frc = { @@ -226,12 +232,12 @@ struct syscore_ops mct_frc_core = { static void __init exynos4_clocksource_init(void) { - u64 initial_time = exynos4_frc_read(&mct_frc); + u64 initial_time = _exynos4_frc_read(); do_div(initial_time, (clk_rate / 1000000)); printk(KERN_INFO "Initial usec timer %llu\n", initial_time); - exynos4_mct_frc_start(0, 0); + exynos4_mct_frc_start(); if (clocksource_register_hz(&mct_frc, clk_rate)) panic("%s: can't register clocksource\n", mct_frc.name); @@ -267,7 +273,7 @@ static void exynos4_mct_comp0_start(enum clock_event_mode mode, exynos4_mct_write(cycles, EXYNOS4_MCT_G_COMP0_ADD_INCR); } - comp_cycle = exynos4_frc_read(&mct_frc) + cycles; + comp_cycle = _exynos4_frc_read() + cycles; exynos4_mct_write((u32)comp_cycle, EXYNOS4_MCT_G_COMP0_L); exynos4_mct_write((u32)(comp_cycle >> 32), EXYNOS4_MCT_G_COMP0_U); -- 1.9.1.423.g4596e3a