diff mbox

[11/15] ARM: OMAP: timer: Interchange clksrc and clkevt for AM33XX

Message ID 1351859566-24818-12-git-send-email-vaibhav.bedia@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vaibhav Bedia Nov. 2, 2012, 12:32 p.m. UTC
AM33XX has only one usable timer in the WKUP domain.
Currently the timer instance in WKUP domain is used
as the clockevent and the timer in non-WKUP domain
as the clocksource. The timer in WKUP domain can keep
running in suspend from a 32K clock and hence serve
as the persistent clock. To enable this, interchange
the timers used as clocksource and clockevent for
AM33XX. A subsequent patch will add suspend-resume
support for the clockevent to ensure that there are
no issues with timekeeping.

Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
---
 arch/arm/mach-omap2/timer.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Kevin Hilman Nov. 3, 2012, 11:43 a.m. UTC | #1
On 11/02/2012 01:32 PM, Vaibhav Bedia wrote:
> AM33XX has only one usable timer in the WKUP domain.
> Currently the timer instance in WKUP domain is used
> as the clockevent and the timer in non-WKUP domain
> as the clocksource. The timer in WKUP domain can keep
> running in suspend from a 32K clock and hence serve
> as the persistent clock. To enable this, interchange
> the timers used as clocksource and clockevent for
> AM33XX.

Doesn't this also mean that you won't get timer wakeups
in idle?  Or are you keeping the domain where the clockevent is
on during idle?

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 3, 2012, 12:47 p.m. UTC | #2
Hi Kevin,

On Sat, Nov 03, 2012 at 17:13:54, Kevin Hilman wrote:
> On 11/02/2012 01:32 PM, Vaibhav Bedia wrote:
> > AM33XX has only one usable timer in the WKUP domain.
> > Currently the timer instance in WKUP domain is used
> > as the clockevent and the timer in non-WKUP domain
> > as the clocksource. The timer in WKUP domain can keep
> > running in suspend from a 32K clock and hence serve
> > as the persistent clock. To enable this, interchange
> > the timers used as clocksource and clockevent for
> > AM33XX.
> 
> Doesn't this also mean that you won't get timer wakeups
> in idle?  Or are you keeping the domain where the clockevent is
> on during idle?
> 

The lowest idle state that we are targeting will have MPU powered
off with external memory in self-refresh mode. Peripheral domain
with the clockevent will be kept on.

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman Nov. 3, 2012, 1:04 p.m. UTC | #3
T
On 11/03/2012 12:47 PM, Bedia, Vaibhav wrote:
> Hi Kevin,
>
> On Sat, Nov 03, 2012 at 17:13:54, Kevin Hilman wrote:
>> On 11/02/2012 01:32 PM, Vaibhav Bedia wrote:
>>> AM33XX has only one usable timer in the WKUP domain.
>>> Currently the timer instance in WKUP domain is used
>>> as the clockevent and the timer in non-WKUP domain
>>> as the clocksource. The timer in WKUP domain can keep
>>> running in suspend from a 32K clock and hence serve
>>> as the persistent clock. To enable this, interchange
>>> the timers used as clocksource and clockevent for
>>> AM33XX.
>>
>> Doesn't this also mean that you won't get timer wakeups
>> in idle?  Or are you keeping the domain where the clockevent is
>> on during idle?
>>
>
> The lowest idle state that we are targeting will have MPU powered
> off with external memory in self-refresh mode. Peripheral domain
> with the clockevent will be kept on.

Is this a limitation of the hardware?  or the software?

Kevin

P.S.  I haven't yet made my way through the TRM, so I'll probably figure
this out when I read it, but having this summarized in the changelog would
be helpful since even after I read the TRM, I'll forget.  I'm saving the 
TRM
reading for entertainment on upcoming flight home from Linaro Connect.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 3, 2012, 1:48 p.m. UTC | #4
On Sat, Nov 03, 2012 at 18:34:30, Kevin Hilman wrote:
[...]
> >>
> >> Doesn't this also mean that you won't get timer wakeups
> >> in idle?  Or are you keeping the domain where the clockevent is
> >> on during idle?
> >>
> >
> > The lowest idle state that we are targeting will have MPU powered
> > off with external memory in self-refresh mode. Peripheral domain
> > with the clockevent will be kept on.
> 
> Is this a limitation of the hardware?  or the software?
> 

