RE: [PATCH 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
Hi Jon, On Tue, Nov 06, 2012 at 02:50:50, Hunter, Jon wrote: [...] Why is this? How is the dmtimer TIOCP_CFG register configured on AM33xx? Is it using smart-idle? Yes, it is set to smart-idle with wakeup capable mode. (this needs a fixup since this timer is not wakeup capable) but unfortunately this is not sufficient. On AM33xx there's no HW_AUTO mode magic so unless the IPs in PER domain are disabled by s/w, PER domain can't transition. The next one is that the clockevent doesn't generate any further interrupts once the system resumes. We need to restore the pre-suspend configuration. I haven't tried but I guess we could have used the save and restore of timer registers here. It would be interesting to try using an non-wakeup domain timer on OMAP3/4 for clock events and seeing if suspend/resume works. Do you know what the exact problem here is? I understand that the timer context could get lost, but exactly what is not getting restarted by the kernel? For example, the only place we set the interrupt enable is during the clock event init and so if the context is lost, then I could see no more interrupts occurring. So is it enough to just restore the interrupt enable register, do you really need to program the timer again? Just restoring the interrupt enable register works. But since there's no logic retention I think a context save-restore would be better. Regards, Vaibhav -- 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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
On 11/06/2012 01:32 AM, Bedia, Vaibhav wrote:
Hi Jon,
On Tue, Nov 06, 2012 at 02:34:05, Hunter, Jon wrote:
[...]
static struct clock_event_device clockevent_gpt = {
.name = gp_timer,
.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
@@ -142,6 +171,8 @@ static struct clock_event_device clockevent_gpt = {
.rating = 300,
.set_next_event = omap2_gp_timer_set_next_event,
.set_mode = omap2_gp_timer_set_mode,
+ .suspend= omap_clkevt_suspend,
+ .resume = omap_clkevt_resume,
So these suspend/resume callbacks are going to be called for all OMAP2+
and AM devices? I don't think we want that. AFAIK OMAP timers will
idle on their own when stopped and don't require this.
IMO instead of skipping the callback registration we could have checks in the
suspend/resume callbacks to decide what to do.
I'll check if the idling part is AM33xx specific. If not, based on the recent
timer
changes that you did, perhaps checking if the clockevent selected doesn't
have the
ti,timer-alwon capability will be good enough. What do you think?
Yes, I was thinking along the same lines. If I get chance I will try and
test your scenario on an OMAP3 too.
Cheers
Jon
--
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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
On 11/06/2012 03:38 AM, Bedia, Vaibhav wrote: Hi Jon, On Tue, Nov 06, 2012 at 02:50:50, Hunter, Jon wrote: [...] Why is this? How is the dmtimer TIOCP_CFG register configured on AM33xx? Is it using smart-idle? Yes, it is set to smart-idle with wakeup capable mode. (this needs a fixup since this timer is not wakeup capable) but unfortunately this is not sufficient. On AM33xx there's no HW_AUTO mode magic so unless the IPs in PER domain are disabled by s/w, PER domain can't transition. The next one is that the clockevent doesn't generate any further interrupts once the system resumes. We need to restore the pre-suspend configuration. I haven't tried but I guess we could have used the save and restore of timer registers here. It would be interesting to try using an non-wakeup domain timer on OMAP3/4 for clock events and seeing if suspend/resume works. Do you know what the exact problem here is? I understand that the timer context could get lost, but exactly what is not getting restarted by the kernel? For example, the only place we set the interrupt enable is during the clock event init and so if the context is lost, then I could see no more interrupts occurring. So is it enough to just restore the interrupt enable register, do you really need to program the timer again? Just restoring the interrupt enable register works. But since there's no logic retention I think a context save-restore would be better. Ok, we may need to check the order in which events occur following resume. The kernel will restart the clock-events and we just need to make sure we do not restore the context after the clock-events has been restarted. Cheers Jon -- 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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
On Sunday 04 November 2012 08:55 PM, Bedia, Vaibhav wrote:
Hi Santosh,
On Sat, Nov 03, 2012 at 21:22:04, Shilimkar, Santosh wrote:
On Friday 02 November 2012 06:02 PM, Vaibhav Bedia wrote:
From: Vaibhav Hiremath hvaib...@ti.com
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
Actually OMAP also uses only one timer. The clocksource
is taken care by 32K syntimer till OMAP4 and by realtime
counter on OMAP5. There is a clocksource registration of
timer is available but that is not being used in systems.
Yes, I guess the changelog should mention that AM33xx does not
have the 32k synctimer. I'll also add in the OMAP details that
you pointed out so that all the details get captured.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
So you use WKUP domain timer for clocksource and PER
domain one for clock-event ?
Yes, that's correct.
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
---
arch/arm/mach-omap2/timer.c | 31 +++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 6584ee0..e8781fd 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -135,6 +135,35 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode
mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
You can move all the look up stuff in init code and then
suspend resume hooks will be cleaner.
Will do. Kevin also pointed this out.
+
+ omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_enable(oh);
+ __omap_dm_timer_load_start(clkev,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ __omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
+}
+
OK. So since your clk_event stops when PER idles, how do you plan
to support the SOC idle. For CPUIDLE path, you need your clock-event
to wakeup the system based on next timer expiry. So you need your
clock event to be active. Indirectly, you can't let PER idle which
leads npo CORE idle-SOC idle.
How do you plan to address this ? Os is SOC idle is not suppose
to be added for AMXXX ?
We can't really have SOC idle on AM33xx or at least that's what I think.
The deepest that we should be able to support is MPU off with external
memory in self-refresh mode. I mentioned the reasons for that in the
reply to Kevin [1]. If there's any another approach that we could take
that would be great to know.
Thanks for information.
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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
On 11/02/2012 07:32 AM, Vaibhav Bedia wrote:
From: Vaibhav Hiremath hvaib...@ti.com
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
---
arch/arm/mach-omap2/timer.c | 31 +++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 6584ee0..e8781fd 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -135,6 +135,35 @@ static void omap2_gp_timer_set_mode(enum
clock_event_mode mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_enable(oh);
+ __omap_dm_timer_load_start(clkev,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ __omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
+}
+
static struct clock_event_device clockevent_gpt = {
.name = gp_timer,
.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
@@ -142,6 +171,8 @@ static struct clock_event_device clockevent_gpt = {
.rating = 300,
.set_next_event = omap2_gp_timer_set_next_event,
.set_mode = omap2_gp_timer_set_mode,
+ .suspend= omap_clkevt_suspend,
+ .resume = omap_clkevt_resume,
So these suspend/resume callbacks are going to be called for all OMAP2+
and AM devices? I don't think we want that. AFAIK OMAP timers will
idle on their own when stopped and don't require this.
Cheers
Jon
--
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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
On 11/03/2012 08:17 AM, Bedia, Vaibhav wrote: On Sat, Nov 03, 2012 at 17:45:03, Kevin Hilman wrote: On 11/02/2012 01:32 PM, Vaibhav Bedia wrote: From: Vaibhav Hiremath hvaib...@ti.com The current OMAP timer code registers two timers - one as clocksource and one as clockevent. AM33XX has only one usable timer in the WKUP domain so one of the timers needs suspend-resume support to restore the configuration to pre-suspend state. The changelog describes what, but doesn't answer why? Sorry I'll try to take of this in the future. commit adc78e6 (timekeeping: Add suspend and resume of clock event devices) introduced .suspend and .resume callbacks for clock event devices. Leverages these callbacks to have AM33XX clockevent timer which is in not in WKUP domain to behave properly across system suspend. You say it behaves properly without describing what improper behavior is happening. There are two issues. One is that the clockevent timer doesn't get idled which blocks PER domain transition. Why is this? How is the dmtimer TIOCP_CFG register configured on AM33xx? Is it using smart-idle? The next one is that the clockevent doesn't generate any further interrupts once the system resumes. We need to restore the pre-suspend configuration. I haven't tried but I guess we could have used the save and restore of timer registers here. It would be interesting to try using an non-wakeup domain timer on OMAP3/4 for clock events and seeing if suspend/resume works. Do you know what the exact problem here is? I understand that the timer context could get lost, but exactly what is not getting restarted by the kernel? For example, the only place we set the interrupt enable is during the clock event init and so if the context is lost, then I could see no more interrupts occurring. So is it enough to just restore the interrupt enable register, do you really need to program the timer again? Cheers Jon -- 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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
Hi Jon,
On Tue, Nov 06, 2012 at 02:34:05, Hunter, Jon wrote:
[...]
static struct clock_event_device clockevent_gpt = {
.name = gp_timer,
.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
@@ -142,6 +171,8 @@ static struct clock_event_device clockevent_gpt = {
.rating = 300,
.set_next_event = omap2_gp_timer_set_next_event,
.set_mode = omap2_gp_timer_set_mode,
+ .suspend= omap_clkevt_suspend,
+ .resume = omap_clkevt_resume,
So these suspend/resume callbacks are going to be called for all OMAP2+
and AM devices? I don't think we want that. AFAIK OMAP timers will
idle on their own when stopped and don't require this.
IMO instead of skipping the callback registration we could have checks in the
suspend/resume callbacks to decide what to do.
I'll check if the idling part is AM33xx specific. If not, based on the recent
timer
changes that you did, perhaps checking if the clockevent selected doesn't have
the
ti,timer-alwon capability will be good enough. What do you think?
Regards.
Vaibhav
--
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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
Hi Santosh,
On Sat, Nov 03, 2012 at 21:22:04, Shilimkar, Santosh wrote:
On Friday 02 November 2012 06:02 PM, Vaibhav Bedia wrote:
From: Vaibhav Hiremath hvaib...@ti.com
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
Actually OMAP also uses only one timer. The clocksource
is taken care by 32K syntimer till OMAP4 and by realtime
counter on OMAP5. There is a clocksource registration of
timer is available but that is not being used in systems.
Yes, I guess the changelog should mention that AM33xx does not
have the 32k synctimer. I'll also add in the OMAP details that
you pointed out so that all the details get captured.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
So you use WKUP domain timer for clocksource and PER
domain one for clock-event ?
Yes, that's correct.
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
---
arch/arm/mach-omap2/timer.c | 31 +++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 6584ee0..e8781fd 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -135,6 +135,35 @@ static void omap2_gp_timer_set_mode(enum
clock_event_mode mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
You can move all the look up stuff in init code and then
suspend resume hooks will be cleaner.
Will do. Kevin also pointed this out.
+
+ omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_enable(oh);
+ __omap_dm_timer_load_start(clkev,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ __omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
+}
+
OK. So since your clk_event stops when PER idles, how do you plan
to support the SOC idle. For CPUIDLE path, you need your clock-event
to wakeup the system based on next timer expiry. So you need your
clock event to be active. Indirectly, you can't let PER idle which
leads npo CORE idle-SOC idle.
How do you plan to address this ? Os is SOC idle is not suppose
to be added for AMXXX ?
We can't really have SOC idle on AM33xx or at least that's what I think.
The deepest that we should be able to support is MPU off with external
memory in self-refresh mode. I mentioned the reasons for that in the
reply to Kevin [1]. If there's any another approach that we could take
that would be great to know.
Regards,
Vaibhav
[1] http://marc.info/?l=linux-arm-kernelm=135195053104053w=2
--
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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
On 11/02/2012 01:32 PM, Vaibhav Bedia wrote:
From: Vaibhav Hiremath hvaib...@ti.com
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
The changelog describes what, but doesn't answer why?
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
You say it behaves properly without describing what improper
behavior is happening.
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
---
arch/arm/mach-omap2/timer.c | 31 +++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 6584ee0..e8781fd 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -135,6 +135,35 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode
mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
hard-coded timer ID? the rest of the code is using timer_id
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_enable(oh);
+ __omap_dm_timer_load_start(clkev,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ __omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
+}
I don't like the sprintf/hwmod_lookup for every suspend/resume. Just add a
new file-global static 'struct omap_hwmod clockevt_oh' along side
clockevent_gpt,
and assign it at init time in dmtimer_init_one. Then you don't have to
do this
sprintf/lookup on every suspend/resume.
Kevin
static struct clock_event_device clockevent_gpt = {
.name = gp_timer,
.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
@@ -142,6 +171,8 @@ static struct clock_event_device clockevent_gpt = {
.rating = 300,
.set_next_event = omap2_gp_timer_set_next_event,
.set_mode = omap2_gp_timer_set_mode,
+ .suspend= omap_clkevt_suspend,
+ .resume = omap_clkevt_resume,
};
static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
--
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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
On Sat, Nov 03, 2012 at 17:45:03, Kevin Hilman wrote:
On 11/02/2012 01:32 PM, Vaibhav Bedia wrote:
From: Vaibhav Hiremath hvaib...@ti.com
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
The changelog describes what, but doesn't answer why?
Sorry I'll try to take of this in the future.
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
You say it behaves properly without describing what improper
behavior is happening.
There are two issues. One is that the clockevent timer doesn't
get idled which blocks PER domain transition. The next one is that
the clockevent doesn't generate any further interrupts once the
system resumes. We need to restore the pre-suspend configuration.
I haven't tried but I guess we could have used the save and restore
of timer registers here.
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
---
arch/arm/mach-omap2/timer.c | 31 +++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 6584ee0..e8781fd 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -135,6 +135,35 @@ static void omap2_gp_timer_set_mode(enum
clock_event_mode mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
hard-coded timer ID? the rest of the code is using timer_id
Will fix.
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_enable(oh);
+ __omap_dm_timer_load_start(clkev,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ __omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
+}
I don't like the sprintf/hwmod_lookup for every suspend/resume. Just add a
new file-global static 'struct omap_hwmod clockevt_oh' along side
clockevent_gpt,
and assign it at init time in dmtimer_init_one. Then you don't have to
do this
sprintf/lookup on every suspend/resume.
Ok. Will make the changes accordingly.
Regards,
Vaibhav
--
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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
On 11/03/2012 01:17 PM, Bedia, Vaibhav wrote: On Sat, Nov 03, 2012 at 17:45:03, Kevin Hilman wrote: On 11/02/2012 01:32 PM, Vaibhav Bedia wrote: From: Vaibhav Hiremath hvaib...@ti.com The current OMAP timer code registers two timers - one as clocksource and one as clockevent. AM33XX has only one usable timer in the WKUP domain so one of the timers needs suspend-resume support to restore the configuration to pre-suspend state. The changelog describes what, but doesn't answer why? Sorry I'll try to take of this in the future. Thanks. Here's a general rule. Assume you (or I) will be reading this a year from now and will have forgotten the details. The changelog then serves as our long-term memory. :) commit adc78e6 (timekeeping: Add suspend and resume of clock event devices) introduced .suspend and .resume callbacks for clock event devices. Leverages these callbacks to have AM33XX clockevent timer which is in not in WKUP domain to behave properly across system suspend. You say it behaves properly without describing what improper behavior is happening. There are two issues. One is that the clockevent timer doesn't get idled which blocks PER domain transition. The next one is that the clockevent doesn't generate any further interrupts once the system resumes. Please include both in the next rev of the changelog. We need to restore the pre-suspend configuration. I haven't tried but I guess we could have used the save and restore c timer registers here. Yes, please try with that. Won't that be necessary anyways for situations where the powerdomain goes off? Kevin -- 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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
On Sat, Nov 03, 2012 at 19:11:54, Kevin Hilman wrote: [...] Yes, please try with that. Won't that be necessary anyways for situations where the powerdomain goes off? Yes, we probably got lucky with the minimal resume routine. Regards, Vaibhav -- 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 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
On Friday 02 November 2012 06:02 PM, Vaibhav Bedia wrote:
From: Vaibhav Hiremath hvaib...@ti.com
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
Actually OMAP also uses only one timer. The clocksource
is taken care by 32K syntimer till OMAP4 and by realtime
counter on OMAP5. There is a clocksource registration of
timer is available but that is not being used in systems.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
So you use WKUP domain timer for clocksource and PER
domain one for clock-event ?
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
---
arch/arm/mach-omap2/timer.c | 31 +++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 6584ee0..e8781fd 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -135,6 +135,35 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode
mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
You can move all the look up stuff in init code and then
suspend resume hooks will be cleaner.
+
+ omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_enable(oh);
+ __omap_dm_timer_load_start(clkev,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ __omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
+}
+
OK. So since your clk_event stops when PER idles, how do you plan
to support the SOC idle. For CPUIDLE path, you need your clock-event
to wakeup the system based on next timer expiry. So you need your
clock event to be active. Indirectly, you can't let PER idle which
leads npo CORE idle-SOC idle.
How do you plan to address this ? Os is SOC idle is not suppose
to be added for AMXXX ?
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
[PATCH 12/15] ARM: OMAP: timer: Add suspend-resume callbacks for clockevent device
From: Vaibhav Hiremath hvaib...@ti.com
The current OMAP timer code registers two timers -
one as clocksource and one as clockevent.
AM33XX has only one usable timer in the WKUP domain
so one of the timers needs suspend-resume support
to restore the configuration to pre-suspend state.
commit adc78e6 (timekeeping: Add suspend and resume
of clock event devices) introduced .suspend and .resume
callbacks for clock event devices. Leverages these
callbacks to have AM33XX clockevent timer which is
in not in WKUP domain to behave properly across system
suspend.
Signed-off-by: Vaibhav Hiremath hvaib...@ti.com
Signed-off-by: Vaibhav Bedia vaibhav.be...@ti.com
---
arch/arm/mach-omap2/timer.c | 31 +++
1 files changed, 31 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 6584ee0..e8781fd 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -135,6 +135,35 @@ static void omap2_gp_timer_set_mode(enum clock_event_mode
mode,
}
}
+static void omap_clkevt_suspend(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_idle(oh);
+}
+
+static void omap_clkevt_resume(struct clock_event_device *unused)
+{
+ char name[10];
+ struct omap_hwmod *oh;
+
+ sprintf(name, timer%d, 2);
+ oh = omap_hwmod_lookup(name);
+ if (!oh)
+ return;
+
+ omap_hwmod_enable(oh);
+ __omap_dm_timer_load_start(clkev,
+ OMAP_TIMER_CTRL_ST | OMAP_TIMER_CTRL_AR, 0, 1);
+ __omap_dm_timer_int_enable(clkev, OMAP_TIMER_INT_OVERFLOW);
+}
+
static struct clock_event_device clockevent_gpt = {
.name = gp_timer,
.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
@@ -142,6 +171,8 @@ static struct clock_event_device clockevent_gpt = {
.rating = 300,
.set_next_event = omap2_gp_timer_set_next_event,
.set_mode = omap2_gp_timer_set_mode,
+ .suspend= omap_clkevt_suspend,
+ .resume = omap_clkevt_resume,
};
static int __init omap_dm_timer_init_one(struct omap_dm_timer *timer,
--
1.7.0.4
--
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
