From patchwork Thu Sep 6 15:20:27 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Hunter, Jon" X-Patchwork-Id: 1417101 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) by patchwork1.kernel.org (Postfix) with ESMTP id 66EE23FC71 for ; Thu, 6 Sep 2012 16:43:41 +0000 (UTC) Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.76 #1 (Red Hat Linux)) id 1T9esC-00087f-AV; Thu, 06 Sep 2012 16:24:09 +0000 Received: from comal.ext.ti.com ([198.47.26.152]) by merlin.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1T9dsd-0000RI-Oc for linux-arm-kernel@lists.infradead.org; Thu, 06 Sep 2012 15:20:34 +0000 Received: from dlelxv30.itg.ti.com ([172.17.2.17]) by comal.ext.ti.com (8.13.7/8.13.7) with ESMTP id q86FKSnR021185; Thu, 6 Sep 2012 10:20:28 -0500 Received: from DLEE74.ent.ti.com (dlee74.ent.ti.com [157.170.170.8]) by dlelxv30.itg.ti.com (8.13.8/8.13.8) with ESMTP id q86FKSKQ023025; Thu, 6 Sep 2012 10:20:28 -0500 Received: from [192.157.144.139] (192.157.144.139) by DLEE74.ent.ti.com (157.170.170.8) with Microsoft SMTP Server id 14.1.323.3; Thu, 6 Sep 2012 10:20:27 -0500 Message-ID: <5048BF3B.8050302@ti.com> Date: Thu, 6 Sep 2012 10:20:27 -0500 From: Jon Hunter User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120827 Thunderbird/15.0 MIME-Version: 1.0 To: Vaibhav Hiremath Subject: Re: [PATCH 01/10] ARM: OMAP3+: Implement timer workaround for errata i103 and i767 References: <1346871872-24413-1-git-send-email-jon-hunter@ti.com> <1346871872-24413-2-git-send-email-jon-hunter@ti.com> <50482FAC.3080807@ti.com> <5048ADD0.4050804@ti.com> <5048B642.2070200@ti.com> In-Reply-To: <5048B642.2070200@ti.com> X-Originating-IP: [192.157.144.139] X-Spam-Note: CRM114 invocation failed X-Spam-Score: -6.9 (------) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-6.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -5.0 RCVD_IN_DNSWL_HI RBL: Sender listed at http://www.dnswl.org/, high trust [198.47.26.152 listed in list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Tony Lindgren , Kevin Hilman , Paul Walmsley , linux-omap , linux-arm X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org On 09/06/2012 09:42 AM, Jon Hunter wrote: > > On 09/06/2012 09:06 AM, Jon Hunter wrote: >> >> On 09/06/2012 12:07 AM, Vaibhav Hiremath wrote: >>> >>> >>> On 9/6/2012 12:34 AM, Jon Hunter wrote: >>>> Errata Titles: >>>> i103: Delay needed to read some GP timer, WD timer and sync timer registers >>>> after wakeup (OMAP3/4) >>>> i767: Delay needed to read some GP timer registers after wakeup (OMAP5) >>>> >>>> Description (i103/i767): >>>> If a General Purpose Timer (GPTimer) is in posted mode (TSICR [2].POSTED=1), >>>> due to internal resynchronizations, values read in TCRR, TCAR1 and TCAR2 >>>> registers right after the timer interface clock (L4) goes from stopped to >>>> active may not return the expected values. The most common event leading to >>>> this situation occurs upon wake up from idle. >>>> >>>> GPTimer non-posted synchronization mode is not impacted by this limitation. >>>> >>>> Workarounds: >>>> 1). Disable posted mode >>>> 2). Use static dependency between timer clock domain and MPUSS clock domain >>>> 3). Use no-idle mode when the timer is active >>>> >>>> Workarounds #2 and #3 are not pratical from a power standpoint and so >>>> workaround #1 has been implemented. Disabling posted mode adds some CPU overhead >>>> for configuring the timers as the CPU has to wait for the write to complete. >>>> However, disabling posted mode guarantees correct operation. >>>> >>>> Please note that it is safe to use posted mode for timers if the counter (TCRR) >>>> and capture (TCARx) registers will never be read. An example of this is the >>>> clock-event system timer. This is used by the kernel to schedule events however, >>>> the timers counter is never read and capture registers are not used. Given that >>>> the kernel configures this timer often yet never reads the counter register it >>>> is safe to enable posted mode in this case. Hence, for the timer used for kernel >>>> clock-events, posted mode is enabled by overriding the errata for devices that >>>> are impacted by this defect. >>>> >>>> Although both dmtimers and watchdogs are impacted by this defect this patch only >>>> implements the workaround for the dmtimer. Currently the watchdog driver does >>>> not read the counter register and so no workaround is necessary. >>>> >>>> Confirmed with Vaibhav Hiremath that this bug also impacts AM33xx devices. >>>> >>> >>> Thanks for pinging me on this and getting it confirmed. >>> >>>> Signed-off-by: Jon Hunter >>>> --- >>>> arch/arm/mach-omap2/timer.c | 9 +++++++ >>>> arch/arm/plat-omap/dmtimer.c | 2 ++ >>>> arch/arm/plat-omap/include/plat/dmtimer.h | 39 +++++++++++++++++++++++++++++ >>>> 3 files changed, 50 insertions(+) >>>> >>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c >>>> index 2ff6d41..5471706 100644 >>>> --- a/arch/arm/mach-omap2/timer.c >>>> +++ b/arch/arm/mach-omap2/timer.c >>>> @@ -208,6 +208,13 @@ static void __init omap2_gp_clockevent_init(int gptimer_id, >>>> { >>>> int res; >>>> >>>> + /* >>>> + * For clock-event timers we never read the timer counter and >>>> + * so we are not impacted by errata i103 and i767. Therefore, >>>> + * we can safely ignore this errata for clock-event timers. >>>> + */ >>>> + __omap_dm_timer_populate_errata(&clkev, OMAP_TIMER_ERRATA_I103_I767); >>>> + >>> >>> Couple of points, >>> >>> 1. It is confusing to me, as you are passing the errata flag so i expect >>> api should set it. Why can't we do reverse way, you pass 0 here, since >>> you don't want to set and pass this flag every other places where you >>> want to enable this errata. >> >> Per the design of the __omap_dm_timer_populate_errata function, the 2nd >> argument is called override to allow us to override an errata. I am not >> a huge fan of this, but I wanted to be explicit in the code that we are >> intentionally allowing posted mode for the clock-events timer. >> >> I did not wish to pass the flags we want to set because if there more >> flags added in the future then we will have to keep changing the calls >> to the populate_errata function to add these. > > By the way, your proposal could work nicely if we could pass errata > flags from HWMOD. However, I am not sure if Paul or Benoit would go for > this as they want HWMOD data to be auto-generated as much as possible > and so I am not sure how that would work for errata which are not > expected by design ;-) Another alternative would be to drop the override argument altogether and just do something like the following for the clock-events timer ... diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index 43da595..f59e797 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -206,12 +206,14 @@ static void __init omap2_gp_clockevent_init(int gptimer_id, { int res; + __omap_dm_timer_populate_errata(&clkev); + /* * For clock-event timers we never read the timer counter and * so we are not impacted by errata i103 and i767. Therefore, * we can safely ignore this errata for clock-event timers. */ - __omap_dm_timer_populate_errata(&clkev, OMAP_TIMER_ERRATA_I103_I767); + clkev.errata &= ~OMAP_TIMER_ERRATA_I103_I767; Cheers Jon