Well, making the lowest idle state same as the suspend state will require
us to involve WKUP_M3 in the idle path and wakeup sources get limited to
the IPs in the WKUP domain alone. There's no IO daisy chaining in AM33XX
so that's one big difference compared to OMAP. 

The other potential problem is that the IPC mechanism that we have
uses interrupts. Assuming that the lowest idle state, say Cx, is the same
as the suspend state, we'll need to communicate with the WKUP_M3 using
interrupts once we decide to enter Cx. I am not sure if we can do something
in the cpuidle implementation to work around the "interrupt for idle"
problem. We could probably not wait for an ACK when we want to enter Cx,
but the problem of limited wakeup sources remains. If we let the various
drivers block the entry to Cx, since almost all the IPs are in the
peripheral domain a system which uses anything other than UART and Timer
in WKUP domain will probably never be able enter Cx.

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman Nov. 3, 2012, 4:22 p.m. UTC | #5
On 11/02/2012 12:32 PM, Vaibhav Bedia wrote:
> AM33XX has only one usable timer in the WKUP domain.

After reading the TRM, it seems there are two: DMTIMER0 and DMTIMER1.

Looking at the hwmod data though, I don't see a hwmod for DMTIMER0.  Can
you explain a little about why DMTIMER0 is missing/broken?

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Santosh Shilimkar Nov. 3, 2012, 4:31 p.m. UTC | #6
On Friday 02 November 2012 06:02 PM, Vaibhav Bedia wrote:
> AM33XX has only one usable timer in the WKUP domain.
> Currently the timer instance in WKUP domain is used
> as the clockevent and the timer in non-WKUP domain
> as the clocksource. The timer in WKUP domain can keep
> running in suspend from a 32K clock and hence serve
> as the persistent clock. To enable this, interchange
> the timers used as clocksource and clockevent for
> AM33XX. A subsequent patch will add suspend-resume
> support for the clockevent to ensure that there are
> no issues with timekeeping.
>
> Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> ---
>   arch/arm/mach-omap2/timer.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 565e575..6584ee0 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -460,7 +460,7 @@ OMAP_SYS_TIMER(3_secure)
>   #endif
>
>   #ifdef CONFIG_SOC_AM33XX
> -OMAP_SYS_TIMER_INIT(3_am33xx, 1, OMAP4_MPU_SOURCE, 2, OMAP4_MPU_SOURCE)
> +OMAP_SYS_TIMER_INIT(3_am33xx, 2, OMAP4_MPU_SOURCE, 1, OMAP4_MPU_SOURCE)
>   OMAP_SYS_TIMER(3_am33xx)
>   #endif
>
As mentioned on other patch comment, I think this might break your
SOC idle.

Regards
Santosh

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 4, 2012, 3:26 p.m. UTC | #7
On Sat, Nov 03, 2012 at 22:01:46, Shilimkar, Santosh wrote:
> >
> As mentioned on other patch comment, I think this might break your
> SOC idle.
> 

Unfortunately we don't have SOC idle.

Regards,
Vaibhav 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 5, 2012, 4:40 a.m. UTC | #8
Hi Kevin,

On Sat, Nov 03, 2012 at 21:52:00, Kevin Hilman wrote:
> On 11/02/2012 12:32 PM, Vaibhav Bedia wrote:
> > AM33XX has only one usable timer in the WKUP domain.
> 
> After reading the TRM, it seems there are two: DMTIMER0 and DMTIMER1.
> 
> Looking at the hwmod data though, I don't see a hwmod for DMTIMER0.  Can
> you explain a little about why DMTIMER0 is missing/broken?
> 

DMTimer0 is usable only in secure devices only. This timer by default runs
from an inaccurate RC oscillator the frequency of which can be anywhere
from 16-60KHz based on process variations. There is a mux to change the
clock source but the register for the mux can't be modified in GP devices.
(will add this in the changelog)

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kevin Hilman Nov. 5, 2012, 6:03 p.m. UTC | #9
"Bedia, Vaibhav" <vaibhav.bedia@ti.com> writes:

