* [PATCH V2 03/14] ARM: OMAP3+: Implement timer workaround for errata i103 and i767
From: Jon Hunter @ 2012-11-07 23:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <509ADD28.20707@ti.com>
On 11/07/2012 04:14 PM, Santosh Shilimkar wrote:
> Looks sensible considering alternative WAs.
>
> Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
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 <jon-hunter@ti.com>
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 <jon-hunter@ti.com>
---
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 0xfffff000
/*
- * 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,
const char *fck_source,
- const char *property)
+ const char *property,
+ int posted)
{
char name[10]; /* 10 = sizeof("gptXX_Xck0") */
const char *oh_name;
@@ -319,10 +329,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 (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;
return res;
@@ -334,7 +349,17 @@ static void __init omap2_gp_clockevent_init(int gptimer_id,
{
int res;
- res = omap_dm_timer_init_one(&clkev, gptimer_id, fck_source, property);
+ 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);
omap2_gp_timer_irq.dev_id = &clkev;
@@ -461,7 +486,10 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
{
int res;
- res = omap_dm_timer_init_one(&clksrc, gptimer_id, fck_source, NULL);
+ omap_dm_timer_populate_errata(&clksrc);
+
+ res = omap_dm_timer_init_one(&clksrc, gptimer_id, fck_source, NULL,
+ OMAP_CLKSRC_POSTEDMODE);
BUG_ON(res);
__omap_dm_timer_load_start(&clksrc,
diff --git a/arch/arm/plat-omap/dmtimer.c b/arch/arm/plat-omap/dmtimer.c
index b09e556..4abbbe5 100644
--- a/arch/arm/plat-omap/dmtimer.c
+++ b/arch/arm/plat-omap/dmtimer.c
@@ -131,8 +131,8 @@ static void omap_dm_timer_reset(struct omap_dm_timer *timer)
}
__omap_dm_timer_reset(timer, 0, 0);
+ omap_dm_timer_enable_posted(timer);
omap_dm_timer_disable(timer);
- timer->posted = 1;
}
int omap_dm_timer_prepare(struct omap_dm_timer *timer)
@@ -176,6 +176,61 @@ int omap_dm_timer_reserve_systimer(int id)
return 0;
}
+/**
+ * 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())
+ return;
+
+ timer->errata = OMAP_TIMER_ERRATA_I103_I767;
+}
+
+/**
+ * omap_dm_timer_override_errata - override errata flags for a timer
+ * @timer: pointer to timer handle
+ * @errata: errata flags to be ignored
+ *
+ * For a given timer, override a timer errata by clearing the flags specified
+ * by the errata argument. A specific erratum should only be overridden for a
+ * timer if the timer is used in such a way the erratum has no impact.
+ */
+void omap_dm_timer_override_errata(struct omap_dm_timer *timer, u32 errata)
+{
+ timer->errata &= ~errata;
+}
+
+/*
+ * omap_dm_timer_enable_posted - enables write posted mode
+ * @timer: pointer to timer instance handle
+ *
+ * Enables the write posted mode for the timer. When posted mode is enabled
+ * writes to certain timer registers are immediately acknowledged by the
+ * internal bus and hence prevents stalling the CPU waiting for the write to
+ * complete. Enabling this feature can improve performance for writing to the
+ * timer registers.
+ */
+void omap_dm_timer_enable_posted(struct omap_dm_timer *timer)
+{
+ if (timer->posted)
+ return;
+
+ if (timer->errata & OMAP_TIMER_ERRATA_I103_I767)
+ return;
+
+ omap_dm_timer_write_reg(timer, OMAP_TIMER_IF_CTRL_REG,
+ OMAP_TIMER_CTRL_POSTED);
+ timer->context.tsicr = OMAP_TIMER_CTRL_POSTED;
+ timer->posted = OMAP_TIMER_POSTED;
+}
+
struct omap_dm_timer *omap_dm_timer_request(void)
{
struct omap_dm_timer *timer = NULL, *t;
@@ -803,6 +858,8 @@ static int __devinit omap_dm_timer_probe(struct platform_device *pdev)
timer->irq = irq->start;
timer->pdev = pdev;
+ omap_dm_timer_populate_errata(timer);
+
/* Skip pm_runtime_enable for OMAP1 */
if (!(timer->capability & OMAP_TIMER_NEEDS_RESET)) {
pm_runtime_enable(dev);
diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h b/arch/arm/plat-omap/include/plat/dmtimer.h
index 835c3bdf..ef93017 100644
--- a/arch/arm/plat-omap/include/plat/dmtimer.h
+++ b/arch/arm/plat-omap/include/plat/dmtimer.h
@@ -36,6 +36,7 @@
#include <linux/delay.h>
#include <linux/io.h>
#include <linux/platform_device.h>
+#include <plat/cpu.h>
#ifndef __ASM_ARCH_DMTIMER_H
#define __ASM_ARCH_DMTIMER_H
@@ -66,6 +67,16 @@
#define OMAP_TIMER_NEEDS_RESET 0x10000000
#define OMAP_TIMER_HAS_DSP_IRQ 0x08000000
+/*
+ * timer errata flags
+ *
+ * Errata i103/i767 impacts all OMAP3/4/5 devices including AM33xx. This
+ * errata prevents us from using posted mode on these devices, unless the
+ * timer counter register is never read. For more details please refer to
+ * the OMAP3/4/5 errata documents.
+ */
+#define OMAP_TIMER_ERRATA_I103_I767 0x80000000
+
struct omap_timer_capability_dev_attr {
u32 timer_capability;
};
@@ -101,6 +112,9 @@ struct dmtimer_platform_data {
};
int omap_dm_timer_reserve_systimer(int id);
+void omap_dm_timer_populate_errata(struct omap_dm_timer *timer);
+void omap_dm_timer_override_errata(struct omap_dm_timer *timer, u32 errata);
+void omap_dm_timer_enable_posted(struct omap_dm_timer *timer);
struct omap_dm_timer *omap_dm_timer_request(void);
struct omap_dm_timer *omap_dm_timer_request_specific(int timer_id);
struct omap_dm_timer *omap_dm_timer_request_by_cap(u32 cap);
@@ -271,6 +285,7 @@ struct omap_dm_timer {
int ctx_loss_count;
int revision;
u32 capability;
+ u32 errata;
struct platform_device *pdev;
struct list_head node;
};
@@ -342,10 +357,6 @@ static inline void __omap_dm_timer_reset(struct omap_dm_timer *timer,
l |= 1 << 2;
__raw_writel(l, timer->io_base + OMAP_TIMER_OCP_CFG_OFFSET);
-
- /* Match hardware reset default of posted mode */
- __omap_dm_timer_write(timer, OMAP_TIMER_IF_CTRL_REG,
- OMAP_TIMER_CTRL_POSTED, 0);
}
static inline int __omap_dm_timer_set_source(struct clk *timer_fck,
--
1.7.9.5
^ permalink raw reply related
* [PATCH] cpuidle: add Calxeda SOC idle support
From: Rob Herring @ 2012-11-07 23:23 UTC (permalink / raw)
To: linux-arm-kernel
From: Rob Herring <rob.herring@calxeda.com>
Add support for core powergating on Calxeda platforms. Initially, this
supports ECX-1000 (highbank), but support will be added for ECX-2000
later.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Len Brown <len.brown@intel.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
---
It's not really clear where we want ARM cpuidle drivers. We're moving
everything else out of arch/arm, and my understanding is Len doesn't want
them in drivers/idle. It seems kind of silly to me to have the framework
and drivers in 2 places. I've put this in drivers/cpuidle, but it doesn't
make any difference to me.
Rob
drivers/cpuidle/Kconfig | 10 +++
drivers/cpuidle/Makefile | 2 +
drivers/cpuidle/cpuidle-calxeda.c | 161 +++++++++++++++++++++++++++++++++++++
3 files changed, 173 insertions(+)
create mode 100644 drivers/cpuidle/cpuidle-calxeda.c
diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index a76b689..d2d007b 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -21,3 +21,13 @@ config CPU_IDLE_GOV_MENU
config ARCH_NEEDS_CPU_IDLE_COUPLED
def_bool n
+
+if CPU_IDLE
+
+config CPU_IDLE_CALXEDA
+ bool "CPU Idle Driver for Calxeda processors"
+ depends on ARCH_HIGHBANK
+ help
+ Select this to enable cpuidle on Calxeda processors.
+
+endif
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 38c8f69..03ee874 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -4,3 +4,5 @@
obj-y += cpuidle.o driver.o governor.o sysfs.o governors/
obj-$(CONFIG_ARCH_NEEDS_CPU_IDLE_COUPLED) += coupled.o
+
+obj-$(CONFIG_CPU_IDLE_CALXEDA) += cpuidle-calxeda.o
diff --git a/drivers/cpuidle/cpuidle-calxeda.c b/drivers/cpuidle/cpuidle-calxeda.c
new file mode 100644
index 0000000..e1aab38
--- /dev/null
+++ b/drivers/cpuidle/cpuidle-calxeda.c
@@ -0,0 +1,161 @@
+/*
+ * Copyright 2012 Calxeda, Inc.
+ *
+ * Based on arch/arm/plat-mxc/cpuidle.c:
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ * Copyright 2012 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/cpuidle.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/time.h>
+#include <linux/delay.h>
+#include <linux/suspend.h>
+#include <asm/cpuidle.h>
+#include <asm/proc-fns.h>
+#include <asm/smp_scu.h>
+#include <asm/suspend.h>
+#include <asm/cacheflush.h>
+#include <asm/cp15.h>
+
+extern void highbank_set_cpu_jump(int cpu, void *jump_addr);
+extern void *scu_base_addr;
+
+static struct cpuidle_device __percpu *calxeda_idle_cpuidle_devices;
+
+static inline unsigned int get_auxcr(void)
+{
+ unsigned int val;
+ asm("mrc p15, 0, %0, c1, c0, 1 @ get AUXCR" : "=r" (val) : : "cc");
+ return val;
+}
+
+static inline void set_auxcr(unsigned int val)
+{
+ asm volatile("mcr p15, 0, %0, c1, c0, 1 @ set AUXCR"
+ : : "r" (val) : "cc");
+ isb();
+}
+
+static noinline void calxeda_idle_restore(void)
+{
+ set_cr(get_cr() | CR_C);
+ set_auxcr(get_auxcr() | 0x40);
+ scu_power_mode(scu_base_addr, SCU_PM_NORMAL);
+}
+
+static int calxeda_idle_finish(unsigned long val)
+{
+ /* Already flushed cache, but do it again as the outer cache functions
+ * dirty the cache with spinlocks */
+ flush_cache_all();
+
+ set_auxcr(get_auxcr() & ~0x40);
+ set_cr(get_cr() & ~CR_C);
+
+ scu_power_mode(scu_base_addr, SCU_PM_DORMANT);
+
+ cpu_do_idle();
+
+ /* Restore things if we didn't enter power-gating */
+ calxeda_idle_restore();
+ return 1;
+}
+
+static int calxeda_pwrdown_idle(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv,
+ int index)
+{
+ highbank_set_cpu_jump(smp_processor_id(), cpu_resume);
+ cpu_suspend(0, calxeda_idle_finish);
+ return index;
+}
+
+static void calxeda_idle_cpuidle_devices_uninit(void)
+{
+ int i;
+ struct cpuidle_device *dev;
+
+ for_each_possible_cpu(i) {
+ dev = per_cpu_ptr(calxeda_idle_cpuidle_devices, i);
+ cpuidle_unregister_device(dev);
+ }
+
+ free_percpu(calxeda_idle_cpuidle_devices);
+}
+
+static struct cpuidle_driver calxeda_idle_driver = {
+ .name = "calxeda_idle",
+ .en_core_tk_irqen = 1,
+ .states = {
+ ARM_CPUIDLE_WFI_STATE,
+ {
+ .name = "PG",
+ .desc = "Power Gate",
+ .flags = CPUIDLE_FLAG_TIME_VALID,
+ .exit_latency = 30,
+ .power_usage = 50,
+ .target_residency = 200,
+ .enter = calxeda_pwrdown_idle,
+ },
+ },
+ .state_count = 2,
+};
+
+static int __init calxeda_cpuidle_init(void)
+{
+ int cpu_id;
+ int ret;
+ struct cpuidle_device *dev;
+ struct cpuidle_driver *drv = &calxeda_idle_driver;
+
+ if (!of_machine_is_compatible("calxeda,highbank"))
+ return -ENODEV;
+
+ ret = cpuidle_register_driver(drv);
+ if (ret)
+ return ret;
+
+ calxeda_idle_cpuidle_devices = alloc_percpu(struct cpuidle_device);
+ if (calxeda_idle_cpuidle_devices == NULL) {
+ ret = -ENOMEM;
+ goto unregister_drv;
+ }
+
+ /* initialize state data for each cpuidle_device */
+ for_each_possible_cpu(cpu_id) {
+ dev = per_cpu_ptr(calxeda_idle_cpuidle_devices, cpu_id);
+ dev->cpu = cpu_id;
+ dev->state_count = drv->state_count;
+
+ ret = cpuidle_register_device(dev);
+ if (ret) {
+ pr_err("Failed to register cpu %u, error: %d\n",
+ cpu_id, ret);
+ goto uninit;
+ }
+ }
+
+ return 0;
+
+uninit:
+ calxeda_idle_cpuidle_devices_uninit();
+unregister_drv:
+ cpuidle_unregister_driver(drv);
+ return ret;
+}
+module_init(calxeda_cpuidle_init);
--
1.7.10.4
^ permalink raw reply related
* struct sys_timer .suspend/.resume ignored for ARCH_SA1100/ARCH_PXA?
From: Russell King - ARM Linux @ 2012-11-07 23:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <509AE964.8070009@wwwdotorg.org>
On Wed, Nov 07, 2012 at 04:06:12PM -0700, Stephen Warren wrote:
> Russell, Kevin,
>
> In commit 9e4559d "[ARM] 4258/2: Support for dynticks in idle loop" in
> 2007, Kevin applied the following change:
>
> > diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
>
> > -#ifdef CONFIG_PM
> > +#if defined(CONFIG_PM) && !defined(CONFIG_GENERIC_CLOCKEVENTS)
> > static int timer_suspend(struct sys_device *dev, pm_message_t state)
>
> This means that for any architecture that enables GENERIC_CLOCKEVENTS,
> the .suspend/.resume fields of struct sys_timer will be ignored, since
> timer_suspend()/timer_resume() won't be filled into
> arch/arm/kernel/time.c's struct syscore_ops timer_syscore_ops.
Correct.
> Later, in commit 3e238be "[ARM] sa1100: add clock event support" in
> 2008, Russell modified ARCH_SA1100 to select GENERIC_CLOCKEVENTS. I
> believe this means that sa1100_timer_suspend()/resume() haven't been
> used since.
Also correct.
> A similar issue exists for ARCH_PXA.
>
> Should sa1100_timer_suspend(), sa1100_timer_resume(),
> pxa_timer_suspend(), pxa_timer_suspend() simply be deleted since they
> are dead code, or should they be revived somehow; is the ifdef from
> Kevin's change incorrect?
Hmm, that's probably not good for either of those two platforms; it means
that the OSCR and match registers get lost over a suspend/resume. That's
not a real big problem for the clocksource code, but if its being used
for something else (eg, rtc) then it probably means we have a failure
there.
> As background, I'm working on a patch series that will remove all fields
> from struct sys_timer except for .init, and will then replace the ARM
> machine descriptor's .timer struct pointer with a .init_timer function
> pointer. This will allow machines, on an opt-in basis, to call into a
> central function in drivers/clocksource to initialize the required
> timer, as determined by searching device tree for a known device type,
> in much the same way as has been proposed to use a single implementation
> for for the machine descriptor's .init_irq. As part of this, I've been
> looking at moving any use of struct sys_timer .suspend/.resume into e.g.
> struct clock_event_device .suspend/.resume, and found this issue.
Don't forget we still have a number of platforms not converted to
the generic event/clocksource stuff (because they lack the necessary
counters/timers for this 'new' infrastructure.)
^ permalink raw reply
* struct sys_timer .suspend/.resume ignored for ARCH_SA1100/ARCH_PXA?
From: Stephen Warren @ 2012-11-07 23:06 UTC (permalink / raw)
To: linux-arm-kernel
Russell, Kevin,
In commit 9e4559d "[ARM] 4258/2: Support for dynticks in idle loop" in
2007, Kevin applied the following change:
> diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c
> -#ifdef CONFIG_PM
> +#if defined(CONFIG_PM) && !defined(CONFIG_GENERIC_CLOCKEVENTS)
> static int timer_suspend(struct sys_device *dev, pm_message_t state)
This means that for any architecture that enables GENERIC_CLOCKEVENTS,
the .suspend/.resume fields of struct sys_timer will be ignored, since
timer_suspend()/timer_resume() won't be filled into
arch/arm/kernel/time.c's struct syscore_ops timer_syscore_ops.
Later, in commit 3e238be "[ARM] sa1100: add clock event support" in
2008, Russell modified ARCH_SA1100 to select GENERIC_CLOCKEVENTS. I
believe this means that sa1100_timer_suspend()/resume() haven't been
used since.
A similar issue exists for ARCH_PXA.
Should sa1100_timer_suspend(), sa1100_timer_resume(),
pxa_timer_suspend(), pxa_timer_suspend() simply be deleted since they
are dead code, or should they be revived somehow; is the ifdef from
Kevin's change incorrect?
As background, I'm working on a patch series that will remove all fields
from struct sys_timer except for .init, and will then replace the ARM
machine descriptor's .timer struct pointer with a .init_timer function
pointer. This will allow machines, on an opt-in basis, to call into a
central function in drivers/clocksource to initialize the required
timer, as determined by searching device tree for a known device type,
in much the same way as has been proposed to use a single implementation
for for the machine descriptor's .init_irq. As part of this, I've been
looking at moving any use of struct sys_timer .suspend/.resume into e.g.
struct clock_event_device .suspend/.resume, and found this issue.
Thanks for any hints.
^ permalink raw reply
* [PATCH V2 01/14] ARM: OMAP: Add DMTIMER definitions for posted mode
From: Jon Hunter @ 2012-11-07 22:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <509ADE4E.6070106@ti.com>
On 11/07/2012 04:18 PM, Santosh Shilimkar wrote:
> On Wednesday 07 November 2012 04:11 PM, Jon Hunter wrote:
>>
>> On 11/07/2012 04:04 PM, Santosh Shilimkar wrote:
>>> On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote:
>>>> For OMAP2+ devices, when using DMTIMERs for system timers
>>>> (clock-events and
>>>> clock-source) the posted mode configuration of the timers is used. To
>>>> allow
>>>> the compiler to optimise the functions for configuring and reading the
>>>> system
>>>> timers, the posted flag variable is hard-coded with the value 1. To
>>>> make it
>>>> clear that posted mode is being used add some definitions so that it
>>>> is more
>>>> readable.
>>>>
>>>> Add separate definitions for the clock-events and clock-source timers
>>>> so that
>>>> we can change the posted mode of clock-events and clock-source
>>>> independently.
>>>>
>>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>>> ---
>>>> arch/arm/mach-omap2/timer.c | 26
>>>> +++++++++++++++++++-------
>>>> arch/arm/plat-omap/include/plat/dmtimer.h | 4 ++++
>>>> 2 files changed, 23 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>>> index 0758bae..28c6078 100644
>>>> --- a/arch/arm/mach-omap2/timer.c
>>>> +++ b/arch/arm/mach-omap2/timer.c
>>>> @@ -82,6 +82,13 @@
>>>> #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET 0x14
>>>> #define NUMERATOR_DENUMERATOR_MASK 0xfffff000
>>>>
>>>> +/*
>>>> + * For clock-events timer, always use posted mode to
>>>> + * minimise CPU overhead for configuring the timer.
>>>> + */
>>>> +#define OMAP_CLKEVT_POSTEDMODE OMAP_TIMER_POSTED
>>>> +#define OMAP_CLKSRC_POSTEDMODE OMAP_TIMER_POSTED
>>>> +
>>> I don't see need of above defines. Just use OMAP_TIMER_POSTED directly
>>> with API. Rest of the patch looks fine.
>>
>> Yes that's possible, however, in patch #2, I am disabling posted mode
>> for clock-source (see changelog of patch #2 for details). Having these
>> #defines makes it easier to change the posted configuration. That was
>> the real motivation here.
>>
> Sure but that is more confusing because you are flipping
> the meaning of the macro. Better to specify direct
> argument to avoid the confusion.
Hmmm ... I guess I don't see it that way. The intent was that the
definitions OMAP_CLKxxx_POSTEDMODE described the posted configuration
(ie. posted or non-posted) and a user could change/flip it if so desired.
I can use the OMAP_TIMER_POSTED/NONPOSTED directly, but my concern with
that was if someone wanted to changed the posted mode then they have to
change it in multiple places and there is a chance they could miss one.
This way, as long as I have it right to begin with, then no one should
be able to screw it up :-)
Cheers
Jon
^ permalink raw reply
* [PATCH V2 02/14] ARM: OMAP2+: Disable posted mode for the clocksource timer
From: Jon Hunter @ 2012-11-07 22:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <509ADC5F.7020307@ti.com>
On 11/07/2012 04:10 PM, Santosh Shilimkar wrote:
> On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote:
>> When using a DMTIMER as the clock-source timer, posted mode
>> configuration of
>> the DMTIMER is used. Posted mode is only benefical when configuring
>> timers as
>> it allows writes to be posted and does not stall the CPU until the
>> write is
>> complete. The clock-source timer is only configured once on boot and
>> so using
>> posted mode has no benefit. In fact, by using posted mode, it adds
>> overhead
>> to reading the timer. Therefore, by default disable posted mode for
>> DMTIMERs
>> used for clock-source.
>>
>> Using objdump this change reduces the function clocksource_read_cycles()
>> function from a total of 15 instructions (including 3 branches and 7
>> loads)
>> to 5 instructions (including 1 branch and 3 loads). Please note that
>> before
>> the minimum number of instructions that would be executed when calling
>> clocksource_read_cycles() would be 9 instructions (including 2
>> branches and 5
>> loads) where as now it will always be 5 instructions.
>>
>> This change also reduces the function dmtimer_read_sched_clock()
>> function from
>> a total of 17 instructions (including 4 branches and 8 loads) to 6
>> instructions
>> (including 1 branch and 4 loads). Please note that before the minimum
>> number of
>> instructions that would be executed when calling
>> dmtimer_read_sched_clock()
>> would be 11 instructions (including 2 branches and 6 loads) where as
>> now it
>> will always be 6 instructions.
>>
> This isn't right way to calculate the penalty of posted mode. Non-posted
> mode can results in 100 of cycles wait over interconnect
> and that can not be justified with few instructions savings.
Right, I see your point. Non-posted reads are going to add
re-synchronisation overhead. However, our hands are tied here because of
errata i103 we can't use posted mode for reading the counter.
So may be I should squash this change with patch #3 and just make this
part of the errata workaround. I can always re-visit later and do some
profiling to see what the optimal configuration should be.
>> This change removes the configuration of posted mode from the
>> __omap_dm_timer_reset() function and adds a new function called
>> omap_dm_timer_enable_posted() for enabling posted mode. Hence, call
>> omap_dm_timer_enable_posted() where ever we are calling
>> __omap_dm_timer_reset().
>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> ---
> But clock-source doesn't involve much into writes so patch as
> such is fine.
Yes writes are only on init.
Cheers
Jon
^ permalink raw reply
* [PATCH V2 00/14] ARM: OMAP: DMTIMER fixes
From: Santosh Shilimkar @ 2012-11-07 22:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1352314890-22224-1-git-send-email-jon-hunter@ti.com>
Jon,
On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote:
> This series includes several fixes for the OMAP DMTIMER driver. This is
> based upon 3.7-rc4 with the two series adding device-tree support for
> DMTIMERs [1] and the 32kHz Counter [2]
>
> Tested on OMAP5912 OSK, OMAP2420 H4, OMAP3430 Beagle and OMAP4430 Panda.
> Testing includes ...
> 1. Booting kernel on above boards
> 2. Set date and ensuring time of day is correct after 24 hours
> 3. Checking the timer counter is incrementing when configuring and starting
> a timer
> 4. Checking the timer overflow interrupt when timer expires.
> 5. Using different clock sources to operate the timer with.
> 6. Running a loop test overnight that continually runs test #3 and #4 for
> each available timer
>
> This has also been boot tested on the AM335x Beagle Bone.
>
> [1] http://marc.info/?l=linux-omap&m=135065875808614&w=2
> [2] http://marc.info/?l=linux-omap&m=135119308123513&w=2
>
> Jon Hunter (14):
> ARM: OMAP: Add DMTIMER definitions for posted mode
> ARM: OMAP2+: Disable posted mode for the clocksource timer
> ARM: OMAP3+: Implement timer workaround for errata i103 and i767
> ARM: OMAP: Fix timer posted mode support
> ARM: OMAP3: Correct HWMOD DMTIMER SYSC register declarations
> ARM: OMAP2/3: Define HWMOD software reset status for DMTIMERs
> ARM: OMAP2+: Don't use __omap_dm_timer_reset()
> ARM: OMAP: Fix dmtimer reset for timer1
> ARM: OMAP: Don't restore of DMTIMER TISTAT register
> ARM: OMAP: Don't restore DMTIMER interrupt status register
> ARM: OMAP: Fix spurious interrupts when using timer match feature
> ARM: OMAP: Add dmtimer interrupt disable function
> ARM: OMAP: Remove unnecessary call to clk_get()
> ARM: OMAP: Remove __omap_dm_timer_set_source function
>
> arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c | 15 ++-
> arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 41 +++---
> arch/arm/mach-omap2/omap_hwmod_44xx_data.c | 4 +
> arch/arm/mach-omap2/timer.c | 73 ++++++++---
> arch/arm/plat-omap/dmtimer.c | 132 +++++++++++++++-----
> arch/arm/plat-omap/include/plat/dmtimer.h | 47 ++++---
> 6 files changed, 215 insertions(+), 97 deletions(-)
>
Nice work. Apart from some minor comments, this series looks pretty
good to me.
Feel free to add my ack for entire series.
Regards
Santosh
^ permalink raw reply
* [PATCH v4 2/9] pinctrl: single: support gpio request and free
From: Tony Lindgren @ 2012-11-07 22:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1352301582-12244-3-git-send-email-haojian.zhuang@gmail.com>
* Haojian Zhuang <haojian.zhuang@gmail.com> [121107 07:21]:
> Marvell's PXA/MMP silicon also match the behavior of pinctrl-single.
> Each pin binds to one register. A lot of pins could be configured
> as gpio.
>
> Now add these properties in below.
> <gpio range phandle>:
> include "pinctrl-single,gpio" & "pinctrl,gpio-func" properties.
>
> pinctrl-single,gpio: <gpio base, npins in range, register offset>
>
> pinctrl-single,gpio-func: <gpio function value in mux>
>
> pinctrl-single,gpio-ranges: phandle list of gpio range array
This one looks OK to me now:
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
> ---
> drivers/pinctrl/pinctrl-single.c | 100 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 98 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
> index 726a729..a7c5fdd 100644
> --- a/drivers/pinctrl/pinctrl-single.c
> +++ b/drivers/pinctrl/pinctrl-single.c
> @@ -30,6 +30,7 @@
> #define PCS_MUX_BITS_NAME "pinctrl-single,bits"
> #define PCS_REG_NAME_LEN ((sizeof(unsigned long) * 2) + 1)
> #define PCS_OFF_DISABLED ~0U
> +#define PCS_MAX_GPIO_VALUES 3
>
> /**
> * struct pcs_pingroup - pingroups for a function
> @@ -77,6 +78,18 @@ struct pcs_function {
> };
>
> /**
> + * struct pcs_gpio_range - pinctrl gpio range
> + * @range: subrange of the GPIO number space
> + * @gpio_func: gpio function value in the pinmux register
> + * @func_en: need to handle gpio function in the pinmux register
> + */
> +struct pcs_gpio_range {
> + struct pinctrl_gpio_range range;
> + int gpio_func;
> + unsigned func_en:1;
> +};
> +
> +/**
> * struct pcs_data - wrapper for data needed by pinctrl framework
> * @pa: pindesc array
> * @cur: index to current element
> @@ -123,8 +136,10 @@ struct pcs_name {
> * @ftree: function index radix tree
> * @pingroups: list of pingroups
> * @functions: list of functions
> + * @ranges: list of gpio ranges
> * @ngroups: number of pingroups
> * @nfuncs: number of functions
> + * @nranges: number of gpio ranges
> * @desc: pin controller descriptor
> * @read: register read function to use
> * @write: register write function to use
> @@ -148,8 +163,10 @@ struct pcs_device {
> struct radix_tree_root ftree;
> struct list_head pingroups;
> struct list_head functions;
> + struct list_head ranges;
> unsigned ngroups;
> unsigned nfuncs;
> + unsigned nranges;
> struct pinctrl_desc desc;
> unsigned (*read)(void __iomem *reg);
> void (*write)(unsigned val, void __iomem *reg);
> @@ -403,9 +420,27 @@ static void pcs_disable(struct pinctrl_dev *pctldev, unsigned fselector,
> }
>
> static int pcs_request_gpio(struct pinctrl_dev *pctldev,
> - struct pinctrl_gpio_range *range, unsigned offset)
> + struct pinctrl_gpio_range *range, unsigned pin)
> {
> - return -ENOTSUPP;
> + struct pcs_device *pcs = pinctrl_dev_get_drvdata(pctldev);
> + struct pcs_gpio_range *gpio = NULL;
> + int end, mux_bytes;
> + unsigned data;
> +
> + gpio = container_of(range, struct pcs_gpio_range, range);
> + if (!gpio->func_en)
> + return -ENOTSUPP;
> + end = range->pin_base + range->npins - 1;
> + if (pin < range->pin_base || pin > end) {
> + dev_err(pctldev->dev, "pin %d isn't in the range of "
> + "%d to %d\n", pin, range->pin_base, end);
> + return -EINVAL;
> + }
> + mux_bytes = pcs->width / BITS_PER_BYTE;
> + data = pcs->read(pcs->base + pin * mux_bytes) & ~pcs->fmask;
> + data |= gpio->gpio_func;
> + pcs->write(data, pcs->base + pin * mux_bytes);
> + return 0;
> }
>
> static struct pinmux_ops pcs_pinmux_ops = {
> @@ -879,6 +914,62 @@ static void pcs_free_resources(struct pcs_device *pcs)
>
> static struct of_device_id pcs_of_match[];
>
> +static int __devinit pcs_add_gpio_range(struct device_node *node,
> + struct pcs_device *pcs)
> +{
> + struct pcs_gpio_range *gpio;
> + struct device_node *np;
> + const __be32 *list;
> + const char list_name[] = "pinctrl-single,gpio-ranges";
> + const char name[] = "pinctrl-single";
> + u32 gpiores[PCS_MAX_GPIO_VALUES];
> + int ret, size, i, mux_bytes = 0;
> +
> + list = of_get_property(node, list_name, &size);
> + if (!list)
> + return 0;
> + size = size / sizeof(*list);
> + for (i = 0; i < size; i++) {
> + np = of_parse_phandle(node, list_name, i);
> + memset(gpiores, 0, sizeof(u32) * PCS_MAX_GPIO_VALUES);
> + ret = of_property_read_u32_array(np, "pinctrl-single,gpio",
> + gpiores, PCS_MAX_GPIO_VALUES);
> + if (ret < 0)
> + return -ENOENT;
> + gpio = devm_kzalloc(pcs->dev, sizeof(*gpio), GFP_KERNEL);
> + if (!gpio) {
> + dev_err(pcs->dev, "failed to allocate pcs gpio\n");
> + return -ENOMEM;
> + }
> + gpio->range.id = i;
> + gpio->range.base = gpiores[0];
> + gpio->range.npins = gpiores[1];
> + gpio->range.name = devm_kzalloc(pcs->dev, sizeof(name),
> + GFP_KERNEL);
> + if (!gpio->range.name) {
> + dev_err(pcs->dev, "failed to allocate range name\n");
> + return -ENOMEM;
> + }
> + memcpy(&gpio->range.name, name, sizeof(name));
> + mux_bytes = pcs->width / BITS_PER_BYTE;
> + gpio->range.pin_base = gpiores[2] / mux_bytes;
> + memset(gpiores, 0, sizeof(u32) * PCS_MAX_GPIO_VALUES);
> + ret = of_property_read_u32(np, "pinctrl-single,gpio-func",
> + &gpio->gpio_func);
> + if (ret < 0)
> + return -ENOENT;
> + gpio->func_en = 1;
> +
> + mutex_lock(&pcs->mutex);
> + list_add_tail(&gpio->range.node, &pcs->ranges);
> + pcs->nranges++;
> + mutex_unlock(&pcs->mutex);
> +
> + pinctrl_add_gpio_range(pcs->pctl, &gpio->range);
> + }
> + return 0;
> +}
> +
> static int __devinit pcs_probe(struct platform_device *pdev)
> {
> struct device_node *np = pdev->dev.of_node;
> @@ -900,6 +991,7 @@ static int __devinit pcs_probe(struct platform_device *pdev)
> mutex_init(&pcs->mutex);
> INIT_LIST_HEAD(&pcs->pingroups);
> INIT_LIST_HEAD(&pcs->functions);
> + INIT_LIST_HEAD(&pcs->ranges);
>
> PCS_GET_PROP_U32("pinctrl-single,register-width", &pcs->width,
> "register width not specified\n");
> @@ -975,6 +1067,10 @@ static int __devinit pcs_probe(struct platform_device *pdev)
> goto free;
> }
>
> + ret = pcs_add_gpio_range(np, pcs);
> + if (ret < 0)
> + goto free;
> +
> dev_info(pcs->dev, "%i pins at pa %p size %u\n",
> pcs->desc.npins, pcs->base, pcs->size);
>
> --
> 1.7.10.4
>
^ permalink raw reply
* [PATCH V2 04/14] ARM: OMAP: Fix timer posted mode support
From: Santosh Shilimkar @ 2012-11-07 22:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1352314890-22224-5-git-send-email-jon-hunter@ti.com>
On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote:
> Currently the dmtimer posted mode is being enabled when the function
> omap_dm_timer_enable_posted() is called. This function is only being called
> for OMAP1 timers and OMAP2+ timers that are being used as system timers. Hence,
> for OMAP2+ timers that are NOT being used as a system timer, posted mode is
> not enabled but the "timer->posted" variable is still set (incorrectly) in
> the omap_dm_timer_prepare() function.
>
> This is a regression introduced by commit 3392cdd3 (ARM: OMAP: dmtimer:
> switch-over to platform device driver) which was before the
> omap_dm_timer_enable_posted() function was introduced. Although this is a
> regression from the original code it only impacts performance and so is not
> needed for stable.
>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply
* [PATCH V2 01/14] ARM: OMAP: Add DMTIMER definitions for posted mode
From: Santosh Shilimkar @ 2012-11-07 22:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <509ADC99.3020106@ti.com>
On Wednesday 07 November 2012 04:11 PM, Jon Hunter wrote:
>
> On 11/07/2012 04:04 PM, Santosh Shilimkar wrote:
>> On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote:
>>> For OMAP2+ devices, when using DMTIMERs for system timers
>>> (clock-events and
>>> clock-source) the posted mode configuration of the timers is used. To
>>> allow
>>> the compiler to optimise the functions for configuring and reading the
>>> system
>>> timers, the posted flag variable is hard-coded with the value 1. To
>>> make it
>>> clear that posted mode is being used add some definitions so that it
>>> is more
>>> readable.
>>>
>>> Add separate definitions for the clock-events and clock-source timers
>>> so that
>>> we can change the posted mode of clock-events and clock-source
>>> independently.
>>>
>>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>>> ---
>>> arch/arm/mach-omap2/timer.c | 26
>>> +++++++++++++++++++-------
>>> arch/arm/plat-omap/include/plat/dmtimer.h | 4 ++++
>>> 2 files changed, 23 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>>> index 0758bae..28c6078 100644
>>> --- a/arch/arm/mach-omap2/timer.c
>>> +++ b/arch/arm/mach-omap2/timer.c
>>> @@ -82,6 +82,13 @@
>>> #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET 0x14
>>> #define NUMERATOR_DENUMERATOR_MASK 0xfffff000
>>>
>>> +/*
>>> + * For clock-events timer, always use posted mode to
>>> + * minimise CPU overhead for configuring the timer.
>>> + */
>>> +#define OMAP_CLKEVT_POSTEDMODE OMAP_TIMER_POSTED
>>> +#define OMAP_CLKSRC_POSTEDMODE OMAP_TIMER_POSTED
>>> +
>> I don't see need of above defines. Just use OMAP_TIMER_POSTED directly
>> with API. Rest of the patch looks fine.
>
> Yes that's possible, however, in patch #2, I am disabling posted mode
> for clock-source (see changelog of patch #2 for details). Having these
> #defines makes it easier to change the posted configuration. That was
> the real motivation here.
>
Sure but that is more confusing because you are flipping
the meaning of the macro. Better to specify direct
argument to avoid the confusion.
Regards
Santosh
^ permalink raw reply
* [PATCH 07/11] ARM: OMAP: Move omap-pm-noop.c local to mach-omap2
From: Tony Lindgren @ 2012-11-07 22:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <509ADB03.2050909@ti.com>
* Jon Hunter <jon-hunter@ti.com> [121107 14:06]:
> On 10/31/2012 06:11 PM, Tony Lindgren wrote:
> >
> > When it's converted to be a device driver, it can do it
> > using runtime PM calls.
>
> I am not sure if you are referring to runtime pm callbacks here, but if
> so I am not sure I follow. Drivers such as dmtimer and gpio that are
> using runtime pm are still dependent on OMAP specific APIs (such as
> omap_pm_get_dev_context_loss_count()) for determining if the context was
> lost between suspending and resuming the device. So I am not sure how
> runtime pm solves this.
Ah right.
> Speaking with Rob Herring, one solution for DT would be using bus
> notifiers to populate such function pointers when a device is added.
> Given that there are a few devices using this architecture specific API
> for context loss I am wondering if we can do something generic in
> omap_device.c for DT.
Sounds good to me.
Regards,
Tony
^ permalink raw reply
* [PATCH V2 03/14] ARM: OMAP3+: Implement timer workaround for errata i103 and i767
From: Santosh Shilimkar @ 2012-11-07 22:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1352314890-22224-4-git-send-email-jon-hunter@ti.com>
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 <jon-hunter@ti.com>
> ---
Looks sensible considering alternative WAs.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply
* [PATCH V2 01/14] ARM: OMAP: Add DMTIMER definitions for posted mode
From: Jon Hunter @ 2012-11-07 22:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <509ADAFE.4070109@ti.com>
On 11/07/2012 04:04 PM, Santosh Shilimkar wrote:
> On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote:
>> For OMAP2+ devices, when using DMTIMERs for system timers
>> (clock-events and
>> clock-source) the posted mode configuration of the timers is used. To
>> allow
>> the compiler to optimise the functions for configuring and reading the
>> system
>> timers, the posted flag variable is hard-coded with the value 1. To
>> make it
>> clear that posted mode is being used add some definitions so that it
>> is more
>> readable.
>>
>> Add separate definitions for the clock-events and clock-source timers
>> so that
>> we can change the posted mode of clock-events and clock-source
>> independently.
>>
>> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
>> ---
>> arch/arm/mach-omap2/timer.c | 26
>> +++++++++++++++++++-------
>> arch/arm/plat-omap/include/plat/dmtimer.h | 4 ++++
>> 2 files changed, 23 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
>> index 0758bae..28c6078 100644
>> --- a/arch/arm/mach-omap2/timer.c
>> +++ b/arch/arm/mach-omap2/timer.c
>> @@ -82,6 +82,13 @@
>> #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET 0x14
>> #define NUMERATOR_DENUMERATOR_MASK 0xfffff000
>>
>> +/*
>> + * For clock-events timer, always use posted mode to
>> + * minimise CPU overhead for configuring the timer.
>> + */
>> +#define OMAP_CLKEVT_POSTEDMODE OMAP_TIMER_POSTED
>> +#define OMAP_CLKSRC_POSTEDMODE OMAP_TIMER_POSTED
>> +
> I don't see need of above defines. Just use OMAP_TIMER_POSTED directly
> with API. Rest of the patch looks fine.
Yes that's possible, however, in patch #2, I am disabling posted mode
for clock-source (see changelog of patch #2 for details). Having these
#defines makes it easier to change the posted configuration. That was
the real motivation here.
Cheers
Jon
^ permalink raw reply
* [PATCH V2 02/14] ARM: OMAP2+: Disable posted mode for the clocksource timer
From: Santosh Shilimkar @ 2012-11-07 22:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1352314890-22224-3-git-send-email-jon-hunter@ti.com>
On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote:
> When using a DMTIMER as the clock-source timer, posted mode configuration of
> the DMTIMER is used. Posted mode is only benefical when configuring timers as
> it allows writes to be posted and does not stall the CPU until the write is
> complete. The clock-source timer is only configured once on boot and so using
> posted mode has no benefit. In fact, by using posted mode, it adds overhead
> to reading the timer. Therefore, by default disable posted mode for DMTIMERs
> used for clock-source.
>
> Using objdump this change reduces the function clocksource_read_cycles()
> function from a total of 15 instructions (including 3 branches and 7 loads)
> to 5 instructions (including 1 branch and 3 loads). Please note that before
> the minimum number of instructions that would be executed when calling
> clocksource_read_cycles() would be 9 instructions (including 2 branches and 5
> loads) where as now it will always be 5 instructions.
>
> This change also reduces the function dmtimer_read_sched_clock() function from
> a total of 17 instructions (including 4 branches and 8 loads) to 6 instructions
> (including 1 branch and 4 loads). Please note that before the minimum number of
> instructions that would be executed when calling dmtimer_read_sched_clock()
> would be 11 instructions (including 2 branches and 6 loads) where as now it
> will always be 6 instructions.
>
This isn't right way to calculate the penalty of posted mode. Non-posted
mode can results in 100 of cycles wait over interconnect
and that can not be justified with few instructions savings.
> This change removes the configuration of posted mode from the
> __omap_dm_timer_reset() function and adds a new function called
> omap_dm_timer_enable_posted() for enabling posted mode. Hence, call
> omap_dm_timer_enable_posted() where ever we are calling __omap_dm_timer_reset().
>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
But clock-source doesn't involve much into writes so patch as
such is fine.
Regards
Santosh
^ permalink raw reply
* [PATCH 07/11] ARM: OMAP: Move omap-pm-noop.c local to mach-omap2
From: Jon Hunter @ 2012-11-07 22:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121031231115.GG15766@atomide.com>
On 10/31/2012 06:11 PM, Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [121031 16:03]:
>> Hi Tony,
>>
>> On Wednesday 31 October 2012 13:58:50 Tony Lindgren wrote:
>>> * Tony Lindgren <tony@atomide.com> [121030 16:55]:
>>>> This code should be private to mach-omap2.
>>>>
>>>> The only use for it in for omap1 has been in dmtimer.c
>>>> to check for context loss. However, omap1 does not
>>>> lose context during idle, so the code is not needed.
>>>> Further, omap1 timer has OMAP_TIMER_ALWON set, so omap1
>>>> was not hitting omap_pm_get_dev_context_loss_count()
>>>> test.
>>>
>>> Noticed one issue with my test compiles in the
>>> omap-for-v3.8/cleanup-headers branch that can be
>>> fixed along with this patch.
>>>
>>> --- a/drivers/media/platform/omap3isp/ispvideo.c
>>> +++ b/drivers/media/platform/omap3isp/ispvideo.c
>>> @@ -36,7 +36,6 @@
>>> #include <media/v4l2-ioctl.h>
>>> #include <plat/iommu.h>
>>> #include <plat/iovmm.h>
>>> -#include <plat/omap-pm.h>
>>
>> The reason this was included was to call omap_pm_set_min_bus_tput() in earlier
>> versions of the driver. We'll have to discuss what to replace that with, but
>> that's another topic.
>
> OK thanks.
>
>>>> @@ -730,6 +732,7 @@ static int __devinit omap_dm_timer_probe(struct
>>>> platform_device *pdev)>
>>>> timer->reserved = omap_dm_timer_reserved_systimer(timer->id);
>>>> timer->pdev = pdev;
>>>> timer->capability = pdata->timer_capability;
>>>>
>>>> + timer->get_context_loss_count = pdata->get_context_loss_count;
>>>>
>>>> /* Skip pm_runtime_enable for OMAP1 */
>>>> if (!(timer->capability & OMAP_TIMER_NEEDS_RESET)) {
>>>>
>>>> diff --git a/arch/arm/plat-omap/include/plat/dmtimer.h
>>>> b/arch/arm/plat-omap/include/plat/dmtimer.h index 85868e9..3f5b9cf 100644
>>>> --- a/arch/arm/plat-omap/include/plat/dmtimer.h
>>>> +++ b/arch/arm/plat-omap/include/plat/dmtimer.h
>>>> @@ -94,6 +94,7 @@ struct dmtimer_platform_data {
>>>>
>>>> /* set_timer_src - Only used for OMAP1 devices */
>>>> int (*set_timer_src)(struct platform_device *pdev, int source);
>>>> u32 timer_capability;
>>>>
>>>> + int (*get_context_loss_count)(struct device *);
>>
>> That's a step forward for the common zImage, but one step backward for DT
>> support :-) I'm fine with this for now, but do you already have an idea on how
>> to solve that ?
>
> When it's converted to be a device driver, it can do it
> using runtime PM calls.
I am not sure if you are referring to runtime pm callbacks here, but if
so I am not sure I follow. Drivers such as dmtimer and gpio that are
using runtime pm are still dependent on OMAP specific APIs (such as
omap_pm_get_dev_context_loss_count()) for determining if the context was
lost between suspending and resuming the device. So I am not sure how
runtime pm solves this.
Speaking with Rob Herring, one solution for DT would be using bus
notifiers to populate such function pointers when a device is added.
Given that there are a few devices using this architecture specific API
for context loss I am wondering if we can do something generic in
omap_device.c for DT.
Cheers
Jon
^ permalink raw reply
* [PATCH V2 01/14] ARM: OMAP: Add DMTIMER definitions for posted mode
From: Santosh Shilimkar @ 2012-11-07 22:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1352314890-22224-2-git-send-email-jon-hunter@ti.com>
On Wednesday 07 November 2012 01:01 PM, Jon Hunter wrote:
> For OMAP2+ devices, when using DMTIMERs for system timers (clock-events and
> clock-source) the posted mode configuration of the timers is used. To allow
> the compiler to optimise the functions for configuring and reading the system
> timers, the posted flag variable is hard-coded with the value 1. To make it
> clear that posted mode is being used add some definitions so that it is more
> readable.
>
> Add separate definitions for the clock-events and clock-source timers so that
> we can change the posted mode of clock-events and clock-source independently.
>
> Signed-off-by: Jon Hunter <jon-hunter@ti.com>
> ---
> arch/arm/mach-omap2/timer.c | 26 +++++++++++++++++++-------
> arch/arm/plat-omap/include/plat/dmtimer.h | 4 ++++
> 2 files changed, 23 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 0758bae..28c6078 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -82,6 +82,13 @@
> #define INCREMENTER_DENUMERATOR_RELOAD_OFFSET 0x14
> #define NUMERATOR_DENUMERATOR_MASK 0xfffff000
>
> +/*
> + * For clock-events timer, always use posted mode to
> + * minimise CPU overhead for configuring the timer.
> + */
> +#define OMAP_CLKEVT_POSTEDMODE OMAP_TIMER_POSTED
> +#define OMAP_CLKSRC_POSTEDMODE OMAP_TIMER_POSTED
> +
I don't see need of above defines. Just use OMAP_TIMER_POSTED directly
with API. Rest of the patch looks fine.
Apart from above one comment,
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
^ permalink raw reply
* [PATCH 2/2] ARM: S3C64XX: Statically define parent clock of the "camera" clock
From: Sylwester Nawrocki @ 2012-11-07 22:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1352325657-28885-1-git-send-email-sylvester.nawrocki@gmail.com>
The "camera" clock defined in arch/arm/mach-s3c64xx/clock.c has null
clock source mux control register as it can have only one parent
clock. In such cases there is a need to configure the parent clock
statically, otherwise s3c_set_clksrc() bails out with an error message
"no parent clock specified" leaving the parent clock not configured.
Define statically the parent clock so it is possible to get or set rate
of the "camera" clock.
Reported-by: In-Bae Jeong <kukyakya@gmail.com>
Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
---
arch/arm/mach-s3c64xx/clock.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-s3c64xx/clock.c b/arch/arm/mach-s3c64xx/clock.c
index 28041e8..85b9cf1 100644
--- a/arch/arm/mach-s3c64xx/clock.c
+++ b/arch/arm/mach-s3c64xx/clock.c
@@ -744,6 +744,7 @@ static struct clksrc_clk clksrcs[] = {
.name = "camera",
.ctrlbit = S3C_CLKCON_SCLK_CAM,
.enable = s3c64xx_sclk_ctrl,
+ .parent = &clk_h2,
},
.reg_div = { .reg = S3C_CLK_DIV0, .shift = 20, .size = 4 },
.reg_src = { .reg = NULL, .shift = 0, .size = 0 },
--
1.7.4.1
^ permalink raw reply related
* [PATCH 1/2] ARM: S3C24XX: Add clkdev entry for camif-upll clock
From: Sylwester Nawrocki @ 2012-11-07 22:00 UTC (permalink / raw)
To: linux-arm-kernel
The s3c-camif driver uses "camera" clock conn_id for the "camif-upll"
(s3c244x) and "camera" (s3c64xx) platform clock. By adding this new
clkdev entry the platform differences are isolated from the driver.
Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
---
arch/arm/mach-s3c24xx/clock-s3c2440.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-s3c24xx/clock-s3c2440.c b/arch/arm/mach-s3c24xx/clock-s3c2440.c
index 4407b17..04b87ec 100644
--- a/arch/arm/mach-s3c24xx/clock-s3c2440.c
+++ b/arch/arm/mach-s3c24xx/clock-s3c2440.c
@@ -161,6 +161,7 @@ static struct clk_lookup s3c2440_clk_lookup[] = {
CLKDEV_INIT(NULL, "clk_uart_baud1", &s3c24xx_uclk),
CLKDEV_INIT(NULL, "clk_uart_baud2", &clk_p),
CLKDEV_INIT(NULL, "clk_uart_baud3", &s3c2440_clk_fclk_n),
+ CLKDEV_INIT("s3c2440-camif", "camera", &s3c2440_clk_cam_upll),
};
static int __init_refok s3c2440_clk_add(struct device *dev, struct subsys_interface *sif)
--
1.7.4.1
^ permalink raw reply related
* [PATCH V2 00/14] ARM: OMAP: DMTIMER fixes
From: Tony Lindgren @ 2012-11-07 21:52 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <509AD621.7060402@ti.com>
* Jon Hunter <jon-hunter@ti.com> [121107 13:45]:
>
> On 11/07/2012 03:22 PM, Tony Lindgren wrote:
> > * Jon Hunter <jon-hunter@ti.com> [121107 11:03]:
> >> This series includes several fixes for the OMAP DMTIMER driver. This is
> >> based upon 3.7-rc4 with the two series adding device-tree support for
> >> DMTIMERs [1] and the 32kHz Counter [2]
> >
> > These look OK to me, I'm assuming you'll do a pull request eventually
> > for these. Might be worth checking if these merge fine also on
> > omap-for-v3.8/cleanup-headers-prepare-multiplatform-v3, rename detection
> > might be able to deal with that for the header.
>
> Yes I was going to wait a few days for people to review before sending a
> pull request. I will make sure it merges on your master. Is Monday too
> late for v3.8?
Probably not for this series as it fixes some issues, but in general
we should have most of the patches queued by -rc4 time.
Regards,
Tony
> >> [1] http://marc.info/?l=linux-omap&m=135065875808614&w=2
> >> [2] http://marc.info/?l=linux-omap&m=135119308123513&w=2
^ permalink raw reply
* [PATCH V2 00/14] ARM: OMAP: DMTIMER fixes
From: Jon Hunter @ 2012-11-07 21:44 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20121107212254.GK6801@atomide.com>
On 11/07/2012 03:22 PM, Tony Lindgren wrote:
> * Jon Hunter <jon-hunter@ti.com> [121107 11:03]:
>> This series includes several fixes for the OMAP DMTIMER driver. This is
>> based upon 3.7-rc4 with the two series adding device-tree support for
>> DMTIMERs [1] and the 32kHz Counter [2]
>
> These look OK to me, I'm assuming you'll do a pull request eventually
> for these. Might be worth checking if these merge fine also on
> omap-for-v3.8/cleanup-headers-prepare-multiplatform-v3, rename detection
> might be able to deal with that for the header.
Yes I was going to wait a few days for people to review before sending a
pull request. I will make sure it merges on your master. Is Monday too
late for v3.8?
Cheers
Jon
>> [1] http://marc.info/?l=linux-omap&m=135065875808614&w=2
>> [2] http://marc.info/?l=linux-omap&m=135119308123513&w=2
^ permalink raw reply
* [PATCH] ARM: OMAP2+: timer: remove CONFIG_OMAP_32K_TIMER
From: Jon Hunter @ 2012-11-07 21:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1352299344-8011-1-git-send-email-grinberg@compulab.co.il>
Hi Igor,
On 11/07/2012 08:42 AM, Igor Grinberg wrote:
> CONFIG_OMAP_32K_TIMER is kind of standing on the single zImage way.
> Make OMAP2+ timer code independant from the CONFIG_OMAP_32K_TIMER
> setting.
> To remove the dependancy, several conversions/additions had to be done:
> 1) Timer structures and initialization functions are named by the platform
> name and the clock source in use. The decision which timer is
> used is done statically from the machine_desc structure. In the
> future it should come from DT.
> 2) Settings under the CONFIG_OMAP_32K_TIMER option are expanded into
> separate timer structures along with the timer init functions.
> This removes the CONFIG_OMAP_32K_TIMER on OMAP2+ timer code.
> 3) Since we have all the timers defined inside machine_desc structure
> and we no longer need the fallback to gp_timer clock source in case
> 32k_timer clock source is unavailable (namely on AM33xx), we no
> longer need the #ifdef around __omap2_sync32k_clocksource_init()
> function. Remove the #ifdef CONFIG_OMAP_32K_TIMER around the
> __omap2_sync32k_clocksource_init() function.
>
> Signed-off-by: Igor Grinberg <grinberg@compulab.co.il>
> Cc: Jon Hunter <jon-hunter@ti.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Cc: Vaibhav Hiremath <hvaibhav@ti.com>
[snip]
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 684d2fc..a4ad7a0 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -63,20 +63,8 @@
> #define OMAP2_32K_SOURCE "func_32k_ck"
> #define OMAP3_32K_SOURCE "omap_32k_fck"
> #define OMAP4_32K_SOURCE "sys_32k_ck"
> -
> -#ifdef CONFIG_OMAP_32K_TIMER
> -#define OMAP2_CLKEV_SOURCE OMAP2_32K_SOURCE
> -#define OMAP3_CLKEV_SOURCE OMAP3_32K_SOURCE
> -#define OMAP4_CLKEV_SOURCE OMAP4_32K_SOURCE
> -#define OMAP3_SECURE_TIMER 12
> #define TIMER_PROP_SECURE "ti,timer-secure"
> -#else
> -#define OMAP2_CLKEV_SOURCE OMAP2_MPU_SOURCE
> -#define OMAP3_CLKEV_SOURCE OMAP3_MPU_SOURCE
> -#define OMAP4_CLKEV_SOURCE OMAP4_MPU_SOURCE
> -#define OMAP3_SECURE_TIMER 1
> -#define TIMER_PROP_SECURE "ti,timer-alwon"
> -#endif
> +#define TIMER_PROP_ALWON "ti,timer-alwon"
Nit-pick, can we drop the TIMER_PROP_SECURE/ALWON and use the
"ti,timer-secure" and "ti,timer-alwon" directly?
Initially, I also defined TIMER_PROP_ALWON and Rob Herring's feedback
was to drop this and use the property string directly to remove any
abstraction.
> #define REALTIME_COUNTER_BASE 0x48243200
> #define INCREMENTER_NUMERATOR_OFFSET 0x10
> @@ -216,7 +204,7 @@ void __init omap_dmtimer_init(void)
>
> /* If we are a secure device, remove any secure timer nodes */
> if ((omap_type() != OMAP2_DEVICE_TYPE_GP)) {
> - np = omap_get_timer_dt(omap_timer_match, "ti,timer-secure");
> + np = omap_get_timer_dt(omap_timer_match, TIMER_PROP_SECURE);
> if (np)
> of_node_put(np);
> }
> @@ -378,9 +366,8 @@ static u32 notrace dmtimer_read_sched_clock(void)
> return 0;
> }
>
> -#ifdef CONFIG_OMAP_32K_TIMER
> /* Setup free-running counter for clocksource */
> -static int __init omap2_sync32k_clocksource_init(void)
> +static int __init __omap2_sync32k_clocksource_init(void)
Not sure I follow why you renamed this function here ...
> {
> int ret;
> struct device_node *np = NULL;
> @@ -439,15 +426,9 @@ static int __init omap2_sync32k_clocksource_init(void)
>
> return ret;
> }
> -#else
> -static inline int omap2_sync32k_clocksource_init(void)
> -{
> - return -ENODEV;
> -}
> -#endif
>
> -static void __init omap2_gptimer_clocksource_init(int gptimer_id,
> - const char *fck_source)
> +static void __init omap2_gp_clocksource_init(int gptimer_id,
> + const char *fck_source)
Nit, I personally prefer keeping gptimer, because gp just means
"general-purpose" and does not imply a timer per-se.
> {
> int res;
>
> @@ -466,23 +447,10 @@ static void __init omap2_gptimer_clocksource_init(int gptimer_id,
> gptimer_id, clksrc.rate);
> }
>
> -static void __init omap2_clocksource_init(int gptimer_id,
> - const char *fck_source)
> +static void __init omap2_sync32k_clocksource_init(int gptimer_id,
> + const char *fck_source)
> {
> - /*
> - * First give preference to kernel parameter configuration
> - * by user (clocksource="gp_timer").
> - *
> - * In case of missing kernel parameter for clocksource,
> - * first check for availability for 32k-sync timer, in case
> - * of failure in finding 32k_counter module or registering
> - * it as clocksource, execution will fallback to gp-timer.
> - */
> - if (use_gptimer_clksrc == true)
> - omap2_gptimer_clocksource_init(gptimer_id, fck_source);
> - else if (omap2_sync32k_clocksource_init())
> - /* Fall back to gp-timer code */
> - omap2_gptimer_clocksource_init(gptimer_id, fck_source);
> + __omap2_sync32k_clocksource_init();
> }
... this just appears to be a wrapper function, but I don't see why this
is needed? Do we need this wrapper?
> #ifdef CONFIG_SOC_HAS_REALTIME_COUNTER
> @@ -563,52 +531,64 @@ static inline void __init realtime_counter_init(void)
> {}
> #endif
>
> -#define OMAP_SYS_TIMER_INIT(name, clkev_nr, clkev_src, clkev_prop, \
> - clksrc_nr, clksrc_src) \
> -static void __init omap##name##_timer_init(void) \
> +#define OMAP_SYS_TIMER_INIT(n, clksrc_name, clkev_nr, clkev_src, \
> + clkev_prop, clksrc_nr, clksrc_src) \
> +static void __init omap##n##_##clksrc_name##_timer_init(void) \
> { \
> omap_dmtimer_init(); \
> omap2_gp_clockevent_init((clkev_nr), clkev_src, clkev_prop); \
> - omap2_clocksource_init((clksrc_nr), clksrc_src); \
> + \
> + if (use_gptimer_clksrc) \
> + omap2_gp_clocksource_init((clksrc_nr), clksrc_src); \
> + else \
> + omap2_##clksrc_name##_clocksource_init((clksrc_nr), \
> + clksrc_src); \
Something here seems a little odd. If "clksrc_name" is "gp", then the
if-else parts will call the same function. Or am I missing something here?
I think that I prefer how it works today where we call just
omap2_clocksource_init(), and it determines whether to use the gptimer
or the 32k-sync.
For OMAP I think that it is fine to default to the 32k-sync and then if
the gptimer is selected, it uses the higher frequency sys_clk as the
timer source.
For AMxxx, devices, sync-32k does not exist, and so I understand it does
not work the same.
I am wondering if the use_gptimer_clksrc, should become
use_sysclk_clksrc, and then ...
For OMAP ...
use_sysclk_clksrc = 0 --> use sync-32k (default)
use_sysclk_clksrc = 1 --> use gptimer with sys_clk
For AM33xx ...
use_sysclk_clksrc = 0 --> use gptimer with 32khz clock (default)
use_sysclk_clksrc = 1 --> use gptimer with sys_clk
> }
>
> -#define OMAP_SYS_TIMER(name) \
> -struct sys_timer omap##name##_timer = { \
> - .init = omap##name##_timer_init, \
> -};
> +#define OMAP_SYS_TIMER(n, clksrc) \
> +struct sys_timer omap##n##_##clksrc##_timer = { \
> + .init = omap##n##_##clksrc##_timer_init, \
> +}
>
> #ifdef CONFIG_ARCH_OMAP2
> -OMAP_SYS_TIMER_INIT(2, 1, OMAP2_CLKEV_SOURCE, "ti,timer-alwon",
> - 2, OMAP2_MPU_SOURCE)
> -OMAP_SYS_TIMER(2)
> +OMAP_SYS_TIMER_INIT(2, sync32k, 1, OMAP2_32K_SOURCE, TIMER_PROP_ALWON,
> + 2, OMAP2_MPU_SOURCE);
> +OMAP_SYS_TIMER(2, sync32k);
> +OMAP_SYS_TIMER_INIT(2, gp, 1, OMAP2_MPU_SOURCE, TIMER_PROP_ALWON,
> + 2, OMAP2_MPU_SOURCE);
> +OMAP_SYS_TIMER(2, gp);
It would be good if we can avoid having two timer_init functions for
each OMAP generation.
Cheers
Jon
^ permalink raw reply
* [PATCH 0/6] OMAPDSS: enable DSS for Panda & SDP with devtree
From: Tony Lindgren @ 2012-11-07 21:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <509A3EE2.1080500@ti.com>
* Benoit Cousson <b-cousson@ti.com> [121107 03:00]:
> On 11/07/2012 02:04 AM, Tony Lindgren wrote:
> > * Tomi Valkeinen <tomi.valkeinen@ti.com> [121105 05:16]:
> >> Hi,
> >>
> >> OMAPDSS device tree support is still some way in the future. Tony has requested
> >> to get DSS working for Panda & SDP boards with DT kernel, so that we'll have
> >> fully working boards with DT.
> >>
> >> This series makes a few hacks to get a working display on OMAP4 Panda and SDP
> >> boards. The idea is to setup the omapdss with the non-DT method, creating the
> >> omapdss devices and passing platform data to them. This setup code is called
> >> from board-generic for Panda and SDP boards.
> >>
> >> There was one problem with this approach: omapdss cannot get regulators using
> >> the omapdss's names fro the regulators. Thus there's a hack patch to get the
> >> regulators using the OMAP4 "native" regulator names, thus circumventing the
> >> problem.
> >>
> >> Tony, if these look good, how do you want to merge these? There are three parts
> >> here, and I think they can be merged independently if so wished:
> >>
> >> * .dts changes for the pinmuxing (2 patches)
> >
> > Let's let Benoit queue these.
>
> Done. I'll send you the pull-request tomorrow in case some more DTS
> patches are coming.
OK thanks, I'll apply 3-6 to omap-for-v3.8/board branch.
Regards,
Tony
^ permalink raw reply
* [PATCH V2 00/14] ARM: OMAP: DMTIMER fixes
From: Tony Lindgren @ 2012-11-07 21:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1352314890-22224-1-git-send-email-jon-hunter@ti.com>
* Jon Hunter <jon-hunter@ti.com> [121107 11:03]:
> This series includes several fixes for the OMAP DMTIMER driver. This is
> based upon 3.7-rc4 with the two series adding device-tree support for
> DMTIMERs [1] and the 32kHz Counter [2]
These look OK to me, I'm assuming you'll do a pull request eventually
for these. Might be worth checking if these merge fine also on
omap-for-v3.8/cleanup-headers-prepare-multiplatform-v3, rename detection
might be able to deal with that for the header.
Regards,
Tony
> [1] http://marc.info/?l=linux-omap&m=135065875808614&w=2
> [2] http://marc.info/?l=linux-omap&m=135119308123513&w=2
^ permalink raw reply
* OMAP baseline test results for v3.7-rc4
From: Jon Hunter @ 2012-11-07 20:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1211071728310.20704@utopia.booyaka.com>
On 11/07/2012 11:32 AM, Paul Walmsley wrote:
> On Wed, 7 Nov 2012, Jon Hunter wrote:
>
>> On 11/04/2012 08:46 PM, Paul Walmsley wrote:
>>
>>> * 4460pandaes: boot fails early
>>> - Appears to be due to X-loader problems here
>>> - Need to note the X-loader version so we know it's broken
>>
>> Are you still planning to investigate this further or migrate to a new
>> bootloader?
>
> Yes. The problem with the current boot setup is that the new
> bootloader is breaking as well! :-(
>
> This can be seen by comparing:
>
> http://www.pwsan.com/omap/testlogs/test_v3.7-rc1/20121017205513/boot/4460pandaes/4460pandaes_log.txt
>
> (the old bootloader)
>
> with
>
> http://www.pwsan.com/omap/testlogs/test_v3.7-rc4/20121104142910/boot/4460pandaes/4460pandaes_log.txt
>
> (the new bootloader)
>
> Dealing with this will require hands-on access, and unfortunately, right
> now I am traveling and away from my testbed. Next week, will take a
> closer look.
>
>> With 4460 and 3.7-rc4, I am seeing that resume from suspend is not
>> working (not sure when this broke though). I am just curious if anyone
>> else sees this. My u-boot version is v2012.10.
>
> If you have the chance, it would be useful to know if any of the v3.7-rcs
> worked. Otherwise, will look at this next week.
Kevin pointed me to his pm fixes for 3.8 as it is working there and I
have confirmed suspend is fixed on v3.7-rc4 with the rom code fix [1]
Kevin has queued for v3.8.
Cheers
Jon
[1]
http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commit;h=ff999b8a0983ee15668394ed49e38d3568fc6859
^ permalink raw reply
* vexpress issues in next-20121029
From: Stephen Warren @ 2012-11-07 20:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <508EE610.9050703@wwwdotorg.org>
On 10/29/2012 02:24 PM, Stephen Warren wrote:
> Pawel,
>
> I see two issues on vexpress in next-20121029:
...
> 2) With those calls commented out, I find that
> vexpress_sysreg_init_leds() is device_initcall, and so executes even
> when not running on vexpress HW. This crashes on Tegra (which I have
> converted to single-zImage locally).
This issue still appears to be present in next-20121107.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox