Add support for "new" PTP cross-timestamp mechanism added in kernel version 4.6 #450
Conversation
Linux kernel version 4.6+ adds an additional cross timestamp ioctl for hardware that supports it. The cross-timestamp is exposed through the PTP driver interface. Intel PCH hardware reliably provides sub microsecond accuracy.
The Github automated builds are based on a very old Linux distribution. The precise timestamp ioctl is not available for these builds and the patch will not build. Add ifdef's to prevent compile errors.
Fixes issue Avnu#456 The port parameter link delay is now set *after* the initialization file is parsed
|
How is PTP_SYS_OFFSET_PRECISE ever set? Should there be any comments anywhere describing how to set and use it? |
|
It will be defined in the kernel include file (include/uapi/linux) for kernel 4.6+ and not defined in previous versions. I’m fine with commenting it, but where? Alternatively I could put something in the Makefile to test for this – probably a little messy – and add a wrapper. Now if someone were to grep for said wrapper it would be clear from the Makefile, in the form of a comment there, how it’s set. All of this is somewhat academic because eventually these changes will be included as part of the standard headers and the need to wrap them in an ifdef will go away. From: andrew-elder [mailto:notifications@github.com] How is PTP_SYS_OFFSET_PRECISE ever set? Should there be any comments anywhere describing how to set and use it? — |
christopher-s-hall
force-pushed
the
open-avb-next
branch
from
a194dd3 to
69ab901
Compare
|
Absent feedback I opted for the second. The Makefile explicitly checks for support and sets an internal variable. |
|
Added fix for issue #456 |
| \#ifdef PTP_SYS_OFFSET_PRECISE\\nint main(){return 0;}\\n\#endif" | ||
| HAS_PRECISE_TIME = $(shell echo $(PRECISE_TIME_TEST) | gcc -xc - \ | ||
| -I$(ALTERNATE_LINUX_INCPATH) -o /dev/null > /dev/null 2>&1 ; echo $$?) | ||
|
|
||
| CFLAGS_G = -Wall -std=c++0x -g -I. -I../../common -I../src \ |
There was a problem hiding this comment.
Wow, this is very clever. I personally think it is overkill in response to my "where does PTP_SYS_OFFSET" come from. I would have been 100% ok with a comment block in the .cpp module before the first usage of PTP_SYS_OFFSET_PRECISE explaining that kernels with precise time support have PTP_SYS_OFFSET_PRECISE defined in the linux/ptp_clock.h header file. My 2c. Others may have different opinions.
Sorry I was slow responding to the followup post on this topic.
|
The code is correct and in my opinion pretty clear. I would rather not spend more time on this. I don't think "overkill" should be a reason for code rejection. |
|
@christopher-s-hall - acknowledged. Can anyone else sign off on this? |
|
How come this got closed? I think there was a merge conflict indicated last time I saw it, but I didn't have time at the moment to see why. Aside from that, it looked fine as far as I can tell. |
|
Now pull #462. That has fewer commits (on second look, it didn't make a lot of sense to have so many commits), but is the same. |
Add support for HW timestamping & fix issue where link delay adjustments from .ini file are not applied correctly