* [PATCH] powerpc/time: When starting the decrementer don't zero the other bits in TCR
@ 2011-08-25 12:19 Laurentiu Tudor
2011-09-01 6:27 ` Kumar Gala
0 siblings, 1 reply; 6+ messages in thread
From: Laurentiu Tudor @ 2011-08-25 12:19 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurentiu Tudor
Clearing the other TCR bits might break code that sets them (e.g. to setup
the watchdog or fixed interval timer) before start_cpu_decrementer() gets
called.
Signed-off-by: Laurentiu Tudor <Laurentiu.Tudor@freescale.com>
---
arch/powerpc/kernel/time.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 03b29a6..e8b5cdc 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -721,7 +721,7 @@ void start_cpu_decrementer(void)
mtspr(SPRN_TSR, TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS);
/* Enable decrementer interrupt */
- mtspr(SPRN_TCR, TCR_DIE);
+ mtspr(SPRN_TCR, mfspr(SPRN_TCR) | TCR_DIE);
#endif /* defined(CONFIG_BOOKE) || defined(CONFIG_40x) */
}
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/time: When starting the decrementer don't zero the other bits in TCR
2011-08-25 12:19 [PATCH] powerpc/time: When starting the decrementer don't zero the other bits in TCR Laurentiu Tudor
@ 2011-09-01 6:27 ` Kumar Gala
2011-09-01 8:31 ` Tudor Laurentiu
0 siblings, 1 reply; 6+ messages in thread
From: Kumar Gala @ 2011-09-01 6:27 UTC (permalink / raw)
To: Laurentiu Tudor; +Cc: linuxppc-dev@ozlabs.org list
On Aug 25, 2011, at 7:19 AM, Laurentiu Tudor wrote:
> Clearing the other TCR bits might break code that sets them (e.g. to =
setup
> the watchdog or fixed interval timer) before start_cpu_decrementer() =
gets
> called.
Can you be more specific about the case you're concerned about. I'm not =
sure we want to leave the FIT enabled if it was previously. Watchdog I =
could possibly see leaving alone.
>=20
> Signed-off-by: Laurentiu Tudor <Laurentiu.Tudor@freescale.com>
> ---
> arch/powerpc/kernel/time.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>=20
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index 03b29a6..e8b5cdc 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -721,7 +721,7 @@ void start_cpu_decrementer(void)
> mtspr(SPRN_TSR, TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS);
>=20
> /* Enable decrementer interrupt */
> - mtspr(SPRN_TCR, TCR_DIE);
> + mtspr(SPRN_TCR, mfspr(SPRN_TCR) | TCR_DIE);
> #endif /* defined(CONFIG_BOOKE) || defined(CONFIG_40x) */
> }
>=20
> --=20
> 1.7.1
>=20
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/time: When starting the decrementer don't zero the other bits in TCR
2011-09-01 6:27 ` Kumar Gala
@ 2011-09-01 8:31 ` Tudor Laurentiu
2011-09-01 13:56 ` Kumar Gala
0 siblings, 1 reply; 6+ messages in thread
From: Tudor Laurentiu @ 2011-09-01 8:31 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev@ozlabs.org list
On 9/1/2011 9:27 AM, Kumar Gala wrote:
>
> On Aug 25, 2011, at 7:19 AM, Laurentiu Tudor wrote:
>
>> Clearing the other TCR bits might break code that sets them (e.g. to setup
>> the watchdog or fixed interval timer) before start_cpu_decrementer() gets
>> called.
>
> Can you be more specific about the case you're concerned about. I'm not sure we want to leave the FIT enabled if it was previously. Watchdog I could possibly see leaving alone.
My particular case was this: I was experimenting with having the
watchdog enabled during the boot process, in case something goes wrong
before the normal watchdog mechanism kicks in. That didn't work without
making this modification.
Then, thinking that having a function start_cpu_decrementer() touching
the other timers and not only the decrementer as its name implies might
not be good. So I submitted this patch to the public mailing list to let
the community decide if it's useful.
>> Signed-off-by: Laurentiu Tudor<Laurentiu.Tudor@freescale.com>
>> ---
>> arch/powerpc/kernel/time.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
>> index 03b29a6..e8b5cdc 100644
>> --- a/arch/powerpc/kernel/time.c
>> +++ b/arch/powerpc/kernel/time.c
>> @@ -721,7 +721,7 @@ void start_cpu_decrementer(void)
>> mtspr(SPRN_TSR, TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS);
>>
>> /* Enable decrementer interrupt */
>> - mtspr(SPRN_TCR, TCR_DIE);
>> + mtspr(SPRN_TCR, mfspr(SPRN_TCR) | TCR_DIE);
>> #endif /* defined(CONFIG_BOOKE) || defined(CONFIG_40x) */
>> }
>>
>> --
>> 1.7.1
>>
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/time: When starting the decrementer don't zero the other bits in TCR
2011-09-01 8:31 ` Tudor Laurentiu
@ 2011-09-01 13:56 ` Kumar Gala
2011-09-01 15:38 ` Tudor Laurentiu
0 siblings, 1 reply; 6+ messages in thread
From: Kumar Gala @ 2011-09-01 13:56 UTC (permalink / raw)
To: Tudor Laurentiu; +Cc: linuxppc-dev@ozlabs.org list
On Sep 1, 2011, at 3:31 AM, Tudor Laurentiu wrote:
> On 9/1/2011 9:27 AM, Kumar Gala wrote:
>>=20
>> On Aug 25, 2011, at 7:19 AM, Laurentiu Tudor wrote:
>>=20
>>> Clearing the other TCR bits might break code that sets them (e.g. to =
setup
>>> the watchdog or fixed interval timer) before start_cpu_decrementer() =
gets
>>> called.
>>=20
>> Can you be more specific about the case you're concerned about. I'm =
not sure we want to leave the FIT enabled if it was previously. =
Watchdog I could possibly see leaving alone.
>=20
> My particular case was this: I was experimenting with having the =
watchdog enabled during the boot process, in case something goes wrong =
before the normal watchdog mechanism kicks in. That didn't work without =
making this modification.
> Then, thinking that having a function start_cpu_decrementer() touching =
the other timers and not only the decrementer as its name implies might =
not be good. So I submitted this patch to the public mailing list to let =
the community decide if it's useful.
So I think renaming the function is fine, and I think not touching the =
watchdog bits is also reasonable. So if you update the patch along =
those lines I think we can push in. I do want to make sure that the FIT =
is disabled as we dont expect it.
Probably add a comment about how care needs to be taken if the wathchdog =
is enable be firmware or a previous kernel.
>=20
>>> Signed-off-by: Laurentiu Tudor<Laurentiu.Tudor@freescale.com>
>>> ---
>>> arch/powerpc/kernel/time.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>=20
>>> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
>>> index 03b29a6..e8b5cdc 100644
>>> --- a/arch/powerpc/kernel/time.c
>>> +++ b/arch/powerpc/kernel/time.c
>>> @@ -721,7 +721,7 @@ void start_cpu_decrementer(void)
>>> mtspr(SPRN_TSR, TSR_ENW | TSR_WIS | TSR_DIS | TSR_FIS);
>>>=20
>>> /* Enable decrementer interrupt */
>>> - mtspr(SPRN_TCR, TCR_DIE);
>>> + mtspr(SPRN_TCR, mfspr(SPRN_TCR) | TCR_DIE);
>>> #endif /* defined(CONFIG_BOOKE) || defined(CONFIG_40x) */
>>> }
>>>=20
>>> --
>>> 1.7.1
>>>=20
>>>=20
>>> _______________________________________________
>>> Linuxppc-dev mailing list
>>> Linuxppc-dev@lists.ozlabs.org
>>> https://lists.ozlabs.org/listinfo/linuxppc-dev
>>=20
>>=20
>=20
>=20
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/time: When starting the decrementer don't zero the other bits in TCR
2011-09-01 13:56 ` Kumar Gala
@ 2011-09-01 15:38 ` Tudor Laurentiu
2011-09-01 18:49 ` Kumar Gala
0 siblings, 1 reply; 6+ messages in thread
From: Tudor Laurentiu @ 2011-09-01 15:38 UTC (permalink / raw)
To: linuxppc-dev
On 9/1/2011 4:56 PM, Kumar Gala wrote:
>
> On Sep 1, 2011, at 3:31 AM, Tudor Laurentiu wrote:
>
>> On 9/1/2011 9:27 AM, Kumar Gala wrote:
>>>
>>> On Aug 25, 2011, at 7:19 AM, Laurentiu Tudor wrote:
>>>
>>>> Clearing the other TCR bits might break code that sets them (e.g. to setup
>>>> the watchdog or fixed interval timer) before start_cpu_decrementer() gets
>>>> called.
>>>
>>> Can you be more specific about the case you're concerned about. I'm not sure we want to leave the FIT enabled if it was previously. Watchdog I could possibly see leaving alone.
>>
>> My particular case was this: I was experimenting with having the watchdog enabled during the boot process, in case something goes wrong before the normal watchdog mechanism kicks in. That didn't work without making this modification.
>> Then, thinking that having a function start_cpu_decrementer() touching the other timers and not only the decrementer as its name implies might not be good. So I submitted this patch to the public mailing list to let the community decide if it's useful.
>
> So I think renaming the function is fine, and I think not touching the watchdog bits is also reasonable. So if you update the patch along those lines I think we can push in. I do want to make sure that the FIT is disabled as we dont expect it.
how about:
static void setup_cpu_timers()
?
> Probably add a comment about how care needs to be taken if the wathchdog is enable be firmware or a previous kernel.
OK.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powerpc/time: When starting the decrementer don't zero the other bits in TCR
2011-09-01 15:38 ` Tudor Laurentiu
@ 2011-09-01 18:49 ` Kumar Gala
0 siblings, 0 replies; 6+ messages in thread
From: Kumar Gala @ 2011-09-01 18:49 UTC (permalink / raw)
To: Tudor Laurentiu; +Cc: linuxppc-dev
On Sep 1, 2011, at 10:38 AM, Tudor Laurentiu wrote:
> On 9/1/2011 4:56 PM, Kumar Gala wrote:
>>=20
>> On Sep 1, 2011, at 3:31 AM, Tudor Laurentiu wrote:
>>=20
>>> On 9/1/2011 9:27 AM, Kumar Gala wrote:
>>>>=20
>>>> On Aug 25, 2011, at 7:19 AM, Laurentiu Tudor wrote:
>>>>=20
>>>>> Clearing the other TCR bits might break code that sets them (e.g. =
to setup
>>>>> the watchdog or fixed interval timer) before =
start_cpu_decrementer() gets
>>>>> called.
>>>>=20
>>>> Can you be more specific about the case you're concerned about. =
I'm not sure we want to leave the FIT enabled if it was previously. =
Watchdog I could possibly see leaving alone.
>>>=20
>>> My particular case was this: I was experimenting with having the =
watchdog enabled during the boot process, in case something goes wrong =
before the normal watchdog mechanism kicks in. That didn't work without =
making this modification.
>>> Then, thinking that having a function start_cpu_decrementer() =
touching the other timers and not only the decrementer as its name =
implies might not be good. So I submitted this patch to the public =
mailing list to let the community decide if it's useful.
>>=20
>> So I think renaming the function is fine, and I think not touching =
the watchdog bits is also reasonable. So if you update the patch along =
those lines I think we can push in. I do want to make sure that the FIT =
is disabled as we dont expect it.
>=20
> how about:
>=20
> static void setup_cpu_timers()
>=20
> ?
Sounds fine to me
>=20
>> Probably add a comment about how care needs to be taken if the =
wathchdog is enable be firmware or a previous kernel.
>=20
> OK.
>=20
>=20
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-09-01 18:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-25 12:19 [PATCH] powerpc/time: When starting the decrementer don't zero the other bits in TCR Laurentiu Tudor
2011-09-01 6:27 ` Kumar Gala
2011-09-01 8:31 ` Tudor Laurentiu
2011-09-01 13:56 ` Kumar Gala
2011-09-01 15:38 ` Tudor Laurentiu
2011-09-01 18:49 ` Kumar Gala
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.