[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <02874ECE860811409154E81DA85FBB58564F1C88@ORSMSX104.amr.corp.intel.com>
Date: Tue, 28 May 2013 21:12:02 +0000
From: "Keller, Jacob E" <jacob.e.keller@...el.com>
To: "Vick, Matthew" <matthew.vick@...el.com>,
Richard Cochran <richardcochran@...il.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
David Miller <davem@...emloft.net>,
"e1000-devel@...ts.sourceforge.net"
<e1000-devel@...ts.sourceforge.net>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
Subject: RE: [PATCH net-next 4/4] igb: enable auxiliary PHC functions for
the i210.
> -----Original Message-----
> From: Vick, Matthew
> Sent: Tuesday, May 28, 2013 10:49 AM
> To: Richard Cochran
> Cc: netdev@...r.kernel.org; David Miller; e1000-
> devel@...ts.sourceforge.net; Keller, Jacob E; Kirsher, Jeffrey T
> Subject: Re: [PATCH net-next 4/4] igb: enable auxiliary PHC functions for
> the i210.
>
> On 5/28/13 9:23 AM, "Richard Cochran" <richardcochran@...il.com>
> wrote:
>
> >On Tue, May 28, 2013 at 03:58:07PM +0000, Vick, Matthew wrote:
> >> On 5/27/13 2:21 AM, "Richard Cochran" <richardcochran@...il.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.
>
> Personal preference would dictate a MAC check, but I'm alright with a
> comment. :)
>
> >> >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)
>
> Ah, I didn't realize we had a precedent. Module param is fine with me,
> then. ethtool does seem like the right long-term solution, but I don't
> think that should gate your patchset if we already have a precedent for
> this functionality.
I think ethtool would be good, but what about using sysfs?
- Jake
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists