Re: [PATCH V2 03/14] ARM: OMAP3+: Implement timer workaround for errata i103 and i767
On 11/08/2012 10:55 AM, Jon Hunter wrote:
>
> On 11/07/2012 05:43 PM, Santosh Shilimkar wrote:
>> On Wednesday 07 November 2012 05:28 PM, Jon Hunter wrote:
>>>
>>> On 11/07/2012 04:14 PM, Santosh Shilimkar wrote:
>>>
Looks sensible considering alternative WAs.
Acked-by: Santosh Shilimkar
>>>
>>> Thanks. With further thought I think that it would be best to combine
>>> patches #2 and #3. Really the main motivation here is the errata
>>> workaround and without actually benchmarking the timer read I should
>>> not claim the improvement in overhead as stated in patch #2. So I
>>> have combined #2 and #3 and updated the changelog/comments
>>> appropriately. Let me know if you guys are ok with this.
>>>
>> Yep. Sounds good.
>
> Updated, removing the additional OMAP_CLKEVT/SRC_POSTEDMODE definitions.
Another minor update after rebasing on Tony's linux-omap master. I
needed to fix-up headers included.
Cheers
Jon
>From 0b2e3ef2cea7896f6466e010f9ab22a86a00bba1 Mon Sep 17 00:00:00 2001
From: Jon Hunter
Date: Thu, 27 Sep 2012 12:47:43 -0500
Subject: [PATCH] ARM: OMAP3+: Implement timer workaround for errata i103 and
i767
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 and reading the timers as the CPU has to wait
for accesses to be re-synchronised within the timer. 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.
For drivers using the timers that do not read the counter or capture
registers and wish to use posted mode, can override the errata and
enable posted mode by making the following function calls.
omap_dm_timer_override_errata(timer, OMAP_TIMER_ERRATA_I103_I767);
omap_dm_timer_enable_posted(timer);
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.
Posted mode will be disabled for all OMAP2+ devices (including AM33xx)
using a GP timer as a clock-source timer to guarantee correct operation.
This is not necessary for OMAP24xx devices but the default clock-source
timer for OMAP24xx devices is the 32k-sync timer and not the GP timer
and so should not have any impact. This should be re-visited for future
devices if this errata is fixed.
Confirmed with Vaibhav Hiremath that this bug also impacts AM33xx
devices.
Signed-off-by: Jon Hunter
Acked-by: Santosh Shilimkar
---
arch/arm/mach-omap2/timer.c | 35 +
arch/arm/plat-omap/dmtimer.c | 60 -
arch/arm/plat-omap/include/plat/dmtimer.h | 18 +++--
3 files changed, 100 insertions(+), 13 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index a135d28..23ebe15 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -225,7 +225,8 @@ void __init omap_dmtimer_init(void)
static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
int gptimer_id,
const char *fck_source,
- const char *property)
+ const char *property,
+ int posted)
{
char name[10]; /* 10 = sizeof("gptXX_Xck0") */
const char *oh_name;
@@ -311,10 +312,15 @@ static int __init omap_dm_timer_init_one(struct
omap_dm_timer *
Re: [PATCH V2 03/14] ARM: OMAP3+: Implement timer workaround for errata i103 and i767
On 11/07/2012 05:43 PM, Santosh Shilimkar wrote:
> On Wednesday 07 November 2012 05:28 PM, Jon Hunter wrote:
>>
>> On 11/07/2012 04:14 PM, Santosh Shilimkar wrote:
>>
>>> Looks sensible considering alternative WAs.
>>>
>>> Acked-by: Santosh Shilimkar
>>
>> Thanks. With further thought I think that it would be best to combine
>> patches #2 and #3. Really the main motivation here is the errata
>> workaround and without actually benchmarking the timer read I should
>> not claim the improvement in overhead as stated in patch #2. So I
>> have combined #2 and #3 and updated the changelog/comments
>> appropriately. Let me know if you guys are ok with this.
>>
> Yep. Sounds good.
Updated, removing the additional OMAP_CLKEVT/SRC_POSTEDMODE definitions.
Cheers
Jon
>From 5b55c6c2ca6f41e37f531d8ca0ea80a0e49f3e4d Mon Sep 17 00:00:00 2001
From: Jon Hunter
Date: Thu, 27 Sep 2012 12:47:43 -0500
Subject: [PATCH 2/2] ARM: OMAP3+: Implement timer workaround for errata i103
and i767
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 and reading the timers as the CPU has to wait
for accesses to be re-synchronised within the timer. 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.
For drivers using the timers that do not read the counter or capture
registers and wish to use posted mode, can override the errata and
enable posted mode by making the following function calls.
omap_dm_timer_override_errata(timer, OMAP_TIMER_ERRATA_I103_I767);
omap_dm_timer_enable_posted(timer);
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.
Posted mode will be disabled for all OMAP2+ devices (including AM33xx)
using a GP timer as a clock-source timer to guarantee correct operation.
This is not necessary for OMAP24xx devices but the default clock-source
timer for OMAP24xx devices is the 32k-sync timer and not the GP timer
and so should not have any impact. This should be re-visited for future
devices if this errata is fixed.
Confirmed with Vaibhav Hiremath that this bug also impacts AM33xx
devices.
Signed-off-by: Jon Hunter
---
arch/arm/mach-omap2/timer.c | 35 +
arch/arm/plat-omap/dmtimer.c | 59 -
arch/arm/plat-omap/include/plat/dmtimer.h | 19 --
3 files changed, 100 insertions(+), 13 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index ad427ba..e99b95c 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -224,7 +224,8 @@ void __init omap_dmtimer_init(void)
static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
int gptimer_id,
const char *fck_source,
- const char *property)
+ const char *property,
+ int posted)
{
char name[10]; /* 10 = sizeof("gptXX_Xck0") */
const char *oh_name;
@@ -310,10 +311,15 @@ static int __init omap_dm_timer_init_one(struct
omap_dm_timer *timer,
}
__omap_dm_timer_init_regs(timer);
__omap_dm_timer_reset(timer, 1, 1);
- timer->posted = 1;
- timer->rate = clk_get_rate(timer->fclk);
+ if (p
Re: [PATCH V2 03/14] ARM: OMAP3+: Implement timer workaround for errata i103 and i767
On Wednesday 07 November 2012 05:28 PM, Jon Hunter wrote: On 11/07/2012 04:14 PM, Santosh Shilimkar wrote: Looks sensible considering alternative WAs. Acked-by: Santosh Shilimkar Thanks. With further thought I think that it would be best to combine patches #2 and #3. Really the main motivation here is the errata workaround and without actually benchmarking the timer read I should not claim the improvement in overhead as stated in patch #2. So I have combined #2 and #3 and updated the changelog/comments appropriately. Let me know if you guys are ok with this. Yep. Sounds good. Regards Santosh -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2 03/14] ARM: OMAP3+: Implement timer workaround for errata i103 and i767
On 11/07/2012 04:14 PM, Santosh Shilimkar wrote: > Looks sensible considering alternative WAs. > > Acked-by: Santosh Shilimkar Thanks. With further thought I think that it would be best to combine patches #2 and #3. Really the main motivation here is the errata workaround and without actually benchmarking the timer read I should not claim the improvement in overhead as stated in patch #2. So I have combined #2 and #3 and updated the changelog/comments appropriately. Let me know if you guys are ok with this. Cheers Jon >From 0143aa216ef4c4b7554588bd72c417bc8c614525 Mon Sep 17 00:00:00 2001 From: Jon Hunter Date: Thu, 27 Sep 2012 12:47:43 -0500 Subject: [PATCH] ARM: OMAP3+: Implement timer workaround for errata i103 and i767 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 and reading the timers as the CPU has to wait for accesses to be re-synchronised within the timer. 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. For drivers using the timers that do not read the counter or capture registers and wish to use posted mode, can override the errata and enable posted mode by making the following function calls. omap_dm_timer_override_errata(timer, OMAP_TIMER_ERRATA_I103_I767); omap_dm_timer_enable_posted(timer); 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. Posted mode will be disabled for all OMAP2+ devices (including AM33xx) using a GP timer as a clock-source timer to guarantee correct operation. This is not necessary for OMAP24xx devices but the default clock-source timer for OMAP24xx devices is the 32k-sync timer and not the GP timer and so should not have any impact. This should be re-visited for future devices if this errata is fixed. Confirmed with Vaibhav Hiremath that this bug also impacts AM33xx devices. Signed-off-by: Jon Hunter --- arch/arm/mach-omap2/timer.c | 44 + arch/arm/plat-omap/dmtimer.c | 59 - arch/arm/plat-omap/include/plat/dmtimer.h | 19 -- 3 files changed, 109 insertions(+), 13 deletions(-) diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c index 28c6078..37e3089 100644 --- a/arch/arm/mach-omap2/timer.c +++ b/arch/arm/mach-omap2/timer.c @@ -83,11 +83,20 @@ #define NUMERATOR_DENUMERATOR_MASK 0xf000 /* - * For clock-events timer, always use posted mode to - * minimise CPU overhead for configuring the timer. + * For the clock-events timer, always use posted mode to + * minimise CPU overhead for configuring the timer. This timer + * is never read and so overhead of reading the timer in posted + * mode is not applicable. */ #define OMAP_CLKEVT_POSTEDMODE OMAP_TIMER_POSTED -#define OMAP_CLKSRC_POSTEDMODE OMAP_TIMER_POSTED + +/* + * For the clock-source timer, use non-posted mode default due to + * errata i103/i767 that can cause the timer to return an incorrect + * counter value for OMAP3/4/5 devices. REVISIT this to see what is + * the optimal way to handle for future devices. + */ +#define OMAP_CLKSRC_POSTEDMODE OMAP_TIMER_NONPOSTED /* Clockevent code */ @@ -233,7 +242,8 @@ void __init omap_dmtimer_init(void) static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer, int gptimer_id,
Re: [PATCH V2 03/14] ARM: OMAP3+: Implement timer workaround for errata i103 and i767
On Wednesday 07 November 2012 01:01 PM, 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. For drivers using the timers that do not read the counter or capture registers and wish to use posted mode, can override the errata and enable posted mode by making the following function calls. omap_dm_timer_override_errata(timer, OMAP_TIMER_ERRATA_I103_I767); omap_dm_timer_enable_posted(timer); 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. Please note that now calls to omap_dm_timer_enable_posted() may not able posted mode if the device is impacted by this errata. Therefore, for system-timers check to see if the intended posted mode matches the actual. If it does not then there is a configuration error in the system timers posted configuration. Signed-off-by: Jon Hunter --- Looks sensible considering alternative WAs. Acked-by: Santosh Shilimkar -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 03/14] ARM: OMAP3+: Implement timer workaround for errata i103 and i767
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.
For drivers using the timers that do not read the counter or capture registers
and wish to use posted mode, can override the errata and enable posted mode by
making the following function calls.
omap_dm_timer_override_errata(timer, OMAP_TIMER_ERRATA_I103_I767);
omap_dm_timer_enable_posted(timer);
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.
Please note that now calls to omap_dm_timer_enable_posted() may not able posted
mode if the device is impacted by this errata. Therefore, for system-timers
check to see if the intended posted mode matches the actual. If it does not then
there is a configuration error in the system timers posted configuration.
Signed-off-by: Jon Hunter
---
arch/arm/mach-omap2/timer.c | 15
arch/arm/plat-omap/dmtimer.c | 36 +
arch/arm/plat-omap/include/plat/dmtimer.h | 14 +++
3 files changed, 65 insertions(+)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 78b7712..8b0068c 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -332,6 +332,10 @@ static int __init omap_dm_timer_init_one(struct
omap_dm_timer *timer,
if (posted)
omap_dm_timer_enable_posted(timer);
+ /* Check that the intended posted configuration matches the actual */
+ if (posted != timer->posted)
+ return -EINVAL;
+
timer->rate = clk_get_rate(timer->fclk);
timer->reserved = 1;
@@ -344,6 +348,15 @@ 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_override_errata(&clkev, OMAP_TIMER_ERRATA_I103_I767);
+
res = omap_dm_timer_init_one(&clkev, gptimer_id, fck_source, property,
OMAP_CLKEVT_POSTEDMODE);
BUG_ON(res);
@@ -472,6 +485,8 @@ static void __init omap2_gptimer_clocksource_init(int
gptimer_id,
{
int res;
+ omap_dm_timer_populate_errata(&clksrc);
+
res = omap_dm_timer_init_one(&clksrc, gptimer_id, fck_source, NULL,
OMAP_CLKSRC_POSTEDMODE);
BUG_ON(res);
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index 699570b..4abbbe5 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -177,6 +177,37 @@ int omap_dm_timer_reserve_systimer(int id)
}
/**
+ * omap_dm_timer_populate_errata - populate errata flags for a timer
+ * @timer: pointer to timer handle
+ *
+ * For a given timer, populate the timer errata flags that are specific to the
+ * OMAP device being used.
+ */
+void omap_dm_timer_populate_errata(struct omap_dm_timer *timer)
+{
+ timer->errata = 0;
+
+ if (cpu_class_is_omap1() || cpu_is_omap24xx(
