[dpdk-dev] [PATCH v5 1/7] ethdev: add additional ieee1588 support functions

[Thomas Monjalon]

2015-11-10 11:36, Mcnamara, John:
> From: Thomas Monjalon
> > I'll try to fix it now to be sure it will be one of the first series ready
> > for the 2.3 cycle.
> 
> These comments are minor and could be fixed now.

After having a closer look in the drivers change, it seems to be restricted to
the PTP functions of the Intel drivers.
So you can ask to the Intel validation team if they are OK to add it in RC2.
I think it would be a wrong idea because we need to stop moving the ethdev and
drivers code, and focus on other DPDK areas for the RC2.

> > > +extern int rte_eth_timesync_time_get(uint8_t port_id,
> > > +	      struct timespec *time);
> > 
> > How is it different from rte_eth_timesync_read_rx_timestamp() and
> > rte_eth_timesync_read_tx_timestamp()?
> > 
> > Why repeating the word time? Why not rte_eth_timesync_get()?
> 
> In the context of PTP there is a difference between the time (os or NIC) and the timestamp (either in the mbuf, a register or as part of the payload).

Do you think we can make it clear in the definition of these functions?

More wording comments:
- rte_eth_timesync_time_get
- rte_eth_timesync_read_rx_timestamp
Why is it "get" in a case and "read" in another?
Why the verb is at the end in the first and before the complement in the latter?