diff mbox

[net-next,2/2] e100: enable transmit time stamping.

Message ID 33c61d135f4fd62e5128466633302414ead0092e.1331400160.git.richardcochran@gmail.com
State Awaiting Upstream, archived
Delegated to: David Miller
Headers show

Commit Message

Richard Cochran March 10, 2012, 5:29 p.m. UTC
This patch enables software (and phy device) transmit time stamping.
Tested on an old PIII laptop with built in NIC.

Cc: Alex Duyck <alexander.h.duyck@intel.com>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Cc: Greg Rose <gregory.v.rose@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: John Ronciak <john.ronciak@intel.com>
Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>

Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
 drivers/net/ethernet/intel/e100.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Comments

Kirsher, Jeffrey T March 11, 2012, 9:56 a.m. UTC | #1
On Sat, 2012-03-10 at 18:29 +0100, Richard Cochran wrote:
> This patch enables software (and phy device) transmit time stamping.
> Tested on an old PIII laptop with built in NIC.
> 
> Cc: Alex Duyck <alexander.h.duyck@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Cc: Greg Rose <gregory.v.rose@intel.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: John Ronciak <john.ronciak@intel.com>
> Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
>  drivers/net/ethernet/intel/e100.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-) 

Thanks Richard, I will add the patch to my queue.
Kirsher, Jeffrey T April 4, 2012, 9:04 a.m. UTC | #2
On Sat, 2012-03-10 at 09:29 -0800, Richard Cochran wrote:
> This patch enables software (and phy device) transmit time stamping.
> Tested on an old PIII laptop with built in NIC.
> 
> Cc: Alex Duyck <alexander.h.duyck@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Cc: Greg Rose <gregory.v.rose@intel.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: John Ronciak <john.ronciak@intel.com>
> Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> ---
>  drivers/net/ethernet/intel/e100.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-) 

Richard-

I had this patch in my queue, but during testing we found some serious
issues with some adapters.  With every 82559 based adapter Aaron tried
the system would panic on driver load.  This included at least one
sampling of D101S and D101M.  Every other e100 chipset Aaron tried
(82557, 82558, 82550) appeared to work fine.

So I am dropping this patch as is due to the kernel panics.

Cheers,
Jeff
Richard Cochran April 4, 2012, 9:30 a.m. UTC | #3
On Wed, Apr 04, 2012 at 02:04:43AM -0700, Jeff Kirsher wrote:
> 
> I had this patch in my queue, but during testing we found some serious
> issues with some adapters.  With every 82559 based adapter Aaron tried
> the system would panic on driver load.  This included at least one
> sampling of D101S and D101M.  Every other e100 chipset Aaron tried
> (82557, 82558, 82550) appeared to work fine.
> 
> So I am dropping this patch as is due to the kernel panics.

Okay, fine. That driver is strangely written, with weird callbacks and
passing function pointers around.

Taking a second look, I think I might have found the problem. The
drivers casts a firmware pointer onto a skb, and my patch then tries
to timestamp the firmware blob.

If I rework this, are you willing to give it another try?

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
Kirsher, Jeffrey T April 4, 2012, 9:34 a.m. UTC | #4
On Wed, 2012-04-04 at 11:30 +0200, Richard Cochran wrote:
> On Wed, Apr 04, 2012 at 02:04:43AM -0700, Jeff Kirsher wrote:
> > 
> > I had this patch in my queue, but during testing we found some serious
> > issues with some adapters.  With every 82559 based adapter Aaron tried
> > the system would panic on driver load.  This included at least one
> > sampling of D101S and D101M.  Every other e100 chipset Aaron tried
> > (82557, 82558, 82550) appeared to work fine.
> > 
> > So I am dropping this patch as is due to the kernel panics.
> 
> Okay, fine. That driver is strangely written, with weird callbacks and
> passing function pointers around.
> 
> Taking a second look, I think I might have found the problem. The
> drivers casts a firmware pointer onto a skb, and my patch then tries
> to timestamp the firmware blob.
> 
> If I rework this, are you willing to give it another try?
> 
> Thanks,
> Richard

Yeah, we appreciate the work your doing.  Just send me an updated patch
and I will add it to my queue.

That was our thoughts as well regarding the firmware blobs.
Richard Cochran April 7, 2012, 12:49 p.m. UTC | #5
Changes in V2:
- Avoid time stamping of firmware blobs

This patch adds SO_TIMESTAMPING in the transmit path for the Ethernet
MACs found in my older laptop.


Richard Cochran (1):
  e100: enable transmit time stamping.

 drivers/net/ethernet/intel/e100.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
diff mbox

Patch

diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index e498eff..8a1ba85 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -918,6 +918,8 @@  static int e100_exec_cb(struct nic *nic, struct sk_buff *skb,
 		}
 	}
 
+	if (skb)
+		skb_tx_timestamp(skb);
 err_unlock:
 	spin_unlock_irqrestore(&nic->cb_lock, flags);