Hi,
On Thu, 2009-11-12 at 21:17 +0100, Ralf Baechle wrote:
[...]
> > And also, this external timer is also needed by the next Cpufreq support,
> > 'Cause the frequency of the MIPS Timer is half of the cpu frequency, if
> > we use it with Cpufreq support, the sytem time will become not accuracy.
[...]
> > Signed-off-by: Wu Zhangjin <wuzhangjin@xxxxxxxxx>
> > diff --git a/arch/mips/loongson/Kconfig b/arch/mips/loongson/Kconfig
> > index 029360f..648c47d 100644
> > --- a/arch/mips/loongson/Kconfig
> > +++ b/arch/mips/loongson/Kconfig
> > @@ -34,10 +34,10 @@ config LEMOTE_MACH2F
> > select ARCH_SPARSEMEM_ENABLE
> > select BOARD_SCACHE
> > select BOOT_ELF32
> > - select CEVT_R4K
> > + select CEVT_R4K if !CS5536_MFGPT
> > select CPU_HAS_WB
> > select CS5536
> > - select CSRC_R4K
> > + select CSRC_R4K if !CS5536_MFGPT
>
> A bit inelegant to trust on a user to pick the right value for CS5536_MFGPT.
>
> A non-fixed clock for c0_count is an obvious reason to avoid using CEVT_R4K
> and CSRC_R4K; we probably need something like cpu_has_fixed_c0_count_clock
> to make that decission at runtime.
Do you mean enable both of them when compiling, and at runtime select
the external one when something like cpu_has_fixed_c0_count_clock is
undefined and the CPUFreq Driver is loaded?
This maybe a good idea to allow users to choose their need in userspace
via /sys/devices/system/clocksource/clocksource0/current_clocksource,
and hide the details from the users.
But we may meet some problems:
1. At first, we must ensure the external one is used when the CPUFreq
Driver is loaded.
Just have a look at the source code in kernel/time/clocksource.c,
Seems we can finish this job via clocksource_change_rating().
we can try to increase the rating of the MFGPT Timer(external timer,
in Gdium, they use I8253), but perhaps we need to add a flag to this
external timer to mask them, otherwise, we will not know which one is
the external one, what about these flags in include/linux/clocksource.h:
/*
* Clock source flags bits:
*/
#define CLOCK_SOURCE_IS_CONTINUOUS 0x01
#define CLOCK_SOURCE_MUST_VERIFY 0x02
#define CLOCK_SOURCE_WATCHDOG 0x10
#define CLOCK_SOURCE_VALID_FOR_HRES 0x20
#define CLOCK_SOURCE_UNSTABLE 0x40
Seems no suitable flag to mask them, what about 0? we just set the
flag as 0, 'Cause the flag of MIPS Timer is set as
CLOCK_SOURCE_IS_CONTINUOUS, we will be able to distinguish them by this
flag. so, this problem will be fixed.
Of course, if there is no external timer found, we have to exit from
the CPUFreq Driver. and also, we need to resume the Timer to the MIPS
when we unload the CPUFreq Driver.
2. And another problem is: people will choose their own
current_clocksource when they want, if they choose MIPS, we must disable
the CPUFreq Driver asap, otherwise, the system time will be broken.
It will be hard to detect when will people change the current clock
source, and just checked the clocksource_select() in
kernel/time/clocksource.c, seems there is a timekeeping_notify() called:
timekeeping_notify()
--> tick_clock_notify()
--> set_bit(0, &per_cpu(tick_cpu_sched, cpu).check_clocks);
perhaps we can check the check_clocks bit in the CPUFreq Driver, for
example we do it in loongson2_cpufreq_target(), but we can not protect,
which cpu frequency currently we are, if we are not in the normal
level(normally it is 797MHz, the mult/shift of MIPS timer is calculated
with mips_hpt_frequency when initialization), we will also break the
system time. So, if we really want to fix this problem, perhaps we need
to touch the source code of the clocksource subsystem,
kernel/time/timekeeping.c:
/**
* timekeeping_notify - Install a new clock source
* @clock: pointer to the clock source
*
* This function is called from clocksource.c after a new, better clock
* source has been registered. The caller holds the clocksource_mutex.
*/
void timekeeping_notify(struct clocksource *clock)
{
if (timekeeper.clock == clock)
return;
stop_machine(change_clocksource, clock, NULL);
tick_clock_notify();
}
We can add a early-notify before changing the clocksource to tell the
relative drivers to do some necessary preparation.
void timekeeping_notify(struct clocksource *clock)
{
if (timekeeper.clock == clock)
return;
+ tick_clock_early_notify(clock);
stop_machine(change_clocksource, clock, NULL);
tick_clock_notify();
}
we must ensure the drivers can register a notifier to that
tick_clock_early_notify can called by it.
and then, in the CPUFreq driver, we can check who is the target clock,
if it is MIPS, the CPUFreq driver must change it's Frequency to a normal
level and then the whole problem will be fixed.
Seems this will make the problem be more complex.
> > select DMA_NONCOHERENT
> > select GENERIC_HARDIRQS_NO__DO_IRQ
> > select GENERIC_ISA_DMA_SUPPORT_BROKEN
> > @@ -62,6 +62,13 @@ endchoice
> > config CS5536
> > bool
> >
> > +config CS5536_MFGPT
> > + bool "Using cs5536's MFGPT as system clock"
> > + depends on CS5536
> > + help
> > + This is needed when cpufreq or oprofile is enabled in Lemote Loongson2F
> > + family machines
>
> A patch to deal with interaction between oprofile and the cp0 count / compare
> was queued recently. So is there any remaining issue or is the comment here
> just outdated?
>
Okay, Will remove the out-of-date part.
Thanks & Regards,
Wu Zhangjin
