| Message ID | 1345715813-20757-14-git-send-email-jeffrey.t.kirsher@intel.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Aug 23, 2012 at 02:56:53AM -0700, Jeff Kirsher wrote: > From: Matthew Vick <matthew.vick@intel.com> > > Change the name of the adapter in the PTP struct to enable easier > correlation between interface and PTP device. You want to put the MAC address into the clock name? This is wrong. Besides, there is no need for this. The ethool method already makes the correlation crystal clear. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: Richard Cochran [mailto:richardcochran@gmail.com] > Sent: Thursday, August 23, 2012 3:46 AM > To: Kirsher, Jeffrey T > Cc: davem@davemloft.net; Vick, Matthew; netdev@vger.kernel.org; > gospo@redhat.com; sassmann@redhat.com > Subject: Re: [net-next 13/13] igb: Store the MAC address in the name in > the PTP struct. > > On Thu, Aug 23, 2012 at 02:56:53AM -0700, Jeff Kirsher wrote: > > From: Matthew Vick <matthew.vick@intel.com> > > > > Change the name of the adapter in the PTP struct to enable easier > > correlation between interface and PTP device. > > You want to put the MAC address into the clock name? This is wrong. > > Besides, there is no need for this. The ethool method already makes the > correlation crystal clear. > > Thanks, > Richard Actually, the name today is wrong. "igb-82580" is inapplicable to I350, I210, and I211 devices. This patch aims to bring some sense to the naming while fixing it. ixgbe implements the same naming scheme. Cheers, Matthew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2012-08-23 at 12:45 +0200, Richard Cochran wrote: > On Thu, Aug 23, 2012 at 02:56:53AM -0700, Jeff Kirsher wrote: > > From: Matthew Vick <matthew.vick@intel.com> > > > > Change the name of the adapter in the PTP struct to enable easier > > correlation between interface and PTP device. > > You want to put the MAC address into the clock name? This is wrong. > > Besides, there is no need for this. The ethool method already makes > the correlation crystal clear. The name field is described as 'A short name to identify the clock'. It is not stated whether this is meant to be the name of the clock *device* or the clock *driver*. If it's the name of the device then some unique ID such as the permanent MAC address is required. The ixgbe driver uses MAC addresses and so did the last submitted version of PHC support for the sfc driver. Ben.
On Thu, Aug 23, 2012 at 10:35:37PM +0100, Ben Hutchings wrote: > > The name field is described as 'A short name to identify the clock'. It > is not stated whether this is meant to be the name of the clock *device* > or the clock *driver*. If it's the name of the device then some unique > ID such as the permanent MAC address is required. In an ideal world, there is only one clock device per system, so the original concept was to use the driver name. The hardware designers have ignored or not understood the "one clock per system" model, and so, unfortunately, we have to deal with it. A clock device can and should be connected to more than one network device (see gianfar, dp83640), and to give it a MAC address is misleading. The ethtool solution works perfectly to go from interface->clock, which is all the information that a user space PTP stack needs. I think for sysfs and the ioctl it is nicer to have the driver name, since it is associated with the clock and its capabilities. Thanks, Richard -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] > On Behalf Of Richard Cochran > Sent: Thursday, August 23, 2012 11:20 PM > To: Ben Hutchings > Cc: Kirsher, Jeffrey T; davem@davemloft.net; Vick, Matthew; > netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com; Stuart > Hodgson > Subject: Re: [net-next 13/13] igb: Store the MAC address in the name in > the PTP struct. > > In an ideal world, there is only one clock device per system, so the > original concept was to use the driver name. The hardware designers have > ignored or not understood the "one clock per system" model, and so, > unfortunately, we have to deal with it. > > A clock device can and should be connected to more than one network device > (see gianfar, dp83640), and to give it a MAC address is misleading. > > The ethtool solution works perfectly to go from interface->clock, which is > all the information that a user space PTP stack needs. > > I think for sysfs and the ioctl it is nicer to have the driver name, since > it is associated with the clock and its capabilities. > Correct. MAC address is misleading. However driver name for our parts is misleading because each driver creates one clock per port. In either case, one driver handling multiple cards would also have to create multiple ptp devices and that is just as misleading. The ethtool method is definitely preferred, and is the correct way to identify the clock device correlation. - Jake > Thanks, > Richard > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in the > body of a message to majordomo@vger.kernel.org More majordomo info at > http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c index 34e0d69..e69555f 100644 --- a/drivers/net/ethernet/intel/igb/igb_ptp.c +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c @@ -547,14 +547,15 @@ int igb_ptp_hwtstamp_ioctl(struct net_device *netdev, void igb_ptp_init(struct igb_adapter *adapter) { struct e1000_hw *hw = &adapter->hw; + struct net_device *netdev = adapter->netdev; switch (hw->mac.type) { case e1000_i210: case e1000_i211: case e1000_i350: case e1000_82580: + snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr); adapter->ptp_caps.owner = THIS_MODULE; - strcpy(adapter->ptp_caps.name, "igb-82580"); adapter->ptp_caps.max_adj = 62499999; adapter->ptp_caps.n_ext_ts = 0; adapter->ptp_caps.pps = 0; @@ -570,10 +571,9 @@ void igb_ptp_init(struct igb_adapter *adapter) /* Enable the timer functions by clearing bit 31. */ wr32(E1000_TSAUXC, 0x0); break; - case e1000_82576: + snprintf(adapter->ptp_caps.name, 16, "%pm", netdev->dev_addr); adapter->ptp_caps.owner = THIS_MODULE; - strcpy(adapter->ptp_caps.name, "igb-82576"); adapter->ptp_caps.max_adj = 1000000000; adapter->ptp_caps.n_ext_ts = 0; adapter->ptp_caps.pps = 0; @@ -589,7 +589,6 @@ void igb_ptp_init(struct igb_adapter *adapter) /* Dial the nominal frequency. */ wr32(E1000_TIMINCA, INCPERIOD_82576 | INCVALUE_82576); break; - default: adapter->ptp_clock = NULL; return;