On 21.05.21 03:27, Chen, Hongzhan wrote:
>
>>
>> -----Original Message-----
>> From: Jan Kiszka <jan.kis...@siemens.com>
>> Sent: Thursday, May 20, 2021 11:54 PM
>> To: Chen, Hongzhan <hongzhan.c...@intel.com>; xenomai@xenomai.org
>> Subject: Re: [PATCH V3 1/3] rtdm/testing: latmus: introduce latmus driver
>>
>> On 21.04.21 07:05, hongzha1 via Xenomai wrote:
>>> To support the latmus application for determining the best
>>> gravity values for the cobalt core clock, and measuring
>>> the response time to timer events.
>>>
>>> Signed-off-by: hongzha1 <hongzhan.c...@intel.com>
>>> ---
>>> include/rtdm/uapi/testing.h | 63 ++
>>> kernel/drivers/testing/Kconfig | 10 +
>>> kernel/drivers/testing/Makefile | 3 +
>>> kernel/drivers/testing/latmus.c | 1237 +++++++++++++++++++++++++++++++
>>> 4 files changed, 1313 insertions(+)
>>> create mode 100644 kernel/drivers/testing/latmus.c
>>>
>>
>> ...
>>
>>> diff --git a/kernel/drivers/testing/latmus.c
>>> b/kernel/drivers/testing/latmus.c
>>> new file mode 100644
>>> index 000000000..bef662260
>>> --- /dev/null
>>> +++ b/kernel/drivers/testing/latmus.c
>>> @@ -0,0 +1,1237 @@
>>> +/*
>>> + * SPDX-License-Identifier: GPL-2.0
>>> + *
>>> + * Derived from Xenomai Cobalt's autotune driver, https://xenomai.org/
>>> + * Copyright (C) 2014, 2018 Philippe Gerum <r...@xenomai.org>
>>> + */
>>> +
>>> +#include <linux/types.h>
>>> +#include <linux/init.h>
>>> +#include <linux/module.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/sort.h>
>>> +#include <linux/cdev.h>
>>> +#include <linux/fs.h>
>>> +#include <linux/fcntl.h>
>>> +#include <cobalt/kernel/pipe.h>
>>> +#include <cobalt/kernel/sched.h>
Do we need those two? RTDM should be the interface when possible. And if
some RTDM header is need of any of them, that would be a bug and a
separate patch.
>>> +#include <rtdm/ipc.h>
>>> +#include <rtdm/testing.h>
>>> +#include <rtdm/driver.h>
>>> +#include <rtdm/compat.h>
>>> +
>>> +#define ONE_BILLION 1000000000
>>> +#define TUNER_SAMPLING_TIME 500000000UL
>>> +#define TUNER_WARMUP_STEPS 10
>>> +#define TUNER_RESULT_STEPS 40
>>> +
>>> +#define progress(__runner, __fmt, __args...)
>>> \
>>> + do { \
>>> + if ((__runner)->verbosity > 1) \
>>> + printk(XENO_INFO "latmus(%s) " __fmt "\n", \
>>> + (__runner)->name, ##__args); \
>>> + } while (0)
>>> +
>>> +#define cobalt_init_xntimer_on_cpu(__timer, __cpu, __handler)
>>> \
>>> + rtdm_timer_init_on_cpu(__timer, __handler, \
>>> + #__handler, __cpu)
>>
>> Why this wrapper? Why not calling rtdm_timer... directly?
>
> Of course, we can call rtdm_timer... directly. My reason is :
>
> 1. When I ported it from evl , I just want to align with original as I can
> to pass same number of parameters here to replace evl_init_timer_on_cpu.
> 2. In addition ,when use this wrapper, caller may not need to care about
> name when it does not matter for user because define would handle it by
> default.
>
The current code is abstracting a single use case (you can't use the
code over evl as-is) - that is usually not a good idea. Once you have
two or more active use cases, abstraction can still be done.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
