Patchwork [resending] SPARC32: forced setting of mode of SUN4M per-cpu timers

login
register
mail settings
Submitter Kirill Tkhai
Date Jan. 5, 2012, 2:27 a.m.
Message ID <772801325730475@web77.yandex.ru>
Download mbox | patch
Permalink /patch/134406/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Kirill Tkhai - Jan. 5, 2012, 2:27 a.m.
SPARC32: forced setting of mode of SUN4M per-cpu timers

SUN4M per-cpu timers have two modes of work. These are timer mode and counter mode.
SPARC32 doesn't write anything to the register, which is connected with mode choice.
So, the mode is chosen by bootloader. This patch forces to use timer mode from the
kernel and to be independent of bootloader.

I had this problem with OpenBIOS. Timers didn't use to tick and kernel on QEMU used
to fail, when it's compiled with SMP support. The patch fixes problem.

Signed-off-by: Tkhai Kirill <tkhai@yandex.ru>
---

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg - Jan. 5, 2012, 6:13 a.m.
Hi Kirill.

Glad to see that you are looking into timer stuff on sparc32!

On Thu, Jan 05, 2012 at 06:27:54AM +0400, Kirill Tkhai wrote:
> SPARC32: forced setting of mode of SUN4M per-cpu timers
The subject only belongsi n the subject: line of the mail - do
not repeat it in the mail body.

Patches that touch both sparc32 and sparc64 shall be prefixed:
sparc: bla bla

Patches that touches only sparc32 shall be prefixed:
sparc32: bla bla

So if you use lower-case sparc32 I am happy.


> SUN4M per-cpu timers have two modes of work. These are timer mode and counter mode.
> SPARC32 doesn't write anything to the register, which is connected with mode choice.
> So, the mode is chosen by bootloader. This patch forces to use timer mode from the
> kernel and to be independent of bootloader.
> 
> I had this problem with OpenBIOS. Timers didn't use to tick and kernel on QEMU used
> to fail, when it's compiled with SMP support. The patch fixes problem.

Good description!

> 
> Signed-off-by: Tkhai Kirill <tkhai@yandex.ru>
> ---
> 
> diff --git a/arch/sparc/kernel/sun4m_irq.c b/arch/sparc/kernel/sun4m_irq.c
> index 422c16d..aa0b9df 100644
> --- a/arch/sparc/kernel/sun4m_irq.c
> +++ b/arch/sparc/kernel/sun4m_irq.c
> @@ -414,6 +414,10 @@ static void __init sun4m_init_timers(irq_handler_t counter_fn)
>  
>  	for (i = 0; i < num_cpu_timers; i++)
>  		sbus_writel(0, &timers_percpu[i]->l14_limit);
> +#ifdef CONFIG_SMP
> +	/* Timer-mode for every per-cpu timer (bit '0' is timer mode) */
> +	sbus_writel(0x00000000, &timers_global->timer_config);
> +#endif

Does this have to be SMP only - I think not.

It would also be nicer to move this up a few lines so it is
right before:

	sbus_writel((((1000000/HZ) + 1) << 10), &timers_global->l10_limit);

Then we have the config in one place.

I looked in the sun4m manual which says:

"
The value of the User Timer and Count and Limit registers is unspecified after the
corresponding configuration bit has been changed.
It is required for software to initialize the counter after a mode
change by writing to it in order to set the register value and to
clear the limit bit. When the counter is programmed to be
a User Timer the Counter/Timer function is disabled,
and vice–versa; these functions share one counter, so no state is
preserved across mode changes.
"

So we should actually set the config bit before we set the limit.

Your comment says:
> +	/* Timer-mode for every per-cpu timer (bit '0' is timer mode) */

But the sun4m manual specify that the first four bits correspond to each
of the per-cpu timers. So we actually need to set all four lower bits to 0.
Which you do - but the comment is wrong.

	Sam

