| Message ID | 20200506214107.25956-8-Sergey.Semin@baikalelectronics.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clocksource: Fix MIPS GIC and DW APB Timer for Baikal-T1 SoC support | expand |
On Thu, May 07, 2020 at 12:41:07AM +0300, Serge Semin wrote: > Currently neither clocksource nor scheduler clock kernel framework > support the clocks with variable frequency. Needless to say how many > problems may cause the sudden base clocks frequency change. In a > simplest case the system time will either slow down or speed up. > Since on CM2.5 and earlier MIPS GIC timer is synchronously clocked > with CPU we must set some limitations on using it for these frameworks > if CPU frequency may change. First of all it's not safe to have the > MIPS GIC used for scheduler timings. So we shouldn't proceed with > the clocks registration in the sched-subsystem. Secondly we must > significantly decrease the MIPS GIC clocksource rating. This will let > the system to use it only as a last resort. > > Note CM3.x-based systems may also experience the problems with MIPS GIC > if the CPU-frequency change is activated for the whole CPU cluster > instead of using the individual CPC core clocks divider. May be there is no alternative but the code looks a bit hacksih. Isn't possible to do something with the sched_mark_unstable? Or just not use the timer at all ?
Hello Daniel, Thanks for your comment. My response is below. On Fri, May 15, 2020 at 07:10:04PM +0200, Daniel Lezcano wrote: > On Thu, May 07, 2020 at 12:41:07AM +0300, Serge Semin wrote: > > Currently neither clocksource nor scheduler clock kernel framework > > support the clocks with variable frequency. Needless to say how many > > problems may cause the sudden base clocks frequency change. In a > > simplest case the system time will either slow down or speed up. > > Since on CM2.5 and earlier MIPS GIC timer is synchronously clocked > > with CPU we must set some limitations on using it for these frameworks > > if CPU frequency may change. First of all it's not safe to have the > > MIPS GIC used for scheduler timings. So we shouldn't proceed with > > the clocks registration in the sched-subsystem. Secondly we must > > significantly decrease the MIPS GIC clocksource rating. This will let > > the system to use it only as a last resort. > > > > Note CM3.x-based systems may also experience the problems with MIPS GIC > > if the CPU-frequency change is activated for the whole CPU cluster > > instead of using the individual CPC core clocks divider. > > May be there is no alternative but the code looks a bit hacksih. Isn't possible > to do something with the sched_mark_unstable? > > Or just not use the timer at all ? Not using the timer might be better, but not that good alternative either especially in our case due to very slow external timer. Me and Thomas Bogendoerfer discussed the similar commit I've provided to the csrc-r4k driver available on MIPS: https://lkml.org/lkml/2020/5/11/576 To cut it short, you are right. The solution with using clocksource_mark_unstable() is better alternative spied up in x86 tsc implementation. I'll use a similar approach here and submit the updated patch in v3. Could you please proceed with the rest of the series review? I'd like to send the next version with as many comments taken into account as possible. The patchset has been submitted a while ago, but except Rob noone have had any comments.( -Sergey
On 16/05/2020 14:16, Serge Semin wrote: > Hello Daniel, > > Thanks for your comment. My response is below. > > On Fri, May 15, 2020 at 07:10:04PM +0200, Daniel Lezcano wrote: >> On Thu, May 07, 2020 at 12:41:07AM +0300, Serge Semin wrote: >>> Currently neither clocksource nor scheduler clock kernel framework >>> support the clocks with variable frequency. Needless to say how many >>> problems may cause the sudden base clocks frequency change. In a >>> simplest case the system time will either slow down or speed up. >>> Since on CM2.5 and earlier MIPS GIC timer is synchronously clocked >>> with CPU we must set some limitations on using it for these frameworks >>> if CPU frequency may change. First of all it's not safe to have the >>> MIPS GIC used for scheduler timings. So we shouldn't proceed with >>> the clocks registration in the sched-subsystem. Secondly we must >>> significantly decrease the MIPS GIC clocksource rating. This will let >>> the system to use it only as a last resort. >>> >>> Note CM3.x-based systems may also experience the problems with MIPS GIC >>> if the CPU-frequency change is activated for the whole CPU cluster >>> instead of using the individual CPC core clocks divider. >> >> May be there is no alternative but the code looks a bit hacksih. Isn't possible >> to do something with the sched_mark_unstable? >> >> Or just not use the timer at all ? > > Not using the timer might be better, but not that good alternative either > especially in our case due to very slow external timer. Me and Thomas > Bogendoerfer discussed the similar commit I've provided to the csrc-r4k driver > available on MIPS: > https://lkml.org/lkml/2020/5/11/576 > > To cut it short, you are right. The solution with using clocksource_mark_unstable() > is better alternative spied up in x86 tsc implementation. I'll use a similar > approach here and submit the updated patch in v3. > > Could you please proceed with the rest of the series review? I'd like to send > the next version with as many comments taken into account as possible. The > patchset has been submitted a while ago, but except Rob noone have had any > comments.( For me other patches are ok. I can apply patches 1, 2, 4, 5, 6 Will remain patches 3 et 7
Daniel, On Mon, May 18, 2020 at 03:59:16PM +0200, Daniel Lezcano wrote: > On 16/05/2020 14:16, Serge Semin wrote: > > Hello Daniel, > > > > Thanks for your comment. My response is below. > > > > On Fri, May 15, 2020 at 07:10:04PM +0200, Daniel Lezcano wrote: > >> On Thu, May 07, 2020 at 12:41:07AM +0300, Serge Semin wrote: > >>> Currently neither clocksource nor scheduler clock kernel framework > >>> support the clocks with variable frequency. Needless to say how many > >>> problems may cause the sudden base clocks frequency change. In a > >>> simplest case the system time will either slow down or speed up. > >>> Since on CM2.5 and earlier MIPS GIC timer is synchronously clocked > >>> with CPU we must set some limitations on using it for these frameworks > >>> if CPU frequency may change. First of all it's not safe to have the > >>> MIPS GIC used for scheduler timings. So we shouldn't proceed with > >>> the clocks registration in the sched-subsystem. Secondly we must > >>> significantly decrease the MIPS GIC clocksource rating. This will let > >>> the system to use it only as a last resort. > >>> > >>> Note CM3.x-based systems may also experience the problems with MIPS GIC > >>> if the CPU-frequency change is activated for the whole CPU cluster > >>> instead of using the individual CPC core clocks divider. > >> > >> May be there is no alternative but the code looks a bit hacksih. Isn't possible > >> to do something with the sched_mark_unstable? > >> > >> Or just not use the timer at all ? > > > > Not using the timer might be better, but not that good alternative either > > especially in our case due to very slow external timer. Me and Thomas > > Bogendoerfer discussed the similar commit I've provided to the csrc-r4k driver > > available on MIPS: > > https://lkml.org/lkml/2020/5/11/576 > > > > To cut it short, you are right. The solution with using clocksource_mark_unstable() > > is better alternative spied up in x86 tsc implementation. I'll use a similar > > approach here and submit the updated patch in v3. > > > > Could you please proceed with the rest of the series review? I'd like to send > > the next version with as many comments taken into account as possible. The > > patchset has been submitted a while ago, but except Rob noone have had any > > comments.( > > For me other patches are ok. > > I can apply patches 1, 2, 4, 5, 6 > > Will remain patches 3 et 7 That's be great! Thanks. Is patch 3 supposed to be merged in by Rob or by you? I don't see one being in the Rob's repo. He might be waiting for you acknowledgment or something. I'll send the updated patch 3 shortly today. -Sergey > > > -- > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog
On Mon, May 18, 2020 at 05:40:46PM +0300, Serge Semin wrote: > Daniel, > > On Mon, May 18, 2020 at 03:59:16PM +0200, Daniel Lezcano wrote: > > On 16/05/2020 14:16, Serge Semin wrote: > > > Hello Daniel, > > > > > > Thanks for your comment. My response is below. > > > > > > On Fri, May 15, 2020 at 07:10:04PM +0200, Daniel Lezcano wrote: > > >> On Thu, May 07, 2020 at 12:41:07AM +0300, Serge Semin wrote: > > >>> Currently neither clocksource nor scheduler clock kernel framework > > >>> support the clocks with variable frequency. Needless to say how many > > >>> problems may cause the sudden base clocks frequency change. In a > > >>> simplest case the system time will either slow down or speed up. > > >>> Since on CM2.5 and earlier MIPS GIC timer is synchronously clocked > > >>> with CPU we must set some limitations on using it for these frameworks > > >>> if CPU frequency may change. First of all it's not safe to have the > > >>> MIPS GIC used for scheduler timings. So we shouldn't proceed with > > >>> the clocks registration in the sched-subsystem. Secondly we must > > >>> significantly decrease the MIPS GIC clocksource rating. This will let > > >>> the system to use it only as a last resort. > > >>> > > >>> Note CM3.x-based systems may also experience the problems with MIPS GIC > > >>> if the CPU-frequency change is activated for the whole CPU cluster > > >>> instead of using the individual CPC core clocks divider. > > >> > > >> May be there is no alternative but the code looks a bit hacksih. Isn't possible > > >> to do something with the sched_mark_unstable? > > >> > > >> Or just not use the timer at all ? > > > > > > Not using the timer might be better, but not that good alternative either > > > especially in our case due to very slow external timer. Me and Thomas > > > Bogendoerfer discussed the similar commit I've provided to the csrc-r4k driver > > > available on MIPS: > > > https://lkml.org/lkml/2020/5/11/576 > > > > > > To cut it short, you are right. The solution with using clocksource_mark_unstable() > > > is better alternative spied up in x86 tsc implementation. I'll use a similar > > > approach here and submit the updated patch in v3. > > > > > > Could you please proceed with the rest of the series review? I'd like to send > > > the next version with as many comments taken into account as possible. The > > > patchset has been submitted a while ago, but except Rob noone have had any > > > comments.( > > > > For me other patches are ok. > > > > I can apply patches 1, 2, 4, 5, 6 > > > > Will remain patches 3 et 7 > > That's be great! Thanks. Is patch 3 supposed to be merged in by Rob or by you? > I don't see one being in the Rob's repo. He might be waiting for you > acknowledgment or something. Ah, it's about the MIPS GIC IRQchip bindings conversion. No questions tabout patch 3then. I'll ask Thomas about that patch status. -Sergey > > I'll send the updated patch 3 shortly today. > > -Sergey > > > > > > > -- > > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > > <http://twitter.com/#!/linaroorg> Twitter | > > <http://www.linaro.org/linaro-blog/> Blog
diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c index 802b93fe3ae7..095d65b48920 100644 --- a/drivers/clocksource/mips-gic-timer.c +++ b/drivers/clocksource/mips-gic-timer.c @@ -185,7 +185,10 @@ static int __init __gic_clocksource_init(void) gic_clocksource.mask = CLOCKSOURCE_MASK(count_width); /* Calculate a somewhat reasonable rating value. */ - gic_clocksource.rating = 200 + gic_frequency / 10000000; + if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ)) + gic_clocksource.rating = 200 + gic_frequency / 10000000; + else + gic_clocksource.rating = 99; ret = clocksource_register_hz(&gic_clocksource, gic_frequency); if (ret < 0) @@ -239,9 +242,11 @@ static int __init gic_clocksource_of_init(struct device_node *node) /* And finally start the counter */ clear_gic_config(GIC_CONFIG_COUNTSTOP); - sched_clock_register(mips_cm_is64 ? - gic_read_count_64 : gic_read_count_2x32, - 64, gic_frequency); + if (mips_cm_revision() >= CM_REV_CM3 || !IS_ENABLED(CONFIG_CPU_FREQ)) { + sched_clock_register(mips_cm_is64 ? + gic_read_count_64 : gic_read_count_2x32, + 64, gic_frequency); + } return 0; }
Currently neither clocksource nor scheduler clock kernel framework support the clocks with variable frequency. Needless to say how many problems may cause the sudden base clocks frequency change. In a simplest case the system time will either slow down or speed up. Since on CM2.5 and earlier MIPS GIC timer is synchronously clocked with CPU we must set some limitations on using it for these frameworks if CPU frequency may change. First of all it's not safe to have the MIPS GIC used for scheduler timings. So we shouldn't proceed with the clocks registration in the sched-subsystem. Secondly we must significantly decrease the MIPS GIC clocksource rating. This will let the system to use it only as a last resort. Note CM3.x-based systems may also experience the problems with MIPS GIC if the CPU-frequency change is activated for the whole CPU cluster instead of using the individual CPC core clocks divider. Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Cc: Paul Burton <paulburton@kernel.org> Cc: Ralf Baechle <ralf@linux-mips.org> Cc: Alessandro Zummo <a.zummo@towertech.it> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Rob Herring <robh+dt@kernel.org> Cc: linux-mips@vger.kernel.org Cc: linux-rtc@vger.kernel.org Cc: devicetree@vger.kernel.org --- drivers/clocksource/mips-gic-timer.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)