[Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.

[Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.

[Hongzhan Chen]

When there is refined tsc clock, notify Xenomai to apply it.
Linux may schedule a delayed work to refine tsc clock and update
tsc_khz which happen after Xenomai finsih init but tsc_scale and
tsc_shift still keep the value depending on origianl tsc clock
which is outdated. The difference between two clocks may cause
timing issue.

For example:
  [ 0.001731] tsc: Detected 2899.886 MHz TSC
  [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
  cat /sys/module/xenomai/parameters/clockfreq
  2899886000
  After patching, we like to use 2903.999 MHz.

The patchset includes IPIPE patch and cobalt-patch.

Hongzhan Chen (1):
  process: update clockfreq when receive corresponding event.
  x86/tsc: I-PIPE : notify I-PIPE about updated clockfreq.

cobalt side:
 include/cobalt/kernel/init.h                        |  2 ++
 kernel/cobalt/include/asm-generic/xenomai/machine.h |  1 +
 kernel/cobalt/init.c                                | 11 ++++++++++-
 kernel/cobalt/posix/process.c                       |  9 ++++++++-
 4 files changed, 21 insertions(+), 2 deletions(-)

ipipe-side:
 arch/x86/kernel/tsc.c | 5 +++++
 1 file changed, 5 insertions(+)


-- 
2.17.1

[Cobalt Xenoami3.1 PATCH 1/2] process: update clockfreq when receive corresponding event.

[Hongzhan Chen]

1. When there is clockfreq param passed down via command line, we
   do not update clockfreq even if we receive event of updating clockfreq.
   Or else, we update the clockfreq with notified value.
2. At the same time, we would like to update clockfreq param showing
   in sys filesystem after apply updated clockfreq.

Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>

diff --git a/include/cobalt/kernel/init.h b/include/cobalt/kernel/init.h
index 41dd531a8..43144443b 100644
--- a/include/cobalt/kernel/init.h
+++ b/include/cobalt/kernel/init.h
@@ -51,4 +51,6 @@ void cobalt_remove_state_chain(struct notifier_block *nb);
 
 void cobalt_call_state_chain(enum cobalt_run_states newstate);
 
+void cobalt_update_clockfreq_arg(unsigned long updatedclockfreq);
+
 #endif /* !_COBALT_KERNEL_INIT_H_ */
diff --git a/kernel/cobalt/include/asm-generic/xenomai/machine.h b/kernel/cobalt/include/asm-generic/xenomai/machine.h
index 25764f989..aaa3edc97 100644
--- a/kernel/cobalt/include/asm-generic/xenomai/machine.h
+++ b/kernel/cobalt/include/asm-generic/xenomai/machine.h
@@ -61,6 +61,7 @@ struct cobalt_pipeline {
 #ifdef CONFIG_SMP
 	cpumask_t supported_cpus;
 #endif
+	unsigned int passed_clockfreq;
 };
 
 extern struct cobalt_pipeline cobalt_pipeline;
diff --git a/kernel/cobalt/init.c b/kernel/cobalt/init.c
index dbe321c3b..a6cfc1e06 100644
--- a/kernel/cobalt/init.c
+++ b/kernel/cobalt/init.c
@@ -53,6 +53,19 @@ module_param_named(timerfreq, timerfreq_arg, ulong, 0444);
 static unsigned long clockfreq_arg;
 module_param_named(clockfreq, clockfreq_arg, ulong, 0444);
 
+void cobalt_update_clockfreq_arg(unsigned long updatedclockfreq)
+{
+	spl_t s;
+
+	xnlock_get_irqsave(&nklock, s);
+
+	clockfreq_arg = updatedclockfreq;
+
+	xnlock_put_irqrestore(&nklock, s);
+
+}
+EXPORT_SYMBOL_GPL(cobalt_update_clockfreq_arg);
+
 #ifdef CONFIG_SMP
 static unsigned long supported_cpus_arg = -1;
 module_param_named(supported_cpus, supported_cpus_arg, ulong, 0444);
@@ -148,8 +161,11 @@ static int __init mach_setup(void)
 	if (timerfreq_arg == 0)
 		timerfreq_arg = sysinfo.sys_hrtimer_freq;
 
-	if (clockfreq_arg == 0)
+	if (clockfreq_arg == 0) {
 		clockfreq_arg = sysinfo.sys_hrclock_freq;
+		cobalt_pipeline.passed_clockfreq = 0;
+	} else
+		cobalt_pipeline.passed_clockfreq = 1;
 
 	if (clockfreq_arg == 0) {
 		printk(XENO_ERR "null clock frequency? Aborting.\n");
diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
index 6d1c1c427..41ea47b0d 100644
--- a/kernel/cobalt/posix/process.c
+++ b/kernel/cobalt/posix/process.c
@@ -38,6 +38,7 @@
 #include <linux/kallsyms.h>
 #include <linux/ipipe.h>
 #include <linux/ipipe_tickdev.h>
+#include <cobalt/kernel/init.h>
 #include <cobalt/kernel/sched.h>
 #include <cobalt/kernel/heap.h>
 #include <cobalt/kernel/synch.h>
@@ -1382,7 +1383,13 @@ static inline int handle_clockfreq_event(unsigned int *p)
 {
 	unsigned int newfreq = *p;
 
-	xnclock_update_freq(newfreq);
+	/* when there is no para in commandline
+	 * passed down to set clockfreq
+	 */
+	if (!cobalt_pipeline.passed_clockfreq) {
+		xnclock_update_freq(newfreq);
+		cobalt_update_clockfreq_arg(newfreq);
+	}
 
 	return KEVENT_PROPAGATE;
 }
-- 
2.17.1

[I-PIPE Xenoami3.1 PATCH 2/2] x86/tsc: I-PIPE : notify I-PIPE about updated clockfreq

[Hongzhan Chen]

When there is refined tsc clock, notify Xenomai to apply it.
Linux may schedule a delayed work to refine tsc clock and update
tsc_khz which happen after Xenomai finsih init but tsc_scale and
tsc_shift still keep the value depending on origianl tsc clock
which is outdated. The difference between two clocks may cause
unexpected timing drift.

For example:
  [ 0.001731] tsc: Detected 2899.886 MHz TSC
  [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
  cat /sys/module/xenomai/parameters/clockfreq
  2899886000
  After patching, we like to use 2903.999 MHz.

Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>

diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 835856efd71f..e2ca733d76ee 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -1294,6 +1294,7 @@ static void tsc_refine_calibration_work(struct work_struct *work)
 	u64 tsc_stop, ref_stop, delta;
 	unsigned long freq;
 	int cpu;
+	unsigned int ipipe_freq;
 
 	/* Don't bother refining TSC on unstable systems */
 	if (tsc_unstable)
@@ -1345,6 +1346,10 @@ static void tsc_refine_calibration_work(struct work_struct *work)
 	/* Inform the TSC deadline clockevent devices about the recalibration */
 	lapic_update_tsc_freq();
 
+	/* notify xenomai about updated clockfreq */
+	ipipe_freq = tsc_khz * 1000;
+	 __ipipe_report_clockfreq_update(ipipe_freq);
+
 	/* Update the sched_clock() rate to match the clocksource one */
 	for_each_possible_cpu(cpu)
 		set_cyc2ns_scale(tsc_khz, cpu, tsc_stop);
-- 
2.17.1

Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.

[Jan Kiszka]

On 27.05.22 08:22, Hongzhan Chen via Xenomai wrote:
> When there is refined tsc clock, notify Xenomai to apply it.
> Linux may schedule a delayed work to refine tsc clock and update
> tsc_khz which happen after Xenomai finsih init but tsc_scale and
> tsc_shift still keep the value depending on origianl tsc clock
> which is outdated. The difference between two clocks may cause
> timing issue.
> 
> For example:
>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>   cat /sys/module/xenomai/parameters/clockfreq
>   2899886000
>   After patching, we like to use 2903.999 MHz.
> 
> The patchset includes IPIPE patch and cobalt-patch.
> 

Sounds reasonable, but you could help me with reviewing this by already
answering:

 - How does dovetail (and xenomai 3.2 or evl) address this?
 - Why is this tagged "3.1" only?
 - Which I-pipe series is this targeting (5.4, or also 4.19)?

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

RE: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.

[Chen, Hongzhan]

>-----Original Message-----
>From: Jan Kiszka <jan.kiszka@siemens.com> 
>Sent: Thursday, June 2, 2022 5:54 PM
>To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>Subject: Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.
>
>On 27.05.22 08:22, Hongzhan Chen via Xenomai wrote:
>> When there is refined tsc clock, notify Xenomai to apply it.
>> Linux may schedule a delayed work to refine tsc clock and update
>> tsc_khz which happen after Xenomai finsih init but tsc_scale and
>> tsc_shift still keep the value depending on origianl tsc clock
>> which is outdated. The difference between two clocks may cause
>> timing issue.
>> 
>> For example:
>>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>>   cat /sys/module/xenomai/parameters/clockfreq
>>   2899886000
>>   After patching, we like to use 2903.999 MHz.
>> 
>> The patchset includes IPIPE patch and cobalt-patch.
>> 
>
>Sounds reasonable, but you could help me with reviewing this by already
>answering:
>
> - How does dovetail (and xenomai 3.2 or evl) address this?

So far , I have not found similar issue on dovetail-based. Dovetail-based would go vdso uniformly so there is
no such issue but IPIPE would have to depend  on tsc_khz value it got at first to do translation even after tsc clockfreq is refined and changed.

> - Why is this tagged "3.1" only?
> - Which I-pipe series is this targeting (5.4, or also 4.19)?

Currently , I just reproduced this issue and verified the patch on 5.4.133 + xenomai 3.1. But according to [1] reported,
the issue can be found on 4.19 and I think my patch may work but I have not verified on 4.19. 

Regards

Hongzhan Chen


[1]: https://xenomai.org/pipermail/xenomai/2022-May/047770.html

Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.

[Jan Kiszka]

On 02.06.22 14:56, Chen, Hongzhan wrote:
> 
> 
>> -----Original Message-----
>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>> Sent: Thursday, June 2, 2022 5:54 PM
>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>> Subject: Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.
>>
>> On 27.05.22 08:22, Hongzhan Chen via Xenomai wrote:
>>> When there is refined tsc clock, notify Xenomai to apply it.
>>> Linux may schedule a delayed work to refine tsc clock and update
>>> tsc_khz which happen after Xenomai finsih init but tsc_scale and
>>> tsc_shift still keep the value depending on origianl tsc clock
>>> which is outdated. The difference between two clocks may cause
>>> timing issue.
>>>
>>> For example:
>>>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>>>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>>>   cat /sys/module/xenomai/parameters/clockfreq
>>>   2899886000
>>>   After patching, we like to use 2903.999 MHz.
>>>
>>> The patchset includes IPIPE patch and cobalt-patch.
>>>
>>
>> Sounds reasonable, but you could help me with reviewing this by already
>> answering:
>>
>> - How does dovetail (and xenomai 3.2 or evl) address this?
> 
> So far , I have not found similar issue on dovetail-based. Dovetail-based would go vdso uniformly so there is
> no such issue but IPIPE would have to depend  on tsc_khz value it got at first to do translation even after tsc clockfreq is refined and changed.

Right, that is reason...

> 
>> - Why is this tagged "3.1" only?
>> - Which I-pipe series is this targeting (5.4, or also 4.19)?
> 
> Currently , I just reproduced this issue and verified the patch on 5.4.133 + xenomai 3.1. But according to [1] reported,
> the issue can be found on 4.19 and I think my patch may work but I have not verified on 4.19. 
> 

Please check stable/v3.2.x first (or as well) as that is the latest
stable with I-pipe still included. Once the fix is merged there, we can
pick it for 3.1 and possibly even 3.0 as well.

Jan

> Regards
> 
> Hongzhan Chen
> 
> 
> [1]: https://xenomai.org/pipermail/xenomai/2022-May/047770.html
> 

-- 
Siemens AG, Technology
Competence Center Embedded Linux

RE: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.

[Chen, Hongzhan]

>-----Original Message-----
>From: Jan Kiszka <jan.kiszka@siemens.com> 
>Sent: Wednesday, June 8, 2022 11:21 PM
>To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>Subject: Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.
>
>On 02.06.22 14:56, Chen, Hongzhan wrote:
>> 
>> 
>>> -----Original Message-----
>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>> Sent: Thursday, June 2, 2022 5:54 PM
>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>> Subject: Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.
>>>
>>> On 27.05.22 08:22, Hongzhan Chen via Xenomai wrote:
>>>> When there is refined tsc clock, notify Xenomai to apply it.
>>>> Linux may schedule a delayed work to refine tsc clock and update
>>>> tsc_khz which happen after Xenomai finsih init but tsc_scale and
>>>> tsc_shift still keep the value depending on origianl tsc clock
>>>> which is outdated. The difference between two clocks may cause
>>>> timing issue.
>>>>
>>>> For example:
>>>>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>>>>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>>>>   cat /sys/module/xenomai/parameters/clockfreq
>>>>   2899886000
>>>>   After patching, we like to use 2903.999 MHz.
>>>>
>>>> The patchset includes IPIPE patch and cobalt-patch.
>>>>
>>>
>>> Sounds reasonable, but you could help me with reviewing this by already
>>> answering:
>>>
>>> - How does dovetail (and xenomai 3.2 or evl) address this?
>> 
>> So far , I have not found similar issue on dovetail-based. Dovetail-based would go vdso uniformly so there is
>> no such issue but IPIPE would have to depend  on tsc_khz value it got at first to do translation even after tsc clockfreq is refined and changed.
>
>Right, that is reason...
>
>> 
>>> - Why is this tagged "3.1" only?
>>> - Which I-pipe series is this targeting (5.4, or also 4.19)?
>> 
>> Currently , I just reproduced this issue and verified the patch on 5.4.133 + xenomai 3.1. But according to [1] reported,
>> the issue can be found on 4.19 and I think my patch may work but I have not verified on 4.19. 
>> 
>
>Please check stable/v3.2.x first (or as well) as that is the latest
>stable with I-pipe still included. Once the fix is merged there, we can
>pick it for 3.1 and possibly even 3.0 as well.

The code between 3.2 and 3.1 involving this patchset is quite different now. For 3.2, we already dropped code related to clockfreq parameter
in 6d2989b6da73ec52fe8c990798be8a637e4db5b9 by Philippe.
But in my patchset for 3.1, we have to update value of  clockfreq parameter to show correct clock freq after we update to refined clock freq.
To keep consistency between 3.1 and 3.2 for I-PIPE based code, do we need to revert this part of code related to clockfreq parameter for 3.2?

Regards

Hongzhan Chen


>
>Jan
>
>> Regards
>> 
>> Hongzhan Chen
>> 
>> 
>> [1]: https://xenomai.org/pipermail/xenomai/2022-May/047770.html
>> 
>
>-- 
>Siemens AG, Technology
>Competence Center Embedded Linux

Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.

[Jan Kiszka]

On 09.06.22 04:06, Chen, Hongzhan wrote:
>> -----Original Message-----
>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>> Sent: Wednesday, June 8, 2022 11:21 PM
>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>> Subject: Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.
>>
>> On 02.06.22 14:56, Chen, Hongzhan wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>> Sent: Thursday, June 2, 2022 5:54 PM
>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>> Subject: Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.
>>>>
>>>> On 27.05.22 08:22, Hongzhan Chen via Xenomai wrote:
>>>>> When there is refined tsc clock, notify Xenomai to apply it.
>>>>> Linux may schedule a delayed work to refine tsc clock and update
>>>>> tsc_khz which happen after Xenomai finsih init but tsc_scale and
>>>>> tsc_shift still keep the value depending on origianl tsc clock
>>>>> which is outdated. The difference between two clocks may cause
>>>>> timing issue.
>>>>>
>>>>> For example:
>>>>>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>>>>>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>>>>>   cat /sys/module/xenomai/parameters/clockfreq
>>>>>   2899886000
>>>>>   After patching, we like to use 2903.999 MHz.
>>>>>
>>>>> The patchset includes IPIPE patch and cobalt-patch.
>>>>>
>>>>
>>>> Sounds reasonable, but you could help me with reviewing this by already
>>>> answering:
>>>>
>>>> - How does dovetail (and xenomai 3.2 or evl) address this?
>>>
>>> So far , I have not found similar issue on dovetail-based. Dovetail-based would go vdso uniformly so there is
>>> no such issue but IPIPE would have to depend  on tsc_khz value it got at first to do translation even after tsc clockfreq is refined and changed.
>>
>> Right, that is reason...
>>
>>>
>>>> - Why is this tagged "3.1" only?
>>>> - Which I-pipe series is this targeting (5.4, or also 4.19)?
>>>
>>> Currently , I just reproduced this issue and verified the patch on 5.4.133 + xenomai 3.1. But according to [1] reported,
>>> the issue can be found on 4.19 and I think my patch may work but I have not verified on 4.19. 
>>>
>>
>> Please check stable/v3.2.x first (or as well) as that is the latest
>> stable with I-pipe still included. Once the fix is merged there, we can
>> pick it for 3.1 and possibly even 3.0 as well.
> 
> The code between 3.2 and 3.1 involving this patchset is quite different now. For 3.2, we already dropped code related to clockfreq parameter
> in 6d2989b6da73ec52fe8c990798be8a637e4db5b9 by Philippe.
> But in my patchset for 3.1, we have to update value of  clockfreq parameter to show correct clock freq after we update to refined clock freq.
> To keep consistency between 3.1 and 3.2 for I-PIPE based code, do we need to revert this part of code related to clockfreq parameter for 3.2?

Do you see the problem of 3.1+ipipe with 3.2 as well? I suspect so, but
please confirm. Then we do need a solution there, too. How is the next
question, I need to dive into that again.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

RE: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.

[Chen, Hongzhan]

>-----Original Message-----
>From: Jan Kiszka <jan.kiszka@siemens.com> 
>Sent: Thursday, June 9, 2022 1:48 PM
>To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>Subject: Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.
>
>On 09.06.22 04:06, Chen, Hongzhan wrote:
>>> -----Original Message-----
>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>> Sent: Wednesday, June 8, 2022 11:21 PM
>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>> Subject: Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.
>>>
>>> On 02.06.22 14:56, Chen, Hongzhan wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com> 
>>>>> Sent: Thursday, June 2, 2022 5:54 PM
>>>>> To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>>>> Subject: Re: [Cobalt Xenoami3.1 PATCH 0/2] notify Xenomai udpated clockfreq.
>>>>>
>>>>> On 27.05.22 08:22, Hongzhan Chen via Xenomai wrote:
>>>>>> When there is refined tsc clock, notify Xenomai to apply it.
>>>>>> Linux may schedule a delayed work to refine tsc clock and update
>>>>>> tsc_khz which happen after Xenomai finsih init but tsc_scale and
>>>>>> tsc_shift still keep the value depending on origianl tsc clock
>>>>>> which is outdated. The difference between two clocks may cause
>>>>>> timing issue.
>>>>>>
>>>>>> For example:
>>>>>>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>>>>>>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>>>>>>   cat /sys/module/xenomai/parameters/clockfreq
>>>>>>   2899886000
>>>>>>   After patching, we like to use 2903.999 MHz.
>>>>>>
>>>>>> The patchset includes IPIPE patch and cobalt-patch.
>>>>>>
>>>>>
>>>>> Sounds reasonable, but you could help me with reviewing this by already
>>>>> answering:
>>>>>
>>>>> - How does dovetail (and xenomai 3.2 or evl) address this?
>>>>
>>>> So far , I have not found similar issue on dovetail-based. Dovetail-based would go vdso uniformly so there is
>>>> no such issue but IPIPE would have to depend  on tsc_khz value it got at first to do translation even after tsc clockfreq is refined and changed.
>>>
>>> Right, that is reason...
>>>
>>>>
>>>>> - Why is this tagged "3.1" only?
>>>>> - Which I-pipe series is this targeting (5.4, or also 4.19)?
>>>>
>>>> Currently , I just reproduced this issue and verified the patch on 5.4.133 + xenomai 3.1. But according to [1] reported,
>>>> the issue can be found on 4.19 and I think my patch may work but I have not verified on 4.19. 
>>>>
>>>
>>> Please check stable/v3.2.x first (or as well) as that is the latest
>>> stable with I-pipe still included. Once the fix is merged there, we can
>>> pick it for 3.1 and possibly even 3.0 as well.
>> 
>> The code between 3.2 and 3.1 involving this patchset is quite different now. For 3.2, we already dropped code related to clockfreq parameter
>> in 6d2989b6da73ec52fe8c990798be8a637e4db5b9 by Philippe.
>> But in my patchset for 3.1, we have to update value of  clockfreq parameter to show correct clock freq after we update to refined clock freq.
>> To keep consistency between 3.1 and 3.2 for I-PIPE based code, do we need to revert this part of code related to clockfreq parameter for 3.2?
>
>Do you see the problem of 3.1+ipipe with 3.2 as well? I suspect so, but

Yes. On 3.2 + IPIPE, it still use the clock freq at first got to do init but never
update accordingly even after there is refined tsc clock. In cobalt_ticks_init of 
lib/cobalt/ticks.c, it init tsc_scale and tsc_shift when there is non-zero freq passed
but never change and pipeline_update_clock_freq never called to update them when
there is refined tsc clock. 

Regards

Hongzhan Chen

>please confirm. Then we do need a solution there, too. How is the next
>question, I need to dive into that again.
>
>Jan
>
>-- 
>Siemens AG, Technology
>Competence Center Embedded Linux

Re: [Cobalt Xenoami3.1 PATCH 1/2] process: update clockfreq when receive corresponding event.

[Jan Kiszka]

On 27.05.22 08:22, Hongzhan Chen via Xenomai wrote:
> 1. When there is clockfreq param passed down via command line, we
>    do not update clockfreq even if we receive event of updating clockfreq.
>    Or else, we update the clockfreq with notified value.
> 2. At the same time, we would like to update clockfreq param showing
>    in sys filesystem after apply updated clockfreq.
> 
> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
> 
> diff --git a/include/cobalt/kernel/init.h b/include/cobalt/kernel/init.h
> index 41dd531a8..43144443b 100644
> --- a/include/cobalt/kernel/init.h
> +++ b/include/cobalt/kernel/init.h
> @@ -51,4 +51,6 @@ void cobalt_remove_state_chain(struct notifier_block *nb);
>  
>  void cobalt_call_state_chain(enum cobalt_run_states newstate);
>  
> +void cobalt_update_clockfreq_arg(unsigned long updatedclockfreq);

Just "freq" is sufficient as parameter name.

> +
>  #endif /* !_COBALT_KERNEL_INIT_H_ */
> diff --git a/kernel/cobalt/include/asm-generic/xenomai/machine.h b/kernel/cobalt/include/asm-generic/xenomai/machine.h
> index 25764f989..aaa3edc97 100644
> --- a/kernel/cobalt/include/asm-generic/xenomai/machine.h
> +++ b/kernel/cobalt/include/asm-generic/xenomai/machine.h
> @@ -61,6 +61,7 @@ struct cobalt_pipeline {
>  #ifdef CONFIG_SMP
>  	cpumask_t supported_cpus;
>  #endif
> +	unsigned int passed_clockfreq;

bool

But I would rather make this a private flag in cobalt/init.c, see below.

>  };
>  
>  extern struct cobalt_pipeline cobalt_pipeline;
> diff --git a/kernel/cobalt/init.c b/kernel/cobalt/init.c
> index dbe321c3b..a6cfc1e06 100644
> --- a/kernel/cobalt/init.c
> +++ b/kernel/cobalt/init.c
> @@ -53,6 +53,19 @@ module_param_named(timerfreq, timerfreq_arg, ulong, 0444);
>  static unsigned long clockfreq_arg;
>  module_param_named(clockfreq, clockfreq_arg, ulong, 0444);
>  
> +void cobalt_update_clockfreq_arg(unsigned long updatedclockfreq)
> +{
> +	spl_t s;
> +
> +	xnlock_get_irqsave(&nklock, s);
> +
> +	clockfreq_arg = updatedclockfreq;
> +
> +	xnlock_put_irqrestore(&nklock, s);

How is synchronization supposed to work here?

And when is clockfreq_arg supposed to be consumed after mach_setup? By
whom? /sys/module/xenomai/parameters/?

To make this function more useful, it could perform the "was passed"
check and also set clockfreq_arg and call xnclock_update_freq() if it
was not defined via the command line.

> +
> +}
> +EXPORT_SYMBOL_GPL(cobalt_update_clockfreq_arg);

Why exporting this internal helper? Xenomai is always built into the kernel.

> +
>  #ifdef CONFIG_SMP
>  static unsigned long supported_cpus_arg = -1;
>  module_param_named(supported_cpus, supported_cpus_arg, ulong, 0444);
> @@ -148,8 +161,11 @@ static int __init mach_setup(void)
>  	if (timerfreq_arg == 0)
>  		timerfreq_arg = sysinfo.sys_hrtimer_freq;
>  
> -	if (clockfreq_arg == 0)
> +	if (clockfreq_arg == 0) {
>  		clockfreq_arg = sysinfo.sys_hrclock_freq;
> +		cobalt_pipeline.passed_clockfreq = 0;

Global variables are already zero-initialized.

> +	} else
> +		cobalt_pipeline.passed_clockfreq = 1;

cobalt_pipeline.passed_clockfreq = clockfreq_arg != 0;

>  
>  	if (clockfreq_arg == 0) {
>  		printk(XENO_ERR "null clock frequency? Aborting.\n");
> diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
> index 6d1c1c427..41ea47b0d 100644
> --- a/kernel/cobalt/posix/process.c
> +++ b/kernel/cobalt/posix/process.c
> @@ -38,6 +38,7 @@
>  #include <linux/kallsyms.h>
>  #include <linux/ipipe.h>
>  #include <linux/ipipe_tickdev.h>
> +#include <cobalt/kernel/init.h>
>  #include <cobalt/kernel/sched.h>
>  #include <cobalt/kernel/heap.h>
>  #include <cobalt/kernel/synch.h>
> @@ -1382,7 +1383,13 @@ static inline int handle_clockfreq_event(unsigned int *p)
>  {
>  	unsigned int newfreq = *p;
>  
> -	xnclock_update_freq(newfreq);
> +	/* when there is no para in commandline
> +	 * passed down to set clockfreq

Comment does not tell anything that isn't in the code already.

> +	 */
> +	if (!cobalt_pipeline.passed_clockfreq) {
> +		xnclock_update_freq(newfreq);
> +		cobalt_update_clockfreq_arg(newfreq);
> +	}

See my remark above: If you push xnclock_update_freq into
cobalt_update_clockfreq, you can simplify to logic here, just call
cobalt_update_clockfreq unconditionally.

>  
>  	return KEVENT_PROPAGATE;
>  }

And now I understand that none of this is needed for 3.2, only the
I-pipe patch to send the kevent.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

Re: [I-PIPE Xenoami3.1 PATCH 2/2] x86/tsc: I-PIPE : notify I-PIPE about updated clockfreq

[Jan Kiszka]

On 27.05.22 08:22, Hongzhan Chen via Xenomai wrote:
> When there is refined tsc clock, notify Xenomai to apply it.

Xenomai is conceptually not known to I-pipe. This patch is about calling
the well-defined kevent hook when the TSC frequency changes after a
recalibration. ARM does something similar on frequency changes.

> Linux may schedule a delayed work to refine tsc clock and update
> tsc_khz which happen after Xenomai finsih init but tsc_scale and
> tsc_shift still keep the value depending on origianl tsc clock
> which is outdated. The difference between two clocks may cause
> unexpected timing drift.
> 
> For example:
>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>   cat /sys/module/xenomai/parameters/clockfreq
>   2899886000
>   After patching, we like to use 2903.999 MHz.
> 
> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
> 
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 835856efd71f..e2ca733d76ee 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -1294,6 +1294,7 @@ static void tsc_refine_calibration_work(struct work_struct *work)
>  	u64 tsc_stop, ref_stop, delta;
>  	unsigned long freq;
>  	int cpu;
> +	unsigned int ipipe_freq;
>  
>  	/* Don't bother refining TSC on unstable systems */
>  	if (tsc_unstable)
> @@ -1345,6 +1346,10 @@ static void tsc_refine_calibration_work(struct work_struct *work)
>  	/* Inform the TSC deadline clockevent devices about the recalibration */
>  	lapic_update_tsc_freq();
>  
> +	/* notify xenomai about updated clockfreq */

Drop the comment, it's misleading.

> +	ipipe_freq = tsc_khz * 1000;
> +	 __ipipe_report_clockfreq_update(ipipe_freq);

Why not simply

__ipipe_report_clockfreq_update(tsc_khz * 1000);

?

> +
>  	/* Update the sched_clock() rate to match the clocksource one */
>  	for_each_possible_cpu(cpu)
>  		set_cyc2ns_scale(tsc_khz, cpu, tsc_stop);

The patch looks like valid candidate for 5.4, 4.19-cip and even 4.4-cip.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux

RE: [Cobalt Xenoami3.1 PATCH 1/2] process: update clockfreq when receive corresponding event.

[Chen, Hongzhan]

>-----Original Message-----
>From: Jan Kiszka <jan.kiszka@siemens.com> 
>Sent: Wednesday, June 15, 2022 5:28 AM
>To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>Subject: Re: [Cobalt Xenoami3.1 PATCH 1/2] process: update clockfreq when receive corresponding event.
>
>On 27.05.22 08:22, Hongzhan Chen via Xenomai wrote:
>> 1. When there is clockfreq param passed down via command line, we
>>    do not update clockfreq even if we receive event of updating clockfreq.
>>    Or else, we update the clockfreq with notified value.
>> 2. At the same time, we would like to update clockfreq param showing
>>    in sys filesystem after apply updated clockfreq.
>> 
>> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
>> 
>> diff --git a/include/cobalt/kernel/init.h b/include/cobalt/kernel/init.h
>> index 41dd531a8..43144443b 100644
>> --- a/include/cobalt/kernel/init.h
>> +++ b/include/cobalt/kernel/init.h
>> @@ -51,4 +51,6 @@ void cobalt_remove_state_chain(struct notifier_block *nb);
>>  
>>  void cobalt_call_state_chain(enum cobalt_run_states newstate);
>>  
>> +void cobalt_update_clockfreq_arg(unsigned long updatedclockfreq);
>
>Just "freq" is sufficient as parameter name.
>
>> +
>>  #endif /* !_COBALT_KERNEL_INIT_H_ */
>> diff --git a/kernel/cobalt/include/asm-generic/xenomai/machine.h b/kernel/cobalt/include/asm-generic/xenomai/machine.h
>> index 25764f989..aaa3edc97 100644
>> --- a/kernel/cobalt/include/asm-generic/xenomai/machine.h
>> +++ b/kernel/cobalt/include/asm-generic/xenomai/machine.h
>> @@ -61,6 +61,7 @@ struct cobalt_pipeline {
>>  #ifdef CONFIG_SMP
>>  	cpumask_t supported_cpus;
>>  #endif
>> +	unsigned int passed_clockfreq;
>
>bool
>
>But I would rather make this a private flag in cobalt/init.c, see below.

Sorry , I can not find your comments which related to private flag you mentioned in the below.

>
>>  };
>>  
>>  extern struct cobalt_pipeline cobalt_pipeline;
>> diff --git a/kernel/cobalt/init.c b/kernel/cobalt/init.c
>> index dbe321c3b..a6cfc1e06 100644
>> --- a/kernel/cobalt/init.c
>> +++ b/kernel/cobalt/init.c
>> @@ -53,6 +53,19 @@ module_param_named(timerfreq, timerfreq_arg, ulong, 0444);
>>  static unsigned long clockfreq_arg;
>>  module_param_named(clockfreq, clockfreq_arg, ulong, 0444);
>>  
>> +void cobalt_update_clockfreq_arg(unsigned long updatedclockfreq)
>> +{
>> +	spl_t s;
>> +
>> +	xnlock_get_irqsave(&nklock, s);
>> +
>> +	clockfreq_arg = updatedclockfreq;
>> +
>> +	xnlock_put_irqrestore(&nklock, s);
>
>How is synchronization supposed to work here?
>
>And when is clockfreq_arg supposed to be consumed after mach_setup? By
>whom? /sys/module/xenomai/parameters/?

Yes , /sys/module/xenomai/parameters/clockfreq would use consume it.

>
>To make this function more useful, it could perform the "was passed"
>check and also set clockfreq_arg and call xnclock_update_freq() if it
>was not defined via the command line.
>
>> +
>> +}
>> +EXPORT_SYMBOL_GPL(cobalt_update_clockfreq_arg);
>
>Why exporting this internal helper? Xenomai is always built into the kernel.
>
>> +
>>  #ifdef CONFIG_SMP
>>  static unsigned long supported_cpus_arg = -1;
>>  module_param_named(supported_cpus, supported_cpus_arg, ulong, 0444);
>> @@ -148,8 +161,11 @@ static int __init mach_setup(void)
>>  	if (timerfreq_arg == 0)
>>  		timerfreq_arg = sysinfo.sys_hrtimer_freq;
>>  
>> -	if (clockfreq_arg == 0)
>> +	if (clockfreq_arg == 0) {
>>  		clockfreq_arg = sysinfo.sys_hrclock_freq;
>> +		cobalt_pipeline.passed_clockfreq = 0;
>
>Global variables are already zero-initialized.
>
>> +	} else
>> +		cobalt_pipeline.passed_clockfreq = 1;
>
>cobalt_pipeline.passed_clockfreq = clockfreq_arg != 0;
>
>>  
>>  	if (clockfreq_arg == 0) {
>>  		printk(XENO_ERR "null clock frequency? Aborting.\n");
>> diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
>> index 6d1c1c427..41ea47b0d 100644
>> --- a/kernel/cobalt/posix/process.c
>> +++ b/kernel/cobalt/posix/process.c
>> @@ -38,6 +38,7 @@
>>  #include <linux/kallsyms.h>
>>  #include <linux/ipipe.h>
>>  #include <linux/ipipe_tickdev.h>
>> +#include <cobalt/kernel/init.h>
>>  #include <cobalt/kernel/sched.h>
>>  #include <cobalt/kernel/heap.h>
>>  #include <cobalt/kernel/synch.h>
>> @@ -1382,7 +1383,13 @@ static inline int handle_clockfreq_event(unsigned int *p)
>>  {
>>  	unsigned int newfreq = *p;
>>  
>> -	xnclock_update_freq(newfreq);
>> +	/* when there is no para in commandline
>> +	 * passed down to set clockfreq
>
>Comment does not tell anything that isn't in the code already.
>
>> +	 */
>> +	if (!cobalt_pipeline.passed_clockfreq) {
>> +		xnclock_update_freq(newfreq);
>> +		cobalt_update_clockfreq_arg(newfreq);
>> +	}
>
>See my remark above: If you push xnclock_update_freq into
>cobalt_update_clockfreq, you can simplify to logic here, just call
>cobalt_update_clockfreq unconditionally.
>
>>  
>>  	return KEVENT_PROPAGATE;
>>  }
>
>And now I understand that none of this is needed for 3.2, only the
>I-pipe patch to send the kevent.

Yes, only I-pipe-based would have such issue. But actually 3.2 dropped 
/sys/module/xenomai/parameters/clockfreq.  /sys/module/xenomai/parameters/clockfreq 
scheme can keep the way for user at least from debug view to quickly validate such issue via
passing  clockfreq through commandline and know what clockfreq it is using and is useful for I-PIPE-based.
Do we need to revert /sys/module/xenomai/parameters/clockfreq for I-PIPE in 3.2?

Regards

Hongzhan Chen
>
>Jan
>
>-- 
>Siemens AG, Technology
>Competence Center Embedded Linux
>
>
>

RE: [I-PIPE Xenoami3.1 PATCH 2/2] x86/tsc: I-PIPE : notify I-PIPE about updated clockfreq

[Chen, Hongzhan]

>
>Xenomai is conceptually not known to I-pipe. This patch is about calling
>the well-defined kevent hook when the TSC frequency changes after a
>recalibration. ARM does something similar on frequency changes.
>
>> Linux may schedule a delayed work to refine tsc clock and update
>> tsc_khz which happen after Xenomai finsih init but tsc_scale and
>> tsc_shift still keep the value depending on origianl tsc clock
>> which is outdated. The difference between two clocks may cause
>> unexpected timing drift.
>> 
>> For example:
>>   [ 0.001731] tsc: Detected 2899.886 MHz TSC
>>   [ 5.588387] tsc: Refined TSC clocksource calibration: 2903.999 MHz
>>   cat /sys/module/xenomai/parameters/clockfreq
>>   2899886000
>>   After patching, we like to use 2903.999 MHz.
>> 
>> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
>> 
>> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
>> index 835856efd71f..e2ca733d76ee 100644
>> --- a/arch/x86/kernel/tsc.c
>> +++ b/arch/x86/kernel/tsc.c
>> @@ -1294,6 +1294,7 @@ static void tsc_refine_calibration_work(struct work_struct *work)
>>  	u64 tsc_stop, ref_stop, delta;
>>  	unsigned long freq;
>>  	int cpu;
>> +	unsigned int ipipe_freq;
>>  
>>  	/* Don't bother refining TSC on unstable systems */
>>  	if (tsc_unstable)
>> @@ -1345,6 +1346,10 @@ static void tsc_refine_calibration_work(struct work_struct *work)
>>  	/* Inform the TSC deadline clockevent devices about the recalibration */
>>  	lapic_update_tsc_freq();
>>  
>> +	/* notify xenomai about updated clockfreq */
>
>Drop the comment, it's misleading.
>
>> +	ipipe_freq = tsc_khz * 1000;
>> +	 __ipipe_report_clockfreq_update(ipipe_freq);
>
>Why not simply
>
>__ipipe_report_clockfreq_update(tsc_khz * 1000);

There is following error if we apply this.

./include/linux/ipipe.h:177:53: error: lvalue required as unary ‘&’ operand
  177 |         __ipipe_notify_kevent(IPIPE_KEVT_CLOCKFREQ, &(freq))
      |                                                     ^
arch/x86/kernel/tsc.c:1349:10: note: in expansion of macro ‘__ipipe_report_clockfreq_update’
 1349 |          __ipipe_report_clockfreq_update(tsc_khz * 1000);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
scripts/Makefile.build:262: recipe for target 'arch/x86/kernel/tsc.o' failed

Regards

Hongzhan Chen

>
>?
>
>> +
>>  	/* Update the sched_clock() rate to match the clocksource one */
>>  	for_each_possible_cpu(cpu)
>>  		set_cyc2ns_scale(tsc_khz, cpu, tsc_stop);
>
>The patch looks like valid candidate for 5.4, 4.19-cip and even 4.4-cip.
>
>Jan
>
>-- 
>Siemens AG, Technology
>Competence Center Embedded Linux

RE: [Cobalt Xenoami3.1 PATCH 1/2] process: update clockfreq when receive corresponding event.

[Chen, Hongzhan]

>-----Original Message-----
>From: Xenomai <xenomai-bounces@xenomai.org> On Behalf Of Chen, Hongzhan via Xenomai
>Sent: Wednesday, June 15, 2022 9:29 AM
>To: Kiszka, Jan <jan.kiszka@siemens.com>; xenomai@xenomai.org
>Subject: RE: [Cobalt Xenoami3.1 PATCH 1/2] process: update clockfreq when receive corresponding event.
>
>
>
>>-----Original Message-----
>>From: Jan Kiszka <jan.kiszka@siemens.com> 
>>Sent: Wednesday, June 15, 2022 5:28 AM
>>To: Chen, Hongzhan <hongzhan.chen@intel.com>; xenomai@xenomai.org
>>Subject: Re: [Cobalt Xenoami3.1 PATCH 1/2] process: update clockfreq when receive corresponding event.
>>
>>On 27.05.22 08:22, Hongzhan Chen via Xenomai wrote:
>>> 1. When there is clockfreq param passed down via command line, we
>>>    do not update clockfreq even if we receive event of updating clockfreq.
>>>    Or else, we update the clockfreq with notified value.
>>> 2. At the same time, we would like to update clockfreq param showing
>>>    in sys filesystem after apply updated clockfreq.
>>> 
>>> Signed-off-by: Hongzhan Chen <hongzhan.chen@intel.com>
>>> 
>>> diff --git a/include/cobalt/kernel/init.h b/include/cobalt/kernel/init.h
>>> index 41dd531a8..43144443b 100644
>>> --- a/include/cobalt/kernel/init.h
>>> +++ b/include/cobalt/kernel/init.h
>>> @@ -51,4 +51,6 @@ void cobalt_remove_state_chain(struct notifier_block *nb);
>>>  
>>>  void cobalt_call_state_chain(enum cobalt_run_states newstate);
>>>  
>>> +void cobalt_update_clockfreq_arg(unsigned long updatedclockfreq);
>>
>>Just "freq" is sufficient as parameter name.
>>
>>> +
>>>  #endif /* !_COBALT_KERNEL_INIT_H_ */
>>> diff --git a/kernel/cobalt/include/asm-generic/xenomai/machine.h b/kernel/cobalt/include/asm-generic/xenomai/machine.h
>>> index 25764f989..aaa3edc97 100644
>>> --- a/kernel/cobalt/include/asm-generic/xenomai/machine.h
>>> +++ b/kernel/cobalt/include/asm-generic/xenomai/machine.h
>>> @@ -61,6 +61,7 @@ struct cobalt_pipeline {
>>>  #ifdef CONFIG_SMP
>>>  	cpumask_t supported_cpus;
>>>  #endif
>>> +	unsigned int passed_clockfreq;
>>
>>bool
>>
>>But I would rather make this a private flag in cobalt/init.c, see below.
>
>Sorry , I can not find your comments which related to private flag you mentioned in the below.

I think I already understood what you means now. After we push xnclock_update_freq into
cobalt_update_clockfreq  and make conditional check with passed_clockfreq , we can make it a private flag after then.

Regards

Hongzhan Chen
>
>>
>>>  };
>>>  
>>>  extern struct cobalt_pipeline cobalt_pipeline;
>>> diff --git a/kernel/cobalt/init.c b/kernel/cobalt/init.c
>>> index dbe321c3b..a6cfc1e06 100644
>>> --- a/kernel/cobalt/init.c
>>> +++ b/kernel/cobalt/init.c
>>> @@ -53,6 +53,19 @@ module_param_named(timerfreq, timerfreq_arg, ulong, 0444);
>>>  static unsigned long clockfreq_arg;
>>>  module_param_named(clockfreq, clockfreq_arg, ulong, 0444);
>>>  
>>> +void cobalt_update_clockfreq_arg(unsigned long updatedclockfreq)
>>> +{
>>> +	spl_t s;
>>> +
>>> +	xnlock_get_irqsave(&nklock, s);
>>> +
>>> +	clockfreq_arg = updatedclockfreq;
>>> +
>>> +	xnlock_put_irqrestore(&nklock, s);
>>
>>How is synchronization supposed to work here?
>>
>>And when is clockfreq_arg supposed to be consumed after mach_setup? By
>>whom? /sys/module/xenomai/parameters/?
>
>Yes , /sys/module/xenomai/parameters/clockfreq would use consume it.
>
>>
>>To make this function more useful, it could perform the "was passed"
>>check and also set clockfreq_arg and call xnclock_update_freq() if it
>>was not defined via the command line.
>>
>>> +
>>> +}
>>> +EXPORT_SYMBOL_GPL(cobalt_update_clockfreq_arg);
>>
>>Why exporting this internal helper? Xenomai is always built into the kernel.
>>
>>> +
>>>  #ifdef CONFIG_SMP
>>>  static unsigned long supported_cpus_arg = -1;
>>>  module_param_named(supported_cpus, supported_cpus_arg, ulong, 0444);
>>> @@ -148,8 +161,11 @@ static int __init mach_setup(void)
>>>  	if (timerfreq_arg == 0)
>>>  		timerfreq_arg = sysinfo.sys_hrtimer_freq;
>>>  
>>> -	if (clockfreq_arg == 0)
>>> +	if (clockfreq_arg == 0) {
>>>  		clockfreq_arg = sysinfo.sys_hrclock_freq;
>>> +		cobalt_pipeline.passed_clockfreq = 0;
>>
>>Global variables are already zero-initialized.
>>
>>> +	} else
>>> +		cobalt_pipeline.passed_clockfreq = 1;
>>
>>cobalt_pipeline.passed_clockfreq = clockfreq_arg != 0;
>>
>>>  
>>>  	if (clockfreq_arg == 0) {
>>>  		printk(XENO_ERR "null clock frequency? Aborting.\n");
>>> diff --git a/kernel/cobalt/posix/process.c b/kernel/cobalt/posix/process.c
>>> index 6d1c1c427..41ea47b0d 100644
>>> --- a/kernel/cobalt/posix/process.c
>>> +++ b/kernel/cobalt/posix/process.c
>>> @@ -38,6 +38,7 @@
>>>  #include <linux/kallsyms.h>
>>>  #include <linux/ipipe.h>
>>>  #include <linux/ipipe_tickdev.h>
>>> +#include <cobalt/kernel/init.h>
>>>  #include <cobalt/kernel/sched.h>
>>>  #include <cobalt/kernel/heap.h>
>>>  #include <cobalt/kernel/synch.h>
>>> @@ -1382,7 +1383,13 @@ static inline int handle_clockfreq_event(unsigned int *p)
>>>  {
>>>  	unsigned int newfreq = *p;
>>>  
>>> -	xnclock_update_freq(newfreq);
>>> +	/* when there is no para in commandline
>>> +	 * passed down to set clockfreq
>>
>>Comment does not tell anything that isn't in the code already.
>>
>>> +	 */
>>> +	if (!cobalt_pipeline.passed_clockfreq) {
>>> +		xnclock_update_freq(newfreq);
>>> +		cobalt_update_clockfreq_arg(newfreq);
>>> +	}
>>
>>See my remark above: If you push xnclock_update_freq into
>>cobalt_update_clockfreq, you can simplify to logic here, just call
>>cobalt_update_clockfreq unconditionally.
>>
>>>  
>>>  	return KEVENT_PROPAGATE;
>>>  }
>>
>>And now I understand that none of this is needed for 3.2, only the
>>I-pipe patch to send the kevent.
>
>Yes, only I-pipe-based would have such issue. But actually 3.2 dropped 
>/sys/module/xenomai/parameters/clockfreq.  /sys/module/xenomai/parameters/clockfreq 
>scheme can keep the way for user at least from debug view to quickly validate such issue via
>passing  clockfreq through commandline and know what clockfreq it is using and is useful for I-PIPE-based.
>Do we need to revert /sys/module/xenomai/parameters/clockfreq for I-PIPE in 3.2?
>
>Regards
>
>Hongzhan Chen
>>
>>Jan
>>
>>-- 
>>Siemens AG, Technology
>>Competence Center Embedded Linux
>>
>>
>>
>
>