--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg - Jan. 5, 2012, 1:36 p.m.
> >>  +#ifdef CONFIG_SMP
> >>  + /* Timer-mode for every per-cpu timer (bit '0' is timer mode) */
> >>  + sbus_writel(0x00000000, &timers_global->timer_config);
> >>  +#endif
> >
> > Does this have to be SMP only - I think not.
> 
> There is no difference of which mode is used in case of UP. If it is timer mode, it doesn't tick because limit is 0. If it is counter mode, it doesn't tick too. And kernel doesn't touch per-cpu timer registers in other way, it has a deal with interrupts only.
> 
> Ok?

In that case just always set the mode.
We prefer minimal ifdefs in the code.

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Jan. 5, 2012, 7:28 p.m.
From: Kirill Tkhai <tkhai@yandex.ru>
Date: Thu, 05 Jan 2012 17:52:09 +0400

>> In that case just always set the mode.
>> We prefer minimal ifdefs in the code.
>>
>>         Sam
> 
> Ok, #ifdef was removed
> 
> ---

Please post patches you intend me to apply as fresh list postings,
rather than replies to other discussions.

Because of the way you posted this, it didn't get archived by our
patch management system at:

	http://patchwork.ozlabs.org/project/sparclinux/list/

In particular having two lines of "---" threw it off and made it
ignore your posting thinking it was not a patch at all.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sam Ravnborg - Jan. 5, 2012, 7:32 p.m.
On Thu, Jan 05, 2012 at 05:52:09PM +0400, Kirill Tkhai wrote:
> > In that case just always set the mode.
> > We prefer minimal ifdefs in the code.
> >
> >         Sam
> 
> Ok, #ifdef was removed

Good, you can add my:
Acked-by: Sam Ravnborg <sam@ravnborg.org>


	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Julian Calaby - Jan. 5, 2012, 9:35 p.m.
Kirill,

On Fri, Jan 6, 2012 at 06:28, David Miller <davem@davemloft.net> wrote:
> From: Kirill Tkhai <tkhai@yandex.ru>
> Date: Thu, 05 Jan 2012 17:52:09 +0400
>
>>> In that case just always set the mode.
>>> We prefer minimal ifdefs in the code.
>>>
>>>         Sam
>>
>> Ok, #ifdef was removed
>>
>> ---
>
> Please post patches you intend me to apply as fresh list postings,
> rather than replies to other discussions.
>
> Because of the way you posted this, it didn't get archived by our
> patch management system at:
>
>        http://patchwork.ozlabs.org/project/sparclinux/list/
>
> In particular having two lines of "---" threw it off and made it
> ignore your posting thinking it was not a patch at all.

Also, did you send it to the list? I seem to have missed it here.

Thanks,
Kirill Tkhai - Jan. 5, 2012, 11:05 p.m.
06.01.2012, 01:35, "Julian Calaby" <julian.calaby@gmail.com>:
> Kirill,
>
> Also, did you send it to the list? I seem to have missed it here.
>

Yes, I did.

Kirill
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Julian Calaby - Jan. 5, 2012, 11:13 p.m.
On Fri, Jan 6, 2012 at 10:05, Kirill Tkhai <tkhai@yandex.ru> wrote:
> 06.01.2012, 01:35, "Julian Calaby" <julian.calaby@gmail.com>:
>> Kirill,
>>
>> Also, did you send it to the list? I seem to have missed it here.
>>
>
> Yes, I did.

Odd, clearly gmail ate it or something.

Thanks!

Patch

diff --git a/arch/sparc/kernel/sun4m_irq.c b/arch/sparc/kernel/sun4m_irq.c
index 422c16d..aa0b9df 100644
--- a/arch/sparc/kernel/sun4m_irq.c
+++ b/arch/sparc/kernel/sun4m_irq.c
@@ -414,6 +414,10 @@  static void __init sun4m_init_timers(irq_handler_t counter_fn)
 
 	for (i = 0; i < num_cpu_timers; i++)
 		sbus_writel(0, &timers_percpu[i]->l14_limit);
+#ifdef CONFIG_SMP
+	/* Timer-mode for every per-cpu timer (bit '0' is timer mode) */
+	sbus_writel(0x00000000, &timers_global->timer_config);
+#endif
 	if (num_cpu_timers == 4)
 		sbus_writel(SUN4M_INT_E14, &sun4m_irq_global->mask_set);