Re: [E1000-devel] [PATCH net-next 4/4] igb: enable auxiliary PHC functions for the i210.

[Alexander Duyck <alexander.h.duyck@intel.com>]

On 05/28/2013 09:23 AM, Richard Cochran wrote:
> On Tue, May 28, 2013 at 03:58:07PM +0000, Vick, Matthew wrote:
>> On 5/27/13 2:21 AM, "Richard Cochran" <richardcochran@gmail.com> wrote:
>  
>> I would prefer it if we did a MAC check before these two TSICR checks,
>> since we're making some assumptions about the hardware within the
>> interrupt cases. At the very least, a comment that these are only
>> applicable to I210/I211 would be nice.
> I can respin with a comment that the additional bits are i210 only. I
> think this is better than adding more checks into ISR. Since we only
> enable these bits for the i210, the checks would be redundant.
>
>>> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c
>>> b/drivers/net/ethernet/intel/igb/igb_ptp.c
>>> index 5944de0..8cf4b8a 100644
>>> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
>>> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
>>> @@ -23,6 +23,15 @@
>>>
>>> #include "igb.h"
>>>
>>> +static int igb_input_sdp = 0;
>>> +static int igb_output_sdp = 1;
>>> +module_param(igb_input_sdp, int, 0444);
>>> +module_param(igb_output_sdp, int, 0444);
>>> +MODULE_PARM_DESC(igb_input_sdp,
>>> +		 "The SDP used as an input, to time stamp external events");
>>> +MODULE_PARM_DESC(igb_output_sdp,
>>> +		 "The SDP used to output the programmable periodic signal");
>>> +
>> Is there any other mechanism we could use to control this? I would imagine
>> not, but I know module parameters are generally frowned upon.
> This the way I handled it for the PHYTER, and I think it is the best
> way from our three choices:
>
> 1. kconfig option (inflexible)
> 2. module param
> 3. ethtool (can o'worms)

I really think the ethtool approach would be preferred since it is the
most flexible.  The problem with module parameters is that it only
really works with one port.

Couldn't you use the ethtool private flags interface or something
similar to achieve the same result?

Thanks,

Alex