> On Sat, Nov 03, 2012 at 18:34:30, Kevin Hilman wrote:
> [...]
>> >>
>> >> Doesn't this also mean that you won't get timer wakeups
>> >> in idle?  Or are you keeping the domain where the clockevent is
>> >> on during idle?
>> >>
>> >
>> > The lowest idle state that we are targeting will have MPU powered
>> > off with external memory in self-refresh mode. Peripheral domain
>> > with the clockevent will be kept on.
>> 
>> Is this a limitation of the hardware?  or the software?
>> 
>
> Well, making the lowest idle state same as the suspend state will
> require us to involve WKUP_M3 in the idle path and wakeup sources get
> limited to the IPs in the WKUP domain alone. There's no IO daisy
> chaining in AM33XX so that's one big difference compared to OMAP.  The
> other potential problem is that the IPC mechanism that we have uses
> interrupts.

It can still interrupt the M3, it's only the interrupt back to the MPU
that is the issue, right?  That being said, there's no reason it
couldn't use polling in the idle path, right?  

> Assuming that the lowest idle state, say Cx, is the same as the
> suspend state, we'll need to communicate with the WKUP_M3 using
> interrupts once we decide to enter Cx. I am not sure if we can do
> something in the cpuidle implementation to work around the "interrupt
> for idle" problem. 
>
> We could probably not wait for an ACK when we want to enter Cx, 

why not?

Are the response times from the M3 really up to 500ms (guessing based on
the timeout you used in the suspend path.)  That seems rather unlikely.

Hmm, but as I think about it.  Why does the MPU need to wait for an ACK
at all?  Why not just send the cmd and WFI?

> but the problem of limited wakeup sources remains. If we let the
> various drivers block the entry to Cx, since almost all the IPs are in
> the peripheral domain a system which uses anything other than UART and
> Timer in WKUP domain will probably never be able enter Cx.

Even so, I think the system needs to be designed to hit the same power
states in idle and suspend.  Then, the states can be restricted based
wakeup capabilities as you described.  This would be easy to do in the
runtime PM implementation for this device.

IMO, assuming that idle will not be useful from the begining is leading
down the path to poor design choices that will be much more difficult to
fixup down the road in order to add idle support later.  We need to
design both idle and suspend at the same time.

Also, don't forget about GPIO0.  Systems could easily be built such that
peripherals which want to wakeup but don't have native wakeup
capabilities could use a GPIO in bank 0 to wake the system.

Similarily, I2C0 is in WKUP, and brought out to capes, so some simple
designs with with I2C devices on a cape might be perfectly capable of
hitting deep power states in idle.

Kevin








--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Santosh Shilimkar Nov. 5, 2012, 9:59 p.m. UTC | #10
On Monday 05 November 2012 11:33 PM, Kevin Hilman wrote:
> "Bedia, Vaibhav" <vaibhav.bedia@ti.com> writes:
>
>> On Sat, Nov 03, 2012 at 18:34:30, Kevin Hilman wrote:
>> [...]
>>>>>
>>>>> Doesn't this also mean that you won't get timer wakeups
>>>>> in idle?  Or are you keeping the domain where the clockevent is
>>>>> on during idle?
>>>>>
>>>>
>>>> The lowest idle state that we are targeting will have MPU powered
>>>> off with external memory in self-refresh mode. Peripheral domain
>>>> with the clockevent will be kept on.
>>>
>>> Is this a limitation of the hardware?  or the software?
>>>
>>
>> Well, making the lowest idle state same as the suspend state will
>> require us to involve WKUP_M3 in the idle path and wakeup sources get
>> limited to the IPs in the WKUP domain alone. There's no IO daisy
>> chaining in AM33XX so that's one big difference compared to OMAP.  The
>> other potential problem is that the IPC mechanism that we have uses
>> interrupts.
>
> It can still interrupt the M3, it's only the interrupt back to the MPU
> that is the issue, right?  That being said, there's no reason it
> couldn't use polling in the idle path, right?
>
>> Assuming that the lowest idle state, say Cx, is the same as the
>> suspend state, we'll need to communicate with the WKUP_M3 using
>> interrupts once we decide to enter Cx. I am not sure if we can do
>> something in the cpuidle implementation to work around the "interrupt
>> for idle" problem.
>>
>> We could probably not wait for an ACK when we want to enter Cx,
>
> why not?
>
> Are the response times from the M3 really up to 500ms (guessing based on
> the timeout you used in the suspend path.)  That seems rather unlikely.
>
> Hmm, but as I think about it.  Why does the MPU need to wait for an ACK
> at all?  Why not just send the cmd and WFI?
>
>> but the problem of limited wakeup sources remains. If we let the
>> various drivers block the entry to Cx, since almost all the IPs are in
>> the peripheral domain a system which uses anything other than UART and
>> Timer in WKUP domain will probably never be able enter Cx.
>
> Even so, I think the system needs to be designed to hit the same power
> states in idle and suspend.  Then, the states can be restricted based
> wakeup capabilities as you described.  This would be easy to do in the
> runtime PM implementation for this device.
>
> IMO, assuming that idle will not be useful from the begining is leading
> down the path to poor design choices that will be much more difficult to
> fixup down the road in order to add idle support later.  We need to
> design both idle and suspend at the same time.
>
I agree with Kevin. Not supporting CPUIDLE deep states can hit the
active power numbers dearly. I just don't know why the SOCs don't share
the standard and must have design choices. But thats another discussion.

