| Message ID | 5048BF3B.8050302@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Sep 06, 2012 at 20:50:27, Hunter, Jon wrote: > > 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 ... > I would still vote for adding this info to hwmod data, I think that is the right place for all such hw related information. Thanks, Viabhav > 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 >
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