RE: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.

-----Original Message-----
From: Richard Cochran [mailto:richardcochran@gmail.com]
Sent: Friday, August 24, 2012 11:20 PM
To: Keller, Jacob E
Cc: Ben Hutchings; Vick, Matthew; Kirsher, Jeffrey T; davem@davemloft.net;
netdev@vger.kernel.org; gospo@redhat.com; sassmann@redhat.com
Subject: Re: [net-next 10/13] igb: Tidy up wrapping for CONFIG_IGB_PTP.

On Fri, Aug 24, 2012 at 07:38:38PM +0000, Keller, Jacob E wrote:

quoted

The IGP_PTP is necessary otherwise you have to do something like #if
defined(CONFIG_PTP_1588_CLOCK), or #ifdef CONFIG_PTP_1588_CLOCK \
#ifdef CONFIG_PTP_1588_CLOCK_MODULE

Yes, but maybe use IGP_PTP as Ben described?

That's exactly what I was saying :) That's the reason why IGP_PTP is necessary.

quoted

The main reason that I wouldn't want to un-wrap the timestamping is
because the hwtstamp ioctl is somewhat problematic because it is
almost all ptp only. Also, some of the parts for igb driver don't
support timestamp all, they only support ptp only packets, and this
would be a lot more confusing since I would say still only allow that
if ptp is on.. (since those values are useless except with the PHC
clock)

I keep trying to explain that receive time stamping (HWTSTAMP_FILTER_ALL)
is useful all by itself. It has nothing to do with PTP, and the 82580 can
do this with very little ** overhead.

I myself have used this feature when debugging and profiling quasi real
time Ethernet protocols like EtherCAT and IEC61850-9-2. The other drivers
that offer this (gianfar and vxge) do so without any CONFIG conditionals.

IMHO, the Rx time stamping feature should always be available. It is
really not much different than the vlan feature.

IMO it should be but only for parts with HWTSTAMP_FILTER_ALL, and only for that mode (other modes should be ignored) because timestamping only PTP packets is a PTP feature, so this change should still disable other modes if they exist.

Thanks,
Richard

** Both igb_rx_hwtstamp and igb_tx_hwtstamp only add a single test
   when time stamping is not enabled.