How about leaving the timer choices as is. PER timer for clock source
and wakeuptimer for clock event. Anyway in suspend the clock-source
can be suspended and that is evident from recent discussion. The only
downside is you won't count time in suspend which is any way the case.

Vaibhav,
Do you guys see any implementation bottleneck for above ?

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 6, 2012, 2:33 p.m. UTC | #11
Hi Kevin,

On Mon, Nov 05, 2012 at 23:33:07, Kevin Hilman wrote:
> "Bedia, Vaibhav" <vaibhav.bedia@ti.com> writes:
> 
> > On Sat, Nov 03, 2012 at 18:34:30, Kevin Hilman wrote:
> > [...]
> >> >>
> >> >> Doesn't this also mean that you won't get timer wakeups
> >> >> in idle?  Or are you keeping the domain where the clockevent is
> >> >> on during idle?
> >> >>
> >> >
> >> > The lowest idle state that we are targeting will have MPU powered
> >> > off with external memory in self-refresh mode. Peripheral domain
> >> > with the clockevent will be kept on.
> >> 
> >> Is this a limitation of the hardware?  or the software?
> >> 
> >
> > Well, making the lowest idle state same as the suspend state will
> > require us to involve WKUP_M3 in the idle path and wakeup sources get
> > limited to the IPs in the WKUP domain alone. There's no IO daisy
> > chaining in AM33XX so that's one big difference compared to OMAP.  The
> > other potential problem is that the IPC mechanism that we have uses
> > interrupts.
> 
> It can still interrupt the M3, it's only the interrupt back to the MPU
> that is the issue, right?  That being said, there's no reason it
> couldn't use polling in the idle path, right?  
> 

Yes we could use polling but I think we have a bigger problem in the
chip architecture.

> > Assuming that the lowest idle state, say Cx, is the same as the
> > suspend state, we'll need to communicate with the WKUP_M3 using
> > interrupts once we decide to enter Cx. I am not sure if we can do
> > something in the cpuidle implementation to work around the "interrupt
> > for idle" problem. 
> >
> > We could probably not wait for an ACK when we want to enter Cx, 
> 
> why not?
> 
> Are the response times from the M3 really up to 500ms (guessing based on
> the timeout you used in the suspend path.)  That seems rather unlikely.
> 

No 500ms is too high. Actual delays would be much lower, I need to check
with the design team on the expected number.

> Hmm, but as I think about it.  Why does the MPU need to wait for an ACK
> at all?  Why not just send the cmd and WFI?
> 

I have myself being going back and forth on this. There are lot of things
that we do in software, DDR being one of them. We can't do some of the
DDR related stuff unless memory enter self-refresh AND EMIF gets disabled.
Doing so essentially means that the drivers have entered sort of suspend
state. Given this h/w limitation I don't see how we could handle without
impacting a running system.

> > but the problem of limited wakeup sources remains. If we let the
> > various drivers block the entry to Cx, since almost all the IPs are in
> > the peripheral domain a system which uses anything other than UART and
> > Timer in WKUP domain will probably never be able enter Cx.
> 
> Even so, I think the system needs to be designed to hit the same power
> states in idle and suspend.  Then, the states can be restricted based
> wakeup capabilities as you described.  This would be easy to do in the
> runtime PM implementation for this device.
> 
> IMO, assuming that idle will not be useful from the begining is leading
> down the path to poor design choices that will be much more difficult to
> fixup down the road in order to add idle support later.  We need to
> design both idle and suspend at the same time.
> 

