On Mon, Feb 01, 2021 at 05:55:41PM -0500, vincent.cheng...@renesas.com wrote:
> + /* Set up timer expire notification mechanism */
> + se.sigev_notify = SIGEV_THREAD;
> + se.sigev_value.sival_ptr = (void *)c;
> + se.sigev_notify_function = clock_clear_free_running;
> + se.sigev_notify_attributes = NULL;
> +
> + if (timer_create(CLOCK_MONOTONIC, &se, &c->step_timer_id)) {
> + pr_err("failed to create step timer_id");
> + return NULL;
> + }
I agree with the premise of this patch, but I dislike the
implementation as it uses a timer with asynchronous signal.
The code already has multiple timers, and these are managed using the
timerfd API. This is the modern way of handling asynchronous timers.
I can see two better ways of implementing this new feature.
1. If you really want a to use a timer and keep the option value as a
time value, then increase N_CLOCK_PFD and add the timer to
clock.pollfd.
If you choose this option, please increase the resolution of the
step_window to milliseconds. After all, the window of expected
stale time stamps may be much shorter than one second!
2. You could define the step_window in terms of the number of Sync
events to skip. That reduces the implementation to a simple
counter. The option would still relate to time, because a Sync
rate of X means 2^X seconds.
I prefer #2 because it is much simpler.
Thoughts?
Thanks,
Richard
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
