| Message ID | 772801325730475@web77.yandex.ru |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
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
> >> +#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
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
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
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,
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
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!
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);
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