On Thu, Dec 13, 2018 at 09:52:50AM -0800, dwesterg@xxxxxxxxx wrote:
> Changes from V1:
> -> Remove debugfs for tod manipulation
> -> Reorder variable declarations in functions to order
> by length of declaration, largest to smallest
> -> Rename altera_ptp to intel_fpga_tod
> -> Rename functions in intel_fpga_tod so that they are not
> generic
> -> Use imply instead of select in Kconfig
> -> Re-write adjust time function to remove while loop with
> 64 bit divide.
Overall this is looking better. A few more comments follow...
> +static int tse_get_ts_info(struct net_device *dev,
> + struct ethtool_ts_info *info)
> +{
> + struct altera_tse_private *priv = netdev_priv(dev);
> +
> + if (priv->ptp_enable) {
> + if (priv->ptp_priv.ptp_clock)
> + info->phc_index =
> + ptp_clock_index(priv->ptp_priv.ptp_clock);
else
info->phc_index = -1;
> + info->so_timestamping = SOF_TIMESTAMPING_TX_HARDWARE |
> + SOF_TIMESTAMPING_RX_HARDWARE |
> + SOF_TIMESTAMPING_RAW_HARDWARE;
> +
> + info->tx_types = (1 << HWTSTAMP_TX_OFF) |
> + (1 << HWTSTAMP_TX_ON);
This fits on one line ^^^
> + info->rx_filters = (1 << HWTSTAMP_FILTER_NONE) |
> + (1 << HWTSTAMP_FILTER_ALL);
Funky indentation here. One extra level is enough:
info->rx_filters = (1 << HWTSTAMP_FILTER_NONE) |
(1 << HWTSTAMP_FILTER_ALL);
> + return 0;
> + } else {
> + return ethtool_op_get_ts_info(dev, info);
> + }
> +}
> +
...
> @@ -609,7 +613,14 @@ static netdev_tx_t tse_start_xmit(struct sk_buff *skb, struct net_device *dev)
> if (ret)
> goto out;
>
> - skb_tx_timestamp(skb);
> + /* Provide a hardware time stamp if requested.
> + */
> + if (unlikely((skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) &&
> + priv->hwts_tx_en))
> + /* declare that device is doing timestamping */
Please drop those two comments. They are redundant because they only
restate what the code does.
> + skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
> + else
> + skb_tx_timestamp(skb);
>
> priv->tx_prod++;
> dev->stats.tx_bytes += skb->len;
Thanks,
Richard