Getting PER to transition on a running system is something I can't figure out.
Maybe MPU OFF is the lowest we can go.

> Also, don't forget about GPIO0.  Systems could easily be built such that
> peripherals which want to wakeup but don't have native wakeup
> capabilities could use a GPIO in bank 0 to wake the system.
> 
> Similarily, I2C0 is in WKUP, and brought out to capes, so some simple
> designs with with I2C devices on a cape might be perfectly capable of
> hitting deep power states in idle.
> 

Ok this is interesting. AFAIK I2C wakeup requires the device to be operating
in slave mode. If so, is this something that's already supported on OMAP?

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 6, 2012, 2:38 p.m. UTC | #12
Hi Santosh,

On Tue, Nov 06, 2012 at 03:29:22, Shilimkar, Santosh wrote:

[...]

> >
> > IMO, assuming that idle will not be useful from the begining is leading
> > down the path to poor design choices that will be much more difficult to
> > fixup down the road in order to add idle support later.  We need to
> > design both idle and suspend at the same time.
> >
> I agree with Kevin. Not supporting CPUIDLE deep states can hit the
> active power numbers dearly. I just don't know why the SOCs don't share
> the standard and must have design choices. But thats another discussion.
> 

Yes, active power numbers are not comparable to OMAP :(

> How about leaving the timer choices as is. PER timer for clock source
> and wakeuptimer for clock event. Anyway in suspend the clock-source
> can be suspended and that is evident from recent discussion. The only
> downside is you won't count time in suspend which is any way the case.
> 
> Vaibhav,
> Do you guys see any implementation bottleneck for above ?
> 

Looking at the timekeeping code I see one more potential reason for making
this change. OMAP registers the 32k sync timer as the persistent clock and
since there's no 32k sync timer in AM33xx it doesn't register a persistent
clock right now. Based on what I understood, we need to have to register
one and DMTimer1 is the only clock that can serve as the persistent clock
in suspend state. When we do so we might as well use it as the clocksource.

A related question that I had was, is there a mechanism to handle the 32k
counter (DMTimer or sync timer) wraparound condition in suspend?

Regards,
Vaibhav

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Vaibhav Bedia Nov. 8, 2012, 1:15 p.m. UTC | #13
Hi Santosh,

On Tue, Nov 06, 2012 at 20:05:40, Bedia, Vaibhav wrote:
> Hi Santosh,
> 
> On Tue, Nov 06, 2012 at 03:29:22, Shilimkar, Santosh wrote:
> 
> [...]
> 
> > >
> > > IMO, assuming that idle will not be useful from the begining is leading
> > > down the path to poor design choices that will be much more difficult to
> > > fixup down the road in order to add idle support later.  We need to
> > > design both idle and suspend at the same time.
> > >
> > I agree with Kevin. Not supporting CPUIDLE deep states can hit the
> > active power numbers dearly. I just don't know why the SOCs don't share
> > the standard and must have design choices. But thats another discussion.
> > 
> 
> Yes, active power numbers are not comparable to OMAP :(
> 
> > How about leaving the timer choices as is. PER timer for clock source
> > and wakeuptimer for clock event. Anyway in suspend the clock-source
> > can be suspended and that is evident from recent discussion. The only
> > downside is you won't count time in suspend which is any way the case.
> > 
> > Vaibhav,
> > Do you guys see any implementation bottleneck for above ?
> > 
> 
> Looking at the timekeeping code I see one more potential reason for making
> this change. OMAP registers the 32k sync timer as the persistent clock and
> since there's no 32k sync timer in AM33xx it doesn't register a persistent
> clock right now. Based on what I understood, we need to have to register
> one and DMTimer1 is the only clock that can serve as the persistent clock
> in suspend state. When we do so we might as well use it as the clocksource.
> 
> A related question that I had was, is there a mechanism to handle the 32k
> counter (DMTimer or sync timer) wraparound condition in suspend?
> 

Does interchanging the clksrc and clkevt look fine to you?

Regards,
Vaibhav
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hunter, Jon Nov. 8, 2012, 8:41 p.m. UTC | #14
On 11/02/2012 07:32 AM, Vaibhav Bedia wrote:
> AM33XX has only one usable timer in the WKUP domain.
> Currently the timer instance in WKUP domain is used
> as the clockevent and the timer in non-WKUP domain
> as the clocksource. The timer in WKUP domain can keep
> running in suspend from a 32K clock and hence serve
> as the persistent clock. To enable this, interchange
> the timers used as clocksource and clockevent for
> AM33XX. A subsequent patch will add suspend-resume
> support for the clockevent to ensure that there are
> no issues with timekeeping.
> 
> Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> ---
>  arch/arm/mach-omap2/timer.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 565e575..6584ee0 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -460,7 +460,7 @@ OMAP_SYS_TIMER(3_secure)
>  #endif
>  
>  #ifdef CONFIG_SOC_AM33XX
> -OMAP_SYS_TIMER_INIT(3_am33xx, 1, OMAP4_MPU_SOURCE, 2, OMAP4_MPU_SOURCE)
> +OMAP_SYS_TIMER_INIT(3_am33xx, 2, OMAP4_MPU_SOURCE, 1, OMAP4_MPU_SOURCE)
>  OMAP_SYS_TIMER(3_am33xx)
>  #endif

By the way, for v3.8 (assuming that timer DT patches go in, currently
queued in Tony's master), when booting with DT the clock-source and
clock-events will be selected by timer feature and not ID. So you may
wish to rebase on top of Tony's master.

Cheers
Jon
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Nov. 12, 2012, 10:54 p.m. UTC | #15
* Jon Hunter <jon-hunter@ti.com> [121108 12:43]:
> 
> On 11/02/2012 07:32 AM, Vaibhav Bedia wrote:
> > AM33XX has only one usable timer in the WKUP domain.
> > Currently the timer instance in WKUP domain is used
> > as the clockevent and the timer in non-WKUP domain
> > as the clocksource. The timer in WKUP domain can keep
> > running in suspend from a 32K clock and hence serve
> > as the persistent clock. To enable this, interchange
> > the timers used as clocksource and clockevent for
> > AM33XX. A subsequent patch will add suspend-resume
> > support for the clockevent to ensure that there are
> > no issues with timekeeping.
> > 
> > Signed-off-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> > ---
> >  arch/arm/mach-omap2/timer.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> > index 565e575..6584ee0 100644
> > --- a/arch/arm/mach-omap2/timer.c
> > +++ b/arch/arm/mach-omap2/timer.c
> > @@ -460,7 +460,7 @@ OMAP_SYS_TIMER(3_secure)
> >  #endif
> >  
> >  #ifdef CONFIG_SOC_AM33XX
> > -OMAP_SYS_TIMER_INIT(3_am33xx, 1, OMAP4_MPU_SOURCE, 2, OMAP4_MPU_SOURCE)
> > +OMAP_SYS_TIMER_INIT(3_am33xx, 2, OMAP4_MPU_SOURCE, 1, OMAP4_MPU_SOURCE)
> >  OMAP_SYS_TIMER(3_am33xx)
> >  #endif
> 
> By the way, for v3.8 (assuming that timer DT patches go in, currently
> queued in Tony's master), when booting with DT the clock-source and
> clock-events will be selected by timer feature and not ID. So you may
> wish to rebase on top of Tony's master.

Please don't base anything going upstream on the master branch
except for testing. The master branch is just a merge of the
upstream heading branches, and cannot be used as a base for
pulling into anything.

In this case the right branch to use as a base is omap-for-v3.8/dt.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 565e575..6584ee0 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -460,7 +460,7 @@  OMAP_SYS_TIMER(3_secure)
 #endif
 
 #ifdef CONFIG_SOC_AM33XX
-OMAP_SYS_TIMER_INIT(3_am33xx, 1, OMAP4_MPU_SOURCE, 2, OMAP4_MPU_SOURCE)
+OMAP_SYS_TIMER_INIT(3_am33xx, 2, OMAP4_MPU_SOURCE, 1, OMAP4_MPU_SOURCE)
 OMAP_SYS_TIMER(3_am33xx)
 #endif