[PATCH 3/8] dovetail/clock: implement pipeline_set_timer_shot to trigger tick shot

[Chen, Hongzhan]

>-----Original Message-----
>From: Philippe Gerum <rpm at xenomai.org> 
>Sent: Saturday, January 16, 2021 7:56 PM
>To: Chen, Hongzhan <hongzhan.chen at intel.com>
>Cc: xenomai at xenomai.org
>Subject: Re: [PATCH 3/8] dovetail/clock: implement pipeline_set_timer_shot to trigger tick shot
>
>
>The shortlog should mention the subsystem and the pipeline variant to
>allow for a quick comprehension of the scope; that would be
>"cobalt/tick: dovetail: ..." in this case.
>
>hongzha1 via Xenomai <xenomai at xenomai.org> writes:
>
>> Signed-off-by: hongzha1 <hongzhan.chen at intel.com>
>>
>> diff --git a/include/cobalt/kernel/dovetail/pipeline/clock.h b/include/cobalt/kernel/dovetail/pipeline/clock.h
>> index 28d89b44b..db982d969 100644
>> --- a/include/cobalt/kernel/dovetail/pipeline/clock.h
>> +++ b/include/cobalt/kernel/dovetail/pipeline/clock.h
>> @@ -17,17 +17,7 @@ static inline u64 pipeline_read_cycle_counter(void)
>>  	return  ktime_get_raw_fast_ns();
>>  }
>>  
>> -static inline void pipeline_set_timer_shot(unsigned long cycles)
>> -{
>> -	/*
>> -	 * N/A. Revisit: xnclock_core_local_shot() should go to the
>> -	 * I-pipe section, we do things differently on Dovetail via
>> -	 * the proxy tick device. As a consequence,
>> -	 * pipeline_set_timer_shot() should not be part of the
>> -	 * pipeline interface.
>> -	 */
>> -	TODO();
>> -}
>> +inline void pipeline_set_timer_shot(unsigned long cycles);
>>  
>>  static inline const char *pipeline_timer_name(void)
>>  {
>> diff --git a/kernel/cobalt/dovetail/tick.c b/kernel/cobalt/dovetail/tick.c
>> index 6a196496c..55039f96e 100644
>> --- a/kernel/cobalt/dovetail/tick.c
>> +++ b/kernel/cobalt/dovetail/tick.c
>> @@ -16,6 +16,43 @@ extern struct xnintr nktimer;
>>  
>>  static DEFINE_PER_CPU(struct clock_proxy_device *, proxy_device);
>>  
>> +inline void pipeline_set_timer_shot(unsigned long cycles)
>> +{
>> +	/*
>> +	 * N/A. Revisit: xnclock_core_local_shot() should go to the
>> +	 * I-pipe section, we do things differently on Dovetail via
>> +	 * the proxy tick device. As a consequence,
>> +	 * pipeline_set_timer_shot() should not be part of the
>> +	 * pipeline interface.
>> +	 */
>> +	struct clock_proxy_device *dev = __this_cpu_read(proxy_device);
>> +	struct clock_event_device *real_dev = dev->real_device;
>> +	int ret;
>> +	u64 sumcyc;
>> +	ktime_t t;
>> +
>> +	if (real_dev->features & CLOCK_EVT_FEAT_KTIME) {
>> +		t = ktime_add(cycles, xnclock_core_read_raw());
>> +		real_dev->set_next_ktime(t, real_dev);
>> +	} else {
>> +		if (cycles <= 0)
>> +			cycles = real_dev->min_delta_ns;
>> +		else {
>> +			cycles = min_t(int64_t, cycles,
>> +					real_dev->max_delta_ns);
>> +			cycles = max_t(int64_t, cycles,
>> +					real_dev->min_delta_ns);
>> +		}
>> +		sumcyc = ((u64)cycles * real_dev->mult) >> real_dev->shift;
>> +
>> +		ret = real_dev->set_next_event(sumcyc, real_dev);
>> +		if (ret) {
>> +			ret = real_dev->set_next_event(real_dev->min_delta_ticks,
>> +							real_dev);
>> +		}
>> +	}
>> +}
>> +
>>  static int proxy_set_next_ktime(ktime_t expires,
>>  				struct clock_event_device *proxy_dev)
>>  {
>
>This patch does not apply cleanly, raising a conflict in
>dovetail/pipeline/clock.h. Besides, as discussed recently, the comment   

The patch actually depends on previous 5 patches you already approved 
to merge. One of them modified dovetail/pipeline/clock.h.  

https://xenomai.org/pipermail/xenomai/2021-January/044123.html
https://xenomai.org/pipermail/xenomai/2021-January/044124.html
https://xenomai.org/pipermail/xenomai/2021-January/044125.html
https://xenomai.org/pipermail/xenomai/2021-January/044126.html
https://xenomai.org/pipermail/xenomai/2021-January/044127.html

>mentioning the need to revisit the code may not be accurate anymore. You

Thanks for your suggestions, I will refine it though.

>may want to drop it as part of the fix up.
>
>-- 
>Philippe.