public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: jon-hunter@ti•com (Jon Hunter)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 01/10] ARM: OMAP3+: Implement timer workaround for errata i103 and i767
Date: Thu, 6 Sep 2012 10:20:27 -0500	[thread overview]
Message-ID: <5048BF3B.8050302@ti.com> (raw)
In-Reply-To: <5048B642.2070200@ti.com>


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 <jon-hunter@ti•com>
>>>> ---
>>>>  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

  reply	other threads:[~2012-09-06 15:20 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-05 19:04 [PATCH 00/10] ARM: OMAP: DMTIMER fixes and clean-up Jon Hunter
2012-09-05 19:04 ` [PATCH 01/10] ARM: OMAP3+: Implement timer workaround for errata i103 and i767 Jon Hunter
2012-09-06  5:07   ` Vaibhav Hiremath
2012-09-06 14:06     ` Jon Hunter
2012-09-06 14:42       ` Jon Hunter
2012-09-06 15:20         ` Jon Hunter [this message]
2012-09-13 10:26           ` Hiremath, Vaibhav
2012-09-13 10:24       ` Hiremath, Vaibhav
2012-09-05 19:04 ` [PATCH 02/10] ARM: OMAP: Fix timer posted mode support Jon Hunter
2012-09-06 12:57   ` Vaibhav Hiremath
2012-09-06 14:20     ` Jon Hunter
2012-09-06 16:01       ` Jon Hunter
2012-09-13 10:24         ` Hiremath, Vaibhav
2012-09-05 19:04 ` [PATCH 03/10] ARM: OMAP3: Correct HWMOD DMTIMER SYSC register declarations Jon Hunter
2012-09-05 19:04 ` [PATCH 04/10] ARM: OMAP2/3: Define HWMOD software reset status for DMTIMERs Jon Hunter
2012-09-05 19:04 ` [PATCH 05/10] ARM: OMAP2+: Don't use __omap_dm_timer_reset() Jon Hunter
2012-09-05 19:04 ` [PATCH 06/10] ARM: OMAP: Fix dmtimer reset for timer1 Jon Hunter
2012-09-05 19:04 ` [PATCH 07/10] ARM: OMAP: Clean-up dmtimer reset code Jon Hunter
2012-09-05 19:04 ` [PATCH 08/10] ARM: OMAP: Clean-up timer posted mode support Jon Hunter
2012-09-07 22:22   ` Tony Lindgren
2012-09-10 21:59     ` Jon Hunter
2012-09-11  0:58       ` Tony Lindgren
2012-09-11 16:26         ` Jon Hunter
2012-09-11 16:34           ` Tony Lindgren
2012-09-13  3:26             ` Jon Hunter
2012-09-05 19:04 ` [PATCH 09/10] ARM: OMAP: Add dmtimer interrupt disable function Jon Hunter
2012-09-06 12:58   ` Vaibhav Hiremath
2012-09-06 14:26     ` Jon Hunter
2012-09-05 19:04 ` [PATCH 10/10] ARM: OMAP: Remove unnecessary call to clk_get() Jon Hunter
2012-09-06 12:58 ` [PATCH 00/10] ARM: OMAP: DMTIMER fixes and clean-up Vaibhav Hiremath
2012-09-06 14:30   ` Jon Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5048BF3B.8050302@ti.com \
    --to=jon-hunter@ti$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox