-
Notifications
You must be signed in to change notification settings - Fork 8.2k
drivers: eth: native_posix: Enable gPTP support #8620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8620 +/- ##
==========================================
- Coverage 54.4% 52.22% -2.19%
==========================================
Files 444 195 -249
Lines 42233 24673 -17560
Branches 7996 5124 -2872
==========================================
- Hits 22978 12886 -10092
+ Misses 16048 9721 -6327
+ Partials 3207 2066 -1141
Continue to review full report at Codecov.
|
drivers/ethernet/eth_native_posix.c
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would not be too difficult to provide you a version of the host device time which could be offset. If you want tell, and I can add you a couple of functions in the timer driver. Same about the 2 functions below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting the host device time is probably not needed here, at least for now. The reason being that the native_posix target is typically used when testing things and it might not be ok to change the host time in this case. Of course we could make this optional if someone really wishes to have this kind of feature. I am ok if you send a PR that adds this kind of support thou.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but note that I did not mean that we would change the real host time, but instead that we would provide you a fake time which would be offset compared to the real host one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I misunderstood you then. That kind of fake time would be much welcomed feature so if you have time to add such a feature, I would appreciate that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Roger. I added a note in #6044 about it. Hopefully I will have time in the following days to add it.
aescolar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of comments:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking at this, I would actually think it would be better to not access the host time directly, but to have the timer driver provide you the host time, to have just 1 central place for it.
|
Just in case it is not obvious: My comments, if you would agree with them, could be addressed after this PR would be merged. |
pfalcon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aescolar, I assume that you'd seen the big issues, you gave -1 here. Otherwise, approving, to go on with other gPTP patches.
Allow gPTP code to be run as a linux process and communicate with gPTP daemon running in linux host. Signed-off-by: Jukka Rissanen <[email protected]>
Allow gPTP code to be run as a linux process and communicate
with gPTP daemon running in linux host.
Signed-off-by: Jukka Rissanen [email protected]