Fix broken windows code #377
Conversation
PHY delay defaults to zero if timestamper is NULL
Move init_phy_delay() before HWTimestamper_init() allowing PHY delays to be over-ridden in derived timestamper code (using *new* set_phy_delay()).
Currently, there isn't any native I210 drivers for windows. Remove those PHY delays. Set the defaults to -1. Absent command line or config file over-ride these delays will be set in the derived class code for specific HW. TODO: Add read of configuration file to over-ride defaults in derived timestamper class
| get_phy_delay(&delay_val); | ||
|
|
||
| DeviceClockRateMapping *rate_map = DeviceClockRateMap; | ||
| while (rate_map->device_desc != NULL) |
There was a problem hiding this comment.
Should there be rate_map != NULL here?
There was a problem hiding this comment.
Perhaps rate_map has a entry with device_desc == NULL to terminate?
There was a problem hiding this comment.
Yes. The expectation is that the array has at least one terminating entry where device_desc == NULL. This is for both the PHY delay and clock rate mapping.
|
Overall, looks good to me. |
| default: | ||
| // Unable to identify CPU | ||
| return 0; | ||
| case 0x5E: |
There was a problem hiding this comment.
I understand by the code that these values (0x5e and 0x4e) are processor model identifiers, but I think it would improve readability if we had a define (or a const static uint8_t variable somewhere) for them.
|
Looks good to me. |
Windows drivers interface will by default use QPC for system time rather than "raw" TSC. There are some exceptions for older operating systems on Intel hardware that support ART (Always Running Timer). Nanoseconds units will still be used for upper layers of gPTP and its application interface.
Remove some of the #defines for clock rate, adding table lookup Add table lookup for PHY delays that can be over-ridden by top level daemon_cl code.
|
Ok, so we have a pending format cleanup commit and hopefully one to remove processor ID magic numbers. |
christopher-s-hall
force-pushed
the
open-avb-next
branch
from
ece07e2 to
e8e8214
Compare
|
Any other comments here? |
|
Thanks for the reminder @christopher-s-hall. It looks like you have made all the changes suggested in the various comments, so I'm going to accept the pull request. |
No description provided.