RE: [net-next 04/14] e1000e: add support for IEEE-1588 PTP

-----Original Message-----
From: Richard Cochran [mailto:richardcochran@gmail.com]
Sent: Thursday, January 17, 2013 7:36 AM
To: Kirsher, Jeffrey T
Cc: davem@davemloft.net; Allan, Bruce W; netdev@vger.kernel.org;
gospo@redhat.com; sassmann@redhat.com
Subject: Re: [net-next 04/14] e1000e: add support for IEEE-1588 PTP

On Thu, Jan 17, 2013 at 03:35:09AM -0800, Jeff Kirsher wrote:

quoted

+static struct ptp_clock_info e1000e_ptp_clock_info = {
+	.owner		= THIS_MODULE,

small nit: better to use a static string for .name here than ...

quoted

+	.n_alarm	= 0,
+	.n_ext_ts	= 0,
+	.n_per_out	= 0,
+	.pps		= 0,
+	.adjfreq	= e1000e_phc_adjfreq,
+	.adjtime	= e1000e_phc_adjtime,
+	.gettime	= e1000e_phc_gettime,
+	.settime	= e1000e_phc_settime,
+	.enable		= e1000e_phc_enable,
+};
+
+/**
+ * e1000e_ptp_init - initialize PTP for devices which support it
+ * @adapter: board private structure
+ *
+ * This function performs the required steps for enabling PTP support.
+ * If PTP support has already been loaded it simply calls the cyclecounter
+ * init routine and exits.
+ **/
+void e1000e_ptp_init(struct e1000_adapter *adapter)
+{
+	struct e1000_hw *hw = &adapter->hw;
+
+	adapter->ptp_clock = NULL;
+
+	if (!(adapter->flags & FLAG_HAS_HW_TIMESTAMP))
+		return;
+
+	adapter->ptp_clock_info = e1000e_ptp_clock_info;
+
+	snprintf(adapter->ptp_clock_info.name,
+		 sizeof(adapter->ptp_clock_info.name), "%pm",
+		 adapter->netdev->perm_addr);

... putting any kind of address here. After some back and forth,
discussing what the 'name' field should be, we decided on

 * @name:      A short "friendly name" to identify the clock and to
 *             help distinguish PHY based devices from MAC based ones.
 *             The string is not meant to be a unique id.

and most other drivers put the name of the driver here.

Other than that, this new driver looks good to me. I'll try it out
soon.

Acked-by: Richard Cochran <richardcochran@gmail.com>

Thanks for the review Richard.